Skip to content
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

Fluid Typography: Backport PHP changes for custom font sizes #3431

Conversation

andrewserong
Copy link
Contributor

@andrewserong andrewserong commented Oct 11, 2022

Backports the PHP elements of the following Fluid Typography bug fixes from Gutenberg:

Note: from the above PRs, any changes to files in block-library have not been included, as they will form part of a backport PR for the JS/npm packages changes.

Trac ticket: https://core.trac.wordpress.org/ticket/56467

Gutenberg tracking issue: WordPress/gutenberg#44758

To test:

  • With a theme with settings.typography.fluid set to true (e.g. tests with TT3 theme), create a post and set a custom font size on a heading or paragraph block. Save the post and view on the site frontend, if you inspect the markup, the font-size inline style for those blocks should be a clamp() value with the fluid rules applied.
  • In global styles, set the Typography H1 setting to a custom font-size (e.g. 4rem). Save the global styles, then on the site frontend, inspect the markup for the post or page title. The h1 element should have a styling rule associated with it with the clamp() based rules applied.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@andrewserong andrewserong changed the title Fluid Typography: Backport PHP changes Fluid Typography: Backport PHP changes for custom font sizes Oct 11, 2022
@andrewserong andrewserong marked this pull request as ready for review October 11, 2022 01:49
Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good!

Should we add the changes from https://github.com/WordPress/gutenberg/pull/44807/files too?

We'll probably also want to add WordPress/gutenberg#44847 once it's merged.

@andrewserong
Copy link
Contributor Author

Should we add the changes from https://github.com/WordPress/gutenberg/pull/44807/files too?
We'll probably also want to add WordPress/gutenberg#44847 once it's merged.

Yes, it'd be good to add those in. It overlaps a little with #3428, but since WordPress/gutenberg#44847 appears to supersede 3428, I think once the latter has landed we can pull both changes into this PR. I'll take a look once 44847 has merged.

@tellthemachines
Copy link
Contributor

Forgot to mention, but I also tested these changes on TT3 with both global and block-level custom font sizes, on static and dynamic blocks, and everything works as expected! (Except for the dynamic blocks with exceptional behaviour that require block-library changes, those will only work after the package updates.)

@andrewserong
Copy link
Contributor Author

Thanks for reviewing @tellthemachines!

In 05f8898, I've had a go at adding in changes from the following:

Please let me know if I missed anything @ramonjd!

@ramonjd
Copy link
Member

ramonjd commented Oct 11, 2022

This is working well for me. Custom and preset fluid sizes are appearing as expected in the frontend, including:

  • theme.json fontsize presets
  • global style changes to elements and blocks
  • block support styles (inline)

Also checked the PHP unit tests for 8.2 compat warnings ;)

* Tests that a block element's custom font size in the inline style attribute
* is replaced with a fluid value when "settings.typography.fluid" is set to `true`,
* and the correct block content is generated.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also be added to @ticket 56467?

@ramonjd
Copy link
Member

ramonjd commented Oct 11, 2022

Please let me know if I missed anything @ramonjd!

Awesome stuff @andrewserong, thank you!

I think you've covered all the backend changes and PHP unit tests.

@ramonjd
Copy link
Member

ramonjd commented Oct 11, 2022

Tested in Chrome, Safari and Firefox

2022-10-11.16.45.24.mp4

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates look good, and it's still testing well!

Copy link
Contributor

@dream-encode dream-encode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweaks

src/wp-includes/block-supports/typography.php Outdated Show resolved Hide resolved
src/wp-includes/block-supports/typography.php Outdated Show resolved Hide resolved
src/wp-includes/block-supports/typography.php Outdated Show resolved Hide resolved
@@ -244,15 +248,42 @@ function wp_typography_get_preset_inline_style_value( $style_value, $css_propert
return sprintf( 'var(--wp--preset--%s--%s);', $css_property, $slug );
}

/**
* Renders typography styles/content to the block wrapper.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @since 6.1.0 for new function

@andrewserong
Copy link
Contributor Author

Thank you so much for the reviews and feedback everyone, and for all the updates and landing this one in time @dream-encode! Much appreciated 🙇

Since these changes have been committed in a874e5f#diff-5a0816c84e79d4906f109a954660c8548243cf0aad0f0aa06fadf1f204d608c4, I'll close out this PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

5 participants