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.

Sign up for GitHub

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: Try adding a ceiling to the calculated minimum font size #49247

Closed
wants to merge 5 commits into from
Closed

Fluid Typography: Try adding a ceiling to the calculated minimum font size #49247

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
357f5ad
Fluid Typography: Try adding a ceiling to the calculated minimum font…
andrewserong Mar 22, 2023
519eba4
Update existing tests, add a couple of additional tests to cover the …
andrewserong Mar 22, 2023
6fdd51f
Add test to ensure ceiling is skipped when min font size is provided
andrewserong Mar 22, 2023
72a5feb
Fix whitespace issue
andrewserong Mar 22, 2023
09ad429
Try reducing the default minimum viewport width
andrewserong Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/block-supports/typography.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,14 @@ function gutenberg_get_typography_font_size_value( $preset, $should_use_fluid_ty
)
);

// Sets a ceiling for the minimum font size.
$minimum_font_size_ceiling = gutenberg_get_typography_value_and_unit(
'64px',
Copy link
Member

Choose a reason for hiding this comment

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

Do you think this could be added up near the default var declarations? E.g.,

$minimum_font_size_ceiling = '64px';?

That way we can open it up for theme.json config later (if we decide that's a good thing to do of course!)

array(
'coerce_to' => $preferred_size['unit'],
)
);

// Don't enforce minimum font size if a font size has explicitly set a min and max value.
if ( ! empty( $minimum_font_size_limit ) && ( ! $minimum_font_size_raw && ! $maximum_font_size_raw ) ) {
/*
Expand All @@ -540,6 +548,12 @@ function gutenberg_get_typography_font_size_value( $preset, $should_use_fluid_ty
if ( ! $minimum_font_size_raw ) {
$calculated_minimum_font_size = round( $preferred_size['value'] * $default_minimum_font_size_factor, 3 );
Copy link
Member

@ramonjd ramonjd Apr 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah linear doesn't appear to cut it.

Maybe we need a plot a growth curve so that y (the calculated min size) grows at different rates depending on the value of x (the give custom size).

I'm no expert by the way, but I can try to play around with equations to see if we can get a good fit.

Simple exponential growth seems to work okay as well, e.g.,

$calculated_minimum_font_size = round( pow( $preferred_size['value'], $default_minimum_font_size_factor ), 3 );

It seems to scale better when the scale factor is bumped from 0.75 to 0.875, which is the ratio of 14px/16px, that is, $default_minimum_font_size_limit / root size

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple exponential growth seems to work okay as well, e.g.,

Oh, very cool idea! Feel free to commit to this branch or fork if you think it'd help for us to have multiple approaches on the go. I reckon a non-linear curve is a good thing to try out 👍

I should have a bit of time before the end of the week to have a play with this if you don't beat me to it!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I'll have a play this week. 🍺


// Ensure calculated minimum font size is not greater than the ceiling.
// This is to prevent the font size from being too large in smaller viewports.
if ( $calculated_minimum_font_size > $minimum_font_size_ceiling['value'] ) {
$calculated_minimum_font_size = $minimum_font_size_ceiling['value'];
}

// Only use calculated min font size if it's > $minimum_font_size_limit value.
if ( ! empty( $minimum_font_size_limit ) && $calculated_minimum_font_size <= $minimum_font_size_limit['value'] ) {
$minimum_font_size_raw = $minimum_font_size_limit['value'] . $minimum_font_size_limit['unit'];
Expand Down
18 changes: 17 additions & 1 deletion packages/block-editor/src/components/font-sizes/fluid-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,14 @@ export function getComputedFluidTypographyValue( {
}
);

// Sets a ceiling for the minimum font size.
const minimumFontSizeCeilingParsed = getTypographyValueAndUnit(
'64px',
{
coerceTo: fontSizeParsed.unit,
}
);

// Don't enforce minimum font size if a font size has explicitly set a min and max value.
if (
!! minimumFontSizeLimitParsed?.value &&
Expand All @@ -106,11 +114,19 @@ export function getComputedFluidTypographyValue( {
* the given font size multiplied by the min font size scale factor.
*/
if ( ! minimumFontSize ) {
const calculatedMinimumFontSize = roundToPrecision(
let calculatedMinimumFontSize = roundToPrecision(
fontSizeParsed.value * minimumFontSizeFactor,
3
);

// Ensure calculated minimum font size is not greater than the ceiling.
// This is to prevent the font size from being too large in smaller viewports.
if (
calculatedMinimumFontSize > minimumFontSizeCeilingParsed.value
) {
calculatedMinimumFontSize = minimumFontSizeCeilingParsed.value;
}

// Only use calculated min font size if it's > $minimum_font_size_limit value.
if (
!! minimumFontSizeLimitParsed?.value &&
Expand Down
50 changes: 46 additions & 4 deletions packages/block-editor/src/components/global-styles/test/typography-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ describe( 'typography utils', () => {
{
message: 'should return clamp value for floats',
preset: {
size: '100.175px',
size: '70.175px',
},
typographySettings: {
fluid: true,
},
expected:
'clamp(75.131px, 4.696rem + ((1vw - 7.68px) * 3.01), 100.175px)',
'clamp(52.631px, 3.289rem + ((1vw - 7.68px) * 2.109), 70.175px)',
},

{
Expand All @@ -133,14 +133,14 @@ describe( 'typography utils', () => {
{
message: 'should coerce float to `px` and returns clamp value',
preset: {
size: 100.23,
size: 70.175,
fluid: true,
},
typographySettings: {
fluid: true,
},
expected:
'clamp(75.173px, 4.698rem + ((1vw - 7.68px) * 3.012), 100.23px)',
'clamp(52.631px, 3.289rem + ((1vw - 7.68px) * 2.109), 70.175px)',
},

{
Expand Down Expand Up @@ -382,6 +382,48 @@ describe( 'typography utils', () => {
},
expected: '15px',
},

{
message:
'should use ceiling of 4rem for minimum font size when custom min font size is not set',
preset: {
size: '12rem',
},
typographySettings: {
fluid: true,
},
expected:
'clamp(4rem, 4rem + ((1vw - 0.48rem) * 15.385), 12rem)',
},

{
message:
'should use ceiling of 64px for minimum font size when custom min font size is not set',
preset: {
size: '200px',
},
typographySettings: {
fluid: true,
},
expected:
'clamp(64px, 4rem + ((1vw - 7.68px) * 16.346), 200px)',
},

{
message:
'should not use ceiling for minimum font size when custom min font size is set',
preset: {
size: '200px',
fluid: {
min: '100px',
},
},
typographySettings: {
fluid: true,
},
expected:
'clamp(100px, 6.25rem + ((1vw - 7.68px) * 12.019), 200px)',
},
].forEach( ( { message, preset, typographySettings, expected } ) => {
it( `${ message }`, () => {
expect(
Expand Down
35 changes: 31 additions & 4 deletions phpunit/block-supports/typography-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,10 +392,10 @@ public function data_generate_font_size_preset_fixtures() {

'returns clamp value for floats' => array(
'font_size' => array(
'size' => '100.175px',
'size' => '70.175px',
),
'should_use_fluid_typography' => true,
'expected_output' => 'clamp(75.131px, 4.696rem + ((1vw - 7.68px) * 3.01), 100.175px)',
'expected_output' => 'clamp(52.631px, 3.289rem + ((1vw - 7.68px) * 2.109), 70.175px)',
),

'coerces integer to `px` and returns clamp value' => array(
Expand All @@ -408,10 +408,10 @@ public function data_generate_font_size_preset_fixtures() {

'coerces float to `px` and returns clamp value' => array(
'font_size' => array(
'size' => 100.23,
'size' => 70.175,
),
'should_use_fluid_typography' => true,
'expected_output' => 'clamp(75.173px, 4.698rem + ((1vw - 7.68px) * 3.012), 100.23px)',
'expected_output' => 'clamp(52.631px, 3.289rem + ((1vw - 7.68px) * 2.109), 70.175px)',
),

'returns clamp value when `fluid` is empty array' => array(
Expand Down Expand Up @@ -551,6 +551,33 @@ public function data_generate_font_size_preset_fixtures() {
'should_use_fluid_typography' => true,
'expected_output' => 'clamp(30px, 1.875rem + ((1vw - 7.68px) * 1), 30px)',
),

'should use ceiling of 4rem for minimum font size when custom min font size is not set' => array(
'font_size' => array(
'size' => '12rem',
),
'should_use_fluid_typography' => true,
'expected_output' => 'clamp(4rem, 4rem + ((1vw - 0.48rem) * 15.385), 12rem)',
),

'should use ceiling of 64px for minimum font size when custom min font size is not set' => array(
'font_size' => array(
'size' => '200px',
),
'should_use_fluid_typography' => true,
'expected_output' => 'clamp(64px, 4rem + ((1vw - 7.68px) * 16.346), 200px)',
),

'should not use ceiling for minimum font size when custom min font size is set' => array(
'font_size' => array(
'size' => '200px',
'fluid' => array(
'min' => '100px',
),
),
'should_use_fluid_typography' => true,
'expected_output' => 'clamp(100px, 6.25rem + ((1vw - 7.68px) * 12.019), 200px)',
),
);
}

Expand Down