Closed Bug 1590432 Opened 5 years ago Closed 27 days ago

Filters button colors could be better

Categories

(DevTools :: Shared Components, task, P2)

task

Tracking

(Accessibility Severity:s2, firefox128 fixed)

RESOLVED FIXED
128 Branch
Accessibility Severity s2
Tracking Status
firefox128 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

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.

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

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

Flags: needinfo?(florens)

Should we try to implement this for the 72 cycle?

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.
Flags: needinfo?(florens)

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

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

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

Assignee: nobody → dhruvibutti9477
Status: NEW → ASSIGNED

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

Flags: needinfo?(dhruvibutti9477)

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

Assignee: dhruvibutti9477 → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(dhruvibutti9477)

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.

Assignee: nobody → florens

Florens, should I unassign you?

Flags: needinfo?(florens)

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

Flags: needinfo?(florens)
Assignee: florens → nobody

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

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.

Assignee: nobody → clintonadeleke
Status: NEW → ASSIGNED

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?

(In reply to Clinton from comment #15)

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

Yes :)

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?

Flags: needinfo?(victoria)
Flags: needinfo?(nchevobbe)

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

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

Flags: needinfo?(nchevobbe)

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.

Flags: needinfo?(victoria)

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

Assignee: V1KT1M → nobody
Status: ASSIGNED → NEW
Component: CSS and Themes → Shared Components
Severity: normal → S3

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

Severity: S3 → S2
Keywords: access
Accessibility Severity: --- → s2
Severity: S2 → S3
Priority: P3 → P2

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.

Flags: needinfo?(odvarko)
Flags: needinfo?(odvarko)

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

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Attachment #9368624 - Attachment description: Bug 1590432 - [devtools] Fix contrast for active toggle buttons. r=#devtools-reviewers. → Bug 1590432 - [devtools] Update style for active toggle buttons. r=#devtools-reviewers.
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c4e846a4ebf
[devtools] Update style for active toggle buttons. r=devtools-reviewers,ochameau.

Backed out for causing dt failures in browser_parsable_css.js

Flags: needinfo?(nchevobbe)

(In reply to Cristian Tuns from comment #27)

Backed out for causing dt failures in browser_parsable_css.js

removed now unused css variables

Flags: needinfo?(nchevobbe)
Attachment #9248028 - Attachment is obsolete: true
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/07f9dcdde980
[devtools] Update style for active toggle buttons. r=devtools-reviewers,ochameau.
Status: ASSIGNED → RESOLVED
Closed: 27 days ago
Resolution: --- → FIXED
Target Milestone: --- → 128 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: