Closed Bug 1014999 Opened 10 years ago Closed 10 years ago

Remote tabs panel setup/verification for tablets

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox32 verified, firefox33 verified)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox32 --- verified
firefox33 --- verified

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(5 files, 3 obsolete files)

I realized you didn't get to review all of the UI changes in the previous bug - perhaps you want to audit all of the layout code here.
Attachment #8430340 - Flags: review?(lucasr.at.mozilla)
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Attached image Setup horizontal (obsolete) (deleted) —
Anthony, how's this? If this is acceptable to ship, I'd like to do fine tuning in bug 1007442 because I'll also have to figure out all the l10n issues.
Attachment #8430346 - Flags: feedback?(alam)
Attached image Setup vertical (deleted) —
I assume these next two are okay so I'm not flagging for feedback.
Comment on attachment 8430340 [details] [diff] [review] Add remote tabs setup/verification panels for tablet. Review of attachment 8430340 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Please audit your patch for any redundant style definitions. ::: mobile/android/base/resources/values-large-land-v11/styles.xml @@ +59,5 @@ > + <item name="android:paddingRight">0dp</item> > + </style> > + > + <style name="RemoteTabsSection" parent="RemoteTabsSectionBase"> > + <!-- To override the values-land style. --> You don't set any values in values-land's RemoteTabsSection either. This seems a bit redundant, remove it. @@ +63,5 @@ > + <!-- To override the values-land style. --> > + </style> > + > + <style name="RemoteTabsItem" parent="RemoteTabsItemBase"> > + <!-- To override the values-land style. --> Ditto, this seems redundant as you're setting attribute in values-land's RemoteTabsItem either. Remove it.
Attachment #8430340 - Flags: review?(lucasr.at.mozilla) → review+
Comment on attachment 8430346 [details] Setup horizontal Could we shrink the font size on the get started button down to the same as the body text here?
Attachment #8430346 - Flags: feedback?(alam) → feedback-
Attachment #8430347 - Flags: feedback?(alam) → feedback+
Attached image Setup horizontal (obsolete) (deleted) —
Attachment #8430346 - Attachment is obsolete: true
Attachment #8431753 - Flags: feedback?(alam)
Comment on attachment 8431753 [details] Setup horizontal Looks better, almost there! :) Can we adjust the width of the CTA button here to be the width of the title "Welcome to Sync"? A nice rounded number will work.
Attachment #8431753 - Flags: feedback?(alam) → feedback-
(In reply to Lucas Rocha (:lucasr) from comment #6) > You don't set any values in values-land's RemoteTabsSection either. This > seems a bit redundant, remove it. RemoteTabsSection has no definitions in values/, has defs in values-land/ (layout_weight), and has no defs in values-large-land-v11/. We want to inherit some attr and override others in landscape, hence the use of RemoteTabsSectionBase and the emptiness of values/. However, on tablet, without values-large-land-v11/, values-land/ will be read if we have the device in landscape, when we actually want values/ (or more accurately, the base style). Thus I defined an empty style in values-large-land-v11/. Is there a better paradigm for this? > Ditto, this seems redundant as you're setting attribute in values-land's > RemoteTabsItem either. Remove it. See above.
Attached image Setup horizontal (deleted) —
Attachment #8431753 - Attachment is obsolete: true
Attachment #8431767 - Flags: feedback?(alam)
Comment on attachment 8431767 [details] Setup horizontal Looking good
Attachment #8431767 - Flags: feedback?(alam) → feedback+
Comment on attachment 8430340 [details] [diff] [review] Add remote tabs setup/verification panels for tablet. ># HG changeset patch ># User Michael Comella <michael.l.comella@gmail.com> ># Date 1401312719 25200 ># Wed May 28 14:31:59 2014 -0700 ># Node ID b90d60ee8249065d10f2c401469b0ee32dc5ab47 ># Parent bc643e9ee8422e3a52258b613fedf1b36d42b023 >Bug 1014999 - Add remote tabs setup/verification panels for tablet. > >diff --git a/mobile/android/base/resources/values-large-land-v11/styles.xml b/mobile/android/base/resources/values-large-land-v11/styles.xml >--- a/mobile/android/base/resources/values-large-land-v11/styles.xml >+++ b/mobile/android/base/resources/values-large-land-v11/styles.xml >@@ -46,9 +46,40 @@ > > <style name="Widget.Home.HistoryTabWidget"> > <item name="android:showDividers">beginning|middle|end</item> > <item name="android:dividerPadding">0dp</item> > <item name="android:paddingLeft">100dp</item> > <item name="android:paddingTop">30dp</item> > </style> > >+ <!-- Remote tabs panel --> >+ <style name="RemoteTabsPanelChild" parent="RemoteTabsPanelChildBase"> >+ <item name="android:paddingTop">32dp</item> >+ <!-- Additional spacing set via margins on RemoteTabsSection. --> >+ <item name="android:paddingLeft">0dp</item> >+ <item name="android:paddingRight">0dp</item> >+ </style> >+ >+ <style name="RemoteTabsSection" parent="RemoteTabsSectionBase"> >+ <!-- To override the values-land style. --> >+ </style> >+ >+ <style name="RemoteTabsItem" parent="RemoteTabsItemBase"> >+ <!-- To override the values-land style. --> >+ </style> >+ >+ <style name="RemoteTabsItem.Button" parent="RemoteTabsItem.ButtonBase"> >+ <item name="android:paddingTop">12dp</item> >+ <item name="android:paddingBottom">12dp</item> >+ <item name="android:paddingLeft">24dp</item> >+ <item name="android:paddingRight">24dp</item> >+ </style> >+ >+ <style name="RemoteTabsItem.TextAppearance.Header.FXAccounts"> >+ <item name="android:visibility">gone</item> >+ </style> >+ >+ <style name="RemoteTabsItem.TextAppearance.Linkified.Resend"> >+ <item name="android:layout_height">wrap_content</item> >+ </style> >+ > </resources> >diff --git a/mobile/android/base/resources/values/styles.xml b/mobile/android/base/resources/values/styles.xml >--- a/mobile/android/base/resources/values/styles.xml >+++ b/mobile/android/base/resources/values/styles.xml >@@ -447,43 +447,44 @@ > <item name="android:childDivider">@drawable/remote_tabs_child_divider</item> > <item name="android:groupIndicator">@android:color/transparent</item> > </style> > > <!-- Remote tabs panel --> > <style name="RemoteTabsPanelChildBase"> > <item name="android:paddingLeft">16dp</item> > <item name="android:paddingRight">16dp</item> >+ <item name="android:paddingTop">62dp</item> >+ <item name="android:orientation">vertical</item> > </style> > > <style name="RemoteTabsPanelChild" parent="RemoteTabsPanelChildBase"> >- <item name="android:orientation">vertical</item> >- <item name="android:paddingTop">62dp</item> >+ <!-- We set values in landscape. --> > </style> > > <style name="RemoteTabsSectionBase"> > <item name="android:orientation">vertical</item> > <item name="android:layout_marginLeft">16dp</item> > <item name="android:layout_marginRight">16dp</item> > </style> > > <style name="RemoteTabsSection" parent="RemoteTabsSectionBase"> > <!-- We set values in landscape. --> > </style> > >- <style name="RemoteTabsSection.Resend" parent="RemoteTabsSectionBase"> >- <!-- We set values in landscape. --> >- </style> >- >- <style name="RemoteTabsItem"> >+ <style name="RemoteTabsItemBase"> > <item name="android:layout_marginBottom">28dp</item> > <item name="android:layout_gravity">center</item> > <item name="android:gravity">center</item> > </style> > >+ <style name="RemoteTabsItem" parent="RemoteTabsItemBase"> >+ <!-- We set values in landscape. --> >+ </style> >+ > <style name="RemoteTabsItem.ButtonBase"> > <item name="android:background">@drawable/remote_tabs_setup_button_background</item> > <item name="android:textColor">#FFFEFF</item> > <item name="android:textSize">20sp</item> > </style> > > <style name="RemoteTabsItem.Button" parent="RemoteTabsItem.ButtonBase"> > <item name="android:paddingTop">18dp</item> >diff --git a/mobile/android/base/tabspanel/RemoteTabsSetupPanel.java b/mobile/android/base/tabspanel/RemoteTabsSetupPanel.java >--- a/mobile/android/base/tabspanel/RemoteTabsSetupPanel.java >+++ b/mobile/android/base/tabspanel/RemoteTabsSetupPanel.java >@@ -6,17 +6,16 @@ package org.mozilla.gecko.tabspanel; > > import java.util.Locale; > > import org.mozilla.gecko.R; > import org.mozilla.gecko.Tabs; > import org.mozilla.gecko.fxa.FirefoxAccounts; > import org.mozilla.gecko.fxa.activities.FxAccountCreateAccountActivity; > import org.mozilla.gecko.tabspanel.TabsPanel.PanelView; >-import org.mozilla.gecko.util.HardwareUtils; > > import android.content.Context; > import android.content.Intent; > import android.util.AttributeSet; > import android.view.View; > import android.widget.LinearLayout; > > /** >@@ -64,21 +63,16 @@ class RemoteTabsSetupPanel extends Linea > > @Override > public void setTabsPanel(TabsPanel panel) { > tabsPanel = panel; > } > > @Override > public void show() { >- // We don't have a tablet implementation of this panel. >- if (HardwareUtils.isTablet()) { >- return; >- } >- > setVisibility(View.VISIBLE); > } > > @Override > public void hide() { > setVisibility(View.GONE); > } > >diff --git a/mobile/android/base/tabspanel/RemoteTabsVerificationPanel.java b/mobile/android/base/tabspanel/RemoteTabsVerificationPanel.java >--- a/mobile/android/base/tabspanel/RemoteTabsVerificationPanel.java >+++ b/mobile/android/base/tabspanel/RemoteTabsVerificationPanel.java >@@ -3,17 +3,16 @@ > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > package org.mozilla.gecko.tabspanel; > > import org.mozilla.gecko.R; > import org.mozilla.gecko.fxa.FirefoxAccounts; > import org.mozilla.gecko.fxa.login.State; > import org.mozilla.gecko.tabspanel.TabsPanel.PanelView; >-import org.mozilla.gecko.util.HardwareUtils; > > import android.content.Context; > import android.util.AttributeSet; > import android.util.Log; > import android.view.View; > import android.widget.LinearLayout; > import android.widget.TextView; > >@@ -93,21 +92,16 @@ class RemoteTabsVerificationPanel extend > > @Override > public void setTabsPanel(TabsPanel panel) { > tabsPanel = panel; > } > > @Override > public void show() { >- // We don't have a tablet implementation of this panel. >- if (HardwareUtils.isTablet()) { >- return; >- } >- > refresh(); > setVisibility(View.VISIBLE); > } > > @Override > public void hide() { > setVisibility(View.GONE); > }
Attachment #8430340 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Verified as fixed in Build: Firefox for Android 33.0a2 (2014-08-19) Device: Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
(In reply to Cristina Madaras, QA [:CristinaM] from comment #17) > Verified as fixed in > Build: Firefox for Android 33.0a2 (2014-08-19) > Device: Asus Transformer Pad TF300T (Android 4.2.1) I meant Firefox for Android 32 Beta 8. Sorry for this!
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: