-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable alpha on Block Inspector Color ToolsPanel #37731
Conversation
Size Change: +146 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
Took this one for a quick spin, and it works as intended. Before: After: Honestly, this seems fine to me. The picker is designed to manage the prominence of both the hue and alpha selectors, and it can easily survive being visible forever. Are there any reasons why we would not want to surface alpha as an option? Are there any cases where changing the opacity of a color would actually not work? I can't think of any, can you @jameskoster? Otherwise, this one seems fine to me! Nice work. |
Allowing alpha for backgrounds makes sense. However, I'm not so sure about the text & link colors. Using semi-transparent text would make the text a lot harder to read, and would be an a11y issue. |
Good thought. Do you think we could handle that with the same type of accessibility notice you receive if you choose badly contrasting colors? A semitransparent text color when blended into the background feels very comparable in that it's about comparing two colors for contrast. If it's technically tricky to compare the contrast of semitransparent text on a background, we could just blanket output a warning as soon as the opacity isn't 100%. Someting like "Be careful with transparent text, contrast is likely going to be insufficient." |
Comparing the contrast for semi-transparent text would be extremely difficult... Theoretically, if alpha = 0.6 then we'd mix the text-color with the background-color with a ratio of 60-40 to get a hex value for the text color, which can then be compared against a hex for the background. But if the background itself has a transparency - or is an image, then any computation will be invalid. |
Yeah, text is the only thing I can think of that might be tricky. A blanket warning seems like a sensible idea. |
Or you just check if a text block has a text color with an alpha lower than 1. You don't have to check against background's alphas for that to trigger an accessibility warning, because every text with an alpha can be considered as less readable than normal text. |
Thanks for the feedback, folks. I lean towards offering the toolbox to allows users make their own decisions, so a blanket warning when the alpha for text moves beyond I'll take a shot at it. 👍 Later, we still might want to look at building alpha support into block supports, so we can control where the alpha control appears (e.g., only for backgrounds). In a block's block.json we could have something like (example only): "supports": {
"color": {
"background" : {
"opacity" : true,
},
"text" : true,
}
} Whether alpha/opacity/whatever is disabled or enabled) by default is another question. 😄 |
dd7085d
to
281effa
Compare
@@ -71,16 +83,19 @@ function ContrastChecker( { | |||
const colordTextColor = colord( textColor || fallbackTextColor ); | |||
const hasTransparency = | |||
colordBackgroundColor.alpha() !== 1 || colordTextColor.alpha() !== 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we reduce the minimum alpha threshold here where __experimentalEnableAlphaChecker === true
? Say to 0.8
, so the warning will only show when the transparency of either text or background falls below this?
Good idea, thanks. This might reduce notice fatigue too. 😄 |
…r component from the color tools panel.
… color has transparency. Adding tests. Updating README.md to reflect the reality that colord does not check the contrast of colors with alpha transparency in our implementation.
only show the transparency warning where the text color alpha is < 1 ensure the readability/contrast warning takes precedence over the transparency warning
9f136d3
to
5f667f4
Compare
I've updated the code and the screencast in the test description.
Removing the "for people" part would be good. It seems redundant. To my ear, something along the lines of "Transparent text may be hard to read." rings better. We don't mention contrast sufficiency elsewhere I guess because it's leaning towards the technical too much? 🤷 I dunno, I'm just shouting at the forest. 😄 For now I've left it at Also, I noticed the contrast checker doesn't take link colors into account. I see there's an issue up already. I think I have a way forward on this. I'll add another PR. |
This tested well for me. It will be interesting to see what this means over time for the likes of the Cover block explicit dimRatio setting - I suspect there might still be the need for some blocks to expose this option at a higher level still? Is it likely to be changeable enough to warrant giving an |
Thanks for testing @glendaviesnz
The Cover Block is an interesting one - it explicitly allows background images and the opacity layer. Though some sites out there do some rough calculations to guesstimate the readability of transparent backgrounds - see for example https://contrast-ratio.com/ which seems to be doing something similar to what was described in this comment - it's not accurate. The color library we use appears to flat out ignore alpha for testing readability and contrast too. Throw in background images, RGBA, opacity and unlimited layers of semi-transparent, nested blocks and 😱 Ultimately, we could try to extend the colord library with a fudge and at least attempt to advise/warn users of potentially unreadable color mixes, but it runs the risk of become noisy.
That's a good question. I added the I think I agree that we can ditch it. |
I appreciate the input, everyone! 🙇 |
I am not seeing this alpha option in block editor for group or other blocks that support color, using wordpress 5.9 RC3 and gutenberg 12.4.1, will this feature be available in the future release ? or I have to do some changes in theme.json to enable it ? |
HI @uniquesolution ! This change was merged a few days ago so, unless something catastrophic occurs like a worldwide internet failure or coffee shortages, it should be part of Gutenberg 12.5. 👍 |
Resolves #18095
Description
This PR add alpha controls to Color Picker panels in the Block Inspector.
We're doing this by passing enableAlpha=true to the underlying ColorPicker component from the color tools panel.
Where text color alpha transparency goes below a minimum threshold (currently 1), we'll also show a warning about readability where the text is otherwise "readable" according to the colord library.
Changes to background color transparency are ignored due to the difficultly in accounting for multiple alpha layers or image backgrounds and so on.
See: #18095 (comment)
Jan-20-2022.13-28-31.mp4
Considerations
Here we're enabling alpha controls for all blocks and all colors.
Is the availability to adjust alpha something we'd want to enable at a block level, e.g., via color supports and/or for certain colors, e.g., background, text, link?
Or is it okay to leave it on as a permanent citizen of the color panel?
How has this been tested?
Insert any block that has color support, for example a group block!
Select a background color, or any color. Click on the custom color picker, then play with the alpha slider.
The colors in the editor should match the frontend.
Take note of the warning.
The warning should show where only the text color has transparency AND the color panel does not show any contrast warnings otherwise.
Types of changes
Enabling an existing feature.
Checklist:
*.native.js
files for terms that need renaming or removal).