Cover block: background element's classname consistency #38392
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Fixtures won't pass because the deprecation can't yet handle the classname changes 🤷 |
|
Size Change: +2.81 kB (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
|
@ramonjd I think the issue was you had included |
❤️ Thank you! I was staring at this file for eons trying to work out the issue. |
ramonjd
added
[Block] Cover
ramonjd
changed the title
renaming `wp-block-cover__gradient-background` to `wp-block-cover__background` given that the former class is applied regardless of whether there's a gradient. Removing duplicate gradientClass
ramonjd
force-pushed
the
update/cover-block-background-classname
branch
from
f193def to
99547c4
Compare
|
There are 4 plugins And possibly any block pattern in the block pattern directory that uses cover blocks. That is fewer than I would have guessed, but do the developers need to take any action? |
|
Thanks a lot for the ping @carolinan, and for highlighting the potential effect on existing themes and plugins.
Before the change in October 2021, which made the class a permanent citizen of the Cover Block markup, the logic to display an element with the classname url && gradientValue && dimRatio !== 0So before October, We could, and probably should, reinstate the class for this particular scenario, which would mean that only those who have changed their code to rely exclusively on the permanence of this class in the last 3 months would have to take action. What do you think? Looking at a lot of existing usage, I think this would be a reasonable mitigation against backwards incompatibility. We're also keeping the class rules for The only, minor downside is that we're left with a redundant class. No biggie. A couple of action points for me:
Also cc @glendaviesnz for thoughts.
Yes! All manual and integration tests indicate that it's working as expected 👍 |
Yes, my thinking is that the majority of usages will be expecting that class to only be in place if there is a gradient applied. I expect that the number of plugins/styles that have added that selector in the last 3 months on the expectation that it is always there will be 0, so reverting it back to only being applied when relevant should prevent more issues than it causes - in theory ... but in theory the Titanic was unsinkable 😄 |
…had media, a gradient and a dim for backwards compatibility. See #38392#issuecomment-1028434423
I've tested with a few themes and plugins, and looked at usages. Most are using Leaving But it might be worth doing a shout out on the make blog for anyone who, in the last three months, is relying on that class for other purposes, for example as a general, always-there, selector. |
|
@ramonjd I wonder if in changing If we just revert to applying when |
Yeah fair question, thanks. I originally added the new class But I guess nothing is stopping folks from doing something like We've already reverted to applying
The only risk scenario I saw is if the theme authors are relying on the persistency of That's the "regression" we had to have in my mind. I guess leaving out What do you reckon? |
|
@ramonjd Sorry, I was looking at the code sample in the PR description, which shows |
|
Seems to be something not quite right with opacity when only background color set. The block on the left is from latest GB but running on frontend with this PR with no migration run. The one on right is from this PR once the migration has run: block code is: Will take a closer look and see if I can work out what is going on here. |
Oooh, thanks for spotting. Maybe a CSS calamity. I see the |
For some reason this is not being applied, but can't see the reason why for looking! |
|
Edit: I think I was missing @for $i from 1 through 10 {
&.has-background-dim.has-background-dim-#{ $i * 10 } {
&:not(.has-background-gradient)::before,
.wp-block-cover__background,
.wp-block-cover__gradient-background {
opacity: $i * 0.1;
}
}
} |
glendaviesnz
approved these changes
Feb 8, 2022
There was a problem hiding this comment.
LGTM. I tested with a whole range of options:
Non-migrated blocks viewed in frontend with this PR
<!-- wp:cover {"overlayColor":"secondary","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-secondary-background-color has-background-dim-100 wp-block-cover__gradient-background has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just a color</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"gradient":"cool-to-warm-spectrum","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-background-dim-100 wp-block-cover__gradient-background has-cool-to-warm-spectrum-gradient-background has-background-dim has-background-gradient has-cool-to-warm-spectrum-gradient-background"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just a gradient</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":50} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__gradient-background has-background-dim"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just an image</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"dimRatio":30,"overlayColor":"secondary","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-secondary-background-color has-background-dim-30 wp-block-cover__gradient-background has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just a color with custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"dimRatio":60,"gradient":"cool-to-warm-spectrum","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-background-dim-60 wp-block-cover__gradient-background has-cool-to-warm-spectrum-gradient-background has-background-dim has-background-gradient has-cool-to-warm-spectrum-gradient-background"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just Gradient with custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":30} -->
<div class="wp-block-cover"><span aria-hidden="true" class="has-background-dim-30 wp-block-cover__gradient-background has-background-dim"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":70,"overlayColor":"secondary","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-secondary-background-color has-background-dim-70 wp-block-cover__gradient-background has-background-dim"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with color and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":70,"gradient":"cool-to-warm-spectrum"} -->
<div class="wp-block-cover"><span aria-hidden="true" class="has-background-dim-70 wp-block-cover__gradient-background has-cool-to-warm-spectrum-gradient-background has-background-dim has-background-gradient"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with gradient and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"dimRatio":70,"customOverlayColor":"#df6e03","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-background-dim-70 wp-block-cover__gradient-background has-background-dim" style="background-color:#df6e03"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Custom color and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"dimRatio":20,"customGradient":"linear-gradient(135deg,rgb(6,147,227) 0%,rgb(229,2,2) 31%,rgb(155,81,224) 100%)","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-background-dim-20 wp-block-cover__gradient-background has-background-dim has-background-gradient" style="background:linear-gradient(135deg,rgb(6,147,227) 0%,rgb(229,2,2) 31%,rgb(155,81,224) 100%)"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Custom gradient and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->New blocks added via editor with this PR
<!-- wp:cover {"overlayColor":"secondary","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="wp-block-cover__background has-secondary-background-color has-background-dim-100 has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just a color</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"gradient":"cool-to-warm-spectrum","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-100 has-background-dim has-background-gradient has-cool-to-warm-spectrum-gradient-background"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just a gradient</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":50} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Just an image</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":30,"overlayColor":"secondary"} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-secondary-background-color has-background-dim-30 has-background-dim"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with color and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":70,"gradient":"cool-to-warm-spectrum"} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-70 has-background-dim wp-block-cover__gradient-background has-background-gradient has-cool-to-warm-spectrum-gradient-background"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with gradient and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":70,"customOverlayColor":"#e5740b","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-70 has-background-dim" style="background-color:#e5740b"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with custom color and custom opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:cover {"url":"http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg","id":82,"dimRatio":70,"customGradient":"linear-gradient(135deg,rgb(6,147,227) 0%,rgb(223,20,20) 22%,rgb(155,81,224) 93%)"} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-70 has-background-dim wp-block-cover__gradient-background has-background-gradient" style="background:linear-gradient(135deg,rgb(6,147,227) 0%,rgb(223,20,20) 22%,rgb(155,81,224) 93%)"></span><img class="wp-block-cover__image-background wp-image-82" alt="" src="http://localhost:4759/wp-content/uploads/2022/02/tree2-7.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Image with custom gradient and opacity</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->And all seemed to work as expected
8 tasks
talldan
reviewed
Feb 11, 2022
| { | ||
| "clientId": "_clientId_0", | ||
| "name": "core/cover", | ||
| "isValid": false, |
There was a problem hiding this comment.
There was a problem hiding this comment.
@talldan Thanks for the heads up. I've taken a look and it appears to be human error (mine).
The attribute "isDark":false should have a corresponding is-light classname on the container.
Getting a PR together to fix this now.
This was referenced Feb 11, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Following on from conversations in: #38362 (comment)
Here is a Pull Request that updates the classnames on the Cover Block background span element in order to:
gradientClasswhere there is no media url<span />ofwp-block-cover__backgroundto replacewp-block-cover__gradient-background, since the former latter applied to the element all the time (even where there is no gradient)wp-block-cover__gradient-backgroundto the logic before the the change in October 2021Achieve World Peace ☮️So, in the save.js, the element is switching from:
To:
We've added an extra deprecation and updated the fixtures and other tests.
Testing Instructions
Here are some test blocks created using trunk. Paste them in the code view of the block editor and ensure that there are no validation errors.
Test blocks
Types of changes
Block classname changes and deprecation
Checklist:
*.native.jsfiles for terms that need renaming or removal).