769237 - (ICS) Move overflow menu back to right edge.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ian Barlow (:ibarlow)]

Attachment: Mockup

This is the bug to track the work involved in returning the Android menu button to the far right corner, similar to what exists in our GA release today. 

After a couple of weeks of playing with the new custom placement, Madhava and I both agree that while from a design standpoint it makes sense to group the menu button with the url bar, and for it to travel down the screen when the tabs tray is open, it is simply not as usable. It's harder to hit consistently, and it also pushes us just a bit past the "enhanced design that is still Android-like" feeling that we want to portray. 

A mockup is attached, and Sriram and I are going to try to hammer this out shortly.

Comment 2

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch (1/4): Move the menu button to right

This patch moves the menu button to the right.
"layout-v14/" takes care of this -- only for ICS devices (they are the ones that don't have h/w menu button).
"drawable-xhdpi-v14" takes care of the resources.

As a part of this change, the url bar is split into a curvy portion and a repeating portion. This is now supplied by two views in the background (curve, and background). The tab is aligned to the right of it.
Note: There might be a small visual glitch where these two meet in hdpi devices. I need Ian's help to cut it properly. This change might not land "before beta cutoff".

If's and But's:
# This currently supports only XHDPI ICS devices. If the support is needed for HDPI ICS devices (which may pop up in the market) we need more resources dumped into drawable-hdpi-v14 and drawable-mdpi-v14 folders.
# Devices like Samsung S3 or Galaxy Note (though XHDPI) have a h/w menu button. We may need to look into renaming resources, and loading different layout files based on those. (gecko_app.xml and gecko_app_menu.xml).
# Already there is a problem with Galaxy Note! All our resources are mentioned as "drawable-xhdpi-v11" -- which satisfies only honeycomb+. Note is a XHDPI device running Gingerbread. Though we have all resources, Note is not supplied with the proper resources. We need some cleanup there.
# HDPI devices like Nexus S running ICS are unaffected by this change. But, when we support "drawable-hdpi-v14", we should take special care for those. #2 holds there.
# Tablets -- though have address_bar_bg_curve file added to them, they don't currently use them. I will have to wait for Ian's newer resources to make that change there. (or probably remove them). They are added for the sake of completion (and not to forget).

Probable Followup:
Splitting drawable-xhdpi-v11 into drawable-xhpi and drawable-xhdpi-v11 -- to be consistent with mdpi and hdpi. This will help support Note properly. However, I'm not sure if we have all resources for Gingerbread based XHDPI device -- say the menu items!
I'm not sure if we want this change "before beta cutoff" (and, I'm not pretty confident about it). But sometime in the future, we surely need this.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch (2/4): Remove tab carats

In the expanded state, the tabs counter isn't shown, and the tab carats aren't needed there. Hence removing the resources only for phones.

Also, renaming tabs_expanded_normal.png to tabs_button_expanded.png -- as there is not "pressed" state when expanded on phones.

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch (3/4): Menu arrow is fixed

The overflow menu button is at the rightmost corner on both phones and tablets. Hence, it doesn't make sense to keep calculating the position. This patch removes calculations and provides a fixed position in XML.

Comment 5

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch (4/4): New plus and synced-tab icons

The "plus" and synced tab icons weren't centered in its space. This was ugly when placed near the menu button. This has been changed to be centered -- hence just replacement of resources.

Comment 6

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7c2897ce1b53
http://hg.mozilla.org/integration/mozilla-inbound/rev/ce6955c27e17
http://hg.mozilla.org/integration/mozilla-inbound/rev/7564ef1532bd
http://hg.mozilla.org/integration/mozilla-inbound/rev/35ea8db0ca39

Comment 7

[Sriram Ramasubramanian [:sriram]]

Had missed a | hg remove |:
http://hg.mozilla.org/integration/mozilla-inbound/rev/24a3959ab893

Comment 8

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch (5/5): Remove expanded tabs button

While debugging why the curve doesnt fit properly, I found that Ian ( :( :( :( ) had given me resources of different sizes between testing and final set! :(
I have made sure the layout uses the same.
Also, the curve is supported by the url, which leaves the tabs not to provide the same again. Hence removed the expanded state for tabs button in phones.

Comment 9

[Sriram Ramasubramanian [:sriram]]

Patch (5/5) also has a replacement for one image -- as the one that was pushed, was from testing resources, and it's now replaced with the newer sized image.

Comment 10

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/4248dbd85972

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7c2897ce1b53
https://hg.mozilla.org/mozilla-central/rev/ce6955c27e17
https://hg.mozilla.org/mozilla-central/rev/7564ef1532bd
https://hg.mozilla.org/mozilla-central/rev/35ea8db0ca39
https://hg.mozilla.org/mozilla-central/rev/24a3959ab893
https://hg.mozilla.org/mozilla-central/rev/4248dbd85972

Comment 12

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 640107 [details] [diff] [review]
Patch (1/4): Move the menu button to right

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI change.
User impact if declined: Menu button won't be consistent with other android apps.
Testing completed (on m-c, etc.): Landed on m-c on 07/09
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

Comment 13

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 640108 [details] [diff] [review]
Patch (2/4): Remove tab carats

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI change.
User impact if declined: Menu button won't be consistent with other android apps.
Testing completed (on m-c, etc.): Landed on m-c on 07/09
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

Comment 14

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 640109 [details] [diff] [review]
Patch (3/4): Menu arrow is fixed

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI change.
User impact if declined: Menu button won't be consistent with other android apps.
Testing completed (on m-c, etc.): Landed on m-c on 07/09
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

Comment 15

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 640110 [details] [diff] [review]
Patch (4/4): New plus and synced-tab icons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI change.
User impact if declined: Menu button won't be consistent with other android apps.
Testing completed (on m-c, etc.): Landed on m-c on 07/09
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

Comment 16

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 640394 [details] [diff] [review]
Patch (5/5): Remove expanded tabs button

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI change.
User impact if declined: Menu button won't be consistent with other android apps.
Testing completed (on m-c, etc.): Landed on m-c on 07/09
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

Comment 17

[Thomas Stache]

Attachment: Broken Tab/Menu button rendering in 2012-07-11 nightly, HTC One V

(In reply to Sriram Ramasubramanian [:sriram] from comment #2)
> If's and But's:
> # This currently supports only XHDPI ICS devices. If the support is needed
> for HDPI ICS devices (which may pop up in the market) we need more resources
> dumped into drawable-hdpi-v14 and drawable-mdpi-v14 folders.

Hmm, does that refer to 800x480px screens? Then these devices are already in the market, e.g. the HTC One V, maybe the Galaxy S2 with its ICS update. Nightly build 2012-07-11 renders badly on the One V. Was there a follow-up bug filed for the lower resolution assets?

Comment 18

[Dão Gottwald [:dao]]

(In reply to Thomas Stache from comment #17)
> Created attachment 641400 [details]
> Broken Tab/Menu button rendering in 2012-07-11 nightly, HTC One V
> 
> (In reply to Sriram Ramasubramanian [:sriram] from comment #2)
> > If's and But's:
> > # This currently supports only XHDPI ICS devices. If the support is needed
> > for HDPI ICS devices (which may pop up in the market) we need more resources
> > dumped into drawable-hdpi-v14 and drawable-mdpi-v14 folders.
> 
> Hmm, does that refer to 800x480px screens? Then these devices are already in
> the market, e.g. the HTC One V, maybe the Galaxy S2 with its ICS update.
> Nightly build 2012-07-11 renders badly on the One V. Was there a follow-up
> bug filed for the lower resolution assets?

I can confirm this for the Galaxy S2. This certainly can't be uplifted to Aurora in this form.

Comment 19

[Sriram Ramasubramanian [:sriram]]

(In reply to Dão Gottwald [:dao] from comment #18)
> (In reply to Thomas Stache from comment #17)
> > Created attachment 641400 [details]
> > Broken Tab/Menu button rendering in 2012-07-11 nightly, HTC One V
> > 
> > (In reply to Sriram Ramasubramanian [:sriram] from comment #2)
> > > If's and But's:
> > > # This currently supports only XHDPI ICS devices. If the support is needed
> > > for HDPI ICS devices (which may pop up in the market) we need more resources
> > > dumped into drawable-hdpi-v14 and drawable-mdpi-v14 folders.
> > 
> > Hmm, does that refer to 800x480px screens? Then these devices are already in
> > the market, e.g. the HTC One V, maybe the Galaxy S2 with its ICS update.
> > Nightly build 2012-07-11 renders badly on the One V. Was there a follow-up
> > bug filed for the lower resolution assets?
> 
> I can confirm this for the Galaxy S2. This certainly can't be uplifted to
> Aurora in this form.

This is tracked by bug 772684. I wouldn't be uplifting the patches here (on approval) until that gets approved too.

Comment 20

[Dão Gottwald [:dao]]

You know, we have a dependency system to track these kind of things :)

Comment 21

[Alex Keybl [:akeybl]]

Along with these aurora approvals, I've also pre-approved bug 772684 for landing on Aurora 15 (unorthodox, I know, but we're very close to merge).

Comment 22

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8fd1c9d8d8ce
https://hg.mozilla.org/releases/mozilla-aurora/rev/6e7ca3d28efe
https://hg.mozilla.org/releases/mozilla-aurora/rev/224e24ad8dcf
https://hg.mozilla.org/releases/mozilla-aurora/rev/3c3b194d733a
https://hg.mozilla.org/releases/mozilla-aurora/rev/6351a93db01e
https://hg.mozilla.org/releases/mozilla-aurora/rev/69227eac6748