-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Try: Cover default-overlay color regression fix. #26564
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
Conversation
Possibly fixes #26545.
|
CC: @nosolosw @sirreal @stokesman just because of the semi-urgency of this issue, I'd really love your thoughts on how to best approach this. To copy from the ticket, here's the crux. We can either:
So either we have unscoped Is there a third solution I'm not seeing, which addresses the Cover problem? |
|
Size Change: +23 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
|
Thanks @jasmussen! This may be redundant but I'll add my notes I get up to speed on this:
Regarding the possible changes:
This would return to the situation prior to #25290 and #25290 would be an issue again, is that correct? Initially, that seems undesirable but is preferable to a regression in existing content.
This seems reasonable although the name is pretty generic. To mitigate the generic name we could make the
I've been thinking about this and have a third proposal. It depends on the following assumption (please think carefully about whether we expect this to hold!) We can apply the .wp-block-cover.has-background-dim:not([class*="-background-color"]) {
background-color: #000;
} |
|
It seems that the color classes for the overlay follow that pattern ( |
|
I'm preparing a PR with the alternative I suggested for comparison. |
|
Alternative: #26569 |
|
I much prefer #26569, closing this one in favor of that. We can always revisit. Thanks Jon! |
Possibly fixes #26545.
The Cover block is most of the time, used as a photo with a dim black overlay to ensure text on top is readable. This is augmented by the fact the block has a classname, "has-background-dim". However this regressed, and no dimming was applied until you explicitly set a color:
This both causes a less than optimal default state of the block, but also affects every existing cover block published.
The regression happened to fix #25290, and the issue boils down to this:
.wp-block-cover.has-background-dim { ... }has higher specificity than.has-contrast-yellow-background-color. So previously, users could simply not choose the color of the overlay at all:As far as I can tell, there are only two ways of fixing this.
has-background-dimThis PR does the former, and in doing so fixes the issue so that by default, there's a background dim, but if a user chooses a different color, that color works too:
The obvious downside is that
has-background-dimis now totally unscoped, and affects any element with this class. But as far as I can tell, only the Cover block uses this class.I'd love your thoughts on which of the two approaches above you would prefer.