Closed
Bug 1100464
Opened 10 years ago
Closed 10 years ago
Add back button to top left of the tabs panel header for new tablet
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(firefox36 verified, firefox37 verified)
VERIFIED
FIXED
Firefox 37
People
(Reporter: mhaigh, Assigned: mhaigh)
References
Details
Attachments
(3 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
application/zip
|
Details | |
(deleted),
patch
|
lucasr
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
No description provided.
Updated•10 years ago
|
Priority: -- → P1
Comment 2•10 years ago
|
||
Attaching preview.
Assets to follow.
Comment 4•10 years ago
|
||
Might be worth mentioning..
Totally my fault but I hadn't noticed that we didn't resize our buttons on top to use the purple and yellow specs here: https://bugzilla.mozilla.org/attachment.cgi?id=8508946&action=edit
So it might look different from my mock (since my mock does use that sizing). I'll file a follow up bug to see if we can adjust the tabs tray button sizes I mentioned above.
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8529069 -
Flags: review?(lucasr.at.mozilla)
Comment 6•10 years ago
|
||
Comment on attachment 8529069 [details] [diff] [review]
Add back button to top left of the tabs panel header for new tablet
Review of attachment 8529069 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good overall. I thought we only want the back button in the new tablet UI? This patch will add the back button everywhere, unless I'm missing something.
::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +295,4 @@
> <!ENTITY page "Page">
> <!ENTITY tools "Tools">
> <!ENTITY new_tab "New Tab">
> +<!ENTITY nav_back "Back">
Don't we have the "Back" string already?
Attachment #8529069 -
Flags: review?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #6)
>
> Looks good overall. I thought we only want the back button in the new tablet
> UI? This patch will add the back button everywhere, unless I'm missing
> something.
Ah yes - the layout.xml file caught me out (again!)
Attachment #8529069 -
Attachment is obsolete: true
Attachment #8529737 -
Flags: review?(lucasr.at.mozilla)
Comment 8•10 years ago
|
||
Comment on attachment 8529737 [details] [diff] [review]
Add back button to top left of the tabs panel header for new tablet
Review of attachment 8529737 [details] [diff] [review]:
-----------------------------------------------------------------
Yeah, stubbing is the way to go here. This needs some changes:
1. Move new_tablet_back_button (rename it to new_tablet_tabs_panel_back_button for clarity) to mobile/android/base/newtablet/res/layout-large-v11 and add a matching entry for this new in mobile/android/base/resources/values/layout.xml (take new_tablet_tab_strip.xml as an example)
2. The arrow is vertically misaligned with the icons in IconTabWidget and has difference width. Make sure it has same width and is vertically centered.
Attachment #8529737 -
Flags: review?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8529737 -
Attachment is obsolete: true
Attachment #8530261 -
Flags: review?(lucasr.at.mozilla)
Comment 10•10 years ago
|
||
Comment on attachment 8530261 [details] [diff] [review]
Add back button to top left of the tabs panel header for new tablet
Review of attachment 8530261 [details] [diff] [review]:
-----------------------------------------------------------------
Awesome.
::: mobile/android/base/tabs/TabsPanel.java
@@ +167,5 @@
> });
> +
> + if(NewTabletUI.isEnabled(getContext())) {
> + ViewStub backButtonStub = (ViewStub) findViewById(R.id.nav_back_stub);
> + mNavBackButton = (ImageButton) backButtonStub.inflate( );
nit: remove space between ()'s
Attachment #8530261 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 11•10 years ago
|
||
Drive-by: Martyn, did you run trimage on these new assets?
Flags: needinfo?(mhaigh)
Comment 12•10 years ago
|
||
I ran trimage on the images, just in case.
https://hg.mozilla.org/integration/fx-team/rev/383988745eb2
Comment 13•10 years ago
|
||
Comment on attachment 8530261 [details] [diff] [review]
Add back button to top left of the tabs panel header for new tablet
Approval Request Comment
[Feature/regressing bug #]: New tablet UI (bug 1014156)
[User impact if declined]: Key part of the new interaction with the tabs panel. We need this in 36.
[Describe test coverage new/current, TBPL]: local tests only, looks fine.
[Risks and why]: Low, simply adds a back button to the tabs panel when the new tablet UI is active. Let's bake it in Nightly for a couple days and then uplift it.
[String/UUID change made/needed]: n/a
Attachment #8530261 -
Flags: approval-mozilla-aurora?
Comment 14•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Comment 15•10 years ago
|
||
Verified as fixed on latest Nightly on Nexus 7 (Android 5.0.1) and Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(mhaigh)
Updated•10 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Updated•10 years ago
|
Attachment #8530261 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 16•10 years ago
|
||
Comment 18•10 years ago
|
||
Verified as fixed on latest Aurora on Nexus 7 (Android 5.0.1)
Updated•10 years ago
|
Target Milestone: Firefox 37 → Firefox 36
Updated•10 years ago
|
Target Milestone: Firefox 36 → Firefox 37
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
•