WordPress / gutenberg

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

Cover Block regression: Text which was previously white by default is now black and not visible (WP 5.9 beta1) #37031

Open briceduclos opened 2 years ago

briceduclos commented 2 years ago

Description

For certain images, the useCoverIsDark hook returns a false value while the Cover is dark.

For the example below, there is no reason to change the text to black. It lowers the contrast ratio, and the content isn’t visible anymore.

Step-by-step reproduction instructions

  1. Inside the block editor, paste the following code:

    <!-- wp:cover {"url":"https://s.w.org/images/core/5.5/don-quixote-06.jpg"} -->
    <div class="wp-block-cover has-background-dim"><img class="wp-block-cover__image-background" alt="" src="https://s.w.org/images/core/5.5/don-quixote-06.jpg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:heading {"textAlign":"center"} -->
    <h2 class="has-text-align-center">Heading</h2>
    <!-- /wp:heading --></div></div>
    <!-- /wp:cover -->
  2. Observe that by default the text is black.

  3. As a result, the text color of a Cover block which was white in WP5.8 is now black in WP5.9.

Screenshots, screen recording, code snippet

WordPress 5.8.2 WordPress 5.9 beta 1
58 59

Environment info

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

glendaviesnz commented 2 years ago

Looks like the method that is calculating whether the background is dark or not needs some adjustment - currently it doesn't see that particular image as dark until the overlay is at opacity 60

cover-text

Update - turned that this is because when dimRatio > 50 the background color is used to calculate darkness instead of the image

glendaviesnz commented 2 years ago

It looks like there are a couple of issues here:

  1. The underlying fast-average-color library is calculating the average color of the Don Quixote image as being light so applying a dark text color, but the portion of the image that the text is sitting over is quite dark in relation to the rest of the image. If an actual dark image is used it works as expected. I am not sure how we would get around this as it would be complex to try and work out exactly which part of the image the text was going to sit over in order to get a more accurate default setting.

    Screen Shot 2021-12-02 at 4 45 30 PM
  2. Using a remote url for the image causes fast-average-color to fail with a DOMException: Failed to execute 'getImageData' on 'CanvasRenderingContext2D': The canvas has been tainted by cross-origin data. error which causes it to fall back to #ffffff as the average color, which again is light, so text set to dark.

glendaviesnz commented 2 years ago

Also, as background, the reason for this change is outlined here

hellofromtonya commented 2 years ago

Was reported on Trac too https://core.trac.wordpress.org/ticket/54791.

cbirdsong commented 2 years ago

I've just encountered this issue on a production site when upgrading it to Wordpress 5.9.

pattyok commented 2 years ago

Most of the text on the home page of a recently launched site when white (on white) because of this change! It doesn't appear that it is doing any color detection it is purely based on the opacity level selected.

I have a light colored image, using a white overlay. And if the opacity of the overlay is under 50% all of the text goes white.

image

image

I will add that in my theme I was taking advantage of the has-white-background-color class that was attached to the block. In my themes, I have css rules that set the default text color based on the class added to the block. That rule goes out the window with this change as well.

There's some good discussion on how changes like this impact theme developers. In this issue: https://github.com/WordPress/gutenberg/issues/38694#issuecomment-1036878391

mrwweb commented 2 years ago

As I mentioned in my comment on the Core block refactor dev not: For any sites that don't support the full color controls, the recommended workaround is not feasible. On a site I maintain, we don't allow editors to apply colors through the standard color tools, so this isn't a hypothetical. For now, my solution was to override the new CSS rules with higher specificity selectors that revert the behavior.

Given how many people are finding that the color calculations for light/dark are not working well with their specific colors and images, a filter or theme.json setting to disable this behavior would be really useful.

glendaviesnz commented 2 years ago

@scruffian, do you have any thoughts on ways to mitigate the problems noted above due the change in Cover block background settings?

glendaviesnz commented 2 years ago

Most of the text on the home page of a recently launched site when white (on white) because of this change!

@pattyok thanks for reporting this. I wasn't able to replicate this on 5.9 with TwentyTwentyTwo theme: cover-text-color

It would be good to try and narrow down what is different in your theme that is causing the issue for you. Is it a publicly available theme that we could do some testing on? If not if you have time to see what the difference might be between your theme and TwentyTwentyTwo that would be really helpful to try and track down what is causing the issue for you, and to help with coming up with a fix.

scruffian commented 2 years ago

This is unfortunate, but I still think it is preferable to the previous situation where the text would always be white, even if the block/image was white making the text invisible.

One thing to bear in mind is that user still have control over the color of the text in the cover block - the idea of setting the color to black/white for them is to try to provide some clever defaults, but ultimately we want the user to have control over that color, which they can do using the block controls. Would the best course of action here might be to encourage those users who are impacted to manually set the text color when contrast doesn't work for their image?

glendaviesnz commented 2 years ago

Would the best course of action here might be to encourage those users who are impacted to manually set the text color when contrast doesn't work for their image?

@scruffian, I think the main issue with this is the case that @mrwweb notes where a theme prevents users from changing the colors.

scruffian commented 2 years ago

Ah yes I see. I'm not sure if a filter to disable this behaviour is a solution either though, as problem would still exist when the image was light...

mrwweb commented 2 years ago

I'm not sure if a filter to disable this behaviour is a solution either though, as problem would still exist when the image was light...

It's mostly the case that block settings can be opted out of and it feels like this should be the case here as well. In our case, black is the only overlay color allowed and so there is no circumstance where white text is desirable. I have no doubt I'm not the only person in this situation.

pinksharpii commented 2 years ago

We are also experiencing this on at least 2 sites we have developed. We define the color scheme for the site, only allowing brand colors to be used so it's not a question of color contrast. Selecting black or dark blue overlay should always output white text regardless of opacity. Previously we handled this in our theme by stating .has-black-background-color { color: #fff; } but 5.9.1 removing the background color from the parent and adding in a forced is-white or is-dark-theme really makes theme development difficult to do when it comes to cover blocks. Having a filter or an option to not add that class, and have it be defined by the theme would be great. Putting the background color class back in the parent wrapper (so we as theme devs can easily target the text in CSS) would be ideal but I understand that's probably not the direction this will go.

patrickwc commented 2 years ago

We are experiencing issues with this at our agency. We not add a filter which makes it possible for theme authors to change the dimRatio > 50 value mentioned above by @glendaviesnz ? @scruffian can I ask why was 50 decided upon? I feel like this update only took into account solid background colours not background images with overlay settings applied to them. A dark overlay with 50 opacity is still pretty dark, so much so that a "light" image with 50% dark opacity now has illegible text after upgrading. Having to tell users to manually change the text colour on all posts where cover blocks are used after updating WP is not an option for us - we have clients who have thousands of pages.

I have managed to get around this issue on our themes by overriding the CSS:

.wp-block-cover.is-light:has(> .has-black-background-color),
.wp-block-cover.is-light .has-black-background-color ~ .wp-block-cover__inner-container {
    color: #fff;
}

I cannot think of a scenario where if .has-black-background-color is applied, you would want WordPress to force your text to be dark.

jonathan-dejong commented 2 years ago

This is a wider issue with visually breaking changes in many core blocks in combination with being static blocks.. but please, for this specific issue, at least play ball with us in finding a code solution that does not require us telling clients to go update all their content.

scruffian commented 2 years ago

@scruffian can I ask why was 50 decided upon

This felt like the right compromise between two difficult positions. If we tweak it either way, then the other case will be even more broken.