682412 - Pretty up the Control Panels

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: 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)]

Ian, how should these look in honeycomb?

Comment 2

[Matt Brubeck (:mbrubeck)]

Ian provided this mock-up:
https://secure.flickr.com/photos/61892693@N03/6101214910/in/photostream/lightbox/

Comment 3

[Ian Barlow (:ibarlow)]

Attachment: 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)]

Attachment: Graphic assets for control panels

Comment 5

[Ian Barlow (:ibarlow)]

Attachment: New backgrounds to hopefully fix our banding issues

Comment 6

[Gabriela [:gaby2300]]

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?)]

Attachment: 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?)]

Also, it's no secret that I don't like gradients and large images. Especially large gradient images. :)

Comment 9

[Ian Barlow (:ibarlow)]

Sure, we could do that. I'll mock something up and we can talk about it.

Comment 10

[Ian Barlow (:ibarlow)]

Attachment: 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)]

Attachment: Graphic assets for control panels

Updated assets to go with the new light background design

Comment 12

[Ian Barlow (:ibarlow)]

Attachment: Font / colour specs

Comment 13

[Ian Barlow (:ibarlow)]

Attachment: Margins and spacing measurements

Comment 14

[Ian Barlow (:ibarlow)]

Attachment: Font sizes, relative to the rest of our UI

Comment 15

[Sriram Ramasubramanian [:sriram]]

Attachment: 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)]

Attachment: mockup for downloads

Comment 17

[Ian Barlow (:ibarlow)]

Attachment: mockup for addons

Comment 18

[Ian Barlow (:ibarlow)]

Attachment: specs for buttons

Comment 19

[Sriram Ramasubramanian [:sriram]]

Attachment: WIP

Comment 20

[Sriram Ramasubramanian [:sriram]]

(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)]

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]]

Attachment: 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]]

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?)]

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]]

Attachment: 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?)]

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]]

(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]]

Attachment: 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]]

Attachment: 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?)]

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]]

(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]]

Attachment: Control Panel patch

The secondary-richlistitem has been renamed now. And few more fixes have been made.

Comment 33

[Sriram Ramasubramanian [:sriram]]

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?)]

Comment on attachment 563125 [details] [diff] [review]
Control Panel patch

Good job. I like minimal changes.

Comment 35

[Wesley Johnston (:wesj)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/ed87178d3ee2

Comment 36

[Michael Wu [:mwu]]

https://hg.mozilla.org/mozilla-central/rev/ed87178d3ee2

Comment 37

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

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]]

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)]

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)]

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?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad21e8e83543

Comment 42

[Aaron Train [:aaronmt]]

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]]

(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)]

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.