Closed
Bug 1100021
Opened 10 years ago
Closed 10 years ago
New tablet toolbar buttons are broken on devices with hardware menu buttons
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, defect, P1)
Tracking
(firefox36 affected, fennec36+)
VERIFIED
FIXED
Firefox 36
People
(Reporter: kohei, Assigned: mcomella)
References
Details
Attachments
(3 files, 3 obsolete files)
See the attached screenshot. The reload, star, menu buttons are missing. The tab counter button is misplaced. The latest x86 Nightly build on Galaxy Tab 3.
Updated•10 years ago
|
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Comment 2•10 years ago
|
||
Looks like one of these cases where our UI is getting confused about whether or not the device is considered a 'tablet'?
Reporter | ||
Comment 3•10 years ago
|
||
Umm, I don't know but the older UI looks good to me on the same 10.1-inch tablet.
Updated•10 years ago
|
Priority: -- → P1
Assignee | ||
Comment 4•10 years ago
|
||
We have an x86 Galaxy Tab 3 in the office - I'll check it out when I get in.
Status: NEW → ASSIGNED
Flags: needinfo?(michael.l.comella)
Comment 5•10 years ago
|
||
I am unable to reproduce on my 10-inch ASUS Memopad, so it could be tablet-specific, yes.
Reporter | ||
Updated•10 years ago
|
Summary: New tablet UI: toolbar buttons are broken → New tablet UI: toolbar buttons are broken on x86 Galaxy Tab 3
Assignee | ||
Comment 7•10 years ago
|
||
I can confirm that I see this issue on my x86 Galaxy Tab 3. If I had to guess, it's because there is a hardware menu key so the software menu button is missing (which is a configuration I have not tested).
Flags: needinfo?(michael.l.comella)
Assignee | ||
Updated•10 years ago
|
Summary: New tablet UI: toolbar buttons are broken on x86 Galaxy Tab 3 → New tablet toolbar buttons are broken on devices with hardware menu buttons
Assignee | ||
Comment 8•10 years ago
|
||
Anthony, what do you think of the margin on the right of the tabs panel button?
Attachment #8524941 -
Flags: feedback?(alam)
Assignee | ||
Comment 9•10 years ago
|
||
My patch which dynamically adds the same margin to both the menu button and tabs panel button, waiting on antlam's confirmation.
Comment 10•10 years ago
|
||
Comment on attachment 8524941 [details]
Tablet w/o soft menu key
Ah, thats why.. (hardware menu) Nice catch!
This looks ok to me! thanks Mike
Attachment #8524941 -
Flags: feedback?(alam) → feedback+
Assignee | ||
Updated•10 years ago
|
Attachment #8524944 -
Attachment is obsolete: true
Comment 12•10 years ago
|
||
Comment on attachment 8525612 [details] [diff] [review] Correct new tablet toolbar layout for devices with hardware menu key Review of attachment 8525612 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: mobile/android/base/toolbar/BrowserToolbarNewTablet.java @@ +63,5 @@ > + final RelativeLayout.LayoutParams lp = > + (RelativeLayout.LayoutParams) rightmostActionBarView.getLayoutParams(); > + lp.rightMargin = lp.rightMargin + > + res.getDimensionPixelOffset(R.dimen.new_tablet_browser_toolbar_menu_right_margin); > + rightmostActionBarView.setLayoutParams(lp); Given that you're changing the current LayoutParams instance, you can just call rightmostActionBarrequestLayout.requestLayout() instead of setLayoutParams() here. nit: factor out this code into a separate method called resolveRightmostMargin() or something.
Attachment #8525612 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 13•10 years ago
|
||
I wonder if you could avoid the dynamic margin code by just setting padding = 6dp on the toolbar container instead?
Updated•10 years ago
|
tracking-fennec: ? → 36+
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #13) > I wonder if you could avoid the dynamic margin code by just setting padding > = 6dp on the toolbar container instead? Good point. We can't do this statically because we have to set it on the parent container in gecko_app and we still have old tablet to worry about. This patch sets the padding dynamically, which still cleans up the logic.
Assignee | ||
Updated•10 years ago
|
Attachment #8525612 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8526222 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Attachment #8526222 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8526276 -
Flags: review+
Comment 19•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/75599db3d71d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Reporter | ||
Comment 20•10 years ago
|
||
Forgot to say: the issue is gone, the current Nightly looks pretty. Thanks <3
Status: RESOLVED → VERIFIED
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•