1055536 - Too many differing background greys throughout the browser

Status: VERIFIED FIXED

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

Description

[Aaron Train [:aaronmt]]

Attachment: screenshot.png

See screenshot.

With holo/material design in mind there should be some consistency here.

Comment 1

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

Hey Aaron,

Great catch :) I've got a color swatch library that I've been using myself to keep things visually consistent and I'm trying to establish it into a more concise guide. So, it just hasn't made it to these other pages yet.

This is also a part of Project Chameleon to unify these visuals with folks from Desktop, and OS.

That being said, feel free to file these bugs as a part of the meta bug we're using to track these visual/interactive changes - bug 1052004 :)

Comment 2

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

As a general note, the #CED7DE in the toolbar was changed to #EBEBF0 so that might be a good starting point if we can try that out. :)

Comment 3

[:Margaret Leibovic]

I think we should aim to address this as part of the toolbar changes.

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #3)
> I think we should aim to address this as part of the toolbar changes.

I agree. Taking.

Comment 5

[Lucas Rocha (:lucasr)]

antlam, we need a new version of these images with the new shade of grey:
https://dl.dropboxusercontent.com/u/1187037/menu-popup-bg.zip

Comment 6

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

Attachment: prev_menubg_shadow_compare.png

Here's a preview of the new 9 patches for this. I've also brought in the shadows a bit so the light sources doesn't feel so close to the menu. I feel like this matches up with the new toolbar a bit more and feels less fuzzy/ cramped. :)

9 patch files coming!

Comment 7

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

Attachment: menu-popup-bg-new.zip

Here you go! through in XXHDPI just in case / for the future :)

Comment 8

[Lucas Rocha (:lucasr)]

Attachment: Screenshot (tablets)

I'd liked the more pronounced shadow we had. This looks fine too. Up to you.

Comment 9

[Lucas Rocha (:lucasr)]

Attachment: Screenshot (phones)

We need to tweak the menu header (the one with the back/forward/refresh buttons) color too I think.

Comment 10

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

Comment on attachment 8476628 [details]
Screenshot (tablets)

Yeah , this doesn't come up as well when the menu is larger. Let me kick it up a bit and re-attach :)

Comment 11

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

Comment on attachment 8476631 [details]
Screenshot (phones)

Yeah that looks weird cause it's still blue-ish.

Also, whats with the clipping happening on the right hand side?

Comment 12

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

Attachment: menu-popup-bg-new2.zip

try these! :)

Comment 13

[Lucas Rocha (:lucasr)]

(In reply to Anthony Lam (:antlam) from comment #11)
> Comment on attachment 8476631 [details]
> Screenshot (phones)
> 
> Yeah that looks weird cause it's still blue-ish.

What color should we use then?

> Also, whats with the clipping happening on the right hand side?

The clipping is bug 1020505. Ignore it :-)

Comment 14

[Lucas Rocha (:lucasr)]

Hey Anthony, I noticed that the shadows in the images you sent are quite different for each density. Is that intended? More especifically, mdpi and hdpi have a much more pronounced shadow than xhdpi and xxhdpi.

Comment 15

[Lucas Rocha (:lucasr)]

Also, the assets you sent are not valid 9-patch images. Make sure the shadow pixels don't bleed into the outer frame where you draw the control pixels.

Comment 16

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

Attachment: menu-popup-bg-new3.zip

I figured out the sizing difference issue but because the shadow is so faint, I can't tell 100% if it's within the black "control pixels" but this should be good. Give these a try! :)

Comment 17

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

Attachment: prev_menu_greys2.png

Try this for the different grey? (#E1E1E6) I tried to find a color that both worked well with the current icon colors and was a derivative of the new toolbar grey. 

It might not be obvious enough but I also have another darker one in the palette we could use if that's the case (that doesn't match things like the current button dividers as much)

(attaching quick mockup I used to compare the colors)

Comment 18

[Lucas Rocha (:lucasr)]

I still think the shadow could be a bit more pronounced btw:
https://dl.dropboxusercontent.com/u/1187037/Screenshot_2014-08-26-13-55-06.png
https://dl.dropboxusercontent.com/u/1187037/2014-08-26%2013.56.10.png

Up to you.

Comment 19

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

Attachment: menu-popup-bg-new4.zip

I'd agree :) it's hard to see the full effects from my mocks unfortunately. So we might have to try this a couple more times.

I've made these more prominent, let me know what you think! Could I get a build to check these out too?

Comment 20

[Lucas Rocha (:lucasr)]

Looks good to me:
https://dl.dropboxusercontent.com/u/1187037/Screenshot_2014-08-27-14-02-06.png

APK: https://dl.dropboxusercontent.com/u/1187037/shadow.apk

Comment 21

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

This looks great.

I noticed the dividers are still a bit blue, how easy is it to change them to #D7D9DB ?

Comment 22

[Lucas Rocha (:lucasr)]

Attachment: Use consistent grey tone in overflow menu (r=margaret)

Comment 23

[Lucas Rocha (:lucasr)]

Comment on attachment 8480473 [details] [diff] [review]
Use consistent grey tone in overflow menu (r=margaret)

Checked all the divider, they seem look fine. The divider color change is very subtle anyway.

Comment 24

[:Margaret Leibovic]

Comment on attachment 8480473 [details] [diff] [review]
Use consistent grey tone in overflow menu (r=margaret)

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

This change looks fine, but I think we can make this better by factoring out these shared color values.

::: mobile/android/base/resources/drawable/divider_horizontal.xml
@@ +5,5 @@
>  
>  <shape xmlns:android="http://schemas.android.com/apk/res/android"
>         android:shape="rectangle">
>  
> +    <solid android:color="#D7D9DB"/>

I think we should pull this value into a generic color resource, maybe something like @color/divider_grey.

That way it's obvious that we're using a single color consistently, and we can easily use it in any new views we add.

Comment 25

[Lucas Rocha (:lucasr)]

Attachment: Consolidate divider colors (r=margaret)

Comment 26

[:Margaret Leibovic]

Comment on attachment 8481223 [details] [diff] [review]
Consolidate divider colors (r=margaret)

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

Nice.

Comment 27

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/d4527b684366
https://hg.mozilla.org/integration/fx-team/rev/e18ff990cb6d

Comment 28

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/d4527b684366
https://hg.mozilla.org/mozilla-central/rev/e18ff990cb6d

Comment 29

[Lucas Rocha (:lucasr)]

Comment on attachment 8480473 [details] [diff] [review]
Use consistent grey tone in overflow menu (r=margaret)

Approval Request Comment
[Feature/regressing bug #]: bug 1052004
[User impact if declined]: Inconsistent color in the UI. Make us look a bit amateur.
[Describe test coverage new/current, TBPL]: Local testing only. Let's just confirm it looks good in the next Nightly and uplift.
[Risks and why]: Low. Simply tweaks an image and some color values.
[String/UUID change made/needed]: n/a

Comment 30

[Lucas Rocha (:lucasr)]

Comment on attachment 8481223 [details] [diff] [review]
Consolidate divider colors (r=margaret)

Approval Request Comment
[Feature/regressing bug #]: bug 1052004
[User impact if declined]: Inconsistent color in the UI. Make us look a bit amateur.
[Describe test coverage new/current, TBPL]: Local testing only. Let's just confirm it looks good in the next Nightly and uplift.
[Risks and why]: Low. Simply tweaks an image and some color values.
[String/UUID change made/needed]: n/a

Comment 31

[Lawrence Mandel [:lmandel] (use needinfo)]

Lucas - Please comment once you've confirmed on Nightly and I'll approve the patches for uplift.

Comment 32

[Lucas Rocha (:lucasr)]

(In reply to Lawrence Mandel [:lmandel] from comment #31)
> Lucas - Please comment once you've confirmed on Nightly and I'll approve the
> patches for uplift.

Yeah, looking fine in Nightly.

Comment 33

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8480473 [details] [diff] [review]
Use consistent grey tone in overflow menu (r=margaret)

Aurora+

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/17a7d908475b
https://hg.mozilla.org/releases/mozilla-aurora/rev/ad6d9cc71ddf

Comment 35

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

NI-ing Lucas to check on the dividers blu-ness later

Comment 36

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

Anthony, it seems Lucas checked on the dividers in comment 23 - does something still appear off?

Comment 37

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

Maybe it's just these old eyes... 

I also just checked the dividers in our about: pages and it looks like they're #D7D9DB and I assume they're the same everywhere so it's all good :)

Comment 38

[Ioana Chiorean]

Attachment: Screenshot_2014-12-30-15-32-27.png

All same grey 
Nexus 5 ( Android 5.0.1) Fennec 35 Beta 8 build 1
ZTE Open ( Android 4.0.4) Fennec 35 Beta 8 build 1

Comment 39

[Ioana Chiorean]

Nexus 5 ( Android 5.0.1) and ZTE Open ( Android 4.0.4) 
Fennec 35 Beta 8 build 1
Aurora 36 ( 2014-12-29)
Nightly 37 ( 2014-12-30)

I can only see one type of grey - the blue-ish is not present anymore.