Closed Bug 1063765 Opened 9 years ago Closed 9 years ago
Update Remote Tabs static panel styling for portrait phones
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)
9 years ago
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.
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+
9 years ago
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).
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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Verified fixed on Nightly 36.0a1 (2014-10-19)
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.