-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Heading block] Add a wp-block-heading CSS class #42122
Conversation
@adamziel Regex is express and error prone. Can we add a couple of unit tests please? |
@spacedmonkey I've just added a bunch of tests! @scruffian I got rid of that space. Unfortunately, it required switching from |
Unit tests are very useful. Added some comments. I really hope there unit tests are ported to core on core merged. |
|
If you try removing the block class name via Code Editor, it will be added back, saving the content. @adamziel, any reason for not using block deprecations for this? I know deprecation won't update content without resaving it, but this is how we usually update the block markup. P.S. The Heading block has gutenberg/packages/block-library/src/heading/block.json Lines 29 to 32 in 59fb859
|
The class name needs to be applied both in the editor and on the backend, or else the following mismatch will happen: style.scss: .wp-block-heading {
background-color: #0c88b4;
} theme.json: {
"styles": {
"blocks": {
"core/heading": {
"color": {
"background": "pink"
}
}
}
}
} Editor view, where the CSS Website view, where the CSS
@Mamaduka This PR would only add it when rendering, not when saving the content. Exploring an on save backend hook would be an interesting option, though.
It wouldn't add the class to the blocks that already saved, meaning the styles wouldn't work correctly across your site until you manually migrate all your existing blocks :(
This is great for the editor! |
ab712cc
to
883c811
Compare
It looks like I'll need to revisit a few tests before this one can be merged. |
Size Change: +4.1 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
@adamziel without adding a deprecation, preexisting heading blocks will throw a block validation error. This is now what I'm seeing on trunk due to heading blocks both in patterns and existing content. Checking out the commit prior to this one shows those errors disappear.
Given those errors, I'd say we need a deprecation or temporary revert. |
@@ -29,7 +29,7 @@ | |||
"supports": { | |||
"align": [ "wide", "full" ], | |||
"anchor": true, | |||
"className": false, | |||
"className": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this change might also mean that the className is being serialized to post content. Was that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #42122 (comment)
I've created a PR adding the missing deprecation and fixtures: #46138. If the serialization of the classname was deliberate, as the updates to the existing fixtures suggest, that PR should help sort things out. |
@andrewserong @aaronrobertshaw Oh that’s a good point! I know @mtias wanted to avoid having the class name in the serialized markup, and at the same time I remember there were some challenges related to that. I’ll revisit this choice when I’m at the computer today. If this one derails dev envs, feel free to revert. |
The complication is how setting
The class name could be accidentally removed, but that shouldn't be a problem as @Mamaduka explained earlier in this thread:
To have the className without storing it in the markup, we'd have to invent a new supports API that would:
I'd rather not do that just for the header block – it adds complexity to the code and makes the supports harder to understand. If it deems indispensable for any reason, we could consider adding it later on alongside another deprecation. Let's stick to storing the As for adding a |
Sounds good to me, thanks for following up! Looks like it'll be good to land #46138 then 👍 |
Dev note for 6.2 cc @bph @MaggieCabrera @andrewserong : WordPress 6.2 adds a wp-block-heading CSS class to every Heading BlockIn WordPress 6.2, the h1-h6 elements added via the heading block have a brand new wp-block-heading CSS class. This change enables styling the heading block differently from the regular h1-h6 elements. For example, the following theme.json would add a blue background to all h1 elements and a pink background to only the h1 elements added via the heading block: {
"styles": {
"elements": {
"h1": {
"color": {
"background": "blue"
}
}
},
"blocks": {
"core/heading": {
"elements": {
"h1": {
"color": {
"background": "pink"
}
}
}
}
}
}
} Work is underway to add a CSS class to every Gutenberg block in future WordPress releases. Props to @ajlende for reviewing and editing. |
Thank you, @adamziel. Added Dev Note to Misc Editor post. |
The dev note example is not working for individual heading elements with the block's class. See Trac 57904. Is it just a mistake in the example? |
Since this will work for any block except headings, I suggest we change the dev note to use a different block. |
Let's use this example for the dev note:
This will mean that H1 elements inside a cover block will have a pink background, but all other H1s will have a blue background. |
I've dropped the ball here, thank you so much @scruffian! |
What?
This PR adds the
wp-block-heading
CSS class to the heading block.It happens server-side so:
Why?
Without the classname, global styles can't distinguish between the
h1-h6
elements that belong to the heading block vs ones that don't. As a result, the styles set on thecore/heading
block of theme.json gets applied to all h1, h2, h3, etcHow?
This PR transforms the stored
heading
block markup into one with an additional class name using WP_HTML_Tag_Processor.Alternatives considered
I explored leaning on
get_block_wrapper_attributes
, but it falls short:style
andclass
properties, meaning a heading block with an anchor set would lose itsid
.$content
.Testing Instructions
wp-block-heading
css classcc @draganescu @getdave @scruffian