Last Comment Bug 682412 - Pretty up the Control Panels
: Pretty up the Control Panels
Status: VERIFIED FIXED
: verified-aurora
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: ARM Android
: P3 normal (vote)
: Firefox 9
Assigned To: Sriram Ramasubramanian [:sriram]
:
Mentors:
Depends on: 685309 685334 690849 690870
Blocks: 655762
  Show dependency treegraph
 
Reported: 2011-08-26 14:10 PDT by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2011-10-06 15:51 PDT (History)
13 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
screenshot (156.75 KB, image/png)
2011-08-26 14:10 PDT, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
Font / colour specs (137.50 KB, image/png)
2011-09-08 08:33 PDT, Ian Barlow (:ibarlow)
no flags Details
Graphic assets for control panels (65.19 KB, application/zip)
2011-09-08 08:53 PDT, Ian Barlow (:ibarlow)
no flags Details
New backgrounds to hopefully fix our banding issues (114.87 KB, application/zip)
2011-09-09 11:55 PDT, Ian Barlow (:ibarlow)
no flags Details
gmail on honeycomb (91.34 KB, image/png)
2011-09-09 15:04 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details
Updated settings UI (42.79 KB, image/png)
2011-09-19 06:54 PDT, Ian Barlow (:ibarlow)
no flags Details
Graphic assets for control panels (19.90 KB, application/zip)
2011-09-19 07:28 PDT, Ian Barlow (:ibarlow)
no flags Details
Font / colour specs (62.46 KB, image/png)
2011-09-19 07:57 PDT, Ian Barlow (:ibarlow)
no flags Details
Margins and spacing measurements (48.77 KB, image/png)
2011-09-19 14:54 PDT, Ian Barlow (:ibarlow)
no flags Details
Font sizes, relative to the rest of our UI (86.86 KB, image/png)
2011-09-20 11:23 PDT, Ian Barlow (:ibarlow)
no flags Details
VWIP (25.52 KB, patch)
2011-09-21 17:48 PDT, Sriram Ramasubramanian [:sriram]
no flags Details | Diff | Review
mockup for downloads (52.35 KB, image/png)
2011-09-22 12:18 PDT, Ian Barlow (:ibarlow)
no flags Details
mockup for addons (56.00 KB, image/png)
2011-09-22 12:19 PDT, Ian Barlow (:ibarlow)
no flags Details
specs for buttons (12.55 KB, image/png)
2011-09-22 12:20 PDT, Ian Barlow (:ibarlow)
no flags Details
WIP (30.88 KB, patch)
2011-09-22 16:57 PDT, Sriram Ramasubramanian [:sriram]
wjohnston2000: review+
Details | Diff | Review
Control Panel patch (31.17 KB, patch)
2011-09-23 12:58 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review-
Details | Diff | Review
Control Panel patch (32.76 KB, patch)
2011-09-23 16:39 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review-
Details | Diff | Review
Control Panel patch (32.70 KB, patch)
2011-09-26 12:23 PDT, Sriram Ramasubramanian [:sriram]
no flags Details | Diff | Review
Control Panel patch (33.76 KB, patch)
2011-09-26 16:18 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review-
Details | Diff | Review
Control Panel patch (34.24 KB, patch)
2011-09-28 11:33 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-08-26 14:10:33 PDT
Created attachment 556127 [details]
screenshot

Error console with the All, Messages, Warnings, Errors does not seem to match the color selection scheme for the Panel selections on the left hand side.

Mozilla/5.0 (Android; Linux armv7I; rv9.0a1) Gecko/20110826 Firefox/9.0a1 Fennec/9.0a1
Device: Toshiba Thrive
OS Android 3.1
Comment 1 Matt Brubeck (:mbrubeck) 2011-08-29 10:44:56 PDT
Ian, how should these look in honeycomb?
Comment 2 Matt Brubeck (:mbrubeck) 2011-09-07 16:58:42 PDT
Ian provided this mock-up:
https://secure.flickr.com/photos/61892693@N03/6101214910/in/photostream/lightbox/
Comment 3 Ian Barlow (:ibarlow) 2011-09-08 08:33:07 PDT
Created attachment 559171 [details]
Font / colour specs

Mockup for landscape mode http://www.flickr.com/photos/61892693@N03/6101214910/in/photostream/lightbox/
Mockup for portrait mode http://www.flickr.com/photos/61892693@N03/6101205906/in/photostream/lightbox/

Specs are attached here. Visual assets will follow shortly.
Comment 4 Ian Barlow (:ibarlow) 2011-09-08 08:53:30 PDT
Created attachment 559177 [details]
Graphic assets for control panels
Comment 5 Ian Barlow (:ibarlow) 2011-09-09 11:55:10 PDT
Created attachment 559529 [details]
New backgrounds to hopefully fix our banding issues
Comment 6 Gabriela [:gaby2300] 2011-09-09 14:49:04 PDT
Today's Nightly doesn't show the Control Panel as the one in this bug.

Mozilla/5.0 (Android; Linux armv7l; rv:9.0a 1)Gecko/20110909 Firefox/9.0a1 Fennec/9.0a1
Samsung Galaxy Tab 10.1
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-09 15:04:22 PDT
Created attachment 559588 [details]
gmail on honeycomb

This shows the GMail setting/preferences page. I am posting this since we are using a similar to GMail, white-chrome style in the main application area. Wouldn't we want to use a preferences UI that is white-chrome too?
Comment 8 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-09 15:05:08 PDT
Also, it's no secret that I don't like gradients and large images. Especially large gradient images. :)
Comment 9 Ian Barlow (:ibarlow) 2011-09-13 13:32:26 PDT
Sure, we could do that. I'll mock something up and we can talk about it.
Comment 10 Ian Barlow (:ibarlow) 2011-09-19 06:54:16 PDT
Created attachment 560891 [details]
Updated settings UI

Here's a mockup of the new styling on the settings pages. I'll post new assets and specs shortly.
Comment 11 Ian Barlow (:ibarlow) 2011-09-19 07:28:23 PDT
Created attachment 560893 [details]
Graphic assets for control panels

Updated assets to go with the new light background design
Comment 12 Ian Barlow (:ibarlow) 2011-09-19 07:57:02 PDT
Created attachment 560908 [details]
Font / colour specs
Comment 13 Ian Barlow (:ibarlow) 2011-09-19 14:54:27 PDT
Created attachment 561031 [details]
Margins and spacing measurements
Comment 14 Ian Barlow (:ibarlow) 2011-09-20 11:23:30 PDT
Created attachment 561251 [details]
Font sizes, relative to the rest of our UI
Comment 15 Sriram Ramasubramanian [:sriram] 2011-09-21 17:48:05 PDT
Created attachment 561633 [details] [diff] [review]
VWIP

Very wet in paint patch.
Works in both portrait and landscape mode.

Need to add changes to downloads and addons.
Comment 16 Ian Barlow (:ibarlow) 2011-09-22 12:18:59 PDT
Created attachment 561826 [details]
mockup for downloads
Comment 17 Ian Barlow (:ibarlow) 2011-09-22 12:19:53 PDT
Created attachment 561827 [details]
mockup for addons
Comment 18 Ian Barlow (:ibarlow) 2011-09-22 12:20:27 PDT
Created attachment 561828 [details]
specs for buttons
Comment 19 Sriram Ramasubramanian [:sriram] 2011-09-22 16:57:13 PDT
Created attachment 561929 [details] [diff] [review]
WIP
Comment 20 Sriram Ramasubramanian [:sriram] 2011-09-22 16:58:32 PDT
(In reply to Sriram Ramasubramanian [:sriram] from comment #19)
> Created attachment 561929 [details] [diff] [review]
> WIP

This contains the patch for preferences and downloads pane.
The addons pane needs to be cleared a bit.
Comment 21 Wesley Johnston (:wesj) 2011-09-22 19:16:22 PDT
Comment on attachment 561929 [details] [diff] [review]
WIP

Review of attachment 561929 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. I haven't tested it yet. Mostly just some comments below, but nothing I think we should hold up landing this for.

::: mobile/themes/core/honeycomb/browser.css
@@ +1450,5 @@
> +  padding: @padding_snormal@ 0 0 0;
> +}
> +
> +setting[type="menulist"] .setting-label > .preferences-description {
> +  display: none;

Are we sure we're never going to want to show this?

::: mobile/themes/core/honeycomb/defines.inc
@@ +56,5 @@
>  %define font_tiny 2.48mozmm
>  %define font_xtiny 2.27mozmm
>  %define font_xxtiny 1.93mozmm
>  
> +%define touch_row 10.478mozmm

lol. changed from 10.41 to 10.478?

::: mobile/themes/core/honeycomb/platform.css
@@ +817,5 @@
> +
> +#panel-controls > .panel-row-button {
> +  border-top: @border_width_tiny@ solid @color_background_settings@ !important;
> +  border-bottom: @border_width_large@ solid !important;
> +  -moz-border-bottom-colors: @color_panel_row_border@ @color_background_settings@ !important;

Can you file a bug for us to go through our theme code and evaluate all of these !important rules. I'd like to clean them up sometime.

::: mobile/themes/core/jar.mn
@@ +394,5 @@
>    skin/honeycomb/images/check-30.png                  (images/check-30.png)
>    skin/honeycomb/images/check-selected-hdpi.png       (honeycomb/images/check-selected-hdpi.png)
>    skin/honeycomb/images/check-unselected-hdpi.png     (honeycomb/images/check-unselected-hdpi.png)
> +  skin/honeycomb/images/check-selected-tap-hdpi.png   (honeycomb/images/check-selected-tap-hdpi.png)
> +  skin/honeycomb/images/check-unselected-tap-hdpi.png (honeycomb/images/check-unselected-tap-hdpi.png)

We should also check if we really need 6 different checkmark images in this theme anymore.
Comment 22 Sriram Ramasubramanian [:sriram] 2011-09-23 12:58:30 PDT
Created attachment 562141 [details] [diff] [review]
Control Panel patch

This patch includes all changes.

Todos:
1. The addons header needs to be revisited.
2. The Back button bar on top needs to be added.
Comment 23 Sriram Ramasubramanian [:sriram] 2011-09-23 13:15:05 PDT
https://bugzilla.mozilla.org/show_bug.cgi?id=685334 - requires the patch in this to be applied for the above patch to work.
Comment 24 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-23 14:04:50 PDT
Comment on attachment 562141 [details] [diff] [review]
Control Panel patch

>+#addons-list richlistitem.section-header {
>+  margin: 0;
>+  padding: @padding_xxnormal@ @padding_large@;
>+}
>+
>+#addons-list richlistitem:not([class="section-header"])[selected="true"] {
>+  background-color: @color_background_settings@;
>+  border-top: @border_width_tiny@ solid @color_text_button@;
>+  border-bottom: @border_width_tiny@ solid @color_text_button@;
>+}
>+
>+#addons-list richlistitem:not([class="section-header"]):not([selected="true"]):hover:active {
>+  background-color: @color_background_highlight@;
>+}

I am wondering why we want add-on section-headers should be different than other section-headers? Can't we tweak the platform.css richlistitem.section-header rules?

>+richlistitem[typeName="local"],
>+richlistitem[typeName="search"],
>+richlistitem[typeName="searchplugin"],
>+richlistitem[typeName="banner"] {
>+  padding: @padding_xxxnormal@ 0 @padding_tiny@ @padding_xlarge@; 
>+  border-bottom: @border_width_tiny@ solid @color_button_border@;
>+}
>+
>+richlistitem[typeName="local"] image,
>+richlistitem[typeName="search"] image,
>+richlistitem[typeName="searchplugin"] image,
>+richlistitem[typeName="banner"] image {
>+  margin-right: @margin_xxxnormal@;

for RTL safety, use -moz-margin-end

>+
>+richlistitem[typeName="local"] label.normal,
>+richlistitem[typeName="local"] description.normal,
>+richlistitem[typeName="search"] label.normal,
>+richlistitem[typeName="search"] description.normal,
>+richlistitem[typeName="searchplugin"] label.normal,
>+richlistitem[typeName="searchplugin"] description.normal,
>+richlistitem[typeName="banner"] label.normal,
>+richlistitem[typeName="banner"] description.normal {
>+  color: @color_subtext_settings@;
>+}

You do the same 3 types of rules for downloads. Maybe we could add a class and get them all instead of listing so many and listing them in add-ons and downloads.

> .addon-image,
> .searchplugin-image {

>-  -moz-margin-start: @margin_normal@;
>   -moz-margin-end: @margin_normal@;
>+  margin-left: @margin_large@;

For RTL safety, use -moz-margin-start

> /* downloads panel UI   ---------------------------------------------------- */
>+#downloads-list richlistitem[selected="true"] {
>+  background-color: @color_background_settings@;
>+}
>+
>+#downloads-list richlistitem:hover:active {
>+  background-color: @color_background_highlight@;
>+}
>+
>+richlistitem[typeName="download"] {
>+  padding: @padding_xxxnormal@ 0 @padding_tiny@ @padding_xlarge@; 
>+  border-bottom: @border_width_tiny@ solid @color_button_border@;
>+}
>+
>+richlistitem[typeName="download"] image {
>+  margin-right: @margin_xxxnormal@;

For RTL safety, use -moz-margin-end

>+}
>+
>+richlistitem[typeName="download"] label.normal,
>+richlistitem[typeName="download"] description.normal {
>+  color: @color_subtext_settings@;
>+}

Move this to platform? I commented there too.

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

> .toolbarbutton-icon[label]:not([label=""]),
> .toolbarbutton-icon[type="menu"] {
>   -moz-margin-end: @margin_tiny@;
>+  margin-left: @margin_xxxnormal@;
>+  margin-right: @margin_xnormal@;

For RTL safetly, use -moz-margin-end and -moz-margin-start

> richlistitem label.normal,
> richlistitem description.normal {
>   color: @color_subtext_default@;

You could safely change this to use color_settings_default and not add the rules for add-ons and downloads


>+#panel-controls > .panel-row-button {

>+  border-left: 0 solid; 
>+  border-right: 0 solid; 

For RTL safety, -moz-border-start and -moz-border-end

r- for the RTL stuff and I want to see if we can move more of these changes to platform.css
Comment 25 Sriram Ramasubramanian [:sriram] 2011-09-23 16:39:51 PDT
Created attachment 562190 [details] [diff] [review]
Control Panel patch

Most possible settings have been moved in platform.css.
This didn't affect anything as much as I checked.
RTL support specific "-moz-*-start" and "-moz-*-end" have been added.

I would like to move the settings to platform.css
Comment 26 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-26 11:30:48 PDT
Comment on attachment 562190 [details] [diff] [review]
Control Panel patch

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

> .options-box {
>-  -moz-margin-start: 28px;  /* sized based on the 32px addon image */
>+  -moz-margin-start: @margin_xlarge@;

This worries me a little since the image is still based on pixels

>-#appmenu-popup-appcommands richlistitem:hover:active {
>-  background-color: @color_background_highlight_overlay@;
>-}
>-

Leave this. We need it.

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

> button:focus > .button-box {
>-  border: @border_width_tiny@ solid transparent;
> }

If we don't need this, remove the entire rule

> toolbarbutton {
>   min-width: @touch_button_small@ !important; /* primary button size */
>-  min-height: @touch_button_small@ !important; /* primary button size */
>+  min-height: @touch_button_tiny@ !important; /* primary button size */
>   -moz-appearance: none !important;
>   padding: @padding_xsmall@;
>   -moz-margin-end: @margin_xxxnormal@;
> }

This is causing trouble

> .toolbarbutton-icon[label]:not([label=""]),
> .toolbarbutton-icon[type="menu"] {
>-  -moz-margin-end: @margin_tiny@;
>+  -moz-margin-start: @margin_xxxnormal@;
>+  -moz-margin-end: @margin_xnormal@;
> }

This could cause trouble

> radio {
>   -moz-appearance: none;
>   min-width: @touch_button_small@ !important; /* button size */
>-  min-height: @touch_button_small@ !important; /* button size */
>+  min-height: @touch_button_tiny@ !important; /* button size */

I'm less worried about this one

> richlistitem {
>   -moz-user-focus: ignore;
>   min-height: @touch_row@; /* row size */
>-  padding: @padding_small@;
>+  padding-top: @padding_xxxnormal@;
>+  padding-bottom: @padding_tiny@;
>+  -moz-padding-start: @padding_xlarge@; 
>+  -moz-padding-end: 0;

Do we really need this level of tweaks to the padding?

> .prompt-button {
>   min-width: 33%;
>+  min-height: @touch_button_small@;

Doers this need an "!important" ?


>+@media (@orientation@: portrait) and (min-width: @tablet_panel_minwidth@) {
>+  #panel-controls > .panel-row-button {
>+    min-width: @tablet_panel_controls@ !important;
>+  }
>+  
>+  #panel-items {
>+    max-width: @tablet_panel_item_width@ !important;
>+  }
>+}
>+
>+@media (@orientation@: landscape) and (min-width: @tablet_panel_minwidth@) {
>+  #panel-controls > .panel-row-button {
>+    min-width: @tablet_panel_controls_wide@ !important;
>+  }
>+  
>+  #panel-items {
>+    max-width: @tablet_panel_item_width_wide@ !important;
>+  }
>+}
>+

These rules should be in browser.css since they are specific to UI in browser.xul and are not general purpose rules.

>+#panel-container {
>+  background-color: @color_background_settings@ !important;
>+}
>+
> #panel-container-inner {
>   -moz-box-orient: vertical;
>+  background-color: transparent !important;
>+}
>+
>+#panel-container-inner > vbox {
>+  border: @border_width_large@ solid @color_background_active@; 
>+  box-shadow: 0 0 @shadow_width_tiny@ @shadow_width_medium@ @color_background_button_overlay@;  
> }
> 
> #panel-controls {
>   -moz-box-orient: horizontal;
>+  background-color: transparent !important;
>+  padding: @padding_xxxnormal@ 0 !important;
> }
> 
>+#panel-controls .toolbarbutton-text {
>+  font-size: @font_snormal@ !important;
>+}
>+
>+#panel-controls > .panel-row-button {
>+  border-top: @border_width_tiny@ solid @color_background_settings@ !important;
>+  border-bottom: @border_width_large@ solid !important;
>+  -moz-border-bottom-colors: @color_background_button@ @color_background_settings@ !important;
>+  -moz-border-start: 0 solid; 
>+  -moz-border-end: 0 solid; 
>+}
>+
>+#panel-controls > .panel-row-button[checked="true"] {
>+  background-color: @color_background_highlight@;
>+  color: @color_text_default@;
>+}
>+
>+.panel-list {
>+  background: transparent;
>+}
>+
>+#panel-items richlistitem:not([class="section-header"])[selected="true"] {
>+  background-color: @color_background_settings@;
>+}
>+
>+#panel-items richlistitem label.normal,
>+#panel-items richlistitem description.normal {
>+  color: @color_subtext_settings@;
>+}
>+
>+
> @media (min-width: @tablet_panel_minwidth@) {
>   #panel-container-inner {
>     -moz-box-orient: horizontal;
>     -moz-box-pack: center;
>   }
> 
>   #panel-items {
>-    max-width: @tablet_panel_minwidth@;
>     min-width: 0px !important;
>   }
>   
>   /* This will affect the prefs screen, but not the awesome screen */
>   #panel-controls {
>     -moz-box-orient: vertical !important;
>     -moz-box-align: start;
>   }
>   
>   #panel-controls > .panel-row-button {
>+    margin: 0;
>     -moz-box-orient: horizontal;
>     -moz-box-flex: 0;
>-    min-width: @tablet_panel_controls@ !important;
>   }
> 
>   #panel-controls .toolbarbutton-text {
>     display: -moz-box !important;
>     -moz-box-flex: 1;
>   }
>   
>   #panel-container {
>     -moz-box-pack: center;
>-    padding: @padding_xlarge@ 0px;
>+    padding: @padding_xxxlarge@ 0 @padding_xlarge@ 0;
>+  }
>+
>+  #panel-container-inner > vbox {
>+    padding: @padding_xlarge@ @padding_xxlarge@;
>   }
> }

These rules should be in browser.css since they are specific to UI in browser.xul and are not general purpose rules.
(I see some of this was already here. That makes me sad)
Comment 27 Sriram Ramasubramanian [:sriram] 2011-09-26 11:42:51 PDT
(In reply to Mark Finkle (:mfinkle) from comment #26)
> Comment on attachment 562190 [details] [diff] [review] [diff] [details] [review]
> Control Panel patch
> 
> >diff --git a/mobile/themes/core/honeycomb/browser.css b/mobile/themes/core/honeycomb/browser.css
> 
> > .options-box {
> >-  -moz-margin-start: 28px;  /* sized based on the 32px addon image */
> >+  -moz-margin-start: @margin_xlarge@;
> 
> This worries me a little since the image is still based on pixels
> 

The preferences in honeycomb has changed, and it doesn't align with the image, but the hbox containing the image. Hence this wouldn't affect the pixel based position we had earlier.

http://cl.ly/0i3p382S0r3L051n2j2L

> >-#appmenu-popup-appcommands richlistitem:hover:active {
> >-  background-color: @color_background_highlight_overlay@;
> >-}
> >-
> 
> Leave this. We need it.
> 

This is not needed anymore. This is a redundant rule. This is covered under richlistitem already.

> >diff --git a/mobile/themes/core/honeycomb/platform.css b/mobile/themes/core/honeycomb/platform.css
> 
> > button:focus > .button-box {
> >-  border: @border_width_tiny@ solid transparent;
> > }
> 
> If we don't need this, remove the entire rule

I guess, this was added because there was a border showing up due to global.css. I would test it and remove it if not needed.

> 
> > toolbarbutton {
> >   min-width: @touch_button_small@ !important; /* primary button size */
> >-  min-height: @touch_button_small@ !important; /* primary button size */
> >+  min-height: @touch_button_tiny@ !important; /* primary button size */
> >   -moz-appearance: none !important;
> >   padding: @padding_xsmall@;
> >   -moz-margin-end: @margin_xxxnormal@;
> > }
> 
> This is causing trouble

I've reverted this back to old one.
> 
> > .toolbarbutton-icon[label]:not([label=""]),
> > .toolbarbutton-icon[type="menu"] {
> >-  -moz-margin-end: @margin_tiny@;
> >+  -moz-margin-start: @margin_xxxnormal@;
> >+  -moz-margin-end: @margin_xnormal@;
> > }
> 
> This could cause trouble
> 
I haven't seen this breaking UI anywhere. I would change it, if we could find any.

> > radio {
> >   -moz-appearance: none;
> >   min-width: @touch_button_small@ !important; /* button size */
> >-  min-height: @touch_button_small@ !important; /* button size */
> >+  min-height: @touch_button_tiny@ !important; /* button size */
> 
> I'm less worried about this one
> 

I've reverted it to old one.

> > richlistitem {
> >   -moz-user-focus: ignore;
> >   min-height: @touch_row@; /* row size */
> >-  padding: @padding_small@;
> >+  padding-top: @padding_xxxnormal@;
> >+  padding-bottom: @padding_tiny@;
> >+  -moz-padding-start: @padding_xlarge@; 
> >+  -moz-padding-end: 0;
> 
> Do we really need this level of tweaks to the padding?
> 

Ibarlow's mockup demands it ;)

> > .prompt-button {
> >   min-width: 33%;
> >+  min-height: @touch_button_small@;
> 
> Doers this need an "!important" ?
> 

I guess, this was overriden by "button", hence had to add the !important.
I'll check it again.

> 
> >+@media (@orientation@: portrait) and (min-width: @tablet_panel_minwidth@) {
> >+  #panel-controls > .panel-row-button {
> >+    min-width: @tablet_panel_controls@ !important;
> >+  }
> >+  
> >+  #panel-items {
> >+    max-width: @tablet_panel_item_width@ !important;
> >+  }
> >+}
> >+
> >+@media (@orientation@: landscape) and (min-width: @tablet_panel_minwidth@) {
> >+  #panel-controls > .panel-row-button {
> >+    min-width: @tablet_panel_controls_wide@ !important;
> >+  }
> >+  
> >+  #panel-items {
> >+    max-width: @tablet_panel_item_width_wide@ !important;
> >+  }
> >+}
> >+
> 
> These rules should be in browser.css since they are specific to UI in
> browser.xul and are not general purpose rules.

Do we do it as a separate bug?
> 
> >+#panel-container {
> >+  background-color: @color_background_settings@ !important;
> >+}
> >+
> > #panel-container-inner {
> >   -moz-box-orient: vertical;
> >+  background-color: transparent !important;
> >+}
> >+
> >+#panel-container-inner > vbox {
> >+  border: @border_width_large@ solid @color_background_active@; 
> >+  box-shadow: 0 0 @shadow_width_tiny@ @shadow_width_medium@ @color_background_button_overlay@;  
> > }
> > 
> > #panel-controls {
> >   -moz-box-orient: horizontal;
> >+  background-color: transparent !important;
> >+  padding: @padding_xxxnormal@ 0 !important;
> > }
> > 
> >+#panel-controls .toolbarbutton-text {
> >+  font-size: @font_snormal@ !important;
> >+}
> >+
> >+#panel-controls > .panel-row-button {
> >+  border-top: @border_width_tiny@ solid @color_background_settings@ !important;
> >+  border-bottom: @border_width_large@ solid !important;
> >+  -moz-border-bottom-colors: @color_background_button@ @color_background_settings@ !important;
> >+  -moz-border-start: 0 solid; 
> >+  -moz-border-end: 0 solid; 
> >+}
> >+
> >+#panel-controls > .panel-row-button[checked="true"] {
> >+  background-color: @color_background_highlight@;
> >+  color: @color_text_default@;
> >+}
> >+
> >+.panel-list {
> >+  background: transparent;
> >+}
> >+
> >+#panel-items richlistitem:not([class="section-header"])[selected="true"] {
> >+  background-color: @color_background_settings@;
> >+}
> >+
> >+#panel-items richlistitem label.normal,
> >+#panel-items richlistitem description.normal {
> >+  color: @color_subtext_settings@;
> >+}
> >+
> >+
> > @media (min-width: @tablet_panel_minwidth@) {
> >   #panel-container-inner {
> >     -moz-box-orient: horizontal;
> >     -moz-box-pack: center;
> >   }
> > 
> >   #panel-items {
> >-    max-width: @tablet_panel_minwidth@;
> >     min-width: 0px !important;
> >   }
> >   
> >   /* This will affect the prefs screen, but not the awesome screen */
> >   #panel-controls {
> >     -moz-box-orient: vertical !important;
> >     -moz-box-align: start;
> >   }
> >   
> >   #panel-controls > .panel-row-button {
> >+    margin: 0;
> >     -moz-box-orient: horizontal;
> >     -moz-box-flex: 0;
> >-    min-width: @tablet_panel_controls@ !important;
> >   }
> > 
> >   #panel-controls .toolbarbutton-text {
> >     display: -moz-box !important;
> >     -moz-box-flex: 1;
> >   }
> >   
> >   #panel-container {
> >     -moz-box-pack: center;
> >-    padding: @padding_xlarge@ 0px;
> >+    padding: @padding_xxxlarge@ 0 @padding_xlarge@ 0;
> >+  }
> >+
> >+  #panel-container-inner > vbox {
> >+    padding: @padding_xlarge@ @padding_xxlarge@;
> >   }
> > }
> 
> These rules should be in browser.css since they are specific to UI in
> browser.xul and are not general purpose rules.
> (I see some of this was already here. That makes me sad)

Do we do it as a separate bug?
This is in other themes as well.
Comment 28 Sriram Ramasubramanian [:sriram] 2011-09-26 12:23:51 PDT
Created attachment 562508 [details] [diff] [review]
Control Panel patch

This patch incorporates the required changes.
The toolbarbutton has its width preserved, fixing the back button issues in previous patch.
Comment 29 Sriram Ramasubramanian [:sriram] 2011-09-26 16:18:26 PDT
Created attachment 562579 [details] [diff] [review]
Control Panel patch

Changed the default richlistitem to have small padding. Reflect the changes in other places using "secondary-richlistitem" class.
Comment 30 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-27 21:37:12 PDT
Comment on attachment 562579 [details] [diff] [review]
Control Panel patch

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

>       item.setAttribute("lastSeconds", Infinity);
>+      item.setAttribute("class", "secondary-richlistitem");

Let's use "panel-listitem" (this is a list item in the panel area)

>diff --git a/mobile/chrome/content/extensions.js b/mobile/chrome/content/extensions.js
>     item.setAttribute("iconURL", aAddon.iconURL);
>+    item.setAttribute("class", "secondary-richlistitem");

Let's use "panel-listitem" (this is a list item in the panel area)

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

>+#tool-back2 image {
>+  -moz-margin-end: 0;
>+}

Do we really need this?

> /* browser tools panel UI   ------------------------------------------------ */
> .panel-header {
>   margin: 0;
>-  padding: @padding_small@ @padding_normal@ @padding_normal@ @padding_normal@;
>-  font-weight: bold;
>-  background-color: @color_background_header@;
>-  color: @color_text_header@ !important;
>-  background-repeat: repeat-x;
>-  background-size: auto 100%;
>+  padding: @padding_small@ @padding_normal@ @padding_xlarge@ @padding_normal@;
>+  border-bottom: @border_width_tiny@ solid @color_text_default@;
>+  background-color: transparent;
>+  color: @color_text_panel_header@ !important;
>+  font-size: @font_snormal@;
> }

Put the rule for .panel-listitem right here, not in platform.css

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

> .button-text,
> .toolbarbutton-text {
>   font-weight: normal;
>-  font-size: @font_snormal@ !important;
>+  font-size: @font_xtiny@ !important;

We need to make sure the prompt buttons are unaffected

> button {
>   -moz-appearance: none;
>   min-width: @touch_button_minwidth@ !important;
>-  min-height: @touch_button_small@ !important; /* button size */
>+  min-height: @touch_button_tiny@ !important; /* button size */

We need to make sure the prompt buttons are unaffected

>+  border-width: 0px;
>+  border-radius: 0px;

0 is unitless, pleased remove the "px"

>+.secondary-richlistitem {
>+  padding-top: @padding_xxxnormal@;
>+  padding-bottom: @padding_tiny@;
>+  -moz-padding-start: @padding_xlarge@; 
>+  -moz-padding-end: 0;
>+}

Renaming to .panel-listitem and moving to browser.css

>+#panel-items richlistitem:not([class="section-header"])[selected="true"] {
>+  background-color: @color_background_settings@;
>+}
>+
>+#panel-items richlistitem label.normal,
>+#panel-items richlistitem description.normal {
>+  color: @color_subtext_settings@;
>+}

Why can't these rules be used with .panel-listitem (secondary-richlistitem) ?

We try to avoid this type of all descendants selector


r-, but getting better
Comment 31 Sriram Ramasubramanian [:sriram] 2011-09-28 09:25:02 PDT
(In reply to Mark Finkle (:mfinkle) from comment #30)
> Comment on attachment 562579 [details] [diff] [review] [diff] [details] [review]
> Control Panel patch
> 
> >diff --git a/mobile/chrome/content/downloads.js b/mobile/chrome/content/downloads.js
> 
> >       item.setAttribute("lastSeconds", Infinity);
> >+      item.setAttribute("class", "secondary-richlistitem");
> 
> Let's use "panel-listitem" (this is a list item in the panel area)

I'll change this.

> 
> >diff --git a/mobile/themes/core/honeycomb/browser.css b/mobile/themes/core/honeycomb/browser.css
> 
> >+#tool-back2 image {
> >+  -moz-margin-end: 0;
> >+}
> 
> Do we really need this?
> 

There is a margin_tiny added to the image. This extends the image size of back button to 48x46.
But still, a '0' is not helping me shrink it to 46x46. There is something else affecting the back button. probably the -moz-calc.
This is needed to make sure the image is 46x46, though.

> > /* browser tools panel UI   ------------------------------------------------ */
> > .panel-header {
> >   margin: 0;
> >-  padding: @padding_small@ @padding_normal@ @padding_normal@ @padding_normal@;
> >-  font-weight: bold;
> >-  background-color: @color_background_header@;
> >-  color: @color_text_header@ !important;
> >-  background-repeat: repeat-x;
> >-  background-size: auto 100%;
> >+  padding: @padding_small@ @padding_normal@ @padding_xlarge@ @padding_normal@;
> >+  border-bottom: @border_width_tiny@ solid @color_text_default@;
> >+  background-color: transparent;
> >+  color: @color_text_panel_header@ !important;
> >+  font-size: @font_snormal@;
> > }
> 
> Put the rule for .panel-listitem right here, not in platform.css

I'll move it here.

> 
> >diff --git a/mobile/themes/core/honeycomb/platform.css b/mobile/themes/core/honeycomb/platform.css
> 
> > .button-text,
> > .toolbarbutton-text {
> >   font-weight: normal;
> >-  font-size: @font_snormal@ !important;
> >+  font-size: @font_xtiny@ !important;
> 
> We need to make sure the prompt buttons are unaffected
> 

I've made necessary changes to the prompt button.

> 
> >+#panel-items richlistitem:not([class="section-header"])[selected="true"] {
> >+  background-color: @color_background_settings@;
> >+}
> >+
> >+#panel-items richlistitem label.normal,
> >+#panel-items richlistitem description.normal {
> >+  color: @color_subtext_settings@;
> >+}
> 
> Why can't these rules be used with .panel-listitem (secondary-richlistitem) ?
> 
> We try to avoid this type of all descendants selector
> 

The richlistitem in preferences don't use ".secondary-richlistitem" (.panel-listitem). Do I add a class there? If I do that, these descendant selectors can be avoided.
Comment 32 Sriram Ramasubramanian [:sriram] 2011-09-28 11:33:32 PDT
Created attachment 563125 [details] [diff] [review]
Control Panel patch

The secondary-richlistitem has been renamed now. And few more fixes have been made.
Comment 33 Sriram Ramasubramanian [:sriram] 2011-09-28 11:37:42 PDT
https://bugzilla.mozilla.org/attachment.cgi?id=563125&action=diff#a/mobile/themes/core/honeycomb/platform.css_sec6

- I would be happy to move these to browser.css. But it is present in Gingerbread also. Hence I would like to make that change as a separate bug instead - which takes care of all themes, to be consistent.
Comment 34 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-28 12:18:03 PDT
Comment on attachment 563125 [details] [diff] [review]
Control Panel patch

Good job. I like minimal changes.
Comment 36 Michael Wu [:mwu] 2011-09-29 01:30:41 PDT
https://hg.mozilla.org/mozilla-central/rev/ed87178d3ee2
Comment 37 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-04 14:02:01 PDT
Comment on attachment 563125 [details] [diff] [review]
Control Panel patch

Get the preferences UI looking better for tablets. It's been on nightly for a bit.
Comment 38 Asa Dotzler [:asa] 2011-10-04 14:44:46 PDT
Comment on attachment 563125 [details] [diff] [review]
Control Panel patch

Naoki, we're taking this late in the game. Can you make sure that we get testing on the newly styled UI bits on all of the tablets we have availbale, as well as a quick check on phones to make sure we're not accidentally busting anything there. (abundance of caution)
Comment 39 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-10-04 16:04:21 PDT
If I'm not mistaken, this should only affect the Honeycomb tablets.

Verified on Toshiba Thrive Android OS 3.1; Samsung Tablet Android OS 3.1
Just to be on the safe side as per suggestion, I also tested on HTC Flyer Android 2.3 and tested on 2.2 Thunderbolt.

Marking qa+ for aurora verification once this lands in Aurora.
Comment 40 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-10-04 16:12:10 PDT
Asa, I sent out an email to the QA-mobile team to take a look at this bug on their own devices once this lands.  I will be testing on my own devices as well.
Comment 41 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-05 20:31:29 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/ad21e8e83543
Comment 42 Aaron Train [:aaronmt] 2011-10-06 07:09:14 PDT
Mozilla/5.0 (Android; Linux armv7l; rv:9.0a2) Gecko/20111006 Firefox/9.0a2 Fennec/9.0a2
Samsung Galaxy Tab 10.01 (Android 2.3.3)
Comment 43 Aaron Train [:aaronmt] 2011-10-06 07:10:31 PDT
(In reply to Aaron Train [:aaronmt] from comment #42)
> Mozilla/5.0 (Android; Linux armv7l; rv:9.0a2) Gecko/20111006 Firefox/9.0a2
> Fennec/9.0a2
> Samsung Galaxy Tab 10.01 (Android 2.3.3)

Whoops, typo, should read: Samsung Galaxy Tab 10.1 (Android 3.1)
Comment 44 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-10-06 15:51:42 PDT
Also verified:
Mozilla/5.0 (Android; Linux armv7l; rv:9.0a2) Gecko/20111006 Firefox/9.0a2
Fennec/9.0a2

Device: HTC Flyer 
OS: Android 2.3

Device: Thunderbolt
OS: Android 2.2

Device: Droid 2 
OS: Android 2.2

Device: Toshiba Thrive
OS Android 3.1


Martijn also took a look.

Note You need to log in before you can comment on or make changes to this bug.