Closed Bug 1063765 Opened 9 years ago Closed 9 years ago

Update Remote Tabs static panel styling for portrait phones


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox35 verified, fennec35+)

Firefox 35
Tracking Status
firefox35 --- verified
fennec 35+ ---


(Reporter: nalexander, Assigned: nalexander)




(1 file)

They're truncated; too wide; and not scrollable.  See also Bug 1063742.
This patch does several things:

1) It wraps static panel content in a ScrollView.

2) It tightens the static panels vertically.  We can fit landscape on
many high-end phones by reducing the space before the header and between
the header and the body text slightly.

3) It makes the body text just a little narrower than the button.  The
idea is for the call to action button to dominate the page horizontally.

4) It caps the maximum width of the content in the ScrollView.  I would
have used maxWidth, but that doesn't exist for LinearLayouts (or
ScrollViews).  Instead, I set a maxWidth for every RemoteTabsPanelItem,
which combined with wrap_content on the containing LinearLayout achieves
the same goal.  It's what I intended to do on tablets.
Attachment #8486145 - Flags: review?(michael.l.comella)
Assignee: nobody → nalexander
mcomella: review ping.  This is small, and addresses this bug and also Bug 1063742.  If you prefer to redirect (to margaret?), please do.
Flags: needinfo?(michael.l.comella)
Comment on attachment 8486145 [details] [diff] [review]
Style Remote Tabs static panels for tablets and portrait phones. r=mcomella

Review of attachment 8486145 [details] [diff] [review]:

Sorry for the delay - lgtm.

For the history books:

<mcomella> nalexander: What is the purpose of capping the width of the content in the ScrollView (re maxWidth)?
<nalexander> mcomella: it looks better both in landscape and on tablets.
<nalexander> mcomella: it kind of centers it out.  Otherwise, you get 4 wide, vertically short lines.
<nalexander> mcomella: and on tablets, the button is *huge*.
<mcomella> nalexander: And for devices smaller than 320dp? It looks the same?
<nalexander> mcomella: yeah, it expands up to 320dp.  That number was kinda-sorta suggested by antlam (he wanted 300dp, IIRC).
<nalexander> mcomella: it would be a maxWidth on the container layout if that was possible.
<nalexander> mcomella: that is, we want as wide as possible up to a max of 320dp.

::: mobile/android/base/resources/values/styles.xml
@@ +847,5 @@
>      <style name="RemoteTabsPanelItem.TextAppearance.Header">
>          <item name="android:textColor">#222222</item>
>          <item name="android:textSize">20sp</item>
> +        <item name="android:layout_marginBottom">8dp</item>
> +	  </style>

nit: indentation.
Attachment #8486145 - Flags: review?(michael.l.comella) → review+
After discussion with antlam, this is more or less okay modulo the top margin.  antlam wants the original margin (48dp, IIRC) in portrait mode and is okay with the new reduced margin (16dp, IIRC) in landscape.  I'll land with this change (via a landscape/portrait @dimen).
tracking-fennec: --- → ?
AaronMT: the top margin for all setup screens (get started, needs password, etc) should be "large" (48dp) on everything except landscape phones, where it should be "tight" (16dp) to try to fit things in.

Horizontally, we should limit to 320dp on all devices.
Whiteboard: [qa+]
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
tracking-fennec: ? → 35+
Verified fixed on Nightly 36.0a1 (2014-10-19)
Whiteboard: [qa+]
Flags: qe-verify+
QA Contact: catalin.suciu
Flags: qe-verify+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.