WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.51k stars 4.2k forks source link

`ColorPalette`: improve color name readability #48119

Closed ciampo closed 1 year ago

ciampo commented 1 year ago

While working on #47373 and #47476, we realised a few additional challenges related to the text contrast for the text displayed on top of the chosen color in ColorPalette:

When discussing this, @mirka made a good point:

[...] I think the alpha issue may require some design changes that should get proper input from the design team. We probably do in fact need to keep the checkered background showing through to signal that the color is semi-transparent. In this case we will always (I think?) run into readability issues at some point in the gradient scale if we put text on top of that checkered pattern. I think it's inevitable that we modify the design so it accommodates this case — for example, placing the text outside of the swatch, or placing the text on a pill-style solid background. The current design really does not take into account things like CSS variables (i.e. longer strings) and transparency.

@brookewp already proposed an alternative design, which would solve the issue by moving the text below the color swatch:

Let's discuss potential alternatives!

cc @jasmussen @jameskoster

espenmn commented 1 year ago

Just an idea: is it lpossible to use 'math' for the color (from the background color, 180 degrees/invert maybe, and then 'round up' to white or black?). I did that in a pluginin for a CMS with python, maybe the same is possible in php ?

jameskoster commented 1 year ago

I'd agree with keeping the checkered background – it's a fairly well established convention for indicating transparency and works better than what we have now.

In terms of the color name, I have a slight preference for the pill overlay idea, mostly because the relationship between swatch and name seems clearer this way.

Screenshot 2023-02-16 at 12 00 23

In terms of the value, what is the purpose of display it when using a theme/default color, can't we just display the color name alone? If so, then there's really no need to display "Custom" when using a custom color either – the presence of the value (and absence of a checkmark in the swatches below) can indicate the custom nature on its own. Just a thought.

ciampo commented 1 year ago

Just an idea: is it possible to use 'math' for the color (from the background color, 180 degrees/invert maybe, and then 'round up' to white or black?).

Yup that is possible:

Other variables to keep in mind:

All of this is (in theory) possible, but what we're really discussing here is that it would impossible to have text with enough contrast in all of its letters if it renders on top of a white/black checkered pattern.

I'd agree with keeping the checkered background – it's a fairly well established convention for indicating transparency and works better than what we have now.

Agreed — a checkered pattern would be very useful in understanding the transparency level of a color

In terms of the color name, I have a slight preference for the pill overlay idea, mostly because the relationship between swatch and name seems clearer this way.

Is there a scenario in which this UI could be confusing? I.e. what if the selected color is exactly the same as the pill's background color?

In terms of the value, what is the purpose of display it when using a theme/default color, can't we just display the color name alone? If so, then there's really no need to display "Custom" when using a custom color either – the presence of the value (and absence of a checkmark in the swatches below) can indicate the custom nature on its own. Just a thought.

I think it would still be useful for a user to know that a color is part of a theme palette vs custom. We also have to keep in mind the case for when a color's value is a CSS variable

jameskoster commented 1 year ago

Is there a scenario in which this UI could be confusing? I.e. what if the selected color is exactly the same as the pill's background color?

I hope the stroke around the pill should structure the UI in a way that avoids confusion. There may be other options to try with the presentation too, perhaps a dark background with light text would work better? Or attaching the pill to the edge of the container?

That's not to rule out displaying the name/value outside of the swatch. That can work too, but it would be good to ideate on the presentation some more.

I think it would still be useful for a user to know that a color is part of a theme palette vs custom

My point was to communicate this via the value as presented. IE when it's part of the theme palette then the name would be displayed e.g. 'Primary' or 'Secondary', additionally one of the swatches in the picker would be checked. Whereas a custom color would always display the value. But perhaps I'm missing a reason for the underlying value to be displayed when a theme color is chosen?

ciampo commented 1 year ago

I hope the stroke around the pill should structure the UI in a way that avoids confusion. There may be other options to try with the presentation too, perhaps a dark background with light text would work better? Or attaching the pill to the edge of the container?

Another thing to keep in mind about this, is the interaction with Theming. In fact, the pill's background color would also need to adapt to whatever "background color" is chosen via theming. For example, as a rule we could decide that for the pill we invert background color and text color (so that if the background is normally light, the pill's background is dark — and viceversa).

That's not to rule out displaying the name/value outside of the swatch. That can work too, but it would be good to ideate on the presentation some more. My point was to communicate this via the value as presented. IE when it's part of the theme palette then the name would be displayed e.g. 'Primary' or 'Secondary', additionally one of the swatches in the picker would be checked. Whereas a custom color would always display the value. But perhaps I'm missing a reason for the underlying value to be displayed when a theme color is chosen?

Absolutely! Personally, I'm not opposed to any of the solutions that were discussed, in principle. We just need to make sure that:

Especially for the latter, I trust that you and the rest of the design/product folks know what's necessary

jameskoster commented 1 year ago

Perhaps the question around whether we always need to display the value is a separate thing. I think it might influence our direction here in the long run (whether to use a pill or not), but in the short term it may be simpler to just display the values outside the swatch in order to fix the issue at hand.

Using roughly the same positioning we have now get's a little funky with long names / css vars:

Screenshot 2023-02-17 at 13 31 36

It might be better to split on to separate rows:

Screenshot 2023-02-17 at 13 33 45

What do you think?

ciampo commented 1 year ago

I think it's a great solution, at least for the short term. Thank you for working on this iteration!

We'll let you know as soon as we get to implement this suggestion

joedolson commented 1 year ago

While looking at https://github.com/WordPress/gutenberg/pull/51197, I saw the manipulation of the hex color codes and wanted to follow up here.

However, with NVDA, this mostly reads OK, except that the initial hyphen (#-0) is read as "minus", so the resulting text is "number minus 0 0 0 0 0 0". This doesn't occur if the first character is a letter, e.g. in #decaf0. I haven't checked the same behavior in VoiceOver.

In JAWS, all punctuation is read, so the reading is 'number d dash e dash c dash a dash f dash 0' or 'number minus zero dash zero dash zero dash zero dash zero dash zero'.

Both screen readers are configured using default settings. Given these experiences, I think that it is better to remove these hyphens and leave the values as written.

Codepen for quick testing: https://codepen.io/joedolson/pen/JjevpZB

alexstine commented 1 year ago

Let's revert this so the user gets the raw hex value announced. This would be better for accessibility than not knowing how each screen reader might announce this differently.

ndiego commented 1 year ago

Just following up here. What needs to be done for 6.3? Is this the line that needs to be reverted?

joedolson commented 1 year ago

Yes, that's one of the lines that needs to be reverted. The same code was also re-used in 51197, and that should be reverted as well.

I don't know if this has been done elsewhere, but I know of those two instances.

t-hamano commented 1 year ago

I see. We can rely on these search results to tell us what needs to be fixed.

Looking at the results, the two things that should be fixed are ColorPalette and BorderControl.

I would like to work on these.

t-hamano commented 1 year ago

Codepen for quick testing: https://codepen.io/joedolson/pen/JjevpZB

I have one concern where I have tested on this page.

There is a link on this page that contains the HEX value #decaf0 as the aria-label. If you focus on this link using a screen reader, it will read as follows:

The string decaf, excluding numerical values, is read out as if it were an English word. What do you think about this?

t-hamano commented 1 year ago

If I separate them with a space instead of a hyphen, it seems to read as intended.

joedolson commented 1 year ago

An important thing to remember is that what a screen reader user should hear is what they're accustomed to, which is more likely to be something like hashtag, decaf, zero than not; because that's how most hashtags around the web will be read. Manipulating how those values are read is of dubious value.

My general opinion is that we should avoid manipulating values in order to tweak how they will be read by a screen reader, so that the user doesn't get something unexpected. However, if @alexstine has a different opinion here, I can go by it.

One distinction to note is that these are viable because these strings are only part of the label; if the color name was the entire label for a control, then we can't do this, because it would create a violation of WCAG 2.5.3: Label in Name, because the visible label (#decaf0) would not be the same as the accessible name (# d e c a f 0). That means that if we use this value in a control by itself at any point, it would need to be pronounced differently than it is in larger strings. So, for overall consistency, I'd still be inclined to leave it unmanipulated.

alexstine commented 1 year ago

I think users will understand the context since they are interacting with a color picker. If this value were to be read in a confusing context, I would support figuring out how to make it more clear. Under this situation, users can simply read the hex with character navigation keys.