Fluid typography: convert server-side block support values #44762

ramonjd · 2022-10-07T01:35:40Z

Parent issue:

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

What?

Let's do this! 🕺

For server rendered typography block support styles, covert font size values to fluid values in the inline styles if fluid typography is activated.

❗ This has effect on the frontend only and not the editor.

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?

Using the existing gutenberg_get_typography_font_size_value() function.

Testing Instructions

Run npm run test:unit:php -- --filter WP_Block_Supports_Typography_Test

Enable fluid typography via theme.json

Here is some test theme.json

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

Testing in the post and site editors, add a Site Title block

Here is some example HTML

<!-- wp:site-title {"style":{"typography":{"fontSize":"62px"}}} /-->

Apply a custom font size to the block and publish the page

In the frontend, check that the inline font size style value has been converted to fluid

2022-10-07.12.28.31.mp4

ramonjd · 2022-10-07T02:27:26Z

lib/block-supports/typography.php

+		$typography_block_styles['fontSize'] = $preset_font_size ? $preset_font_size : gutenberg_get_typography_font_size_value(
+			array(
+				'size'  => $custom_font_size,
+				'fluid' => true,


Ah, we don't actually need this line since gutenberg_get_typography_font_size_value() checks explicitly for false

Suggested change

'fluid' => true,

Yes, good find! Let's ditch it (I think I got a bit confused when I encountered that earlier)

andrewserong

Nice one, this is working well for me @ramonjd, and the tests look good 👍

✅ Setting font sizes by preset still works as expected
✅ Setting a custom font size with fluid set to false results in the direct font-size value being used
✅ Switching settings.typography.fluid to true adjusts the output for server-rendered blocks to use the clampified values

fluid `false`	fluid `true`

Since this PR doesn't cover the editor behaviour / rendering, we might want to hold off on merging until that change is ready to go too? But otherwise this LGTM! ✨

ramonjd · 2022-10-07T02:59:13Z

we might want to hold off on merging until that change is ready to go too?

Good point. Thanks for testing!

tellthemachines

I tested all the server rendered blocks with typography support and they're all working well, except for Nav Link, Submenu, Page List and Search, for which I pushed some custom logic so they should be working now!

Cover block when server rendered (if it has a featured image) doesn't seem to come under this logic. I think we'd best re-test when the work to support non-server rendered blocks is in place, as it might just work then.

jasmussen · 2022-10-07T07:44:14Z

Nice one! Just confirming that this fixes it on the frontend for me, even if not in the editor, which I understand is the goal for this PR:

noisysocks · 2022-10-10T02:53:05Z

packages/block-library/src/search/index.php

+			'font-size: %s;',
+			gutenberg_get_typography_font_size_value(
+				array(
+					'size' => esc_attr( $attributes['style']['typography']['fontSize'] ),


Why does this use esc_attr? $attributes['style']['typography']['fontSize'] isn't a HTML attribute.

From memory, the search block needs to explicitly call esc_attr because it isn't using get_block_wrapper_attributes. When the other server-rendered blocks pass their styles through to that function, it winds up being escaped around this line in core: https://github.com/WordPress/wordpress-develop/blob/5ebe28966e5decbe1862c147bf98db0d8094038e/src/wp-includes/class-wp-block-supports.php#L213 — so the search block needs to do its own escaping before injecting.

It looks like the Search block's style attributes are already being sanitised here using safecss_filter_attr:

gutenberg/packages/block-library/src/search/index.php

Lines 402 to 405 in 6a2d304

'input' => ! empty( $input_styles ) ? sprintf( ' style="%s"', safecss_filter_attr( implode( ' ', $input_styles ) ) ) : '',

'button' => ! empty( $button_styles ) ? sprintf( ' style="%s"', safecss_filter_attr( implode( ' ', $button_styles ) ) ) : '',

'wrapper' => ! empty( $wrapper_styles ) ? sprintf( ' style="%s"', safecss_filter_attr( implode( ' ', $wrapper_styles ) ) ) : '',

'label' => ! empty( $label_styles ) ? sprintf( ' style="%s"', esc_attr( safecss_filter_attr( implode( ' ', $label_styles ) ) ) ) : '',

That should mean all of the esc_attr calls in this function are unnecessary albeit probably harmless.

At the very least let's change this so that esc_attr is called on the output of gutenberg_get_typography_font_size_value, not the input. It's not right at all to escape the input as gutenberg_get_typography_font_size_value wouldn't know what to do with an HTML escape sequence (e.g. $lt; or $quot;) if it encountered one.

At the very least let's change this so that esc_attr is called on the output of gutenberg_get_typography_font_size_value, not the input.

Ah, yes, good point! Agreed.

It looks like the Search block's style attributes are already being sanitised here using safecss_filter_attr:

safecss_filter_attr doesn't handle all circumstances for when a CSS string is used in an inline style, so we still need the esc_attr call.

safecss_filter_attr doesn't handle all circumstances for when a CSS string is used in an inline style, so we still need the esc_attr call.

I'd say move the esc_attr call up to styles_for_block_core_search then. Strangely that's how it works for 'label' but none of the other keys.

Good feedback, hadn't thought that esc_attr might return something the font size function can't process 😅 I'll fix it!

Thanks for fixing that up @tellthemachines! 🙇

ndiego · 2022-10-10T14:48:02Z

@ramonjd @noisysocks @andrewserong I am seeing a couple of approvals. Are we in good shape to merge this? If so, I want to ensure we get this backported ahead of RC1. Thanks!

jffng · 2022-10-10T17:05:40Z

Once more, gave this a spin with TT3 and it's working. I noticed #44764 also cover the site title test case, but this one is needed for the Nav block. Not sure technically why that is the case, but this PR seems to be necessary for some server side blocks 👍

…size values to fluid values in the inline styles if fluid typography is activated. Added unit tests

ramonjd · 2022-10-10T22:26:23Z

@jffng Thanks a lot for testing these 🙇

* For serverside rendered typography block support styles, covert font size values to fluid values in the inline styles if fluid typography is activated. Added unit tests * Add fluid font size to Nav Link * Add fluid typography to Search block * Fix fluid typography for Submenu block with open on click * Fix fluid typography for Page List block * Remove unnecessary parameter * Call esc_attr only once on the whole typography string Co-authored-by: tellthemachines <isabel@tellthemachines.com>

* Fluid typography: convert server-side block support values (#44762) * For serverside rendered typography block support styles, covert font size values to fluid values in the inline styles if fluid typography is activated. Added unit tests * Add fluid font size to Nav Link * Add fluid typography to Search block * Fix fluid typography for Submenu block with open on click * Fix fluid typography for Page List block * Remove unnecessary parameter * Call esc_attr only once on the whole typography string Co-authored-by: tellthemachines <isabel@tellthemachines.com> * Fluid Typography: Fix bug in global styles where fluid clamp rules were not calculated for custom values (#44761) * Fluid Typography: Fix bug where fluid clamp rules were not calculated for custom global styles values * Add inline comments * Add tests for JS changes * Fluid Typography: ensure global styles preset fluid sizes are calculated correctly (#44791) * Forked #44761 * Ensuring the font size picker select box shows the right labels * update comment. Typescript has beaten me. It's much more convenient to use getFontSizeOptions(), but I guess we'll have to refactor. * Adding comment about Typescript bug (YAY, it wasn't me being dumb with TS for once) * Added tests yo * Changeo loggo * Create a new FontSizeSelectOption return type for getSelectedOption to: 1. Clean up type changes in #44791 2. Make TS linter be quiet * Revert accidental changes to emptytheme * Revert type changes and other calamities * Remove additional size value from getToggleGroupOptions test as I believe it is no longer expected Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: ramonjd <ramonjd@gmail.com> * Fluid typography: convert font size inline style attributes to fluid values (#44764) * This commit ensures that custom font size values that appear in the style attribute of block content are converted to fluid typography (if activated) * Adding comments * Bail early if we don't find a custom font size * Adding tests yo * Updating PHP doc to describe incoming type of $raw_value (string|number) * Make custom font sizes appear fluid in the block editor when fluid typography is enabled (#44765) * Make custom font sizes appear fluid in the block editor when fluid typography is enabled * Add tests for fluid utils * update description * You shall not pass with a number, well, yes, but we'll coerce it to `px` and the tests shall pass nonetheless!!! Co-authored-by: Ben Dwyer <ben@scruffian.com> Co-authored-by: ramonjd <ramonjd@gmail.com> * Fluid typography: covert font size number values to pixels (#44807) * Converts incoming raw font size values to value + pixel to remain consistent with the fontsizepicker component. * Updating comments / docs * Fluid typography: ensure fontsizes are strings or integers (#44847) * Updating PHP doc to describe incoming type of $raw_value (string|int) This PR also harmonizes the JS checks And review comments from #44807 (review) These changes have already been backported to Core in WordPress/wordpress-develop#3428 * Update changelog Add extra test for floats Add i18n domain * Font sizes can be string|int|float Updated tests and JSDoc type * Expand tests for gutenberg_get_typography_value_and_unit Fix typo in CHANGELOG.md * Initial commit (#44852) - ensure that we convert fluid font sizes to fluid values in the editor for search block block supports - we do this by passing the getTypographyClassesAndStyles hook a flag to tell it whether to convert Updated CHANGELOG.md Added tests Co-authored-by: tellthemachines <isabel@tellthemachines.com> Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Co-authored-by: Robert Anderson <robert@noisysocks.com> Co-authored-by: Ben Dwyer <ben@scruffian.com>

…ate 1. Merges the following: * [WordPress/gutenberg#44761 Gutenberg PR 44761] - Fluid Typography: Fix bug in global styles where fluid `clamp()` rules were not calculated for custom values * [WordPress/gutenberg#44762 Gutenberg PR 44762] - Fluid Typography: Convert server-side block support values * [WordPress/gutenberg#44764 Gutenberg PR 44764] - Fluid Typography: Convert font size inline style attributes to fluid values * [WordPress/gutenberg#44807 Gutenberg PR 44807] - Fluid Typography: Covert font size number values to pixels * [WordPress/gutenberg#44847 Gutenberg PR 44847] - Fluid Typography: ensure font sizes are strings or integers Follow-up to [54280]. Props andrewserong, isabel_brison, ramonopoly. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54497 602fd350-edb4-49c9-b593-d223f7449a82

…ate 1. Merges the following: * [WordPress/gutenberg#44761 Gutenberg PR 44761] - Fluid Typography: Fix bug in global styles where fluid `clamp()` rules were not calculated for custom values * [WordPress/gutenberg#44762 Gutenberg PR 44762] - Fluid Typography: Convert server-side block support values * [WordPress/gutenberg#44764 Gutenberg PR 44764] - Fluid Typography: Convert font size inline style attributes to fluid values * [WordPress/gutenberg#44807 Gutenberg PR 44807] - Fluid Typography: Covert font size number values to pixels * [WordPress/gutenberg#44847 Gutenberg PR 44847] - Fluid Typography: ensure font sizes are strings or integers Follow-up to [54280]. Props andrewserong, isabel_brison, ramonopoly. See #56467. Built from https://develop.svn.wordpress.org/trunk@54497 git-svn-id: http://core.svn.wordpress.org/trunk@54056 1a063a9b-81f0-0310-95a4-ce76da25c4cd

…ate 1. Merges the following: * [WordPress/gutenberg#44761 Gutenberg PR 44761] - Fluid Typography: Fix bug in global styles where fluid `clamp()` rules were not calculated for custom values * [WordPress/gutenberg#44762 Gutenberg PR 44762] - Fluid Typography: Convert server-side block support values * [WordPress/gutenberg#44764 Gutenberg PR 44764] - Fluid Typography: Convert font size inline style attributes to fluid values * [WordPress/gutenberg#44807 Gutenberg PR 44807] - Fluid Typography: Covert font size number values to pixels * [WordPress/gutenberg#44847 Gutenberg PR 44847] - Fluid Typography: ensure font sizes are strings or integers Follow-up to [54280]. Props andrewserong, isabel_brison, ramonopoly. See #56467. Built from https://develop.svn.wordpress.org/trunk@54497 git-svn-id: https://core.svn.wordpress.org/trunk@54056 1a063a9b-81f0-0310-95a4-ce76da25c4cd

ockham · 2022-10-17T09:27:48Z

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).

…ate 1. Merges the following: * [WordPress/gutenberg#44761 Gutenberg PR 44761] - Fluid Typography: Fix bug in global styles where fluid `clamp()` rules were not calculated for custom values * [WordPress/gutenberg#44762 Gutenberg PR 44762] - Fluid Typography: Convert server-side block support values * [WordPress/gutenberg#44764 Gutenberg PR 44764] - Fluid Typography: Convert font size inline style attributes to fluid values * [WordPress/gutenberg#44807 Gutenberg PR 44807] - Fluid Typography: Covert font size number values to pixels * [WordPress/gutenberg#44847 Gutenberg PR 44847] - Fluid Typography: ensure font sizes are strings or integers Follow-up to [54280]. Props andrewserong, isabel_brison, ramonopoly. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54497 602fd350-edb4-49c9-b593-d223f7449a82

ramonjd added the [Feature] Typography Font and typography-related issues and PRs label Oct 7, 2022

ramonjd requested review from noisysocks, tellthemachines and andrewserong October 7, 2022 01:35

ramonjd self-assigned this Oct 7, 2022

ramonjd marked this pull request as ready for review October 7, 2022 01:35

ramonjd requested a review from spacedmonkey as a code owner October 7, 2022 01:35

ramonjd mentioned this pull request Oct 7, 2022

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

Closed

10 tasks

ramonjd added [Type] Bug An existing feature does not function as intended Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Oct 7, 2022

ramonjd commented Oct 7, 2022

View reviewed changes

andrewserong approved these changes Oct 7, 2022

View reviewed changes

ramonjd changed the title ~~Fluid typography: covert server-side block support values~~ Fluid typography: convert server-side block support values Oct 7, 2022

tellthemachines requested a review from ajitbohra as a code owner October 7, 2022 04:31

andrewserong mentioned this pull request Oct 7, 2022

Fluid Typography: Fix bug in global styles where fluid clamp rules were not calculated for custom values #44761

Merged

tellthemachines approved these changes Oct 7, 2022

View reviewed changes

andrewserong mentioned this pull request Oct 9, 2022

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

Merged

noisysocks reviewed Oct 10, 2022

View reviewed changes

ramonjd and others added 7 commits October 11, 2022 09:06

For serverside rendered typography block support styles, covert font …

77dcd20

…size values to fluid values in the inline styles if fluid typography is activated. Added unit tests

Add fluid font size to Nav Link

2690b93

Add fluid typography to Search block

4e2d5c2

Fix fluid typography for Submenu block with open on click

3595f9f

Fix fluid typography for Page List block

3ea17bf

Remove unnecessary parameter

5c2176e

Call esc_attr only once on the whole typography string

4039afe

ramonjd force-pushed the add/fluid-typography-block-supports-serverside branch from 071c455 to 4039afe Compare October 10, 2022 22:07

andrewserong merged commit e435413 into trunk Oct 10, 2022

andrewserong deleted the add/fluid-typography-block-supports-serverside branch October 10, 2022 23:35

github-actions bot added this to the Gutenberg 14.4 milestone Oct 10, 2022

andrewserong mentioned this pull request Oct 11, 2022

Fluid Typography: Backport PHP changes for custom font sizes WordPress/wordpress-develop#3431

Closed

ramonjd added the [Priority] High Used to indicate top priority items that need quick attention label Oct 11, 2022

ramonjd mentioned this pull request Oct 11, 2022

WP 6.1: backport fluid typography #44868

Merged

ramonjd mentioned this pull request Oct 12, 2022

Tracking: fluid typography #44888

Closed

ockham removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 17, 2022

oandregal mentioned this pull request Oct 28, 2022

Add WP_Object_Cache to the gutenberg_get_global_settings method #45372

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fluid typography: convert server-side block support values #44762

Fluid typography: convert server-side block support values #44762

ramonjd commented Oct 7, 2022 •

edited

Loading

ramonjd Oct 7, 2022

andrewserong Oct 7, 2022

andrewserong left a comment

ramonjd commented Oct 7, 2022

tellthemachines left a comment

jasmussen commented Oct 7, 2022

noisysocks Oct 10, 2022

andrewserong Oct 10, 2022

noisysocks Oct 10, 2022 •

edited

Loading

andrewserong Oct 10, 2022

noisysocks Oct 10, 2022

tellthemachines Oct 10, 2022

tellthemachines Oct 10, 2022

andrewserong Oct 10, 2022

ndiego commented Oct 10, 2022

jffng commented Oct 10, 2022

ramonjd commented Oct 10, 2022

ockham commented Oct 17, 2022

	'input' => ! empty( $input_styles ) ? sprintf( ' style="%s"', safecss_filter_attr( implode( ' ', $input_styles ) ) ) : '',
	'button' => ! empty( $button_styles ) ? sprintf( ' style="%s"', safecss_filter_attr( implode( ' ', $button_styles ) ) ) : '',
	'wrapper' => ! empty( $wrapper_styles ) ? sprintf( ' style="%s"', safecss_filter_attr( implode( ' ', $wrapper_styles ) ) ) : '',
	'label' => ! empty( $label_styles ) ? sprintf( ' style="%s"', esc_attr( safecss_filter_attr( implode( ' ', $label_styles ) ) ) ) : '',

Fluid typography: convert server-side block support values #44762

Fluid typography: convert server-side block support values #44762

Conversation

ramonjd commented Oct 7, 2022 • edited Loading

What?

Why?

How?

Testing Instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewserong left a comment

Choose a reason for hiding this comment

ramonjd commented Oct 7, 2022

tellthemachines left a comment

Choose a reason for hiding this comment

jasmussen commented Oct 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noisysocks Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndiego commented Oct 10, 2022

jffng commented Oct 10, 2022

ramonjd commented Oct 10, 2022

ockham commented Oct 17, 2022

ramonjd commented Oct 7, 2022 •

edited

Loading

noisysocks Oct 10, 2022 •

edited

Loading