Add a CSS class to all static blocks with className supports

[adamziel]

What?

Ensures all static blocks with supports.className === true are rendered with an appropriate wp-block-$name class name.

It's what #42122 does for the heading block, only generalized to all blocks.

This is possible thanks to WP_HTML_Tag_Processor.

Why?

Precisely targeting blocks with CSS requires having class names that currently aren't there.

How?

By adding a render_block hook with the logic previously created just for the heading block.

Alternatives considered

I explored leaning on get_block_wrapper_attributes, but it falls short:

• It only processes the style and class properties, meaning a heading block with an anchor set would lose its id.
• It ignores any additional attributes stored in $block_content but not block attributes.
• We'd still need to some parsing to replace the old wrapping HTML tag stored in $block_content.
• It infers attributes from global variables.

Testing Instructions

(ignore these for now)

1. Update block-library/src/paragraph/block.json and set supports.className to true
2. Confirm that all the paragraph blocks are now rendered with the wp-block-paragraph CSS class.

cc @mtias @draganescu @spacedmonkey @dmsnell @ZebulanStanphill @getdave @scruffian @Mamaduka @carolinan @luminuu

[draganescu]

This will be the hell of testing in real life. But YES I love this idea.

[luminuu]

So if we want to apply this to the list block as well, I guess the only change that needs to be done is changing the className variable in the block.json file?

[adamziel]

So if we want to apply this to the list block as well, I guess the only change that needs to be done is changing the className variable in the block.json file?

Exactly!

...and updating all the test snapshots :(

[drdogbot7]

YES YES YES!! EVERY BLOCK SHOULD HAVE A CLASS! PLEASE FOR THE LOVE OF GOD I WILL KISS YOU!!

• paragraph
• heading
• list

<p class="wp-block-paragraph"></p>

<h1 class="wp-block-heading wp-block-heading--h1">Level One Heading</h1>
<h2 class="wp-block-heading wp-block-heading--h2">Level One Heading</h2>
<h3 class="wp-block-heading wp-block-heading--h3">Level One Heading</h3>
<h4 class="wp-block-heading wp-block-heading--h4">Level One Heading</h4>
<h5 class="wp-block-heading wp-block-heading--h5">Level One Heading</h5>
<h6 class="wp-block-heading wp-block-heading--h6">Level One Heading</h6>

<ul class="wp-block-list wp-block-list--ul">
  <li class="wp-block-list__list-item">List Item</li>
  <li class="wp-block-list__list-item">List Item</li>
<ul/>

<ol class="wp-block-list wp-block-list--ol">
  <li class="wp-block-list__list-item">List Item</li>
  <li class="wp-block-list__list-item">List Item</li>
<ol/>

EDIT. Got a little carried away there, sorry. I will help test this.

[drdogbot7]

Should we do it at all?

Yes. This would be really helpful for anyone who develops themes. It has caused me no end of headaches not to have a simple and low-specificity way of targeting these core blocks without also targeting every single paragraph, list and heading.

Is filtering on render_block a reasonable solution?

It FEELS a bit wrong, but maybe it's ok.

Is there a more canonical way of solving this problem?

Handling this via block deprecation (as discussed in #42122 (comment)) seems like it's a more 'correct' and probably simpler way to do this, and I wonder if that option has been dismissed too quickly. I get that old blocks aren't automatically migrated, but maybe that's a separate issue that would be better handled by a one-off migration tool.

Re: Classic Block?
It's an edge case, but it's an issue. Paragraphs and headings within a classic block aren't affected by this filter, and the classic block itself isn't wrapped in a div at all.

So currently a paragraph "block" and a paragraph within a classic block are identical on the frontend, but with this pull they're treated differently. It's probably not a good idea to start adding classes to content within a classic block, but there's also no way to target elements within a classic block since the classic block itself isn't tagged. My personal preference would be to wrap the classic block in a .wp-block-classic div, but that would definitely break some themes…

[dmsnell]

function gutenberg_add_class_name_to_wrapping_tag( $block_content, $added_class_name ) {
    return new WP_Html_Modifier( $block_content )
        ->find()
        ->add_class( $added_class_name );
}
``

[dmsnell]

update post-merge of the Tag processor.

we probably don't need a function for this anymore, though it's not a problem. this code should add the class name to the first tag in the block, which is assumed to be the wrapper.

$tags = new WP_HTML_Tag_Processor( $block_content );
if ( $tags->next_tag() ) {
	$tags->add_class( wp_get_block_default_classname( $block->name ) );
}
return $tags->get_updated_html();

[scruffian]

IMO we should pursue this approach. It looks very promising.

[adamziel]

Thank you @Mamaduka ! I still want to make this happen, I just wasn't confident about this PR's approach to parsing HTML so I explored a reliable canonical approach with @dmsnell: WP_HTML_Walker: Inject dynamic data to block HTML markup in PHP #42485.

That proposal is now ready for comments and reviews. I'd like to use it in this PR once it's finalized.

[mtias]

One of the points of making this work dynamically server-side is to be able to toggle it on / off or even filter out what it prints. Should we have a mechanism to opt-in / out via filters? Should it be a flag in theme.json?

[adamziel]

@mtias Filters sound good to me. I'm not sure how a flag in theme.json would work, though. Are you thinking of a global feature switch? Or a per-block flag?

[draganescu]

a per-block flag

in theme.json that is ON by default would be cool

[adamziel]

Now that the [Heading block] Add a wp-block-heading CSS class is merged and we have the WP_HTML_Tag_Processor, this should be much easier.

Here's the solution blueprint:

1. Enable "className" in block.json
2. Filter the block markup server-side
3. Use WP_HTML_Tag_Processor to add the correct class name
4. Update all the test mocks snapshots

The filter could perhaps be global and hinge on the className flag from block.json since JavaScript already does that.

Any volunteers for this one? I am currently focusing on the WordPress sandbox demo and won't have the bandwidth to revisit this PR soon.

[getdave]

What happened here? 😕

[adamziel]

@get I've replaced the original branch with take two, and accidentally closed the PR in the process :-)

[getdave]

@get I've replaced the original branch with take two, and accidentally closed the PR in the process :-)

I think you meant to ping @getdave here right not @get.

[adamziel]

I found an easy way to proceed with this PR. It now just adds the technical capability and does not migrate any block yet. Notice how short it is now!

Should we store the CSS class in the block markup, though? Because this PR requires setting supports.className to true:

By default, the class .wp-block-your-block-name is added to the root element of your saved markup. This helps having a consistent mechanism for styling blocks that themes and plugins can rely on. If, for whatever reason, a class is not desired on the markup, this functionality can be disabled.

Advantage: the class name "just works" in the block editor

Disadvantage: the class name pollutes the block markup

An alternative would be to only add the class name to the rendered HTML without ever storing it in the markup. That would require a new supports API to:

• Add the class name in the block editor preview, but not in the edited markup
• Add the class name server-side when rendering the markup

That hypothetical API could be called, say, "storeClassNameInTheMarkup": false, and work in conjunction with className. I'm not sure if it's worth it, though. What do y'all think? @scruffian @MaggieCabrera @mikachan @draganescu @getdave @mtias @Mamaduka @andrewserong

[getdave]

I love how terse this is now. Great work 👏

What were the concerns about having classnames in the stored markup? Is this concern documented or was it discussed elsewhere?

[andrewserong]

Thanks for the ping!

Should we store the CSS class in the block markup, though?

From memory some of the prior arguments for avoiding storing things in serialized markup where we can include:

• The need for adding deprecations whenever we change markup. Deprecations can be challenging to work with, and once added can't be removed, so ideally it's good to avoid changing markup of core blocks unless we have a good reason to make the change (e.g. adding in features that deliver user value / quality of life improvements for themers, etc).
• If we would like to conditionally change the markup in some way, particularly via theme settings. One of the good things about this PR and the new WP_HTML_Tag_Processor class in general is that it frees us up to potentially make reversible changes in a way that doesn't result in needing to perform changes to things stored in the database. In this regard (despite their complexity) I quite like the layout and fluid typography features we have, because it's possible to update logic between major WP versions without needing to do any block / data migrations.

Is this concern documented or was it discussed elsewhere?

I can't remember where the earlier discussions were, I'm sorry! But the number of PRs and regressions surrounding either missing deprecations, or deprecations that require adjustment, means that I'm typically fairly cautious about changing core block markup. There are lots of times when it's necessary, but it's also pretty cool if we can build dynamic features that augment markup rather than needing it to be baked into post content.

That hypothetical API could be called, say, "storeClassNameInTheMarkup": false, and work in conjunction with className. I'm not sure if it's worth it, though. What do y'all think?

While it would be a new API, it also sounds like it'd be quite a small API, and could potentially avoid having to add additional deprecations? If so, that sounds like it could be worth it to me.

I'll copy in @aaronrobertshaw and @glendaviesnz on the discussion (just for visibility), since they've been looking at deprecations PRs lately.

Thanks for looking into all this!

[andrewserong]

Is this concern documented or was it discussed elsewhere?

I think I found one of the instances, in this comment: #38998 (comment)

I think there's also something prior to that which is ensuring all blocks output .wp-block-{name} on the front-end even if not serialized (headings, lists, and paragraphs don't have them). This might need to be something that can be toggled on / off as needed. (People may also consider that p already provides all the semantic that is needed and that .wp-block-paragraph would noisy and redundant as an addition to it, and I'd agree.)

If it needs to be toggled, then that points to the new API idea of storeClassNameInTheMarkup (or something to that effect) being quite useful.

[aaronrobertshaw]

Thanks for the ping @andrewserong and for all the hard work here @adamziel 👍

As has already been discussed, anything we can do to avoid changing saved markup for blocks and thereby also avoid deprecations is a win.

One thing we should be careful of is creating a greater disparity between the editor and the frontend. It might already be part of the proposed approach but I'd like to see the same classes available on both, just without the wp-block-<name> class being saved to the markup if the proposed storeClassNameInTheMarkup flag is false.

While it might not be a problem, the addition and use of these class names might have some flow-on effects for theme.json and global styles. We have several blocks currently using simple elements as their selector. For example, take the paragraph block that uses p. Any styles added by a theme targeting .wp-block-paragraph would immediately override the paragraph block global styles.

We probably then need to consider updating the block.json selectors as part of the process of adopting className support.

[getdave]

An alternative would be to only add the class name to the rendered HTML without ever storing it in the markup. That would require a new supports API to:

• Add the class name in the block editor preview, but not in the edited markup
• Add the class name server-side when rendering the markup

That hypothetical API could be called, say, "storeClassNameInTheMarkup": false, and work in conjunction with className. I'm not sure if it's worth it, though. What do y'all think? @scruffian @MaggieCabrera @mikachan @draganescu @getdave @mtias @Mamaduka @andrewserong

In reference to the excellent contrbutions to the discussion above, should we then consider what @adamziel suggested above?

That would:

• avoid storing presentation things in serialized block content
• allow for dynamically adding presentational things when rendering block on both editor and front of site

As @aaronrobertshaw says it's important to retain the 1:1 between editor and front end where possible. I fear that if there are classes like wp-heading- on the front end and not in the editor the visual presentation of the block's could easily diverge.

It might be nonsense but could we consider using existing supports.className but allowing it to (optionally) be an configurable object:

supports.className: {
    serialize: false
}

[scruffian]

FWIW I also agree that we should avoid serializing anything unnecessary to the block markup, and I believe that is consistent with the philosophy of Gutenberg.

[sstruemph]

Can we expect that this update will be merged?

[getdave]

Can we expect that this update will be merged?

It's unlikely to make it to WP 6.2.

The last comment indicates we'll need to revise the PR in a new direction. I'm not aware of anyone currently working on this however.

[gziolo]

It might be nonsense but could we consider using existing supports.className but allowing it to (optionally) be an configurable object:

supports.className: {
    serialize: false
}

It could be also as simple as

supports.className: 'dynamic' // or `view`

The question is whether this class should be also injected in useBlockProps implementation so it's available in the editor view for styling purposes. The other question is about all types of previews in the editor where this class might be missing when dynamically injected only on the server.

[RavanH]

Oh yes, please... The List block could do with a dedicated block class too. See #12420 and #50342