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

[WP 6.1] Global Styles: Heading elements UI doesn't output fluid font size #44752

Closed
mayuge3 opened this issue Oct 6, 2022 · 8 comments · Fixed by #44761
Closed

[WP 6.1] Global Styles: Heading elements UI doesn't output fluid font size #44752

mayuge3 opened this issue Oct 6, 2022 · 8 comments · Fixed by #44761
Labels
[Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended

Comments

@mayuge3
Copy link

mayuge3 commented Oct 6, 2022

Description

When I use TT3 and set H1 to XXL at Styles > Typography > Heading, the output is not a fluid font size.

Expected global style output:
h1 {font-size: var(--wp--preset--font-size--xx-large);}
or
h1 {font-size: clamp(4rem,4rem + ((1vw - 0.48rem) * 30.769);}

Happens instead:
h1 {font-size: 10rem}

Step-by-step reproduction instructions

  1. Use WP 6.1 beta 3 and Twenty Twenty Three without gutenberg plugin
  2. Go to the site editor > Styles > Typography > Headings
  3. Set H1 size to XXL

Screenshots, screen recording, code snippet

TT3 theme.json XXL font-size setting:

{
	"size": "10rem",
	"slug": "xx-large",
	"fluid": {
		"min": "4rem",
		"max": "20rem"
	}
}

Styles > Typography > Heading setting:
t5

Global style output:

h1 {font-size: 10rem}

Environment info

WP 6.1-beta3-54396, instawp install
TT3 included in WP 6.1 nightly.
Gutenberg plugin deactivated

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@mikachan mikachan added [Type] Bug An existing feature does not function as intended [Feature] Typography Font and typography-related issues and PRs labels Oct 7, 2022
@mikachan
Copy link
Member

mikachan commented Oct 7, 2022

Verifying that I can also see a non-fluid size on the front end when I set the XXL, large or medium sizes via Styles > Typography > Heading, with Gutenberg deactivated. These all have fluid min and max values defined in theme.json.

I believe #44761 may fix this issue. I've just tested the above with this PR and I'm seeing fluid sizes.

@mikachan
Copy link
Member

mikachan commented Oct 7, 2022

Actually, I don't think the above PR does fix this issue 100%. I'm seeing fluid font sizes, but they're not the ones set in theme.json.

@andrewserong @ramonjd, pinging you in case you haven't seen this issue.

@ramonjd
Copy link
Member

ramonjd commented Oct 8, 2022

I can confirm that #44761 partially fixes this.

@mikachan is right: the values are not the same.

The reason there's a discrepancy between the clamp value added in global styles, and the one generated for the preset --wp--preset--font-size--xx-large is that the preset defined extra parameters, such as the max and min fluid sizes.

Incoming from preset:

{ ["size"]=>  "10rem" ["slug"]=> "xx-large" ["fluid"]=>  { ["min"]=> "4rem" ["max"]=> "20rem" } }

Output:

 "clamp(4rem, 4rem + ((1vw - 0.48rem) * 30.769), 20rem)"

https://github.com/WordPress/gutenberg/pull/44761/files#diff-f28aa79997745a4a51f917012b45a809c9b45d55134b55379a718f5c751dddf4R1052 as it stands does not do this yet, so the clamp value is estimated:

Incoming from font size picker:

{ ["size"]=> "10rem" } 

Output:

"clamp(7.5rem, 7.5rem + ((1vw - 0.48rem) * 14.423), 15rem)"

There is an unrelated UI bug as well. See "UNDEFINED"

Screen Shot 2022-10-08 at 11 59 38 am

@ramonjd
Copy link
Member

ramonjd commented Oct 8, 2022

So, we could check the incoming custom value against the fontSizes presets to grab any fluid parameters, e.g.,

if ( 'font-size' === $css_property ) {
    $font_size_properties = array( 'size' => $value );
    // Checks for an existing preset value.
    $font_size_presets = _wp_array_get( $settings, array( 'typography', 'fontSizes', 'theme' ), null );
    if ( ! empty( $font_size_presets ) ) {
        $selected_font_size_preset_index = array_search( $value, array_column( $font_size_presets, 'size' ), true );
        $font_size_properties            = false !== $selected_font_size_preset_index ? $font_size_presets[ $selected_font_size_preset_index ] : $font_size_properties;
    }

    $value = gutenberg_get_typography_font_size_value( $font_size_properties );
}

This works insofar that it ensures that the clamp values are the same.

The caveat is that if the value is a custom font size (not a preset) and there's a matching preset size, the custom value will be converted to a clamp value using the preset's min and max values.

There's no way, at least for me, to know whether an incoming value is a custom value or a preset value.

This seems to me to be inconsistent, but perhaps an acceptable compromise?

Unless we want to be strict about this and dictate that all custom font size values should be treated differently, even custom font sizes that match possible preset font size values. If that's the case, I'm beginning to believe that it might be best to convert global font-size values before we save them, or, maybe save presets as var(--wp--preset--font-size--xx-large) 🤷

What do you reckon @andrewserong / @noisysocks ?

@ramonjd
Copy link
Member

ramonjd commented Oct 8, 2022

I think I have a fix for all this over at #44791

Still testing...

I think it works!

@mayuge3
Copy link
Author

mayuge3 commented Oct 13, 2022

The fluid font size now works in 6.1-RC1-54506. Thank you!

The output is
h1 {font-size: clamp(4rem, 4rem + ((1vw - 0.48rem) * 30.769), 20rem);}

I hope this value will be var() in the future.

@ramonjd
Copy link
Member

ramonjd commented Oct 13, 2022

I hope this value will be var() in the future.

This is a good point, thanks! I'll add it as a follow up in:

@ramonjd
Copy link
Member

ramonjd commented Oct 13, 2022

Resolved by: #44761

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended
Projects
None yet
3 participants