Closed
Bug 1046419
Opened 10 years ago
Closed 10 years ago
Refine appearance of search widget
Categories
(Firefox for Android Graveyard :: Search Activity, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 34
People
(Reporter: Margaret, Assigned: wesj)
References
Details
Attachments
(5 files, 3 obsolete files)
Follow-up to bug 1035642.
See: https://bug1035642.bugzilla.mozilla.org/attachment.cgi?id=8465031
Comment 1•10 years ago
|
||
Attaching current "simple widget" design for reference and as a starting point.
Comment 2•10 years ago
|
||
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•10 years ago
|
||
(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•10 years ago
|
||
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.
Updated•10 years ago
|
status-firefox34:
--- → affected
Updated•10 years ago
|
status-firefox34:
affected → ---
Comment 5•10 years ago
|
||
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•10 years ago
|
||
(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 :)
Assignee | ||
Comment 7•10 years ago
|
||
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)
Assignee | ||
Comment 8•10 years ago
|
||
Screenshot of new theme. There's a build at http://people.mozilla.com/~wjohnston/widget.apk
Reporter | ||
Comment 9•10 years ago
|
||
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+
Comment 10•10 years ago
|
||
I still think those icons need strings, or I'd at least like to see what that would look like, to compare.
Assignee | ||
Comment 11•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
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.
Assignee | ||
Comment 14•10 years ago
|
||
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)
Assignee | ||
Comment 15•10 years ago
|
||
Attachment #8465043 -
Attachment is obsolete: true
Attachment #8467871 -
Attachment is obsolete: true
Attachment #8467891 -
Attachment is obsolete: true
Reporter | ||
Comment 16•10 years ago
|
||
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+
Comment 17•10 years ago
|
||
could we update the widget with our new icons? :)
Flags: needinfo?(wjohnston)
Assignee | ||
Comment 18•10 years ago
|
||
Added the new icons and landed:
https://hg.mozilla.org/integration/fx-team/rev/2ad9b9def7aa
Flags: needinfo?(wjohnston)
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → wjohnston
Priority: -- → P1
Comment 19•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Updated•7 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•