Update Remote Tabs static panel styling for portrait phones

VERIFIED FIXED in Firefox 35

Status

()

defect
VERIFIED FIXED
5 years ago
9 months ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

Trunk
Firefox 35
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 verified, fennec35+)

Details

Attachments

(1 attachment)

They're truncated; too wide; and not scrollable.  See also Bug 1063742.
Duplicate of this bug: 1064381
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
Status: NEW → ASSIGNED
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+
Flags: needinfo?(michael.l.comella)
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+]
https://hg.mozilla.org/mozilla-central/rev/17388bcf5749
https://hg.mozilla.org/mozilla-central/rev/dbdd635a80bf
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
tracking-fennec: ? → 35+
Verified fixed on Nightly 36.0a1 (2014-10-19)
Status: RESOLVED → VERIFIED
Whiteboard: [qa+]
Flags: qe-verify+
QA Contact: catalin.suciu
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.