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

VERIFIED FIXED in Firefox 36

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: u421692, Assigned: mhaigh)

Tracking

(Blocks 1 bug)

Trunk
Firefox 38
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 verified, firefox37 verified, firefox38 verified)

Details

Attachments

(7 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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?
v1, if we have the cycles.
Blocks: new-tablet-v1
No longer blocks: new-toolbar-v1
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.
Flags: needinfo?(michael.l.comella)
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
Flags: needinfo?(michael.l.comella)
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?
Flags: needinfo?(alam)
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.
Solution #1! Anthony, what do you think?
Attachment #8543129 - Flags: feedback?(alam)
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.
If we go with #1, I think it's a pretty easy win from this point. NI mhaigh to see this through.
Flags: needinfo?(mhaigh)
(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.
Flags: needinfo?(alam)
Comment on attachment 8543129 [details]
Private browsing action mode toolbar after patch

Color looks right !
Attachment #8543129 - Flags: feedback?(alam) → feedback+
(Assignee)

Comment 14

4 years ago
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.
Flags: needinfo?(mhaigh)
Attachment #8546589 - Flags: review?(wjohnston)
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.
Attachment #8546589 - Flags: review?(wjohnston) → review+
(Assignee)

Comment 17

4 years ago
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
Attachment #8546589 - Attachment is obsolete: true
Attachment #8550259 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/b1424a861cca
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Martin, any reason why you didn't request the uplift to aurora?
Assignee: nobody → mhaigh
Flags: needinfo?(mhaigh)
Verified as fixed in Firefox for Android 38.0a1 (2015-01-18);
Device: Asus Transformer Pad TF300T (Android 4.2.1).
(Assignee)

Comment 21

4 years ago
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.
Flags: needinfo?(mhaigh)
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?
Flags: needinfo?(mhaigh)
(Assignee)

Comment 23

4 years ago
Ah, yeah.  I didn't realise I had to uploft to aurora and beta!
Flags: needinfo?(mhaigh)
(Assignee)

Comment 24

4 years ago
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
Attachment #8550259 - Flags: approval-mozilla-aurora?
Comment on attachment 8550259 [details] [diff] [review]
Make action bar background color  consistent with the new tablet tab strip background

[Triage Comment]
Attachment #8550259 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8550259 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.