Fluid typography: migrate fluid typography algorithm to JS for site editor #42688
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
…_typography_font_size_value` to JS. We have to do this because the site editor is contained within an iframe and cannot, therefore, access the global CSS presets in the parent window.
ramonjd
added
[Type] Bug
|
Size Change: +713 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Nice one, thanks for putting this up @ramonjd. I ran out of time to give this a proper review today, but just a drive by comment to say I think the failing Puppeteer test catches a change (or small regression?) in global styles.
With TwentyTwentyTwo active, and the fluid size not opted-into, prior to this PR under Typography in global styles it shows Medium as the default size. After this PR it shows Default:
| Before | After |
|---|---|
 |
 |
Is that intentional?
Happy to test further tomorrow!
|
I'm able to confirm that I see the fluid typography size in the template editor as expected 👍 . |
|
Thank you @andrewserong and @matiasbenedetto for checking this branch out! 🙇 The code is a direct port from the PHP version. They have to be identical (at least as far as output is concerned) so I didn't put much refactoring effort into it. I was pleasantly surprised when it worked first time. 😆
Interesting. Thanks for catching. I'll take a looksy. |
ramonjd
added
the
[Feature] Typography
… that the site editor can fetch values from the preset objects. Most notably these operations takes place in getValueFromPresetVariable() and getValueFromVariable()
I found the problem. To quote the commit message:
|
Ah, nice one! I'll take this for another spin shortly 👍 |
andrewserong
approved these changes
Jul 27, 2022
There was a problem hiding this comment.
This is testing nicely @ramonjd!
✅ From reading the code, it looks like a faithful JS reproduction of the PHP logic to me 👍
✅ Medium (default) font size is still working correctly in the site editor in TwentyTwentyTwo, along with changing to existing font sizes.
✅ Emptytheme without fluid still works the same as on trunk.
✅ Emptytheme with fluid sets the fluid values correctly in the site editor.
Values with fluid set to false |
Values with fluid set to true |
|---|---|
 |
 |
LGTM! I left a tiny comment about a typo in a comment, but feel free to ignore it if there aren't any other things you want to change, so that you can merge without having to re-run the e2e tests 🙂
packages/edit-site/src/components/global-styles/typography-utils.js
Outdated
Show resolved
Hide resolved
…ls.js My blod is typo Co-authored-by: Andrew Serong <[email protected]>
This was referenced Jul 28, 2022
37 tasks
57 tasks
ramonjd
added
the
has dev note
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.
Fixes #42680
Follow up to
What?
Migrate fluid typography algorithm from PHP version of
gutenberg_get_typography_font_size_valueto JS.Why?
We have to do this because the site editor is contained within an iframe and cannot, therefore, access the global CSS presets in the parent window.
How?
Adding a value_func callback called
getTypographyFontSizeValue()Testing Instructions
Enable fluid font sizes. Here is some test theme.json (I'm testing using empty theme).
Example theme.json
{ "version": 2, "settings": { "appearanceTools": true, "typography": { "fluid": true, "fontSizes": [ { "size": "5.75rem", "slug": "colossal", "name": "Colossal" } ] } }, "styles": { "blocks": { "core/post-title": { "typography": { "fontSize": "var(--wp--preset--font-size--colossal)" } }, "core/post-author": { "typography": { "fontSize": "var(--wp--preset--font-size--colossal)" } }, "core/post-navigation-link": { "typography": { "fontSize": "var(--wp--preset--font-size--colossal)" } } } } }Head over to the site editor and add a post title, post author and post navigation link blocks.
The value of
--wp--preset--font-size--colossalin the editor should match the frontend.