Skip to content

Conversation

@jasmussen
Copy link
Contributor

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:

before

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:

Screenshot 2020-10-29 at 10 22 33

As far as I can tell, there are only two ways of fixing this.

  1. Reducing the specificity of has-background-dim
  2. Increasing the specificity of every custom theme color.

This 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:

after

The obvious downside is that has-background-dim is 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.

@jasmussen jasmussen self-assigned this Oct 29, 2020
@jasmussen
Copy link
Contributor Author

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:

  1. Reduce the specificity of has-background-dim
  2. Increase the specificity of every custom theme color.

So either we have unscoped .has-background-dim { ... }, or we go back to having every custom registered color be double-specificity, such as .has-yellow-background-color.has-yellow-background-color { ... }.

Is there a third solution I'm not seeing, which addresses the Cover problem?

@github-actions
Copy link

Size Change: +23 B (0%)

Total Size: 1.21 MB

Filename Size Change
build/block-library/style-rtl.css 7.85 kB +12 B (0%)
build/block-library/style.css 7.85 kB +11 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 kB 0 B
build/api-fetch/index.js 3.45 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.98 kB 0 B
build/block-library/editor.css 8.98 kB 0 B
build/block-library/index.js 146 kB 0 B
build/block-library/theme-rtl.css 837 B 0 B
build/block-library/theme.css 838 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.2 kB 0 B
build/components/style.css 15.2 kB 0 B
build/compose/index.js 9.81 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 772 B 0 B
build/data/index.js 8.77 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.46 kB 0 B
build/edit-navigation/index.js 11.2 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.38 kB 0 B
build/edit-post/style.css 6.37 kB 0 B
build/edit-site/index.js 22.1 kB 0 B
build/edit-site/style-rtl.css 3.86 kB 0 B
build/edit-site/style.css 3.85 kB 0 B
build/edit-widgets/index.js 26.4 kB 0 B
build/edit-widgets/style-rtl.css 3.1 kB 0 B
build/edit-widgets/style.css 3.1 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 43.1 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 7.7 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.34 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13.2 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@sirreal
Copy link
Member

sirreal commented Oct 29, 2020

Thanks @jasmussen!

This may be redundant but I'll add my notes I get up to speed on this:

  • Theme-defined background colors were not being respected before Gutenberg 9.2 and custom Cover Block overlay color overwritten by "has-background-dim" #25290
  • The cause was (from description):
    .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 …
  • This manifests with custom backgrounds that themes register. An example is the twentytwentyone theme here.
  • The workaround for themes is to increase specificity, like twentytwentyone has done here. .has-yellow-background-color adds an attribute selector [class] for extra specificity resulting in .has-yellow-background-color[class] (class selector + attribute selector).

Regarding the possible changes:

Increase the specificity of every custom theme color.

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.

Reduce the specificity of has-background-dim

This seems reasonable although the name is pretty generic.

To mitigate the generic name we could make the has-background-dim class more unique without increasing specificity, e.g. adding a prefix to the class itself: wp-block-cover--has-background-dim. This can be used directly without much concern for conflicting class names in selectors as .wp-block-cover--has-background-dim { }.

Is there a third solution I'm not seeing, which addresses the Cover problem?

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 has-background-dim default background only on blocks that do not have another background color class. We can assume that any block that includes -background-color in its class attribute implies that the block has a custom background color that will be set by another style rule matching the has-{{COLORNAME}}-background-color class.

.wp-block-cover.has-background-dim:not([class*="-background-color"]) {
  background-color: #000;
}

@oandregal
Copy link
Member

It seems that the color classes for the overlay follow that pattern (has-COLOR-background-color), so the attribute selector approach ([class='-background-color']) would work nicely. I'm going to ping @jorgefilipecosta as he has some background (:drum:) on how this works and could confirm if there may be any edge cases to how those classes are generated.

@sirreal
Copy link
Member

sirreal commented Oct 29, 2020

I'm preparing a PR with the alternative I suggested for comparison.

@sirreal
Copy link
Member

sirreal commented Oct 29, 2020

Alternative: #26569

@jasmussen
Copy link
Contributor Author

I much prefer #26569, closing this one in favor of that. We can always revisit. Thanks Jon!

@jasmussen jasmussen closed this Oct 29, 2020
@oandregal oandregal deleted the try/cover-color-fix branch October 29, 2020 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cover Block: Overlay background possible regression custom Cover Block overlay color overwritten by "has-background-dim"

4 participants