Make custom font sizes appear fluid in the block editor when fluid typography is enabled #44765
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
noisysocks
added
[Type] Bug
|
Haven't added/updated any tests yet—it's late on a Friday. |
10 tasks
|
Size Change: +270 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
|
Nice work. Can confirm that when
Not on the frontend. But if I'm not entirely wrong, that part is covered by #44764! 🚀 — fast work. |
|
This is also working for me in the post editor and site editor. |
|
I added some tests |
bph
added
[Priority] High
|
Very nice @noisysocks !!! This is also working well for me in testing for the site and block editors: ✅ it only works at the block styles level (not global styles, which is taken care of in #44761 (comment)) ✅ presets are applied in the fontsizepicker dropdown and override any inline styles (the class has ✅ custom font sizes are clamped and added to the inline style attribute but not the block attributes
<!-- wp:paragraph {"style":{"typography":{"fontSize":"14.45rem"}}} -->
<p style="font-size:14.45rem">sdfsdfsdfsdfsdfds</p>
<!-- /wp:paragraph -->
This is not a better idea, but if you're deeply unsatisfied with the approach in this one I was wondering if we couldn't target the block id, e.g., <style>
/* ...other styles */
/* this block has a custom fluid font size */
.editor-styles-wrapper #d580200b-7067-43b6-8bb7-1ef6bb2408c1 {
font-size: clamp(10.8375rem, 10.8375rem + (1vw - 0.48rem) * 20.841, 21.675rem;
}
/* ...other styles */
</style>I haven't tested this thought as usual 😅 |
|
Thanks for the testing and tests! ❤️
Hmm we'd have to use Looking at this with fresh Monday eyes, I'm okay with the hack. At least it's contained to a filter and reasonably well documented. Medium to long term, I think the correct approach would be to refactor this to use style engine. The |
ramonjd
approved these changes
Oct 10, 2022
There was a problem hiding this comment.
If you're happy then I'm happy.
Would also be great to get another set of eyes from @andrewserong and/or @tellthemachines
I was testing this in the dark. 😄
noisysocks
force-pushed
the
try/fluid-custom-font-sizes-in-editor
branch
from
ce2d1b6
to
cfb341c
Compare
|
Maybe rebasing will fix the Performance Tests job. |
andrewserong
approved these changes
Oct 10, 2022
There was a problem hiding this comment.
Nice one, this is testing well for me, too! Without fluid: true the direct font-sizes are used, and with fluid: true the clampified values are used in the editor view, and not serialized to post content 👍
I agree that it'd be good to look into moving the fluid typography logic into the style engine in a follow-up, as it'd be neat if we didn't have to have separate logic for it. Either way, though, I think the abstraction of the addEditPropsForFluidCustomFontSizes function is a good way to contain the logic for now, and the warning inline comment nicely flags what we should improve on in the future.
Using the __experimentalFeatures.typography.fluid check to determine whether or not fluid typography is enabled also sounds reasonable to me for now, since we expect it'll only be flagged globally at the theme level.
LGTM! ✨
noisysocks
force-pushed
the
try/fluid-custom-font-sizes-in-editor
branch
from
cfb341c
to
135ff4e
Compare
andrewserong
added
the
Backport to WP 6.7 Beta/RC
|
With the failing performance tests, it's failing on "Hovering Inserter Items". It looks like the test opens the global inserter and hovers over the paragraph and heading blocks, which appears to be working fine locally when manually replicating those tasks in the editor. From a quick look at the test, it's currently waiting for the
I was wondering if it's worth trying to add Looking through recent commits, though, I can't see much that would obviously affect this test. Though there was a recent CSS update in: #44711, just in case 🤔 |
…px` and the tests shall pass nonetheless!!!
|
Just to follow up on the performance test error mentioned above The browser was throwing an error: TypeError: rawValue.match is not a function
at getTypographyValueAndUnit (http://localhost:8889/wp-content/plugins/gutenberg/build/block-editor/index.min.js?ver=0a50f8b528c395d4fdef:25842:28)
at getComputedFluidTypographyValue (http://localhost:8889/wp-content/plugins/gutenberg/build/block-editor/index.min.js?ver=0a50f8b528c395d4fdef:25752:28)
If we are to support custom font sizes that are numbers we should coerce to This change added in bcca4db |
|
Thanks for fixing that @ramonjd! |
|
Thank you all for the fast bugfixing spree! Something good to follow up on is to test a range of font sizes with the boolean default fluid property, and check that no font sizes ever get too small, whether on tablet or mobile. (Anything smaller than 11px seems too small for the default behavior.) |
ockham
added
Backport to WP 6.7 Beta/RC
ramonjd
added a commit
that referenced
this pull request
Oct 11, 2022
…pography is enabled (#44765) * Make custom font sizes appear fluid in the block editor when fluid typography is enabled * Add tests for fluid utils * update description * You shall not pass with a number, well, yes, but we'll coerce it to `px` and the tests shall pass nonetheless!!! Co-authored-by: Ben Dwyer <ben@scruffian.com> Co-authored-by: ramonjd <ramonjd@gmail.com>
ockham
pushed a commit
that referenced
this pull request
Oct 11, 2022
* Fluid typography: convert server-side block support values (#44762) * For serverside rendered typography block support styles, covert font size values to fluid values in the inline styles if fluid typography is activated. Added unit tests * Add fluid font size to Nav Link * Add fluid typography to Search block * Fix fluid typography for Submenu block with open on click * Fix fluid typography for Page List block * Remove unnecessary parameter * Call esc_attr only once on the whole typography string Co-authored-by: tellthemachines <isabel@tellthemachines.com> * Fluid Typography: Fix bug in global styles where fluid clamp rules were not calculated for custom values (#44761) * Fluid Typography: Fix bug where fluid clamp rules were not calculated for custom global styles values * Add inline comments * Add tests for JS changes * Fluid Typography: ensure global styles preset fluid sizes are calculated correctly (#44791) * Forked #44761 * Ensuring the font size picker select box shows the right labels * update comment. Typescript has beaten me. It's much more convenient to use getFontSizeOptions(), but I guess we'll have to refactor. * Adding comment about Typescript bug (YAY, it wasn't me being dumb with TS for once) * Added tests yo * Changeo loggo * Create a new FontSizeSelectOption return type for getSelectedOption to: 1. Clean up type changes in #44791 2. Make TS linter be quiet * Revert accidental changes to emptytheme * Revert type changes and other calamities * Remove additional size value from getToggleGroupOptions test as I believe it is no longer expected Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: ramonjd <ramonjd@gmail.com> * Fluid typography: convert font size inline style attributes to fluid values (#44764) * This commit ensures that custom font size values that appear in the style attribute of block content are converted to fluid typography (if activated) * Adding comments * Bail early if we don't find a custom font size * Adding tests yo * Updating PHP doc to describe incoming type of $raw_value (string|number) * Make custom font sizes appear fluid in the block editor when fluid typography is enabled (#44765) * Make custom font sizes appear fluid in the block editor when fluid typography is enabled * Add tests for fluid utils * update description * You shall not pass with a number, well, yes, but we'll coerce it to `px` and the tests shall pass nonetheless!!! Co-authored-by: Ben Dwyer <ben@scruffian.com> Co-authored-by: ramonjd <ramonjd@gmail.com> * Fluid typography: covert font size number values to pixels (#44807) * Converts incoming raw font size values to value + pixel to remain consistent with the fontsizepicker component. * Updating comments / docs * Fluid typography: ensure fontsizes are strings or integers (#44847) * Updating PHP doc to describe incoming type of $raw_value (string|int) This PR also harmonizes the JS checks And review comments from #44807 (review) These changes have already been backported to Core in WordPress/wordpress-develop#3428 * Update changelog Add extra test for floats Add i18n domain * Font sizes can be string|int|float Updated tests and JSDoc type * Expand tests for gutenberg_get_typography_value_and_unit Fix typo in CHANGELOG.md * Initial commit (#44852) - ensure that we convert fluid font sizes to fluid values in the editor for search block block supports - we do this by passing the getTypographyClassesAndStyles hook a flag to tell it whether to convert Updated CHANGELOG.md Added tests Co-authored-by: tellthemachines <isabel@tellthemachines.com> Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Co-authored-by: Robert Anderson <robert@noisysocks.com> Co-authored-by: Ben Dwyer <ben@scruffian.com>
|
Removing the "Backport to WP Beta/RC" label since this has been backported as part of #44868 (and sync'd to Core per WordPress/wordpress-develop#3437). |
ockham
removed
the
Backport to WP 6.7 Beta/RC
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.
What?
Part of #44758.
Makes it so that any blocks with a custom font size appear as fluid in the block editor when
typography.fluidis set totrueat the theme level.Why?
See #44758.
How?
Struggled to come up with a good solution for this. This adds a custom
getEditWrapperProps()callback to all block types that support font sizes using theeditor.registerBlockTypefilter. Then, if fluid typography is enabled, this callback will swap any custom font size instyle.fontSizewith a fluid font size (i.e. one that uses clamp()).The code that turns a single font size into a
clamp()already existed, just needed to be moved from@wordpress/global-stylesto@wordpress/block-editorand refactored a little. No problem.The big terrible hacky thing is that we can't use
useSetting()oruseSelect()here becausegetEditWrapperProps()is a function called byBlockListBlockand not a React component with access toBlockListContext.If we use the
editor.BlockListBlockfilter then we can access context but then anystyle.fontSizewill be overwritten by the existingcore/style/addEditPropsfilter. We must run our logic aftercore/style/addEditPropsand so that limits us to usingeditor.registerBlockType.My ideal solution would be to make the style engine do all of this (after all: wasn't its original purpose to turn block attributes into CSS styling?). We can't right now though because the style engine doesn't have access to theme JSON data. I think refactoring it so that this is possible is a good idea but too much work in the context of #44758.
Please let me know if you have any better ideas.
Testing Instructions
See #44758.
Screenshots or screencast