996036 - Confusing Focus Ring UI + Behavior in new in content prefs.

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[John Gruen [:jgruen]]

Attachment: focus-vs-enabled

two related issues: 

Current
--------
Input focus ring is very difficult to distinguish from the enabled radio buttons and check boxes. (see attachment).

Expected
--------
Recommend removing the fuzzy blue ring from radio buttons and check boxes and using a 1 px border in the same blue instead. 

Current
--------
Clicking outside of focused input does not remove focus from element.

Expected
--------
Clicking outside of focused input should remove focus from element.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: NoGlow.png

Michael, what do you think to this? The border is still blue but there is no glow.

Comment 2

[Michael Maslaney [:mmaslaney] ([e-mail])]

Richard, I like this direction. Thanks for tackling the issue.

An official style guide is in the works that will further address this issue, and your solution is closer to what will be officially documented.

Comment 3

[Richard Marti (:Paenglab)]

(In reply to mmaslaney from comment #2)
> Richard, I like this direction. Thanks for tackling the issue.
> 
> An official style guide is in the works that will further address this
> issue, and your solution is closer to what will be officially documented.

Okay, I'm waiting for this guide until I change styles.

Michael, where do you announce the new style guide? In this bug or bug 738796? Only to be sure I'm not missing this announcement.

Comment 4

[Michael Maslaney [:mmaslaney] ([e-mail])]

I'll post phase 1 of the style guide in this bug.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: Bug996036.patch

I followed the Project Chameleon stylings. The glow is only during the radio/checkbox clicking. I also removed the showing of the checkmark on hovering on unchecked checkmarks.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8425225 [details] [diff] [review]
Bug996036.patch

>+checkbox:not([disabled="true"]):hover .checkbox-check {
>+  border-color: #0095dd;
>+}

Use the child selector (here and in similar rules involving .checkbox-check / .radio-check).

> .checkbox-check[checked] {
>-  border-color: #0096dc;
>-  background-image: -moz-image-rect(url("chrome://browser/skin/preferences/in-content/check.png"), 0, 30, 10, 15),
>+  border-color: #0095dd;
>+  background-image: url("chrome://browser/skin/preferences/in-content/check.png"),
>                     /* !important needed to override toolkit !important rule */
>                     linear-gradient(#ffffff, rgba(255,255,255,0.8)) !important;
>-  box-shadow: 0 0 2px 2px rgba(0,150,220,0.35), inset 0 0 2px 0 #0096dc;
> }

Why are you still changing the border color for the checked state?

>-@media (min-resolution: 2dppx) {
>-  checkbox:not([checked]):hover .checkbox-check {
>-    background-size: 15px 10px, auto;
>-    background-image: -moz-image-rect(url("chrome://browser/skin/preferences/in-content/[e-mail]"), 0, 30, 20, 0),
>-                      linear-gradient(#ffffff, rgba(255,255,255,0.8));
>-  }
> 
>   .checkbox-check[checked] {
>     background-size: 15px 10px, auto;
>-    background-image: -moz-image-rect(url("chrome://browser/skin/preferences/in-content/[e-mail]"), 0, 60, 20, 30),
>+    background-image: url("chrome://browser/skin/preferences/in-content/[e-mail]"),
>                       linear-gradient(#ffffff, rgba(255,255,255,0.8)) !important;
>   }
>-}

Why did you remove the media query?

Comment 9

[Richard Marti (:Paenglab)]

Attachment: Bug996036.patch

(In reply to Dão Gottwald [:dao] from comment #8)
> Comment on attachment 8425225 [details] [diff] [review]
> Use the child selector (here and in similar rules involving .checkbox-check
> / .radio-check).

Done

> > .checkbox-check[checked] {
> >-  border-color: #0096dc;
> >-  background-image: -moz-image-rect(url("chrome://browser/skin/preferences/in-content/check.png"), 0, 30, 10, 15),
> >+  border-color: #0095dd;
> 
> Why are you still changing the border color for the checked state?

I changed the border-color according to Project Chameleon.

> >-@media (min-resolution: 2dppx) {
> >-}
> 
> Why did you remove the media query?

This was by accident through testing on a non HiDPI system.

Comment 10

[Dão Gottwald [:dao]]

(In reply to Richard Marti (:Paenglab) from comment #9)
> > > .checkbox-check[checked] {
> > >-  border-color: #0096dc;
> > >-  background-image: -moz-image-rect(url("chrome://browser/skin/preferences/in-content/check.png"), 0, 30, 10, 15),
> > >+  border-color: #0095dd;
> > 
> > Why are you still changing the border color for the checked state?
> 
> I changed the border-color according to Project Chameleon.

I'm not seeing that in attachment 8424811 [details].
Changing the border color for the checked state means that it can still be confused with a focus ring. I don't see why we would want that.

Comment 11

[Richard Marti (:Paenglab)]

(In reply to Dão Gottwald [:dao] from comment #10)
> (In reply to Richard Marti (:Paenglab) from comment #9)
> > > Why are you still changing the border color for the checked state?
> > 
> > I changed the border-color according to Project Chameleon.
> 
> I'm not seeing that in attachment 8424811 [details].
> Changing the border color for the checked state means that it can still be
> confused with a focus ring. I don't see why we would want that.

Not in this attachment but in http://people.mozilla.org/~jgruen/chameleon/ have the selected checkmarks and radios a blue border. For this I'm asking Michael for ui-r to know if this is okay. My attachment 8416908 [details] with the blue border have f+ from him.

Comment 12

[Michael Maslaney [:mmaslaney] ([e-mail])]

Attachment: Preferences - [e-mail]

Radio and Check-box Interaction Rules:

• Elements that are "on" will display a blue glyph, while the rule remains gray (#C1C1C1).

• Elements that are active via toggling will have the highlighted blue rule (#0095DD)

This should solve the visibility problem, when toggling though form elements.

Comment 13

[Michael Maslaney [:mmaslaney] ([e-mail])]

Comment on attachment 8425294 [details] [diff] [review]
Bug996036.patch

Radio and Check-box Interaction Rules:

• Elements that are "on" will display a blue glyph, while the rule remains gray (#C1C1C1).

• Elements that are active via toggling will have the highlighted blue rule (#0095DD)

This should solve the visibility problem, when toggling though form elements.

Comment 14

[Richard Marti (:Paenglab)]

Attachment: Bug996036.patch

Okay, this should do it like you want. I removed the glow on :active and the rule is blue on :hover (or do you want it only blue on :active? Then it would have no feedback when the mouse could switch the button).

On attachment 8425294 [details] [diff] [review] the checkmarks are looking narrower than the ones we have now. If you want to change to them, please could you create new glyphs in both resolutions?

Comment 15

[Michael Maslaney [:mmaslaney] ([e-mail])]

Richard, Let's use the blue rule on :hover. That should provide sufficient feedback.

Thanks for the quick patch. I'll send you the updated glyphs.

Comment 16

[Richard Marti (:Paenglab)]

Comment on attachment 8425545 [details] [diff] [review]
Bug996036.patch

This is still with the old glyphs but the CSS code shouldn't change with new glyphs.

Comment 17

[Richard Marti (:Paenglab)]

Attachment: Bug996036.patch

Now with the updated checkbox glyph from bug 1014208.

Comment 19

[Asaf Romano (gone)]

Is proper focus ring feedback tracked somewhere? I filed bug 1016168, but I would expect a patch to address both issues.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8428081 [details] [diff] [review]
Bug996036.patch

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

r=me with the following change made.

::: browser/themes/shared/incontentprefs/preferences.css
@@ +445,5 @@
>    background-image: linear-gradient(#ffffff, rgba(255,255,255,0.80));
>    box-shadow: 0 1px 1px 0 #ffffff, inset 0 2px 0 0 rgba(0,0,0,0.03);
>  }
>  
> +radio:not([selected]):hover > .radio-check {

This should be:
radio:not([disabled="true"]):hover > .radio-check {
to match the checkbox behavior.

Comment 21

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in

Comment addressed.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/7e955351011a

Comment 23

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/7e955351011a

Comment 25

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Mac OSX 10.9.2, Windows 7 64bit and Ubuntu 13.10 32bit using latest Nightly 32.0a1 (buildID: 20140603030220).