655740 - Make the prefs pane tablet friendly

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

Attachment: WIP

Currently our prefs pane expands to be way to big on large screen tablets. As a result there's a lot of white space between the pref descriptions and the buttons themselves. We need a to adapt better to large screen sizes.

I started playing with this over the weekend, trying to match the Honeycomb prefs better (or at least this screenshot I found of them):

http://static.arstechnica.com/honeycombscreens/screenshot10.png

WIP still modifies the color schemes of our current UI as well (and needs a lot of other cleanup). But I'm going to hold off until I have UX feedback. Screenshots (from desktop Fennec) are at:

http://www.flickr.com/photos/digdug2k/5703408977/in/set-72157626558264317/

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

General comments:
* Remove the cruft added to get it to work on desktop (dip stuff)
* Remove all honeycomb theming code. Let's get this to work using the existing theming. Only add CSS that you need to get this to work.
* Trigger the new structure using a media query for screen size. We should use this look on the galaxy tab too, in landscape mode.
* The only difference between landscape and portrait modes for the 10" tablets is the amount of left and right margin. The <deck> ares does flex a bit to take up the addition space in landscape too.

Try to focus on getting the structural parts ready to land. We can do the CSS in a different bug.

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Patch

This is pretty simple, works fairly well for me on desktop and doesn't change anything on my phone. Still trying to find some unlocked tablets to test on, but added new ugly desktop screenshots:
http://www.flickr.com/photos/digdug2k/5705215814/in/set-72157626558264317/

and a build:
http://dl.dropbox.com/u/72157/fennec-tablet.apk

Switches layouts based on a min-width = 194mozmm media query. Will put up for review once I've tested on a tablet.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

I updated this to work better on the Xoom, and then noticed it didn't follow the system style very well. This should hopefully look better. Will test on the Xoom tomorrow at the office. In the meantime, there are some shots of the old stuff in the flickr set.

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch

Works well on Xoom and Droid X (no change on Droid). Screenshots, as well as screenshots of the default browser prefs are in the flickr set. Adding review request.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 531350 [details] [diff] [review]
Patch

>diff --git a/mobile/app/profile/extensions/[e-mail]/content/overlay.xul b/mobile/app/profile/extensions/[e-mail]/content/overlay.xul

>   <box id="panel-controls">
>-    <toolbarbutton id="tool-feedback" class="panel-row-button" autocheck="true" type="radio" group="1" linkedpanel="feedback-container" insertafter="tool-addons"/>
>+    <toolbarbutton id="tool-feedback" class="panel-row-button" autocheck="true" type="radio" label="&feedbackButton.label;"
>+                   group="1" linkedpanel="feedback-container" insertafter="tool-addons"/>

No need to wrap

>diff --git a/mobile/chrome/content/browser-ui.js b/mobile/chrome/content/browser-ui.js

>-  ["panelUI",            "panel-container"],
>+  ["panelUI",            "panel-container-background"],

I don't like "-background". Let's leave it as "panel-container" and make the old "panel-container" -> "panel-container-inner"

>diff --git a/mobile/locales/en-US/chrome/browser.dtd b/mobile/locales/en-US/chrome/browser.dtd

> <!ENTITY addonsHeader.label        "Add-ons">
>+<!ENTITY addonsButton.label        "Add-ons">

Let's just reuse the same entity for now (in all places)


>diff --git a/mobile/locales/en-US/chrome/feedback.dtd b/mobile/locales/en-US/chrome/feedback.dtd

> <!ENTITY feedbackHeader2.label          "Feedback Tools">
>+<!ENTITY feedbackButton.label           "Feedback Tools">

same here

>diff --git a/mobile/locales/en-US/chrome/preferences.dtd b/mobile/locales/en-US/chrome/preferences.dtd
> <!ENTITY prefsHeader.label                         "Preferences">
>+<!ENTITY prefsButton.label                         "Preferences">

and here

>diff --git a/mobile/themes/core/platform.css b/mobile/themes/core/platform.css

> toolbarbutton:not(.show-text) .toolbarbutton-text {
>-  display: none !important;
>+  display: none;

!important wasn't needed? or are we changing the way the rule works?

>+#panel-container {
>+  -moz-box-orient: vertical;
>+}

make sure you get all the panel-container -> panel-container-inner changes

r- for nits and questions, but real close

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Comment 7

[Wesley Johnston (:wesj)]

Looking into the !important thing, but I haven't noticed any regression from removing it. Checking when/why it was added now.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 531382 [details] [diff] [review]
Patch v2

Let's get the ball rolling

Comment 9

[Wesley Johnston (:wesj)]

http://hg.mozilla.org/mozilla-central/rev/dcf3c8a1466f

Comment 10

[Tony Chung [:tchung]]

Verified fix on Motorola Xoom.  Mozilla/5.0 (Android; Linux armv7l; rv:7.0a1)
Gecko/20110610 Firefox/7.0a1 Fennec/7.0a1.   Filed bug 663418 to track font issue in portrait view.