(ICS) Move overflow menu back to right edge.

VERIFIED FIXED in Firefox 15

Status

()

Firefox for Android
General
VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: ibarlow, Assigned: sriram)

Tracking

15 Branch
Firefox 16
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox15 verified, firefox16 verified, firefox17 verified)

Details

Attachments

(7 attachments)

(Reporter)

Description

6 years ago
Created attachment 637492 [details]
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.
(Reporter)

Updated

6 years ago
Duplicate of this bug: 764339
(Assignee)

Comment 2

6 years ago
Created attachment 640107 [details] [diff] [review]
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.
Attachment #640107 - Flags: review?(mark.finkle)
(Assignee)

Comment 3

6 years ago
Created attachment 640108 [details] [diff] [review]
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.
Attachment #640108 - Flags: review?(mark.finkle)
(Assignee)

Comment 4

6 years ago
Created attachment 640109 [details] [diff] [review]
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.
Attachment #640109 - Flags: review?(mark.finkle)
(Assignee)

Comment 5

6 years ago
Created attachment 640110 [details] [diff] [review]
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.
Attachment #640110 - Flags: review?(mark.finkle)
Attachment #640107 - Flags: review?(mark.finkle) → review+
Attachment #640108 - Flags: review?(mark.finkle) → review+
Attachment #640109 - Flags: review?(mark.finkle) → review+
Attachment #640110 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 8

6 years ago
Created attachment 640394 [details] [diff] [review]
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.
Attachment #640394 - Flags: review?(mark.finkle)
(Assignee)

Comment 9

6 years ago
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.
Attachment #640394 - Flags: review?(mark.finkle) → review+

Updated

6 years ago
Depends on: 772421
(Assignee)

Comment 12

6 years ago
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.
Attachment #640107 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

6 years ago
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.
Attachment #640108 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 14

6 years ago
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.
Attachment #640109 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 15

6 years ago
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.
Attachment #640110 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 16

6 years ago
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.
Attachment #640394 - Flags: approval-mozilla-aurora?

Updated

6 years ago
Assignee: nobody → sriram

Comment 17

6 years ago
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?
(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.
(Assignee)

Comment 19

6 years ago
(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.
You know, we have a dependency system to track these kind of things :)
Depends on: 772684

Updated

6 years ago
Attachment #640107 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

6 years ago
Attachment #640108 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

6 years ago
Attachment #640109 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

6 years ago
Attachment #640110 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

6 years ago
Attachment #640394 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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).

Updated

6 years ago
OS: Mac OS X → Android
Hardware: x86 → ARM

Updated

6 years ago
Status: RESOLVED → VERIFIED
status-firefox15: fixed → verified
status-firefox16: fixed → verified
status-firefox17: --- → verified
You need to log in before you can comment on or make changes to this bug.