Fluid typography: convert font size inline style attributes to fluid values

[ramonjd]

Parent issue:

• Fluid typography: custom font sizes should be converted to fluid font sizes values #44758

What?

This commit ensures that custom font size values that appear in the style attribute of block content are converted to fluid typography (if activated)

Why?

Fluid typography for font size presets was added in #39529

However, custom font sizes should also be automatically converted to fluid values. This is a bug

How?

By adding a filter to the render_block hook, then replacing the custom font size via a regex.

Testing Instructions

Enable fluid typography via theme.json

Example theme.json

{
	"version": 2,
	"settings": {
		"appearanceTools": true,
		"typography": {
			"fluid": true
		}
	}
}

Testing in the post and site editors, add a few blocks that have typography support, e.g., text blocks, Group, Columns... there are many 😄

Example block HTML

<!-- wp:paragraph {"style":{"typography":{"fontSize":"2em"}}} -->
<p style="font-size:2em">This is a paragraph</p>
<!-- /wp:paragraph -->

<!-- wp:heading {"style":{"typography":{"fontSize":"4rem","fontStyle":"normal","fontWeight":"600","textTransform":"capitalize","letterSpacing":"29px","textDecoration":"underline"},"spacing":{"padding":{"top":"var:preset|spacing|50","right":"var:preset|spacing|50","bottom":"var:preset|spacing|50","left":"var:preset|spacing|50"},"margin":{"top":"var:preset|spacing|60","right":"var:preset|spacing|60","bottom":"var:preset|spacing|60","left":"var:preset|spacing|60"}},"elements":{"link":{"color":{"text":"var:preset|color|vivid-purple"}}}},"backgroundColor":"vivid-red"} -->
<h2 class="has-vivid-red-background-color has-background has-link-color" style="margin-top:var(--wp--preset--spacing--60);margin-right:var(--wp--preset--spacing--60);margin-bottom:var(--wp--preset--spacing--60);margin-left:var(--wp--preset--spacing--60);padding-top:var(--wp--preset--spacing--50);padding-right:var(--wp--preset--spacing--50);padding-bottom:var(--wp--preset--spacing--50);padding-left:var(--wp--preset--spacing--50);font-size:4rem;font-style:normal;font-weight:600;letter-spacing:29px;text-decoration:underline;text-transform:capitalize">This is a heading</h2>
<!-- /wp:heading -->

<!-- wp:group {"style":{"typography":{"fontSize":"1em"}},"layout":{"type":"constrained"}} -->
<div class="wp-block-group" style="font-size:1em"><!-- wp:paragraph {"style":{"typography":{"fontSize":"1em"}}} -->
<p style="font-size:1em">A paragraph inside a group</p>
<!-- /wp:paragraph -->

<!-- wp:search {"label":"Search","buttonText":"Search","style":{"typography":{"fontSize":"52px"}}} /--></div>
<!-- /wp:group -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:buttons {"style":{"typography":{"fontSize":"47px"}}} -->
<div class="wp-block-buttons has-custom-font-size" style="font-size:47px"><!-- wp:button -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">Test button</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->

For these blocks, add a custom value via the UI control and publish the page

In the frontend, check that the inline font size style values for these blocks have been converted to fluid

Before

After

[ramonjd]

@adamziel 👋 I know you've been looking at anti-patterns for this kind of stuff so just wanted to run it by you.

This lovely line looks at a block's HTML content and searches for a match on font-size: $custom_font_size.

Assuming we can't yet use the HTML_Walker (yet, which I'd love to), do you see any danger here?

[scruffian]

I don't love this approach, but I can't see a better one until the walker is merged. I have tried pretty hard to break it and I haven't found a way.

[ramonjd]

Thanks for testing this @scruffian

Yeah, I had to scrub my keyboard after writing it it felt so dirty. We'll be able to replace with HTML Walker for the plugin, but not 6.1 just yet 🤞

[jasmussen]

Can confirm this appears to work on the frontend, even if not in the editor. From the test instructions, that sounds like it's right for this PR:

Nice work.

One question: it's still possible for a theme to have fluid: true; enabled generally as a settings.typography property, but also then still disable fluid typography on a per-preset basis, right? A la:

... 
		"typography": {
			"dropCap": false,
			"fluid": true,
			"fontSizes": [
				{
					"size": "1.75rem",
					"slug": "large",
					"fluid": false
				},
			]
		}

[scruffian]

LGTM

[ramonjd]

Can confirm this appears to work on the frontend, even if not in the editor.

Thanks for testing @jasmussen This PR attempts to address the editor side of things: #44765

One question: it's still possible for a theme to have fluid: true; enabled generally as a settings.typography property, but also then still disable fluid typography on a per-preset basis, right? A la

Yep, exactly what you said 👍

So themers can choose to have some static presets despite the global flag.

[jasmussen]

Bug fix works as intended then!

[andrewserong]

Thanks for putting this one together @ramonjd! I haven't tested it yet, but just wondering what happens when this PR is applied at the same time as #44762, which adds fluid typography support for server-rendered blocks. Will it attempt to process fluid typography twice, or does it safely ignore existing clamp values?

If it does process fluid typography twice, I wonder if we could add a check that the block opts-in to the typography support, doesn't skip serialization, and also doesn't have a render method / is a static block.

Apologies if I'm overthinking it!

[ramonjd]

Will it attempt to process fluid typography twice, or does it safely ignore existing clamp values?

Excellent question. 10 points to Gryffindor!

We should be fine since values are parsed via gutenberg_get_typography_value_and_unit(), which only accepts value + unit values, e.g., 12.22rem.

But, I will add tests to back up this bald-face assertion.

😄

[andrewserong]

which only accepts value + unit values, e.g., 12.22rem.

Okay, great. Thanks for confirming!

[andrewserong]

This is testing well for me, too, and I wasn't able to break the regex. I did notice that it only targets the first font-size within rendered markup, which I think is the ideal way for this feature to behave. For example, the Search block (which is a server-rendered block), only the first font-size is targeted in the markup. Here's the markup before / after:

Before (each element is 52px)	After (only the first element gets clampified, the rest are 52px)

In this case, from the sounds of it #44762 will take care of the other font-size values, so when the two PRs are combined, it sounds like they'll probably be fine.

If we wanted to be more conservative and not have double handling for server-rendered blocks, then we could always add a couple more checks to the opening if statement of gutenberg_render_typography_support (check that it's not a server-rendered block / check for skip serialization), but since there's already the check to see whether or not the found value is the same as the clampified value, and that the clampification only works on value + unit values, I think doing that's totally optional.

LGTM!

[ramonjd]

If we wanted to be more conservative and not have double handling for server-rendered blocks, then we could always add a couple more checks to the opening if statement of gutenberg_render_typography_support (check that it's not a server-rendered block / check for skip serialization)

I'm happy to be this defensive if you think it's warranted.

[tellthemachines]

Tested with several different blocks, included nested ones with different custom font sizes, and everything seems to work fine ✅

Code looks alright, dodgy regex and all 😄 (jokes aside, it looks pretty safe!)

[andrewserong]

I'm happy to be this defensive if you think it's warranted.

Thanks, I did a little digging, and worked out that we can grab the block type and then check whether or not the block is dynamic, and then skip this callback with something like the following:

	$block_type  = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );

	if ( ! isset( $block_type ) ) {
		return $block_content;
	}

	if (
		! isset( $block['attrs']['style']['typography']['fontSize'] ) ||
		$block_type->is_dynamic()
	) {
		return $block_content;
	}

However, as @tellthemachines pointed out, we can't rely on the fact of a block being a dynamic block to mean that the fluid typography will already be handled. In the case of the cover block, it never calls get_block_wrapper_attributes, and so doesn't receive the server-rendered versions of the block supports, as the server-side code for that block assumes that the block supports were already applied in the JS version when the block was serialized.

So, I think after digging into this a little bit, I'd actually vote in favour of merging this as-is, as it ensures that blocks like Cover, which slip slightly outside of the divide between static and dynamic blocks, are handled.

[ramonjd]

I think after digging into this a little bit, I'd actually vote in favour of merging this as-is, as it ensures that blocks like Cover, which slip slightly outside of the divide between static and dynamic blocks, are handled.

Thanks! I appreciate you looking into it.

[ndiego]

@andrewserong @ramonjd I assume this will need to be backported to 6.1 when complete. Can you confirm?

[jffng]

This is testing well for me, used TT3 and also tested various default patterns which contain inline font sizes.

[ramonjd]

@andrewserong @ramonjd I assume this will need to be backported to 6.1 when complete. Can you confirm?

Thanks for chasing that up @ndiego. The answer is yes 😄

I'd neglected to add the Backport label. Done!

We'll get the backport PRs up to WordPress/wordpress-develop ASAP

[ockham]

Removing the "Backport to WP Beta/RC" label since this has been backported as part of #44868 (and sync'd to Core per WordPress/wordpress-develop#3437).