506730 - Update toggles to use a "thumb"

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

Current toggles show captions for both states (on & off) and use color to indicate the active state. We should change to only show the caption for the active state.

Comment 1

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

Attachment: WIP - needs CSS help

This patch introduces a new radio XBL binding and some CSS to make it work. We need some CSS help making the "thumb" look better.

Comment 2

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

Current toggles are ambiguous and look like crap. We need better before we ship.

Comment 3

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

Attachment: patch (hildon)

This patch uses -moz-border-image to create nice looking on/off toggles and radio toggles:
* Updates the CSS in platform.css to use -moz-border-image for radios, checkbox-radios and dark radios
* Adds new images for the various states
* Changes the Search buttons to be dark toggles
* Updates browser.xul, browser-ui.js and browser.css where needed for the Search buttons change.

Sean Martell will create similar images for wince and I will add a new patch for just those changes.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 391605 [details] [diff] [review]
patch (hildon)

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

>-    var container = this._autocompleteNavbuttons;
>+    var container = document.getElementById("search-buttons");

Why this change? I guess it's only used here, but should remove _autocompleteNavbuttons in that case.

>-      var button = document.createElementNS(kXULNS, "toolbarbutton");
>+      var button = document.createElementNS(kXULNS, "radio");

rename to "radio", perhaps?

>diff --git a/chrome/content/browser.css b/chrome/content/browser.css

>+radio {
>+  -moz-binding: url("chrome://global/content/bindings/radio.xml#radio");
> }

This rule's already in xul.css...

>diff --git a/chrome/content/browser.xul b/chrome/content/browser.xul

>+        <radiogroup id="search-buttons" class="toggle-dark" onmousedown="BrowserUI.doButtonSearch(event.target);">
>+        </radiogroup>

Is oncommand not fired for radios? How about onselect instead?

>diff --git a/themes/hildon/browser.css b/themes/hildon/browser.css

>+#autocomplete_navbuttons autorepeatbutton {
>+  display: none;
> }

Kind of hurts the discoverability of scrolling... guess that's not a big deal.

r=me with those addressed.

Comment 5

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

(In reply to comment #4)
> (From update of attachment 391605 [details] [diff] [review])
> >diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js
> 
> >-    var container = this._autocompleteNavbuttons;
> >+    var container = document.getElementById("search-buttons");
> 
> Why this change? I guess it's only used here, but should remove
> _autocompleteNavbuttons in that case.

Removed. I needed to put the <radio>s in a <radiogroup>

> >-      var button = document.createElementNS(kXULNS, "toolbarbutton");
> >+      var button = document.createElementNS(kXULNS, "radio");
> 
> rename to "radio", perhaps?

It felt better to leave this as "button" since they still really look like buttons.

> >+radio {
> >+  -moz-binding: url("chrome://global/content/bindings/radio.xml#radio");
> > }
> 
> This rule's already in xul.css...

True, but it is overridden for some platforms, so I force it for all platforms. Makes the CSS easier to deal with.
 
> >+        <radiogroup id="search-buttons" class="toggle-dark" onmousedown="BrowserUI.doButtonSearch(event.target);">
> >+        </radiogroup>
> 
> Is oncommand not fired for radios? How about onselect instead?

Something is blocking it. onselect did not work either.

> >+#autocomplete_navbuttons autorepeatbutton {
> >+  display: none;
> > }
> 
> Kind of hurts the discoverability of scrolling... guess that's not a big deal.

The rounded button ends give a simple indication that there is more the the left or right.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #5)
> > >+radio {
> > >+  -moz-binding: url("chrome://global/content/bindings/radio.xml#radio");
> > > }
> > 
> > This rule's already in xul.css...
> 
> True, but it is overridden for some platforms, so I force it for all platforms.
> Makes the CSS easier to deal with.

Where is it overridden?

> Something is blocking it. onselect did not work either.

We'll need to get a bug on file for keyboard access, then, at the very least.

Comment 7

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

(In reply to comment #6)
> (In reply to comment #5)
> > > >+radio {
> > > >+  -moz-binding: url("chrome://global/content/bindings/radio.xml#radio");
> > > > }
> > > 
> > > This rule's already in xul.css...
> > 
> > True, but it is overridden for some platforms, so I force it for all platforms.
> > Makes the CSS easier to deal with.
> 
> Where is it overridden?

http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/global.css#49
http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/gnomestripe/global/global.css#53

> > Something is blocking it. onselect did not work either.
> 
> We'll need to get a bug on file for keyboard access, then, at the very least.

Bug 508195

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #7)
> > Where is it overridden?
> 
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/global.css#49
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/gnomestripe/global/global.css#53

Should add a comment that that's what we're doing, then (and perhaps make it explicitly !important).

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Screenshot

New toggles buttons are very nice!

But they have a white background, and this can lead to some style conflict with Fennec (example in the screenshot) or even with extension.

Comment 10

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

(In reply to comment #9)
> Created an attachment (id=392926) [details]
> Screenshot
> 
> New toggles buttons are very nice!
> 
> But they have a white background, and this can lead to some style conflict with
> Fennec (example in the screenshot) or even with extension.

Yes, this is known. We are not using a transparent image for speed issues. This means the toggles can only really be used with white backgrounds for now.

This particular toggle (in the Find panel) is being removed.

Comment 11

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

Windows Mobile was fixed in bug 512007

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

verified in 20090828 winmo build and 20090827 maemo build