Fluid typography: add font size constraints #44993
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +412 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
ramonjd
force-pushed
the
update/fluid-typography-with-min-font-size-value-constraint
branch
from
3c2a4a1
to
6f9ad91
Compare
There was a problem hiding this comment.
Tested these changes on Chrome, Firefox and Safari on macOS. I used custom font sizes both large and small, defined in px, em and rem, and tested with different browser font size settings. It's all working pretty well across all the browsers!
A few comments below.
| @@ -372,12 +374,17 @@ function gutenberg_get_computed_fluid_typography_value( $args = array() ) { | |||
| ) | |||
| ); | |||
|
|
|||
| // Protect against unsupported units. | |||
| // Checks for mandatory min and max sizes, and protects against unsupported units. | |||
There was a problem hiding this comment.
In the (unchanged) lines above, we're coercing the max size to $font_size_unit but not the min size, is that deliberate?
There was a problem hiding this comment.
It is, and here's the logic (mine at least 😄)
The min and max font size units can theoretically differ, so one can be px and the other rem. This is fine in
the resulting clamp value's first and third args, e.g., clamp(20px, 1.25rem + ((1vw - 7.68px) * 93.75), 50rem)
But in order to calculate a linear factor between viewport boundaries we have to do the math on similar values, that is, values whose units are the same. So we coerce the max value unit to whatever the min value unit is.
I use the min value because, if there isn't a min value specified, e.g., it's a custom font size, we use incoming custom font size to calculate a minimum value, and therefore use its unit.
The functions' internals are ripe for refactor by the way, but since it's all neatly housed I think we can do it in a follow up.
There was a problem hiding this comment.
This is looking pretty good so far, thanks so much for digging in to land a fix @ramonjd! Just left a few nits, but happy to give this a more detailed test once it's ready for a final review 🙂
ramonjd
added
[Type] Bug
- Adds default minimum font size limits so that text does not become smaller than 14px/0.875rem/0.875em. The consequence will be that all fluid font sizes will be no smaller than 14px/0.875rem/0.875em in any viewport width. - Users may define font-sizes that are smaller than 14px/0.875rem/0.875em. In this case the user-defined font size becomes the minimum. For example if a user defines 9px for a block, then the minimum font size for that block will be 9px. - Min font size should not be greater than max font size. - Round to 3 decimal places (JS) - Rounding computed min and max values
…s in em This is to keep it consistent with the original formula and ensure the fluid argument in clamp is working off a size relative to :root Fixing formatting and grammar Rolling back renaming $preset to $font_size in preference to a follow up. Do not clamp value if there is no min size and the font size is less than 14px
ramonjd
force-pushed
the
update/fluid-typography-with-min-font-size-value-constraint
branch
from
1e0d8f3
to
f45b4a5
Compare
@jasmussen @tellthemachines @andrewserong Seeking your counsel here. I've made an assumption about font sizes that are < 14px set by users or in theme.json (essentially any font size without a defined min font size). In this PR they are not clamped, but will return a static size. So, if the font size is
In a previous commit, I had returned a clamped value but had used font sizes that are < 14px as the minimum font size. That seemed a bit strange and against the spirit of what we're trying to acheive. What are your thoughts on how to handle these small values set by the user? My intention was not to create any surprises in the editor or theme.json, by constraining a font size even if the user is setting the value < 14px. It would be strange to set a font size to Also I thought that there may be some design cases for very small font sizes, e.g., where the text is not meant to be readable. It's a long bow to draw, I know. |
From my perspective, the idea of keeping font sizes < I'm not a designer, though, so keen to hear what @jasmussen thinks here, too! Also, I suppose in follow-ups (perhaps for 6.2), we might be able to have an explicit But when it comes to really small font-sizes, the main use cases I can think of are footer areas where folks are intentionally putting a whole bunch of information with a high level of density (especially terms of service info and the like). And that information is likely the kind that isn't intended to really scale up very much. E.g. here are parts of footers on Microsoft and Apple websites:
|
It doesn't seem strange to me. I think it stranger if the fluid behaviour varies depending on font size, because it feels a bit random - why set the limit at 14px? We could also argue that if the user sets a custom font size, they'll expect that size to persist across different viewports, but if that behaviour changes with setting the |
Also a very good point! |
Hmm, excellent observations. So the choice is whether to:
Both give precedence to user choice, which I think we should do. I'm not sure whether we place such a constraint on styles elsewhere so we'd be setting a precedent that should be compatible with other potential constraints, e.g., maximum font size. With that in mind, I suppose turning on Keen to hear what @jasmussen and co. think as well. The good news is that the functionality for both exists in the commit history of this PR, so it's easy to change once we've made up our minds 😄 Later I'd like to revive this PR: And add a configurable minimum font size as well. Maybe even a max font size. |
jasmussen
approved these changes
Oct 17, 2022
There was a problem hiding this comment.
Nice! Approving from a design perspective, because when fluid: true; and no other fluid boundaries defined on a per-preset basis, the minimum font size is 14px in editor and frontend:
That definitely fixes the small-font bug.
I furthermore tested that the manual boundaries (custom min/max) work as intended, and even fluid: false on a per-preset basis works as intended. This makes fluid typography feel true to its boolean "just works" intent, but maintains the room to customize.
I'd appreciate good code reviews in addition to this approval! Thank you for fast work.
|
Thanks again for testing @andrewserong I've made all the changes and added test for the zero value.
I'm also happy with the balance so far and I think we're in a good place. Existing font sizes, when a user flips the fluid switch won't be affected and we can introduce configurables later. I'm grateful for the constant testing and thoughts! 🙇 |
ramonjd
deleted the
update/fluid-typography-with-min-font-size-value-constraint
branch
ramonjd
added a commit
to ramonjd/wordpress-develop
that referenced
this pull request
Oct 18, 2022
- Adds default minimum font size limits so that min font size, where provided, does not become smaller than 14px/0.875rem/0.875em. The consequence will be that all fluid font sizes will be no smaller than 14px/0.875rem/0.875em in any viewport width. - For font sizes of < 14px that have no defined minimum sizes, use the font size to set the floor of the clamp() value.
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Oct 19, 2022
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. See #56467. Built from https://develop.svn.wordpress.org/trunk@54646 git-svn-id: http://core.svn.wordpress.org/trunk@54198 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Oct 19, 2022
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. Reviewed by bernhard-reiter, SergeyBiryukov. Merges [54646] to the 6.1 branch. See #56467. git-svn-id: https://develop.svn.wordpress.org/branches/6.1@54647 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Oct 19, 2022
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. Reviewed by bernhard-reiter, SergeyBiryukov. Merges [54646] to the 6.1 branch. See #56467. Built from https://develop.svn.wordpress.org/branches/6.1@54647 git-svn-id: http://core.svn.wordpress.org/branches/6.1@54199 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot
pushed a commit
to platformsh/wordpress-performance
that referenced
this pull request
Oct 19, 2022
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. See #56467. Built from https://develop.svn.wordpress.org/trunk@54646 git-svn-id: https://core.svn.wordpress.org/trunk@54198 1a063a9b-81f0-0310-95a4-ce76da25c4cd
ramonjd
added a commit
that referenced
this pull request
Oct 19, 2022
* Initial commit: - Adds default minimum font size limits so that text does not become smaller than 14px/0.875rem/0.875em. The consequence will be that all fluid font sizes will be no smaller than 14px/0.875rem/0.875em in any viewport width. - Users may define font-sizes that are smaller than 14px/0.875rem/0.875em. In this case the user-defined font size becomes the minimum. For example if a user defines 9px for a block, then the minimum font size for that block will be 9px. - Min font size should not be greater than max font size. - Round to 3 decimal places (JS) - Rounding computed min and max values * Rounding parsed min and max values * Updating JS rounding function so that values are rounded * Removing type coercion * Updating failing test * Remove reference to preset in favour of "fontSize" * Ensure that rem units are used when calculating min and max boundaries in em This is to keep it consistent with the original formula and ensure the fluid argument in clamp is working off a size relative to :root Fixing formatting and grammar Rolling back renaming $preset to $font_size in preference to a follow up. Do not clamp value if there is no min size and the font size is less than 14px * Oh Linter! You persistent crank. * For font sizes of < 14px that have no defined minimum sizes, use the font size to set the floor of the clamp() value. * Removing max < min check. It wasn't there before. It can be a follow up if required as it requires extra logic. * Checking for a zero-based linear factor. If we find one, default to `1` * Refer to correct JS var in JS file
This was referenced Oct 19, 2022
ockham
pushed a commit
that referenced
this pull request
Oct 25, 2022
* Initial commit: - Adds default minimum font size limits so that text does not become smaller than 14px/0.875rem/0.875em. The consequence will be that all fluid font sizes will be no smaller than 14px/0.875rem/0.875em in any viewport width. - Users may define font-sizes that are smaller than 14px/0.875rem/0.875em. In this case the user-defined font size becomes the minimum. For example if a user defines 9px for a block, then the minimum font size for that block will be 9px. - Min font size should not be greater than max font size. - Round to 3 decimal places (JS) - Rounding computed min and max values * Rounding parsed min and max values * Updating JS rounding function so that values are rounded * Removing type coercion * Updating failing test * Remove reference to preset in favour of "fontSize" * Ensure that rem units are used when calculating min and max boundaries in em This is to keep it consistent with the original formula and ensure the fluid argument in clamp is working off a size relative to :root Fixing formatting and grammar Rolling back renaming $preset to $font_size in preference to a follow up. Do not clamp value if there is no min size and the font size is less than 14px * Oh Linter! You persistent crank. * For font sizes of < 14px that have no defined minimum sizes, use the font size to set the floor of the clamp() value. * Removing max < min check. It wasn't there before. It can be a follow up if required as it requires extra logic. * Checking for a zero-based linear factor. If we find one, default to `1` * Refer to correct JS var in JS file
|
I just cherry-picked this PR to the wp/6.1 branch to get it included in the next release: c3933fa |
ockham
removed
the
Backport to WP 6.7 Beta/RC
Reverting on the |
|
ockham
added
the
Backport to WP Minor Release
ootwch
pushed a commit
to ootwch/wordpress-develop
that referenced
this pull request
Nov 4, 2022
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54646 602fd350-edb4-49c9-b593-d223f7449a82
Mamaduka
pushed a commit
that referenced
this pull request
Nov 10, 2022
* Initial commit: - Adds default minimum font size limits so that text does not become smaller than 14px/0.875rem/0.875em. The consequence will be that all fluid font sizes will be no smaller than 14px/0.875rem/0.875em in any viewport width. - Users may define font-sizes that are smaller than 14px/0.875rem/0.875em. In this case the user-defined font size becomes the minimum. For example if a user defines 9px for a block, then the minimum font size for that block will be 9px. - Min font size should not be greater than max font size. - Round to 3 decimal places (JS) - Rounding computed min and max values * Rounding parsed min and max values * Updating JS rounding function so that values are rounded * Removing type coercion * Updating failing test * Remove reference to preset in favour of "fontSize" * Ensure that rem units are used when calculating min and max boundaries in em This is to keep it consistent with the original formula and ensure the fluid argument in clamp is working off a size relative to :root Fixing formatting and grammar Rolling back renaming $preset to $font_size in preference to a follow up. Do not clamp value if there is no min size and the font size is less than 14px * Oh Linter! You persistent crank. * For font sizes of < 14px that have no defined minimum sizes, use the font size to set the floor of the clamp() value. * Removing max < min check. It wasn't there before. It can be a follow up if required as it requires extra logic. * Checking for a zero-based linear factor. If we find one, default to `1` * Refer to correct JS var in JS file
Mamaduka
pushed a commit
that referenced
this pull request
Nov 10, 2022
* Initial commit: - Adds default minimum font size limits so that text does not become smaller than 14px/0.875rem/0.875em. The consequence will be that all fluid font sizes will be no smaller than 14px/0.875rem/0.875em in any viewport width. - Users may define font-sizes that are smaller than 14px/0.875rem/0.875em. In this case the user-defined font size becomes the minimum. For example if a user defines 9px for a block, then the minimum font size for that block will be 9px. - Min font size should not be greater than max font size. - Round to 3 decimal places (JS) - Rounding computed min and max values * Rounding parsed min and max values * Updating JS rounding function so that values are rounded * Removing type coercion * Updating failing test * Remove reference to preset in favour of "fontSize" * Ensure that rem units are used when calculating min and max boundaries in em This is to keep it consistent with the original formula and ensure the fluid argument in clamp is working off a size relative to :root Fixing formatting and grammar Rolling back renaming $preset to $font_size in preference to a follow up. Do not clamp value if there is no min size and the font size is less than 14px * Oh Linter! You persistent crank. * For font sizes of < 14px that have no defined minimum sizes, use the font size to set the floor of the clamp() value. * Removing max < min check. It wasn't there before. It can be a follow up if required as it requires extra logic. * Checking for a zero-based linear factor. If we find one, default to `1` * Refer to correct JS var in JS file
Mamaduka
removed
the
Backport to WP Minor Release
VenusPR
added a commit
to VenusPR/Wordpress_Richard
that referenced
this pull request
Mar 9, 2023
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. See #56467. Built from https://develop.svn.wordpress.org/trunk@54646 git-svn-id: http://core.svn.wordpress.org/trunk@54198 1a063a9b-81f0-0310-95a4-ce76da25c4cd
VenusPR
added a commit
to VenusPR/Wordpress_Richard
that referenced
this pull request
Mar 9, 2023
This commit: * Adds default minimum font size limits so that min font size, where provided, does not become smaller than `14px`/`0.875rem`/`0.875em`. * For font sizes of `< 14px` that have no defined minimum sizes, uses the font size to set the floor of the `clamp()` value. This bugfix prevents converting existing small font sizes to clamp values that will reduce their font size even further in narrow widths. It therefore improves backward compatibility and accessibility. Original PR from Gutenberg repository: * [WordPress/gutenberg#44993 #44993 Fluid typography: add font size constraints] Follow-up to [54260], [54360], [54497], [54500]. Props ramonopoly, andrewserong, isabel_brison, Joen, bernhard-reiter. Reviewed by bernhard-reiter, SergeyBiryukov. Merges [54646] to the 6.1 branch. See #56467. Built from https://develop.svn.wordpress.org/branches/6.1@54647 git-svn-id: http://core.svn.wordpress.org/branches/6.1@54199 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #44957
WordPress/wordpress-develop patch:
Cherry picked into wp/6.1 branch in this PR:
What?
This PR adds default minimum font size constraint of 14px/0.875rem/0.875em so that a fluid font's minimum font size does not become smaller than the constraint.
❗ For user-defined font sizes that are < 14px we will use the font-size itself as the minimum value. This is so that changes to these custom values are reflected in the editor and frontend. We trust the user here to insert a sensible value
Why and how?
To ensure that minimum font sizes, whether user-defined or calculated by
gutenberg_get_typography_font_size_valueare not less than 14px/0.875rem/0.875em.This is to protect readability and accessibility.
Testing Instructions
In theme.json, add some very small font sizes (<14px or .875rem) to the presets'
fluid.min.Example:
{ "size": "2.8rem", "fluid": { "min": "0.3rem", "max": "30px" }, "slug": "neutrale", "name": "Neutrale" },Add a post and apply the preset.
Check that these font sizes are never less than 14px or .875rem in the editor and frontend.
Next add a custom font size to an element, either in theme.json or via the editor.
If your custom font size is <14px or .875rem, the value of the custom font size will be static.
Otherwise the clamped value's minimum font size should be never be less than 14px or .875rem.
Example theme.json
{ "version": 2, "settings": { "typography": { "fluid": true, "fontSizes": [ { "size": ".9rem", "slug": "piccolino", "name": "Piccolino" }, { "size": "2rem", "fluid": { "min": "1.8rem", "max": "40px" }, "slug": "in-mezzo", "name": "In Mezzo" }, { "size": "2.8rem", "fluid": { "min": "0.3rem", "max": "30px" }, "slug": "neutrale", "name": "Neutrale" }, { "size": "4.75rem", "slug": "grandone", "name": "Grandone" } ] } }, "styles": { "typography": { "fontSize": "12px" }, "elements": { "h1": { "typography": { "fontSize": "4rem" } } }, "blocks": { "core/post-date": { "typography": { "fontSize": "16px" } } } } }Example HTML
2022-10-17.13.00.13.mp4