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: use layout.wideSize as max viewport width #4604

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Jun 13, 2023

Let's use the value of settings.layout.wideSize, if set, as the maximum viewport width for fluid font size calculations.

The default value is 1600px.

Checks against invalid settings.layout.wideSize values to ensure resulting clamp values are calculated correctly. This is s defensive measure to prevent bugs.

Added to the Gutenberg plugin in:

❗ If #4605 is merged first, this PR will need a rebase and possibly the tests updated

Testing

  1. Fire up this branch and use 2023 (a theme that has fluid font sizes enabled)
  2. In the site editor play around with setting the wide size for layouts, as well as adding some custom font sizes.
  3. In the frontend, check that your font sizes should be at their maximum values when viewport is at least as wide as the value of layout.wideSize

Note: this will only work on the frontend. The editor JS changes will be ready once WordPress/gutenberg#51516 is bundled with the npm packages destined for Core

npm run test:php -- --filter Tests_Block_Supports_Typography

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


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.

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 and layout wide size constraint works as expected! I assume the changes from WordPress/gutenberg#51516 aren't testable until the npm packages are updated?

@ramonjd
Copy link
Member Author

ramonjd commented Jun 20, 2023

Thanks for testing @tellthemachines

I assume the changes from WordPress/gutenberg#51516 aren't testable until the npm packages are updated?

Yes. Thanks for mentioning that. I've updated the test description.

Note to self: if this gets merged, I'll update the tests for #4605 (which will be affected due to the change in the way we calculate max viewport size)

@tellthemachines
Copy link
Contributor

Committed in r55946 / c790ae4.

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.

2 participants