Closed Bug 1046419 Opened 5 years ago Closed 5 years ago

Refine appearance of search widget

Categories

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

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 34

People

(Reporter: Margaret, Assigned: wesj)

References

Details

Attachments

(5 files, 3 obsolete files)

Attached image Widget.png (obsolete) —
Attaching current "simple widget" design for reference and as a starting point.
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.
(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.
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.
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.
(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 :)
Attached patch PatchSplinter Review
I've heard enough complaints, I went ahead and just did this with the current mockup. Happy to keep iterating if UX wants though.
Attachment #8467870 - Flags: review?(margaret.leibovic)
Attached image Screenshot (obsolete) —
Screenshot of new theme. There's a build at http://people.mozilla.com/~wjohnston/widget.apk
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.
Attachment #8467870 - Flags: review?(margaret.leibovic) → review+
I still think those icons need strings, or I'd at least like to see what that would look like, to compare.
Attached image With label (obsolete) —
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.
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?
Attached image 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.
Attached patch Patch 2Splinter Review
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.
Attachment #8468574 - Flags: review?(margaret.leibovic)
Attached image Screenshot
Attachment #8465043 - Attachment is obsolete: true
Attachment #8467871 - Attachment is obsolete: true
Attachment #8467891 - Attachment is obsolete: true
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.
Attachment #8468574 - Flags: review?(margaret.leibovic) → review+
Attached file widget_icons.zip
could we update the widget with our new icons? :)
Flags: needinfo?(wjohnston)
Added the new icons and landed:
https://hg.mozilla.org/integration/fx-team/rev/2ad9b9def7aa
Flags: needinfo?(wjohnston)
Assignee: nobody → wjohnston
Priority: -- → P1
https://hg.mozilla.org/mozilla-central/rev/2ad9b9def7aa
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Depends on: 1050356
Depends on: 1050782
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.