Separator block: refactor to use color block supports #38428
Conversation
glendaviesnz
added
[Status] In Progress
|
Size Change: +476 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
glendaviesnz
force-pushed
the
refactor/separator-to-use-color-support
branch
from
73bcebb
to
d562aed
Compare
glendaviesnz
changed the title
glendaviesnz
removed
the
[Status] In Progress
|
@scruffian Not sure if this is a 100% suitable fix for #34637 as the 'Opacity' option isn't as in your face as in the Cover block UX. Is this good enough do you think for the sort of users that are likely to want to tweak this? It is worth migrating to the block to block supports anyway I think separate to the Opacity question. |
|
Thanks for working on this.
I think this way of doing it is fine, this isn't going to be something that most users need, its more useful for themes. One problem with this PR as it stands is that it doesn't remove the opacity CSS from the block - this means that all separators are forced to have the 0.4 opacity setting on top of whatever other settings there are. This is the main issue with the separator block from my perspective as sometimes themes will want a solid separator and then they have to use custom CSS to achieve it. If we could find a way to only apply the 0.4 opacity to older blocks that would be ideal from my perspective. I'm not sure "Background" makes sense for the color option here - since we only have one color setting then I think using "color" is more obvious to users. |
Ah, ok, will take a look at that next week and see what is possible.
I am pretty sure that currently with the color block supports you can only have 'background', 'text', 'link' colours, ie. you can't specify what the label says in the settings bar so just 'color' isn't possible currently. 'text' is probably just as confusing as well? I will follow up next week though and check that I am not overlooking something with the block supports color options. |
Oh yes you're right. In that case background is fine! |
@scruffian, the only way I could think of to do this is to add an additional style to the v2 version of the blocks, and remove the default 0.4 opacity if this class exists. Let me know if you can think of a better approach than this. |
I'm a bit late to this party and am likely missing context, so feel free to disregard this (I can look into my suggestion more fully tomorrow if needed). If the naming of the color control is still a problem, the recent switch of the color panel to leverage the Taking this approach you'd need to apply the color classes and styles manually rather than relying on block supports. This wouldn't be much different than skipping the serialization of those supports. Particularly if the values were stored in the block attributes the same as the block supports would. If stored the same way, you could then also leverage the utils in In terms of generating the ad hoc control, you might find the |
That is what I was anticipating. Thanks |
Thanks for this detail @aaronrobertshaw. I am thinking at the moment because 'Background' is the only color listed that it will be obvious enough that it will set the separator color in some way, so probably not worth going down a custom control path in order to change this label. What do you think @scruffian? |
@scruffian I had to make some changes to this, as looking for a new class in order to not apply the opacity, meant that the existing blocks only maintained the opacity until they were opened in the editor and saved. I figure that opening a post and editing some text, and also having all the separators lose their opacity would be something users won't expect. With the code as it is now any existing blocks should keep the default |
💯 |
|
In order to apply the deprecated opacity in instances where the block has no color applied I needed to force the addition of a new attribute and associated class, eg. This seems to work for all the deprecated block code with twentytwentytwo theme: <!-- wp:separator -->
<hr class="wp-block-separator"/>
<!-- /wp:separator -->
<!-- wp:separator {"className":"is-style-wide"} -->
<hr class="wp-block-separator is-style-wide"/>
<!-- /wp:separator -->
<!-- wp:separator {"className":"is-style-dots"} -->
<hr class="wp-block-separator is-style-dots"/>
<!-- /wp:separator -->
<!-- wp:separator {"color":"vivid-red"} -->
<hr class="wp-block-separator has-text-color has-background has-vivid-red-background-color has-vivid-red-color"/>
<!-- /wp:separator -->
<!-- wp:separator {"customColor":"#ff0000"} -->
<hr class="wp-block-separator has-text-color has-background" style="background-color:#ff0000;color:#ff0000"/>
<!-- /wp:separator -->
<!-- wp:separator {"color":"vivid-red","className":"is-style-dots"} -->
<hr class="wp-block-separator has-text-color has-background has-vivid-red-background-color has-vivid-red-color is-style-dots"/>
<!-- /wp:separator -->
<!-- wp:separator {"customColor":"#ff0000","className":"is-style-dots"} -->
<hr class="wp-block-separator has-text-color has-background is-style-dots" style="background-color:#ff0000;color:#ff0000"/>
<!-- /wp:separator -->The opacity for these stay at 0.4 in editor and frontend, until their color is edited, and any new blocks have no opacity applied unless the alpha channel is set in color picker. It seems slightly messy/hacky to add a new attribute purely to support backwards compatibility, but I can't see another option - open to alternative suggestions. |
|
This issue might be associated: Btw as one is working on a specific feature it would be helpful to also check out other Separator block issues to see what feels natural to also look closer at this time. As one gets into a kind of zone with working on a feature/block. https://github.com/WordPress/gutenberg/issues?q=is%3Aissue+is%3Aopen+label%3A%22%5BBlock%5D+Separator%22+ Thanks! |
ramonjd
force-pushed
the
refactor/separator-to-use-color-support
branch
from
6fba0cd
to
4a9e904
Compare
…oid any unintended backwards compat issues
glendaviesnz
force-pushed
the
refactor/separator-to-use-color-support
branch
from
e0c1c90
to
22ff3a2
Compare
|
@oandregal are you happy with this now with the move to skipSerialization? |
|
👍 |
|
Information for dev note: The Separator block has been updated to allow the setting of a custom opacity for each block via the alpha channel of the block supports color settings. separator-opacity.mp4The HMTL structure of the block is unchanged, all of the existing classes are still in place, but two additional classes have been added - If theme authors have opted in to block styles with hr.wp-block-separator.has-alpha-channel-opacity {
opacity: 0.4;
} |
| .wp-block-separator { | ||
| border: none; | ||
| border-bottom: 2px solid currentColor; | ||
| margin-left: auto; | ||
| margin-right: auto; | ||
| opacity: 0.4; | ||
|
|
||
| &.has-alpha-channel-opacity { |
There was a problem hiding this comment.
@ramonjd do you think we should maybe change this to &:where( .has-alpha-channel-opacity ) so easier for themes to override if needed?
There was a problem hiding this comment.
This sounds appealing but I think it lowers the specificity so that the styles from deprecated.scss, i.e., .wp-block-separator and therefore opacity: 0.4; take precedence.
There was a problem hiding this comment.
Actually, now I look again, I'm not sure where that opacity: 0.4; is coming from.
I'm looking at a new Separator using this branch
<hr class="wp-block-separator has-alpha-channel-opacity is-style-wide">
Yet the scss is
.wp-block-separator {
// V1 version of the block expects a default opactiy of 0.4 to be set.
&.has-css-opacity {
opacity: 0.4;
}
}
|
This is still working as advertised in test description.
On the specificity question, to me |
yeh, this is strange, in TwentyTwentyTwo, somewhere/somehow the following CSS is being set: but I can't work out where, doesn't seem to be anywhere in Gutenberg, and can't find in the TwentyTwentyTwo theme either. With TT1 there is a theme setting of but on thinking about it, maybe we want the |
|
@ramonjd I am going to go ahead and merge and we can circle back on the opacity if needed. |
With this commit, the expected HTML for the separator block is updated to reflect recent changes from #38428. This is done so that the tests pass as expected.
…channel opacity to be set(WordPress#38428) Co-authored-by: Glen Davies <glen.davies@a8c.com> Co-authored-by: ramonjd <ramonjd@gmail.com>
|
@glendaviesnz I am not sure, but the "mysterious" opacity CSS might be related to this issue: #39393. We have found that some core styles are coming through, even when they should be overridden by Gutenberg. |
Description
The separator block is currently using its own color control to set a custom color attribute, which means it can't take advantage of the alpha channel support in the color block supports color picker in order to fix this issue.
This PR refactors the block to use the new color block support.
Testing Instructions
V1 Block code examples
Screenshots
Backwards compatibility considerations
As well as ensuring that the 0.4 opacity was maintained for both static content, and content that has been opened in the editor, the HTML structure and classnames were checked between old and new block versions.
The HTML structure of saved content remains the same, with just the addition of a new classname to cover the opacity backwards compatibility. This change should have no impact on themes and plugins that are applying custom CSS via the existing classes and elements.