Elements: Merge element style and classname generation to single filter

[aaronrobertshaw]

Addresses: #59462
Related: #59533

What?

This is an alternate approach to fixing #59462 and the further issue noted on the original fix in #59533.

TL;DR - Prevents other filters from disrupting the element class name generation while also reducing the chance for class name conflicts.

Why?

While #59533 fixed the original issue it introduced further albeit reduced issues.

How?

• Switch elements block support from using pre_render_block to render_block_data
• Within the new render_block_data filter, generate and store both the elements styles and class name
• Update the render_block filter to double-check if an elements class name has been added to the block attributes then add it to the block content

Testing Instructions

Confirm #59462 is still resolved

1. Edit a post and add a paragraph
2. Create a link with the paragraph
3. Via the block inspector apply a link color to the paragraph
4. Save and view the frontend confirming the correct link color is shown

Confirm nested applications of the same element styles work correctly:

1. Create some nested blocks containing links
2. Apply a set of element styles to the parent block
3. Apply different styles to a child block
4. Apply the first set of element styles to a grand-child block
5. Confirm the correct display of element styles

Example block markup

<!-- wp:group {"style":{"elements":{"link":{"color":{"text":"var:preset|color|accent-3"}}}},"className":"custom-link-color","layout":{"type":"constrained"}} -->
<div class="wp-block-group custom-link-color has-link-color"><!-- wp:paragraph -->
<p>Welcome to <a href="http://www.wordpress.org">WordPress</a>. This is your first post. Edit or delete it, then start writing!</p>
<!-- /wp:paragraph -->

<!-- wp:group {"style":{"elements":{"link":{"color":{"text":"var:preset|color|accent-4"}}}},"layout":{"type":"constrained"}} -->
<div class="wp-block-group has-link-color"><!-- wp:paragraph -->
<p>Secondary paragraph with <a href="http://test.com">link</a></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph {"style":{"elements":{"link":{"color":{"text":"var:preset|color|accent-3"}}}}} -->
<p class="has-link-color"><a href="http://test.test">Third paragraph</a> </p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->

Screenshots or screencast

Before	After

In the before screenshot the third paragraph's link color is not applied correctly.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/block-supports/elements.php
❔ phpunit/block-supports/elements-test.php

[aaronrobertshaw]

The test failures are related to the changes in this PR. I'm working on fixes for them now.

[ramonjd]

wp_render_elements_support_styles has been a public function since WP 6.0.

I searched on wpdirectory.net and there doesn't appear to be any usages, but I'm wondering if we should formally deprecate this one to make sure?

[aaronrobertshaw]

I'm not 100% sure on this one. These filters weren't designed to be reused. They're also not publicly documented as part of our API, and only really useful as a filter for this specific use case.

Given that, I'm leaning towards handling any deprecation requirements in the core backport.

[peterwilsoncc]

I'd recommend handling it consistently in Gutenberg if the intent is to convert the parameter to $deprecated in Core.

It's the kind of thing that can be easily missed when merging to the wp-dev repo so it would be good to avoid any risk.

It's also hooked on to render_block_data with 10, 2 so may end up throwing an error. I'm unable to see if the filter render_block_data has changed in GB but it's pretty old so is probably only in WP-Dev now days.

[tellthemachines]

This seems to be a bug; I'd expect it to not be necessary to add the attributes manually to $block_content. It might be this issue resurfacing. I was meaning to fix it at the time and then forgot all about it 😅

[aaronrobertshaw]

My inclination was that it was due to the block being static and the markup having been generated at the point it was saved rather than render.

[andrewserong]

This looks like a good follow-up fix to me, thanks for digging in @aaronrobertshaw!

✅ Tested that #59462 is still resolved
✅ Nested link styles are now working correctly

There'll likely be a little bit of stuff to deal with in the backport to core process (deprecating the existing function, swapping null coalescing operators for ternaries?), but the approach here looks good to me, and I like the idea of injecting the classname into the attribute, and then ensuring it's output in the render callback/filter.

LGTM! ✨

[ramonjd]

Manual testing works as described for me too 🚀

[peterwilsoncc]

Follow up re: Raymon's comment on back-compat.

And while I was here figured I'd not the coding standards thing.

Sorry for the late notice, as someone who primarily track wp-dev it's easier to track GB commits rather than all the PRs.

[peterwilsoncc]

I'd recommend handling it consistently in Gutenberg if the intent is to convert the parameter to $deprecated in Core.

It's the kind of thing that can be easily missed when merging to the wp-dev repo so it would be good to avoid any risk.

It's also hooked on to render_block_data with 10, 2 so may end up throwing an error. I'm unable to see if the filter render_block_data has changed in GB but it's pretty old so is probably only in WP-Dev now days.

[peterwilsoncc]

/*
 * Multiline comments
 * use this format
 */

Per WP Coding standards for PHP and JS. https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/#5-inline-comments

A common gotcha is to start wiht /** -- please don't as that's used by parsers looking for docblocks

[peterwilsoncc]

As above re comment formatting.

[andrewserong]

I just cherry-picked this PR to the release/17.8 branch to get it included in the next release: 374f652

[tellthemachines]

I'd recommend handling it consistently in Gutenberg if the intent is to convert the parameter to $deprecated in Core.

@peterwilsoncc this is just a filter that was added to the pre_render_block hook and is no longer needed. Does this type of function need to be deprecated? It's not publicly documented and it's hard to imagine it would be of any use to extenders.

[peterwilsoncc]

@tellthemachines typically we deprecate even the private functions as historically extenders have used functions & classes whether they're marked public, private or internal. List Table classes prior to r54378 are the poster child for this but there's other common examples too.

I'm trying to understand how it hooks on to render_block_data -- it used to manipulate $source_block but is now using $parsed_block, is that because the HTML parser Denis and others worked on allows you to avoid re-working the code?

[aaronrobertshaw]

Thanks for the feedback @peterwilsoncc 👍

I'll put together two PRs shortly:

1. To clean up the filters here in Gutenberg, including deprecations
2. A backport including this PRs changes, along with the suggested tweaks, deprecations, and code style fixes

I'm trying to understand how it hooks on to render_block_data

If I follow the question correctly, it isn't clear how the element block supports use to work versus the latest approach in this PR?

Previously, the elements block supports were implemented in two parts:

1. Generating CSS for a block's element styles and storing those in the style engine under a generated class name (pre_render_block)
2. Generating that class name again and injecting that into the block's markup (render_block)

I believe this was originally done this way so that the order the CSS was generated resulted in the correct cascade. If the CSS generation happened in the original render_block filter only, then the order would have been back to front. With the choice to use pre_render_block, that didn't offer the ability to manipulate the block markup to inject the elements CSS class.

To generate a consistent class name between those two filters (render_block and pre_render_block), the $block object was hashed but this broke when the block's data gets filtered within render_block_data such as #59057.

That brings us to this PR and the current approach:

• The work generating the element styles and class name has been moved from the pre_render_block filter to render_block_data.
• This allows the class name to be generated once only and stored in the block attributes so it is robust in the face of filtered attributes.
• The previous pre_render_block filter didn't provide a means to store the class name or change the markup forcing the switch to render_block_data instead.
  • It was an unfortunate oversight of mine to not completely rename the callback functions for cleaner deprecation. My apologies.
• To allow static blocks that already have their markup saved to get the new elements class name, a render_block callback is still needed but it now only checks for an elements class within the latest parsed attributes and adds it to the markup if needed.

I hope that helps clarify the situation some 🤞

If not, I'll get a couple of PRs up addressing the feedback and they might shed further light on the matter.

Thanks again for the feedback and help!

[aaronrobertshaw]

A very quick and rough draft for updating Gutenberg as discussed above can be found in #59538. I might not be able to iterate on that PR and a backport until next week however.

[aaronrobertshaw]

Even more rough draft for a backport is up in WordPress/wordpress-develop#6214. As noted on both the PRs mentioned here, I will have very limited bandwidth until next week to push them across the line.

If there is any urgency to land these and someone would like to pick them up, please do! 🙏

[getdave]

@aaronrobertshaw Thanks for handling the Backport. I wasn't sure whether this is headed for WP 6.5. Could you confirm?

[aaronrobertshaw]

Thanks for double-checking @getdave 👍

I don't believe this is required for 6.5.

It fixes issues introduced in #59533 which in turn was intended to fix a bug exposed by #59057.

#59057 only landed in Gutenberg 17.8.

[aaronrobertshaw]

The changes from this PR and #59538 are combined in a single backport available in WordPress/wordpress-develop#6214.