WordPress
Contrast checker notice should not use the order CSS property
Description
- Go in the post editor or site editor.
- Select a paragraph
- In the inspector panel, set background and text colors to trigger the color contrast checker notice.
- In the browser dev tools, select the element with CSS class
block-editor-contrast-checker - Observee the element has a CSS property
order: 9999;
See the comment in the code at https://github.com/WordPress/gutenberg/blob/f1cd56679bc926ad1d9ec86060616c3579efe95e/packages/block-editor/src/hooks/color.scss#L2-L8
Introduced in https://github.com/WordPress/gutenberg/commit/8675f86f885ca389f3bca39971b22cb897bab61b See the related PR description that mentions this point https://github.com/WordPress/gutenberg/pull/34027
Instead, this PR simply replicates the styling of the ItemGroup and leverages CSS ordering to force contrast checkers to the bottom of the panel.
I'm not sure this order property is still necessary. In any case, visual order and DOM order must always match when they affect 'meaning and operation'. Reference:
WCAG 2.2 1.3.2 Meaningful Sequence (Level A) https://www.w3.org/TR/WCAG22/#meaningful-sequence 2.4.3 Focus Order (Level A) https://www.w3.org/TR/WCAG22/#focus-order
If this CSS property is no longer necessary, we should just remove it. If the order issue mentioned in the oroginal PR is still relevant, another solution must be found as the DOM order must match the reading order.
Cc @aaronrobertshaw
Step-by-step reproduction instructions
See instructions above.
Screenshots, screen recording, code snippet
No response
Environment info
No response
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
Thanks for the ping @afercia 👍
If this CSS property is no longer necessary, we should just remove it.
The CSS property is still required as the color panel can have controls inserted via slot fills from any number of sources.
If the order issue mentioned in the oroginal PR is still relevant, another solution must be found as the DOM order must match the reading order.
The order is still relevant as it keeps the related color controls together. I'm not sure what other solutions are available to us that wouldn't break the UI further. Perhaps we need some fresh design eyes on this one?
WCAG 2.2 1.3.2 Meaningful Sequence (Level A) https://www.w3.org/TR/WCAG22/#meaningful-sequence 2.4.3 Focus Order (Level A) https://www.w3.org/TR/WCAG22/#focus-order
So I'm clear, are you saying the sequence here is meaningful or is there a problem with the focus order?
I think an argument could be made here that the contrast checkers' meaning is independent from the sequence in the DOM. It's unfortunate that we can't control the order of slot fills otherwise their injection to the panel could be separated and moved to the end of the panel.
I'm a little short on bandwidth at the moment, so if there is anyone that can pick this up that would be appreciated. I'd of course be happy to help out with reviews etc.
The CSS property is still required as the color panel can have controls inserted via slot fills from any number of sources.
It would be nice to understand how to replicate this scenario and actually test the UI with altered order.
So I'm clear, are you saying the sequence here is meaningful or is there a problem with the focus order?
It's not just about the focus order. it's also about the reading order.
The order is still relevant as it keeps the related color controls together.
To my understanding, it will only keep the visual order of the color controls together. The DOM order will not match the visual order though.
It would be nice to understand how to replicate this scenario and actually teste the UI with altered order.
I'm not sure I can give an explicit step-by-step recipe for testing. There are so many possibilities, it would change I think based on what your testing focus is.
There are a few key points that I can note that should help you create a tailored test case:
- The controls in the Color panel are rendered via a single slot
- They are added via
<InspectorControls group="color">{ elements for panel go here }</InspectorControls> - Color block supports inject their "standard" color controls via that same means
- Individual blocks can add their own controls e.g. Cover block
- There are no restrictions on the type of elements that can be added to the panel e.g. Cover's overlay opacity control
- Plugins could override and extend a block adding further controls
- The controls added would be dependent on the load order of plugins etc as well
- SlotFills can't guarantee the DOM order of their fills
Again, I appreciate that isn't quite what you might have been chasing but I hope it helps somewhat though.
As the various inspector panels have matured a bit, it would be good to revisit the color panel as a special case. There could be some benefit to exploring multiple slots just for that panel. That could still have some backwards compatibility issues to wrangle as well as the fact that the slots would still be open to 3rd parties. I'm not sure even then we could guarantee controls aren't placed as fills in the wrong slot and/or disrupting meaningful order (visual or DOM).
Unfortunately, I don't have the bandwidth to explore this further. I'd be happy to help test or review any potential solutions though 🙏
@aaronrobertshaw I found some time to wrap my head around this issue. Thankd foru your detailed explanation, much appreciated.
I played a bit with altering the Cover block code, just for testing. Still, this order CSS property doesn't seem ideal to me. The ContrastChecker can also be added manually to a stack of components added via the slot. As a developer, I would expect to see the ContrastChecker being rendered in the exact place where I placed it.
As a user, the ContrastChecker makes sense only when it is rendered right after the setting that has an invalid color pair. Instead, it is always moved at the bottom in the last position. As such, it is 'detached' from the invalid setting. It is confusing and not the best UI because it's not immediate to understand which setting is invalid.
See the example screenshot below.
On the left: the red arrow illustrates where the ContrastChecker is supposed to be shown, right after a control that may have incalid color pair.
Instead (on the right) it is moved as last. Even after the control that is placed last in the stack of controls within the <InspectorControls group="color">.
This is an user experience problem.
Not sure this is ideal. Ideally there should be a way to 'attach' the ContrastChecker to a specific control and not render it via the slot.
As a developer, I would expect to see the ContrastChecker being rendered in the exact place where I placed it.
I'm not sure this could be guaranteed given SlotFills can't guarantee the order in which their fills will be registered and rendered.
As a user, the ContrastChecker makes sense only when it is rendered right after the setting that has an invalid color pair.
It could also be worth factoring into this discussion that the contrast checkers are really related to more than a single control or color value. The warning is based on how the selection relates to other selections for other color controls. In some circumstances, it could be argued that the contrast checker relates to the overall color configuration for the panel.
Not sure this is ideal. Ideally there should be a way to 'attach' the ContrastChecker to a specific control and not render it via the slot.
The example shown is pretty specific to the Cover block because of the two overlay-related controls. The contrast warning there makes sense given it's after the color controls but before the overlay opacity.
If the contrast warning was related to the heading color and was tied to it, it would break the visual representation of grouping the color controls. I'm not arguing for or against anything here, just that that is the consequence. So to make any changes to strictly tie the contrast warning with the control it relates to would need an alternate design.
If we are to try and maintain the current visuals but detach the contrast checker and color controls, then I think it comes back to earlier suggestions made about potentially introducing additional slots for the color panel.
The contrast warning there makes sense given it's after the color controls but before the overlay opacity.
It's before the overlay opacity in the screenshot on the right, where I removed the order CSS property 🙂 Overall, I think this order property should be removed as a first step to improve the usage of the ContrastChecker.
If the contrast warning was related to the heading color and was tied to it, it would break the visual representation of grouping the color controls. I'm not arguing for or against anything here, just that that is the consequence. So to make any changes to strictly tie the contrast warning with the control it relates to would need an alternate design.
Not sure 'breaking the visual representation' would be a huge issue. A notice or warning are supposed to appear in close proximity with the control that is invalid. That is expected. Imagine a form with many input fields. The form has user input validation. Once submitted with an invalid value, the validation error message can certainly be shown at the top or at the bottom of the form but there is the need to identify the invalid value and point users to it. Usually, the validation error is shown close to the invalid field.
Instead, in this csse there's no way to identify which one of the potential multiple color settings is invalid. Imagine a block with multiple color pair settings. The notice only says 'this color combination' but there's more than one and no indication at all of which one is invalid. Plugins can also add more controls and custom blocks may provide multiple color controls and still the ContrastChecked would not indicate which setting is invalid.
On top of that, the visual order and DOM order must match anyways.
I will prepare a quick PR to remove the order property but I totally agree this needs to be improved and probably needs design.
I will prepare a quick PR to remove the order property but I totally agree this needs to be improved and probably needs design.
Before jumping too deep into a PR, it might pay to get that design input upfront on this one. Possible solutions can then be discussed and settled upon here.
I've added the Needs Design Feedback label to get the ball rolling.
Before jumping too deep into a PR, it might pay to get that design input upfront on this one. Possible solutions can then be discussed and settled upon here.
I think a new design and the ordeer CSS property are two separate issues. I will submit a one-line PR to remove the orer property. Anyways, I did some testing and it seems to me the contrast checker within the color group is always rendered last anyways so that I'm not sure the order property is necessary in the first place.
I'd think the need for a new design and new implementation is a separate issue. The way the contrast checker is used ins't great. For example the Navigation block uses two contrast checkers for two color pair settings. When other color pairs are invalid, the UI isn't that great, honestly. Screenshot:
I still think the contrast checker notice should be visually tied to the color pair it is associated to.
Anyways, I did some testing and it seems to me the contrast checker within the color group is always rendered last anyways so that I'm not sure the order property is necessary in the first place.
Apologies for my confusion here @afercia. It appears how the contrast checkers are added changed with https://github.com/WordPress/gutenberg/pull/48893.
I also couldn't replicate the multiple contrast checkers for the navigation block but the contrast checking still appeared a bit broken.
https://github.com/WordPress/gutenberg/assets/60436221/efc3e85a-b799-4d72-a95c-ce7807ef0294
Unfortunately, as noted before, I'm rather short on bandwidth for the foreseeable future, so I'll have to leave this issue with you.
Thanks @aaronrobertshaw I will create a couple seeparate issues as the ContrastCheckeer appears to be buggy anyways.
I also couldn't replicate the multiple contrast checkers for the navigation block
For me, it doesn't work well when I set a color pair the first time. It only works when I change an already set color pair. For the navigation block specifically, I have to set/change color on both pairs to trigger both the notices. See GIF below, from latest trunk:
It appears how the contrast checkers are added changed with #48893.
@aaronrobertshaw thanks. Is this the relevant commit? https://github.com/WordPress/gutenberg/pull/48893/commits/d5297f2f7ec7045b560ddc9625187a7a01e8fd70
Does that mean that, since the contrast check is performed on every render, the notice will always rendered last?
If that's the case, the order CSS property could be removed safely.
Is this the relevant commit? https://github.com/WordPress/gutenberg/commit/d5297f2f7ec7045b560ddc9625187a7a01e8fd70
At a glance, I don't think that's related to where a contrast checker is rendered.
The Navigation block rendering its own additional contrast checkers does show the potential for the fills rendered to the color panel slot to contain additional contrast checkers. Depending on what else is rendered to the panel, those contrast checkers would not be last.
Take an example where a 3rd party block adopts text and background block support which adds text and background controls plus a contrast checker. The block then also injects two custom color controls for an inner area of the block which require their own contrast checking for the a11y of that area. Without being able to guarantee the order of fills for a slot, the custom controls with their contrast checker could come before the block support controls and their contrast checker resulting in something like:
<ColorPanel>
<CustomColor1 />
<CustomColor2 />
<ContrastCheckerFor1Vs2 />
<TextColor />
<BackgroundColor />
<ContrastCheckerForTextVsBackground />
</ColorPanel>
Thanks for the explanation @aaronrobertshaw The potential order you illustrated above:
<CustomColor1 />
<CustomColor2 />
<ContrastCheckerFor1Vs2 />
<TextColor />
<BackgroundColor />
<ContrastCheckerForTextVsBackground />
is exactly what I would like to see. From an usability and accessibility perspective, the contrast checked notice should appear immediately after the color pair it relates to. Otherwise, it wouldn't be clear which color pari the notice is warning about.
Understood 👍
This brings the discussion back to my earlier comments around needing design feedback, as displaying the contrast checker as per the example structure order, would break the "item group" representation of all the color controls.