Closed Bug 1137333 Opened 6 years ago Closed 5 years ago

Clean up UI of "Clear browsing history" button

Categories

(Firefox for Android :: Awesomescreen, defect)

x86
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: antlam, Assigned: vivek, Mentored)

References

Details

(Whiteboard: [lang=xml])

Attachments

(3 files)

Attached image prev_clearhistory1.png
I noticed that our "Clear browsing history" button in the History panel need some visual love.

Basic clean up to UI, namely... 
 - using the "Toolbar grey" for default state, 
 - "Toolbar grey pressed" for the pressed state, 
 - "Text and Tabs tray grey" for the Text ( Roboto Light (14 sp), 
 - adding a divider above the button thats #D7D9DB at 1 dp,
 - and increasing it's size to be 48 dp (matching the toolbar size)
This button lives here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/home_history_panel.xml#17

Cleaning up the style should just involve updating the style for this button:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/styles.xml#266

We have this Widget.Home.ActionButtton style which extends Widget.Home.PageButton, and it looks like both of those styles are only used for this button, so I think we should consider combining those styles.
Mentor: michael.l.comella, margaret.leibovic
Whiteboard: [lang=xml]
I am interested in working on this bug.
Would be great if you could please let me know how could I get started? Thanks!
Flags: needinfo?(alam)
Awesome! Thanks Ruchita. I am going to NI Margaret here as she could better help with that question.
Flags: needinfo?(alam) → needinfo?(margaret.leibovic)
Thanks Anthony!

Margaret, I saw that you already posted some initial information regarding the visual design for this button. Thanks for that!

Are there any additional features that you need besides the ones listed Anthony listed above?

Also would be great if you could please point me to resources on how I can go to pull repo and start working on this bug? Thanks!
Thanks Anthony!

Margaret, I saw that you already posted some initial information regarding the visual design for this button. Thanks for that!

Are there any additional features that you need besides the ones listed Anthony listed above?

Also would be great if you could please point me to resources on how I can go to pull repo and start working on this bug? Thanks!
Hi ruchita! You'll want to start by following the instructions here to get a build environment set up:
https://wiki.mozilla.org/Mobile/Fennec/Android

You also should join #mobile on irc.mozilla.org to ask any questions that might come up while setting up your build environment.

For this bug, we should focus on what antlam listed in comment 0, and there are plenty of other bugs out there where we can work on more features :)
Assignee: nobody → ruchita
Flags: needinfo?(margaret.leibovic)
Thanks for the info Margaret. 
I am working towards setting the build environment setup on my machine and will start working on this bug right away.
(In reply to ruchita from comment #7)
> Thanks for the info Margaret. 
> I am working towards setting the build environment setup on my machine and
> will start working on this bug right away.

Awesome! Thanks ruchita!
Bug 1137333 Clear history button style changes r?margaret
Attachment #8668120 - Flags: review?(margaret.leibovic)
Assignee: ruchita → vivekb.balakrishnan
Status: NEW → ASSIGNED
Comment on attachment 8668120 [details]
MozReview Request: Bug 1137333 Clear history button style changes r?margaret

https://reviewboard.mozilla.org/r/20889/#review18839

Thanks for picking this up. I just have a few comments, but you should make sure to post a screenshot to get UI review from antlam before landing this.

::: mobile/android/base/resources/values/colors.xml:121
(Diff revision 1)
> +  <color name="home_history_clear_button_top_divider">#D7D9DB</color>

Is this a color we use anywhere else? We've been trying to move towards using colors from a standard color palette as much as possible. If this is a color in the palette, we should just use the existing color, rather than creating a new one.

::: mobile/android/base/resources/values/styles.xml:264
(Diff revision 1)
> -        <item name="android:textAppearance">@style/TextAppearance.Widget.Home.PageTitle</item>
> +        <item name="android:textColor">@color/tabs_tray_icon_grey</item>

It looks like this was the only place this style was used, so you could remove it:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/styles.xml#412
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values-v16/styles.xml#16

::: mobile/android/base/resources/values/styles.xml
(Diff revision 1)
> -        <item name="android:textAppearance">@style/TextAppearance.Widget.Home.PageAction</item>

It looks like this was the only place this style was used, so you could remove it:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/styles.xml#414
Attachment #8668120 - Flags: review?(margaret.leibovic) → review+
@antlam

As per Margaret's comment on color palette, I find that we have divider_light defined as FFD7D9DB. This patch uses that.

Please let me know if any other changes are required
Attachment #8668510 - Flags: review?(alam)
Comment on attachment 8668510 [details]
history_clear_button.png

Thanks Vivek!

Spoke on IRC, seems like the type is still not to spec. Vivek will update :)
Attachment #8668510 - Flags: review?(alam) → review-
Comment on attachment 8668120 [details]
MozReview Request: Bug 1137333 Clear history button style changes r?margaret

Bug 1137333 Clear history button style changes r?margaret
New changes:
Fixed the specs according to comment #1
Removed unused styles

@antlam: 
Please find the latest mocks at
https://www.dropbox.com/home/Bug_Attachments/mocks

All caps seems to be a material design spec.
https://www.google.com/design/spec/style/typography.html#typography-styles
The above document mentions that.
Flags: needinfo?(alam)
Hey Vivek!

Thanks for doing this work. 

Unfortunately, I can't get access to that link you posted so I'm just going to assume that it's the same as when we last spoke on IRC (minutes ago). :)

so, WFM!
Flags: needinfo?(alam)
This landed but unfortunately the flags never got updated.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.