1111598 - [Tablet] Make action bar background color consistent with the new tablet tab strip background

Status: VERIFIED FIXED

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

Description

[u421692]

Attachment: device-2014-12-15-155426.png

The color is slightly different between the color of the action bar and the color of the tab in the new tablet UI(see attached screenshot)

http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/actionbar.xml#18  is this the line that has to be changed?

Comment 1

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

v1, if we have the cycles.

Comment 2

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

Attachment: Chrome text selection

If we change the color of the action bar, we may be going against Android convention.

However, we may already be drawing the action bar in the incorrect position, against Android convention. Logically, the design of the action bar appears to have a colored line at the bottom to divide the action bar from the application content (e.g. see the attached Chrome screenshot).

Note that in chrome, the action bar is larger than the tab strip and thus the toolbar animates downwards when the action bar is shown.

---

The color fix is certainly an easier option for 36, however.

Comment 3

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

NI to investigate.

Comment 4

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

According to the docs, both the action bar and the contextual action mode bar should be at the top of the screen:

[1] Beginning with Android 3.0 (API level 11), the action bar appears at the top of an activity's window...

[2] (From Using the contextual action mode) The contextual action mode is a system implementation of ActionMode that focuses user interaction toward performing contextual actions. When a user enables this mode by selecting an item, a contextual action bar appears at the top of the screen to present actions the user can perform on the currently selected item(s).

---

So we're breaking Android convention, but that being said, we don't have to follow it.

[1]: https://developer.android.com/reference/android/support/v7/app/ActionBar.html
[2]: https://developer.android.com/guide/topics/ui/menus.html#context-menu

Comment 5

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

Attachment: Private browsing action mode enabled

Private browsing looks fine.

Comment 6

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

There are two solutions to this bug:

1) Change the background color of the action bar action mode to match the tab color (or to be so different from tab color that it doesn't clash), as per comment 0
2) Move the action bar action mode to the top of the screen, ala Chrome

Note that implementing #1 for 36 and then implementing #2 later is an option.

I personally think just #1 would be good (and bonus: it simplifies the interactions - you can't use the menu and other toolbar actions).

Anthony, are both solutions acceptable and is there one you find ideal? Do you think #2 is necessary?

Comment 7

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

Attachment: (WIP) Change action bar action mode background color to match tab background on tablet

Here's a patch to change the background color - sample images to follow.

This patch will need some work - the assets determine the background color so
they're named poorly (lolol). Also, if we change change the background color,
we should do it on both phone and tablet to avoid the extra resource
consumption (filed bug 1116915 to attempt to be more flexible without assets).

Note: new assets are trimaged.

Comment 8

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

Attachment: ActionBar action mode toolbar after patch

Comment 9

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

Attachment: Private browsing action mode toolbar after patch

Solution #1! Anthony, what do you think?

Comment 10

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

By the way, I ran the following command (using Image Magick <3)from m/a/b/resources/ to create the new assets:

for i in {m,h,xh}; do pushd drawable-${i}dpi/ && convert ab_stacked_transparent_light_holo.9.png -fill "#EBEBF0" -opaque "#ECF0F3"  lolol.9.png; popd; done

The new color, EBEBF0, is background_normal, which is used for the toolbar tabs.

Comment 11

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

If we go with #1, I think it's a pretty easy win from this point. NI mhaigh to see this through.

Comment 12

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

(In reply to Michael Comella (:mcomella) [PTO until 1/14] from comment #6)
> There are two solutions to this bug:
> 
> 1) Change the background color of the action bar action mode to match the
> tab color (or to be so different from tab color that it doesn't clash), as
> per comment 0
> 2) Move the action bar action mode to the top of the screen, ala Chrome
> 
> Note that implementing #1 for 36 and then implementing #2 later is an option.
> 

I think we should do that (Implement #1 and then #2). The top position for this is best cause it offers a way to escape the Action Bar (by tapping the Browser chrome) while not having to deal with issues around persistence (i.e. "shouldn't the Action Bar stay there if I navigate away from tabs and then back?"). I think being more consistent with the OS in this case is the simpler, better option.

> I personally think just #1 would be good (and bonus: it simplifies the
> interactions - you can't use the menu and other toolbar actions).

> Anthony, are both solutions acceptable and is there one you find ideal? Do
> you think #2 is necessary?

Yeah that's true. But let's keep the Action Bar at the top :) I think exposing the URL and such is better than show the tabs on top.

Comment 13

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

Comment on attachment 8543129 [details]
Private browsing action mode toolbar after patch

Color looks right !

Comment 14

[Martyn Haigh (:mhaigh)]

Attachment: 0001-Bug-1111598-Tablet-Make-action-bar-background-color-.patch

Using the assets provided by mcomella, I've renamed these to ab_background, changed all references (phone + tablet) to use this new asset and removed the old assets.

Comment 15

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

Comment on attachment 8546589 [details] [diff] [review]
0001-Bug-1111598-Tablet-Make-action-bar-background-color-.patch

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

Drive-by: lgtm.

Comment 16

[Martyn Haigh (:mhaigh)]

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

Comment 17

[Martyn Haigh (:mhaigh)]

Attachment: Make action bar background color consistent with the new tablet tab strip background

Approval Request Comment
[Feature/regressing bug #]:
Change background of action bar to match tabs so there is no visual disparity between the two when shown.

[User impact if declined]:
Action bar and tabs will be slightly different colours

[Describe test coverage new/current, TBPL]:
None

[Risks and why]: 
Actionbar background on phone and tablet may be wrong as we update both in this patch.

[String/UUID change made/needed]:
N/A

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b1424a861cca

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Martin, any reason why you didn't request the uplift to aurora?

Comment 20

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in Firefox for Android 38.0a1 (2015-01-18);
Device: Asus Transformer Pad TF300T (Android 4.2.1).

Comment 21

[Martyn Haigh (:mhaigh)]

Uplifting to beta as the rest of the new tablet work is in beta and we want to get this in to the same release.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

My question is about 37, currently in aurora. You requested the uplift only to beta and not aurora. I guess aurora/37 is affected too, right?

Comment 23

[Martyn Haigh (:mhaigh)]

Ah, yeah.  I didn't realise I had to uploft to aurora and beta!

Comment 24

[Martyn Haigh (:mhaigh)]

Comment on attachment 8550259 [details] [diff] [review]
Make action bar background color  consistent with the new tablet tab strip background

Requesting uplift to Aurora too as this was missed out before!

Approval Request Comment
[Feature/regressing bug #]:
Change background of action bar to match tabs so there is no visual disparity between the two when shown.

[User impact if declined]:
Action bar and tabs will be slightly different colours

[Describe test coverage new/current, TBPL]:
None

[Risks and why]: 
Actionbar background on phone and tablet may be wrong as we update both in this patch.

[String/UUID change made/needed]:
N/A

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8550259 [details] [diff] [review]
Make action bar background color  consistent with the new tablet tab strip background

[Triage Comment]

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a411a1aaae5
https://hg.mozilla.org/releases/mozilla-beta/rev/3e58a43384cd

Comment 27

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in builds:
Firefox for Android 37.0a2 (2015-01-21);
Firefox for Android 36 Beta 2;
Device: Asus Transformer Pad TF300T (Android 4.2.1).