Closed Bug 1100464 Opened 10 years ago Closed 9 years ago

Add back button to top left of the tabs panel header for new tablet

Categories

(Firefox for Android Graveyard :: General, defect, P1)

All
Android
defect

Tracking

(firefox36 verified, firefox37 verified)

VERIFIED FIXED
Firefox 37
Tracking Status
firefox36 --- verified
firefox37 --- verified

People

(Reporter: mhaigh, Assigned: mhaigh)

References

Details

Attachments

(3 files, 2 obsolete files)

      No description provided.
Priority: -- → P1
Anthony, any mockups?
Flags: needinfo?(alam)
Attaching preview. 

Assets to follow.
Attached file icon_navback.zip
Icons!
Flags: needinfo?(alam)
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.
Attachment #8529069 - Flags: review?(lucasr.at.mozilla)
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+
(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 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+
Attachment #8529737 - Attachment is obsolete: true
Attachment #8530261 - Flags: review?(lucasr.at.mozilla)
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+
Drive-by: Martyn, did you run trimage on these new assets?
Flags: needinfo?(mhaigh)
I ran trimage on the images, just in case.

https://hg.mozilla.org/integration/fx-team/rev/383988745eb2
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?
https://hg.mozilla.org/mozilla-central/rev/383988745eb2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
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
Flags: needinfo?(mhaigh)
Attachment #8530261 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on latest Aurora on Nexus 7 (Android 5.0.1)
Target Milestone: Firefox 37 → Firefox 36
Target Milestone: Firefox 36 → Firefox 37
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.