Separator: disable the contrastChecker via block.json #43357
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ramonjd
added
[Type] Enhancement
ramonjd
requested review from
ellatrix,
ajitbohra,
ajlende,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Size Change: +1.54 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
ajlende
added
the
[Focus] Accessibility (a11y)
| @@ -292,7 +292,7 @@ const getLinkColorFromAttributeValue = ( colors, value ) => { | |||
| export function ColorEdit( props ) { | |||
| const { name: blockName, attributes } = props; | |||
| // Some color settings have a special handling for deprecated flags in `useSetting`, | |||
| // so we can't unwrap them by doing const { ... } = useSetting('color') | |||
| // so we can't unwrap them by doing conwst { ... } = useSetting('color') | |||
There was a problem hiding this comment.
In fact, we can't destructure any return values with conwst { ... }, which is an oversight on the part of the javascript technical committee I think 😄
There was a problem hiding this comment.
I'll propose it at the next mweeting. 😄
| Platform.OS === 'web' && ! gradient && ! style?.color?.gradient; | ||
|
|
||
| const defaultColorControls = getBlockSupport( props.name, [ | ||
| COLOR_SUPPORT_KEY, | ||
| '__experimentalDefaultControls', | ||
| ] ); | ||
|
|
||
| const enableContrastChecking = | ||
| isWebAndColorNotGradient && | ||
| true === |
There was a problem hiding this comment.
Do we need to default support to true somehow, or switch this to a ! false === check as currently when I test this the check doesn't work for any blocks that don't have this set, eg. paragraph block, and it might be easier to turn it off when not needed than toggle it on everywhere it is needed?
There was a problem hiding this comment.
Thanks for spotting this!!
I swear I had ! false === in there but...
🤦
There was a problem hiding this comment.
Fixed with comment added. 🙇
…erimentalCheckContrast === false
glendaviesnz
approved these changes
Aug 22, 2022
ajlende
reviewed
Aug 22, 2022
There was a problem hiding this comment.
Is there a reason for making this option experimental?
From what you've shown here and in the linked PR comments I think
- the use-case is clear—there are a number of blocks that either don't need the contrast checker or implement their own—and
- not all blocks using the support manually add the UI controls, so the primary way the controls can be configured is through options in block.json.
I think this could be a stable API right away unless you have reason to expect it will need to change in the future.
|
@ajlende Thanks for chiming in. It's a good question. I'd originally made it non experimental, but wasn't sure to be perfectly honest so erred on the side of underscore. To add to your points, the use case is very specific to color blocks supports so it doesn't have the footprint of other experimental controls, e.g., |
Added block.json schema docs
|
@glendaviesnz What do you think? I've pushed a change to remove the experimental prefix. Do you see any issues here, and think it's in line with https://make.wordpress.org/core/2022/08/10/proposal-stop-merging-experimental-apis-from-gutenberg-to-wordpress-core/ ? |
Makes sense, and just retested and still working as expected. |
ajlende
approved these changes
Aug 22, 2022
|
Thanks folks! 🙇 This also means that can move forward. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and How?
This PR adds a property
__experimentalCheckContrastto a block.json'ssupports.colorto disable the contrastChecker in the UI.Context:
Why?
There are scenarios where the contrast checker is not appropriate (Separator block), or a block has its own color implementation in combination with color block supports (Social links block).
Testing Instructions
Add a separator block and change the color so that it's the same or similar to the background.
Here is some dummy HTML:
Make sure your theme has a white background for the above test code.
Before:
After:
