1148550 - General menu visual improvements & change to material 3-dot menu/add-tab icons

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

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

Breaking this up from the Meta. 

Basically, right now it doesn't look like it's the right color. Is it just me? Pressed state should be "Toolbar grey pressed" (#D7D7DC).

Comment 1

[Paul [pwd]]

Google states within in the Material Design guidelines that buttons in their pressed state should be 20% #999999

http://www.google.co.uk/design/spec/components/buttons.html#buttons-flat-raised-buttons

Comment 2

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

(In reply to Paul [pwd] from comment #1)
> Google states within in the Material Design guidelines that buttons in their
> pressed state should be 20% #999999
> 
> http://www.google.co.uk/design/spec/components/buttons.html#buttons-flat-
> raised-buttons

Thanks Paul. I am aware of what Google is doing in Material design. Their guidelines are pretty well done and documented. But, these are our colors that are a part of our guidelines. Rest assured, we aren't choosing these arbitrarily/ on a whim either. :)

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Going with the 3-dot menu from bug 1183601 and the visual improvements from bug 1140169:

(In reply to Anthony Lam (:antlam) from bug 1140169 comment #0)
> Namely:
>  - Pressed state should be "Toolbar grey pressed" (#D7D7DC)
...
>  - Add 15dp left padding to labels (I don't think I ever specified this
> value anywhere but 15dp would keep it consistent in other areas of our UI
> that are close to "edges")
>  - Dividers in between each row seem to be (#C9D3DC), we should update this
> to (#D7D9DB) like our toolbar divider.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I think the divider color was already changed [1] – there's additionally no reference to #C9D3DC [2].

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/styles.xml?rev=c6ebcbe6189e#84
[2]: https://mxr.mozilla.org/mozilla-central/search?string=c9d3dc&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot post-patch

Anthony, what do you think?

Comment 6

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

Comment on attachment 8650771 [details]
Screenshot post-patch

MUCH better! Like music to my... eyes!

Is the font color for disabled text (like Find in Page and Page) here consistent with our disabled grey?

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot post-patch (tablet)

Comment 8

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

Comment on attachment 8651326 [details]
Screenshot post-patch (tablet)

Material menu overflow icon? looks good!

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Tabs panel

Tabs panel looks a bit goofy to me – the button needs to be bigger.

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Tabs panel (non-adjusted)

Here's the menu button in the tabs panel if I don't constrain the height.

Comment 11

[Paul [pwd]]

The non-adjusted isn't the most aesthetically pleasing. It's hard for anything to look good next to such a cross as the new tab button though.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Paul [pwd] from comment #11)
> The non-adjusted isn't the most aesthetically pleasing. It's hard for
> anything to look good next to such a cross as the new tab button though.

Google's Material add button [1] looks fairly similar – perhaps it's just a size issue? Anthony, perhaps you have some new size specs?

[1]: https://www.google.com/design/icons/#ic_add

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Anthony Lam (:antlam) from comment #6)
> Is the font color for disabled text (like Find in Page and Page) here
> consistent with our disabled grey?

I've been spending a bit too long on this so I filed bug 1198050.

Comment 14

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

Comment on attachment 8652003 [details]
Tabs panel

Thanks for posting these, I appreciate it!

I think you're right. I designed a + and - icon to use originally in the tablet redesign. 

I'll attach them here.

Comment 15

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

Attachment: icon_add.zip

I think we should update our + icons now. Give this a try and we can file another bug if we want to go through and swap it in all parts of our UI too (I think I filed that bug somewhere before)

Comment 16

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

As for sizing/ dimensions: the "+" is 18dp, then it's centered inside a square container that is created by the action bar height of 48 dp.

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Add tab asset (phone)

Note that we also need assets for private browsing mode.

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Add tab asset in tab strip (tablet)

Turns out the same asset is used in the tabs strip (though I can fork the assets and not change it for an APK size increase).

Comment 19

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

Comment on attachment 8652415 [details]
Add tab asset (phone)

Looks good!

RE: Private browsing asset - I think we should unify here and just use a single "add" icon. 

The small mask is kinda hard to see and especially with the latest UI changes we did in bug 864958, it's pretty obvious that you're in private browsing mode, and thus, will be adding a private browsing tab.

Comment 20

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

Comment on attachment 8652416 [details]
Add tab asset in tab strip (tablet)

Awesome!

Comment 21

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Change menu padding left to 15dp. r=mhaigh

Bug 1148550 - Change menu padding left to 15dp. r=mhaigh

Comment 22

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Update GeckoMenu pressed color. r=mhaigh

Bug 1148550 - Update GeckoMenu pressed color. r=mhaigh

This also involved moving a redundant background declaration to only the
containing style.

Comment 23

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Add new Material 3-dot menu assets. r=mhaigh

Bug 1148550 - Add new Material 3-dot menu assets. r=mhaigh

And replace the older assets.

These are the 36dp icons from Google's material design page:
  https://www.google.com/design/icons/#ic_more_vert

Then we trim off the whitespace with image magick:
  convert icon.png -trim out.png

And compress with ImageOptim.

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Update 3-dot menu icon to Material on toolbar. r=mhaigh

Bug 1148550 - Update 3-dot menu icon to Material on toolbar. r=mhaigh

Other 3-dot menu button locations to follow.

I tested that the colors are correct in the various states (e.g. private
browsing).

This patch additionally:
* Restructured the menu button (which allowed the removal of a setVisibility
  call
* Removed the now unused tablet assets.

Comment 25

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Update 3-dot menu icon to Material in tabs panel. r=mhaigh

Bug 1148550 - Update 3-dot menu icon to Material in tabs panel. r=mhaigh

Comment 26

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Replace new tab asset with Material one. r=mhaigh

Bug 1148550 - Replace new tab asset with Material one. r=mhaigh

Comment 27

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1148550 - Remove add_tab private browsing and dark assets. r=mhaigh

Bug 1148550 - Remove add_tab private browsing and dark assets. r=mhaigh

Replace dark assets with tinting.

Comment 28

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652603 [details]
MozReview Request: Bug 1148550 - Change menu padding left to 15dp. r=mhaigh

https://reviewboard.mozilla.org/r/17221/#review16001

Comment 29

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652604 [details]
MozReview Request: Bug 1148550 - Update GeckoMenu pressed color. r=mhaigh

https://reviewboard.mozilla.org/r/17223/#review16005

Comment 30

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652605 [details]
MozReview Request: Bug 1148550 - Add new Material 3-dot menu assets. r=mhaigh

https://reviewboard.mozilla.org/r/17225/#review16007

Comment 31

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652606 [details]
MozReview Request: Bug 1148550 - Update 3-dot menu icon to Material on toolbar. r=mhaigh

https://reviewboard.mozilla.org/r/17227/#review16011

Comment 32

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652607 [details]
MozReview Request: Bug 1148550 - Update 3-dot menu icon to Material in tabs panel. r=mhaigh

https://reviewboard.mozilla.org/r/17229/#review16013

Comment 33

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652608 [details]
MozReview Request: Bug 1148550 - Replace new tab asset with Material one. r=mhaigh

https://reviewboard.mozilla.org/r/17231/#review16015

Comment 34

[Martyn Haigh (:mhaigh)]

Comment on attachment 8652609 [details]
MozReview Request: Bug 1148550 - Remove add_tab private browsing and dark assets. r=mhaigh

https://reviewboard.mozilla.org/r/17233/#review16017

::: mobile/android/base/resources/color-large-v11/tab_new_tab_strip_colors.xml:1
(Diff revision 1)
> +<?xml version="1.0" encoding="utf-8"?>

Is this file used anywhere?

Comment 35

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/17233/#review16017

> Is this file used anywhere?

Yes, in `layout-large-v11/tab_strip_inner.xml`.

Comment 36

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/f546981ba4dc
https://hg.mozilla.org/integration/fx-team/rev/22580b5776c2
https://hg.mozilla.org/integration/fx-team/rev/0cc28c71117e
https://hg.mozilla.org/integration/fx-team/rev/66f081a2d7d9
https://hg.mozilla.org/integration/fx-team/rev/f4080d494472
https://hg.mozilla.org/integration/fx-team/rev/eb04d3293fed
https://hg.mozilla.org/integration/fx-team/rev/bcccb8af005d

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f546981ba4dc
https://hg.mozilla.org/mozilla-central/rev/22580b5776c2
https://hg.mozilla.org/mozilla-central/rev/0cc28c71117e
https://hg.mozilla.org/mozilla-central/rev/66f081a2d7d9
https://hg.mozilla.org/mozilla-central/rev/f4080d494472
https://hg.mozilla.org/mozilla-central/rev/eb04d3293fed
https://hg.mozilla.org/mozilla-central/rev/bcccb8af005d

Comment 38

[Barbara Bermes [:barbara] - NI please!]

can we combine this with https://bugzilla.mozilla.org/show_bug.cgi?id=1140048 and ship in 44?

Comment 39

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Barbara Bermes [:bbermes] from comment #38)
> can we combine this with
> https://bugzilla.mozilla.org/show_bug.cgi?id=1140048 and ship in 44?

We clarified this via email – we're shipping this bug in 43 (we try not to hold back work) and tracking bug 1140048 in 44.

Comment 40

[Paul [pwd]]

The new icons are beyond awesome. For something seemingly so small, they add so much polish to Fennec. Great work :antlam and Michael.

Comment 41

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Release Note Request (optional, but appreciated)
[Why is this notable]: The UI started to look dated (imo) so we added a little bit of Google's Material design, the latest Android UI design. In particular, we rounded the 3-dot menu icon and adjusted the menu's padding.

[Suggested wording]: Updated toolbar menu with Material design conventions

[Links (documentation, blog post, etc)]: N/A

Comment 42

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot from 2015-09-24 14:42:41.png

Tested using:
Device: LG G4 (Android 5.1)
Build: Firefox for Android 43.0a2 (2015-09-22)

Comment 43

[Liz Henry (:lizzard) (relman/hg->git project)]

Added to release notes for 43.