1590432 - Filters button colors could be better

Status: RESOLVED FIXED

(DevTools :: Shared Components, task, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

At the moment, we have a transparent background when the filter is off, and a grey background when it's on.
But grey can also convey the "disabled" meaning, which can be confusing for the user.

Let's try to see what we can do.

Comment 1

[Victoria Wang [:victoria]]

Here's what I have for dark mode and hover/active states! Florens, any thoughts?

https://www.figma.com/file/ZEBuHEF9Uq3CXYk2NL2pyH/panelfilters_updated2019

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Should we try to implement this for the 72 cycle?

Comment 3

[Florens Verschelde :fvsch]

I like the design overall, color choices look great in both themes.
I have two small concerns, which are not blockers:

1. Size of the clickable area.

Those buttons are 16px-high, which poses a risk of misclicks. I'm not sure if there are guidelines for mouse-driven interfaces, but for touchscreens Apple recommends a minimum height of 44pt and Material Design of 48pt. For mouse-driven desktop UIs I remember reading something about an imprecision of up to 10px when targeting a specific element (e.g. users may target the center of a checkbox and routinely end up registering a click 10px away from that), so as a rule I try to make buttons at least 20px tall (and often around 30px or more when there's enough room).

Two possible solutions:

1. Make the buttons a bit taller (18px or up to 20px)
2. Keep the button background at 16px but make the actual button larger see green hitboxes in:
   https://www.figma.com/file/Y32o4a9Zw5vtyO23tpjjjG/Toggle-Buttons-with-large-hitboxes

That second option can be implemented as:

<button class="devtools-togglebutton">
  <span class="devtools-togglebutton-background">Label</span>
</button>

2. Hover style for "off" button might be confusing?

Using a background color for the off+hover style is a bit confusing IMO. Since the color is the same as the "on" state and the only difference is a subtle change in opacity, users may interpret the style as "on" instead of "off". They have to rely on their short term memory, as in "oh this button was off before and I haven't clicked it yet".

Some possible solutions:

1. No hover style (on/off/active might be enough)
2. Don't use a background-color change for hover styles, but a different signal, e.g. make the text color more contrasted on hover?
3. Use a subtle gray background-color for the off+hover style, to keep a distinction between blue=on and gray/transparent=off.

Comment 4

[Victoria Wang [:victoria]]

Hi, sorry for the delay on this!

1. Size of the clickable area.
Two possible solutions:

1. Make the buttons a bit taller (18px or up to 20px)
2. Keep the button background at 16px but make the actual button larger see green hitboxes in:
   https://www.figma.com/file/Y32o4a9Zw5vtyO23tpjjjG/Toggle-Buttons-with-large-hitboxes

I'm in favor of the hitbox fix either way. Visually, 18px seems fine, but I have a slight preference for 16px because it's more distinct from the other types of buttons, and seems more like a "tag" that's been selected.

2. Hover style for "off" button might be confusing?

Using a background color for the off+hover style is a bit confusing IMO. Since the color is the same as the "on" state and the only difference is a subtle change in opacity, users may interpret the style as "on" instead of "off". They have to rely on their short term memory, as in "oh this button was off before and I haven't clicked it yet".

Some possible solutions:

1. No hover style (on/off/active might be enough)
2. Don't use a background-color change for hover styles, but a different signal, e.g. make the text color more contrasted on hover?
3. Use a subtle gray background-color for the off+hover style, to keep a distinction between blue=on and gray/transparent=off.

Ah, good point! I think 3 is the best solution. We can use the same gray hover style as other buttons this way. (Currently, I think there's a regression in light theme that is giving the buttons an overly pale gray hover background - maybe someone could take a look at that?)

Comment 5

[Dhruvi Butti]

Hey! I would like to work on this issue.
Thank you.
Dhruvi

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Sure Dhruvi, you can go ahead, I assigned the bug to you

Comment 7

[Patrick Brosset <:pbro>]

Hi Dhruvi, the bug is assigned to you. Please let us know if you are having any problems or questions regarding fixing it.

Comment 8

[Patrick Brosset <:pbro>]

I think it's probably safe to unassign this bug now and let other people try to fix it.

Comment 9

[Florens Verschelde :fvsch]

I can pick it up, especially because managing the different combinations of :hover/:active/:focus states and aria-pressed is proving a bit difficult for people who don't know our CSS codebase well. Buttons are small but pack a lot of complexity.

I'd also like to move the .devtools-togglebutton styles to a separate stylesheet, in line with Belén's presentation on modularizing the CSS codebase.

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Florens, should I unassign you?

Comment 12

[Florens Verschelde :fvsch]

Yeah, sounds reasonable. I need to unassign myself from a bunch of bugs, will do that soon.

Comment 13

[Clinton]

Hi, I'd love to work on this, can it please be assigned to me?

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Hello Clinton,

Thanks for offering help, I assigned the bug to you.
Note that it might not be the easiest bug.
There was some discussion about button size here, but let's focus on colors only for now (see Comment 1 and Comment 3)
You'll probably need https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox to inspect devtools and tweak styling.

Comment 15

[Clinton]

I'll take the challenge, if I get stuck I'll ask for help here, just to clarify: I'm only working on the Colors for now right?

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Clinton from comment #15)

just to clarify: I'm only working on the Colors for now right?

Yes :)

Comment 17

[Clinton]

For the Toggle off hover state, the color from Figma is #3C3C3D but this isn't in the color pallet what should I do about this? Is there an alternative for it?

Comment 18

[Clinton]

The grey-70 variable (#38383d) is the closest color to this, do you think I should use this?

Comment 19

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Clinton from comment #18)

The grey-70 variable (#38383d) is the closest color to this, do you think I should use this?

yes, I think it's close enough to work 👍

Comment 20

[Clinton]

Attachment: Bug 1590432 - Filters button colors could be better r=nchevobbe

Comment 21

[Clinton]

Hi, I made the changes, I don't know if this is by design but my changes don't reflect on my local build, I suppose that's why you told me to use The Browser toolbox, so I used that and tweaked the styles there before changing the style declarations, I'm pretty sure I made some mistakes especially regarding the Off-hover state so it would be super useful if you could point them out to me.

Clinton.

Comment 22

[Nicolas Chevobbe [:nchevobbe]]

as discussed on Element, Colin won't have the time to take care of this

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

Note that this is an accessibility issue:

Color contrast is not sufficient for indication of active/pressed state of toggle buttons (such as “Errors”, “CSS”, and others) - the light gray background color has 1.2:1 color contrast ratio against white background or the page and of the non-pressed buttons while 2:1 or higher is expected for active UI

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, the accessibility severity is higher, .
:Honza, could you consider increasing the severity?

For more information, please visit BugBot documentation.

Comment 25

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1590432 - [devtools] Update style for active toggle buttons. r=#devtools-reviewers.

The pressed/active element need to have at least
a 2:1 contrast against the background.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c4e846a4ebf
[devtools] Update style for active toggle buttons. r=devtools-reviewers,ochameau.

Comment 27

[Cristian Tuns]

Backed out for causing dt failures in browser_parsable_css.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | custom property --grey-10-a15 is not referenced -

Comment 28

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Cristian Tuns from comment #27)

Backed out for causing dt failures in browser_parsable_css.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | custom property --grey-10-a15 is not referenced -

removed now unused css variables

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07f9dcdde980
[devtools] Update style for active toggle buttons. r=devtools-reviewers,ochameau.

Comment 30

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/07f9dcdde980