1046419 - Refine appearance of search widget

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect, P1)

Description

[:Margaret Leibovic]

Follow-up to bug 1035642.

See: https://bug1035642.bugzilla.mozilla.org/attachment.cgi?id=8465031

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Widget.png

Attaching current "simple widget" design for reference and as a starting point.

Comment 2

[Ian Barlow (:ibarlow)]

Hey this looks nice. And minimal. 

But maybe too minimal? I feel like the search and newtab buttons might need some text labels to make them clearer.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Ian Barlow (:ibarlow) from comment #2)
> Hey this looks nice. And minimal. 
> 
> But maybe too minimal? I feel like the search and newtab buttons might need
> some text labels to make them clearer.

Maybe not search, but newtab could be hard to understand from just an image.

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Good points Mark and Ian. I was working on that exactly but couldn't figure out a good way to do that yet so I wanted to land something minimal as the scope of the bug was for a "basic" widget at the time. But with this in the pipes I will now continue to refine this.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

I wouldn't mind seeing a variation of Anthony's current mockup land on Nightly. Even just adding "+ New Tab".

For some reason, I like this style more than what we landed, which seems somewhat like a WWF championship belt. I am a long time Hulkamaniac though.

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Mark Finkle (:mfinkle) from comment #5)
> I wouldn't mind seeing a variation of Anthony's current mockup land on
> Nightly. Even just adding "+ New Tab".
> 
> For some reason, I like this style more than what we landed, which seems
> somewhat like a WWF championship belt. I am a long time Hulkamaniac though.

I'm still unsure as to how necessary adding that text is but I'd be ok with giving it a try... designs to follow :)

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Patch

I've heard enough complaints, I went ahead and just did this with the current mockup. Happy to keep iterating if UX wants though.

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Screenshot

Screenshot of new theme. There's a build at http://people.mozilla.com/~wjohnston/widget.apk

Comment 9

[:Margaret Leibovic]

Comment on attachment 8467870 [details] [diff] [review]
Patch

Review of attachment 8467870 [details] [diff] [review]:
-----------------------------------------------------------------

I think there's room for some improvements, but I'd be fine with us landing this for now, since we'll probably iterate on the design anyway.

::: mobile/android/search/res/drawable/widget_button_right.xml
@@ +10,5 @@
> +            <corners android:topLeftRadius="0dp"
> +                     android:topRightRadius="4dp"
> +                     android:bottomLeftRadius="0dp"
> +                     android:bottomRightRadius="0dp"/>
> +            <solid android:color="#3000"/>

What is this color? Are you missing some digits here? Also, since you're using this color in multiple places, I think it would make sense to define it in search_colors.xml.

::: mobile/android/search/res/layout/search_widget.xml
@@ +36,2 @@
>  
>      </LinearLayout>

Instead of wrapping a LinearLayout around the ImageView, could you just set a background on the ImageView and get rid of the LinearLayout? And you could use a scaleType to just center the image in the middle of the ImageView.

Comment 10

[Ian Barlow (:ibarlow)]

I still think those icons need strings, or I'd at least like to see what that would look like, to compare.

Comment 11

[Wesley Johnston (:wesj)]

Attachment: With label

I played with adding one to just "New tab" since we had talked about it in here. This was a vertical attempt. I thought it look... unbalanced and decided to take off my amateur designer hat at that point.

Comment 12

[Ian Barlow (:ibarlow)]

Yeah I think the search button needs a text label too, if not for clarity then at least for better visual symmetry. 

Anthony, any thoughts on styling?

Comment 13

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Widget2.png

Adding the labels in this design.. for the icon and font color, I've used #777777 and they're padded 7dp apart. Both elements of the label (icon and text) are then centered.

Comment 14

[Wesley Johnston (:wesj)]

Attachment: Patch 2

I did this on top of the old one to try and keep it simple. This just updates us to use text labels, and also refines the background image a little.

Comment 15

[Wesley Johnston (:wesj)]

Attachment: Screenshot

Comment 16

[:Margaret Leibovic]

Comment on attachment 8468574 [details] [diff] [review]
Patch 2

Review of attachment 8468574 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

Also please make a FirefoxSearch PR when you land this on fx-team, and I can merge that.

::: mobile/android/search/res/layout/search_widget.xml
@@ +31,5 @@
> +                  android:layout_height="wrap_content"
> +                  android:drawableLeft="@drawable/ic_url_bar_search"
> +                  android:drawablePadding="@dimen/widget_drawable_padding"
> +                  android:text="@string/search_widget_button_label"
> +                  android:contentDescription="@string/search_widget_button_label"

We shouldn't need a contentDescription if this is a TextView with text, right? Especially if we're just using the same string.

@@ +35,5 @@
> +                  android:contentDescription="@string/search_widget_button_label"
> +                  android:gravity="center"
> +                  android:textSize="@dimen/widget_text_size"
> +                  android:textColor="@color/text_color_secondary"
> +                  android:id="@+id/search_button_label"/>

I know these things are already this way, but I usually like having the id at the top of the property list. Maybe something to move around if we make big changes to this layout again.

Comment 17

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: widget_icons.zip

could we update the widget with our new icons? :)

Comment 18

[Wesley Johnston (:wesj)]

Added the new icons and landed:
https://hg.mozilla.org/integration/fx-team/rev/2ad9b9def7aa

Comment 19

[Nigel Babu [:nigelb]]

https://hg.mozilla.org/mozilla-central/rev/2ad9b9def7aa