Pretty up the Control Panels

VERIFIED FIXED in Firefox 9

Status

defect
P3
normal
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: nhirata, Assigned: sriram)

Tracking

({verified-aurora})

Dependency tree / graph

Firefox Tracking Flags

(firefox9 fixed)

Details

Attachments

(12 attachments, 8 obsolete attachments)

156.75 KB, image/png
Details
114.87 KB, application/zip
Details
91.34 KB, image/png
Details
42.79 KB, image/png
Details
19.90 KB, application/zip
Details
62.46 KB, image/png
Details
48.77 KB, image/png
Details
86.86 KB, image/png
Details
52.35 KB, image/png
Details
56.00 KB, image/png
Details
12.55 KB, image/png
Details
34.24 KB, patch
mfinkle
: review+
Details | Diff | Splinter Review
Posted image 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
Priority: -- → P3
Ian, how should these look in honeycomb?
Keywords: uiwanted
Depends on: 685309
Posted image Font / colour specs (obsolete) —
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.
Assignee

Updated

8 years ago
Assignee: nobody → sriram
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
Posted image 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?
Also, it's no secret that I don't like gradients and large images. Especially large gradient images. :)
Sure, we could do that. I'll mock something up and we can talk about it.
No longer blocks: 653136
Posted image Updated settings UI
Here's a mockup of the new styling on the settings pages. I'll post new assets and specs shortly.
Updated assets to go with the new light background design
Attachment #559177 - Attachment is obsolete: true
Posted image Font / colour specs
Attachment #559171 - Attachment is obsolete: true
Posted patch VWIP (obsolete) — Splinter Review
Very wet in paint patch.
Works in both portrait and landscape mode.

Need to add changes to downloads and addons.
Posted patch WIP (obsolete) — Splinter Review
Attachment #561929 - Flags: review?(wjohnston)
Attachment #561929 - Flags: review?(mark.finkle)
(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.
Assignee

Updated

8 years ago
Attachment #561633 - Attachment is obsolete: true
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.
Attachment #561929 - Flags: review?(wjohnston) → review+
Posted patch Control Panel patch (obsolete) — Splinter Review
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.
Attachment #561929 - Attachment is obsolete: true
Attachment #561929 - Flags: review?(mark.finkle)
Attachment #562141 - Flags: review?(wjohnston)
Attachment #562141 - Flags: review?(mark.finkle)
https://bugzilla.mozilla.org/show_bug.cgi?id=685334 - requires the patch in this to be applied for the above patch to work.
Assignee

Updated

8 years ago
Depends on: 685334
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
Attachment #562141 - Flags: review?(mark.finkle) → review-
Posted patch Control Panel patch (obsolete) — Splinter Review
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
Attachment #562141 - Attachment is obsolete: true
Attachment #562141 - Flags: review?(wjohnston)
Attachment #562190 - Flags: review?(wjohnston)
Attachment #562190 - Flags: review?(mark.finkle)
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)
Attachment #562190 - Flags: review?(mark.finkle) → review-
(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.
Posted patch Control Panel patch (obsolete) — Splinter Review
This patch incorporates the required changes.
The toolbarbutton has its width preserved, fixing the back button issues in previous patch.
Attachment #562508 - Flags: review?(wjohnston)
Attachment #562508 - Flags: review?(mark.finkle)
Attachment #562190 - Attachment is obsolete: true
Attachment #562190 - Flags: review?(wjohnston)
Posted patch Control Panel patch (obsolete) — Splinter Review
Changed the default richlistitem to have small padding. Reflect the changes in other places using "secondary-richlistitem" class.
Attachment #562508 - Attachment is obsolete: true
Attachment #562508 - Flags: review?(wjohnston)
Attachment #562508 - Flags: review?(mark.finkle)
Attachment #562579 - Flags: review?(mark.finkle)
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
Attachment #562579 - Flags: review?(mark.finkle) → review-
(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.
The secondary-richlistitem has been renamed now. And few more fixes have been made.
Attachment #562579 - Attachment is obsolete: true
Attachment #563125 - Flags: review?(mark.finkle)
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 on attachment 563125 [details] [diff] [review]
Control Panel patch

Good job. I like minimal changes.
Attachment #563125 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/ed87178d3ee2
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Keywords: uiwanted
Target Milestone: --- → Firefox 10
Depends on: 690849
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.
Attachment #563125 - Flags: approval-mozilla-aurora?
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)
Attachment #563125 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Status: RESOLVED → VERIFIED
Whiteboard: qa+
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.
Depends on: 690870
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)
Keywords: verified-aurora
Whiteboard: qa+
(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)
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.
You need to log in before you can comment on or make changes to this bug.