Skip to content
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

Cover block reports color contrast accessibility error #39692

Closed
webmandesign opened this issue Mar 23, 2022 · 9 comments
Closed

Cover block reports color contrast accessibility error #39692

webmandesign opened this issue Mar 23, 2022 · 9 comments
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@webmandesign
Copy link
Contributor

Description

Since WordPress 5.9.1 the Cover block background setup was moved from block container HTML element into a child element. So, the container element has left with no background color set, while the text color is set to white by default.

Accessibility evaluation tools such as Wave now throw color contrast error.

The solution would be to set the Cover block background color by default, just like it is with its text color. Something like this could work:
.wp-block-cover:not(.has-background) { background: #000; }

But I understand the fix is not for all cases. But hopefully it should cover the majority of cases anyway. What do you say, guys, how we should approach this?

Step-by-step reproduction instructions

  1. You can see this when testing https://themedemos.webmandesign.eu/eimear/ - the "DISCOVER MORE" section in the middle of the page throws an error in Wave https://wave.webaim.org/report#/https://themedemos.webmandesign.eu/eimear/

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.

No

@Mamaduka Mamaduka added [Block] Cover Affects the Cover Block - used to display content laid over a background image [a11y] Color Contrast labels Mar 23, 2022
@cbirdsong
Copy link

cbirdsong commented Mar 23, 2022

I think it would be best to set a complementary background color on the cover block element itself, which would also be good progressive enhancement for cases like a bleeding-edge gradient syntax or the image not loading in fast enough/at all.

Obviously a cover block with a simple color tint could also just apply that color as a background color to the cover block element, but no such easy solution exists for gradients1.

The is-light class used in the editor needs work, but a fixed version of it that added is-light or is-dark classes to the front end would take care of this, and could also automatically apply the theme's actual color palette instead of simply defaulting to white and black, which is what it does right now:

CleanShot 2022-03-23 at 15 08 10

A similar suggestion was made here: #34717 (comment)

Footnotes

  1. Unless registering a gradient also required a fallback color, which is something I've played with when generating gradient CSS to go with registered presets.

@webmandesign
Copy link
Contributor Author

That would be great to have a dedicated Cover block container background option. It would resolve the issue for the most if not all the cases.

And while we are at it, it would be great to have also a text color option for Cover block. (I personally enable this in my themes already so it is easier to set text color for nested blocks within the Cover block content at once.)

@michellecurran
Copy link

michellecurran commented May 19, 2022

I would like to add my experience to this issue, as this is a pretty big accessibility issue that I hope will be soon fixed :)

This change in architecture to the core cover block is also causing accessibility issues for my client. We have used the cover block as heros on all their t1, t2 and most t3 pages, some heros are solid backgrounds and some are image backgrounds. We created patterns for them of those heros for branding consistency. It is a large site. The site went live with v5.9 and accessibility was not an issue on those heros. However, any time they change the content on those pages or create a new page with their hero pattern with the previous html, this new cover block architecture overrides the html and creates an accessibility issue.

As noted by @webmandesign, the implication is that the background color for the cover block no longer cascades to the the inner blocks for headings and content and the background seen by a screen reader is the body’s default background. For example if the default background color is white for the site, a cover block with a blue background and white text will ‘read’ as a white background with white text on a screen reader.

If visuals are helpful...
Cover block prior to v5.9.1
image

New Architecture
image

In the short term, I wrapped the cover block in a group and applied a background color to the group in hopes that it will behave nicely and cascade the background to the inner cover block and all its content. They are going to re-run the report. I will report back if that removed the accessibility error.

@cbirdsong
Copy link

I agree, the shift in location for the has-background classes is an unnecessary regression that makes the block less failure tolerant and more likely to cause accessibility issues. The background should ideally be applied on both the cover element directly and the child span used for an overlay via inherit. Code samples:

<div class="wp-block-cover" class="has-background has-purple-background-color has-background-dim has-background-dim-80 has-background-media">
  <span aria-hidden="true" class="wp-block-cover__gradient-background"></span>
  <!-- img/video, inner_container -->
</div>

<div class="wp-block-cover" class="has-background has-background-dim has-background-dim-60 has-background-media" style="background-color:#016ead">
  <span aria-hidden="true" class="wp-block-cover__gradient-background"></span>
  <!-- img/video, inner_container -->
</div>```

```scss
.wp-block-cover.has-background-media > {
	.wp-block-cover__background,
	.wp-block-cover__gradient-background {
		background-color: inherit;
		background-image: inherit;
	}
}

.wp-block-cover.has-background-dim-XX > {
	.wp-block-cover__background,
	.wp-block-cover__gradient-background {
		opacity: 0.X;
	}
}

This would address any issues the current approach causes for screen readers, testing tools and any other extensions doing calculations based on background color, and would also make cover block readable if images fail to load due to network issues or broken links. Using these two cover blocks as an example:
Cover blocks with images loaded correctly

With broken images they are pretty close to illegible:
CleanShot 2022-05-20 at 11 29 19

With the background color/gradient set on the cover block itself they become legible again:
CleanShot 2022-05-20 at 11 29 24

The editor could use much more of this kind of defensive CSS.

The only regression I can think of is that the cover block would no longer support setting a background color with opacity without wider CSS changes to apply colors using custom properties based on the presence of has-background (see #40335). This could possibly be avoided by duplicating the color/gradient classes on the child span alongside some complicated CSS selectors on the parent, but that's pretty inelegant.

@michellecurran
Copy link

@cbirdsong I agree that

The background should ideally be applied on both the cover element directly and the child span used for an overlay via inherit.

You code examples made good sense to me.

@michellecurran
Copy link

In the short term, I wrapped the cover block in a group and applied a background color to the group in hopes that it will behave nicely and cascade the background to the inner cover block and all its content. They are going to re-run the report. I will report back if that removed the accessibility error.

I received news that applying a group block with a background to the cover block resolves the accessibility issue. I don't like adding that extra html and bulk, but it solves the issue for now.

@michaelbourne
Copy link

I agree, the shift in location for the has-background classes is an unnecessary regression that makes the block less failure tolerant and more likely to cause accessibility issues. The background should ideally be applied on both the cover element directly and the child span used for an overlay via inherit.

100% this. The recent update that moved classes out of the parent wp-block-cover div has been a nightmare for custom themes and custom colors.

To make matters worse, new default styles were added:

.wp-block-cover-image.is-light .wp-block-cover__inner-container,
.wp-block-cover.is-light .wp-block-cover__inner-container {
    color: #000;
}

So if I have a black overlay at under 50% opacity to tint the overlay and darken a bg image for white text on top, this new CSS causes black text on a nearly black bg. I used to be able to target body .has-black-background-color to set the color to white whenever that bg color was used, in any block, site wide. Now I have to target body .wp-block-cover .wp-block-cover__background.has-black-background-color .wp-block-cover__inner-container simply to beat the specificity of core styles.

If the color classes were all moved to the span, why was this is-light class added to the wrapper?

@skorasaurus skorasaurus added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed [a11y] Color Contrast labels Jul 20, 2023
@richtabor
Copy link
Member

It's worth noting that this is dependent on the theme's background color (or the background color of the element the cover block is placed in. It's not possible to account for all variations and setting a fixed background color for the default state of cover block would not be an appropriate solution.

For resetting, there's a bit more of a path. Perhaps when resetting the image (removing it), the overlay value is set higher to engage the light/dark contrast, say 70.

Image
Image

Duplicate of my comment on #65868 (comment).

@richtabor
Copy link
Member

Closing in favor of #65868, which is more actionable/clearly defined.

Let's get this in. 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

No branches or pull requests

7 participants