Search block: ensure font sizes values are converted to fluid in the editor #44852
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
ramonjd
added
[Type] Bug
- 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
ramonjd
force-pushed
the
try/add-fluid-typography-search-block
branch
from
a807b21 to
9ec85dc
Compare
10 tasks
|
Size Change: +86 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
ramonjd
changed the title
andrewserong
approved these changes
Oct 11, 2022
There was a problem hiding this comment.
Thanks for the quick fix @ramonjd, this is testing well for me:
✅ With fluid set to false the search block's font-sizes are not clampified
✅ With fluid set to true the search block's font-sizes are clampified (label, input field, button)
Just left a comment about the function signature change (sounds good to me, just thinking it through). Unless there's any objections / feedback from @noisysocks or @tellthemachines, this LGTM! ✨
| const typographyStyles = attributes?.style?.typography || {}; | ||
| export function getTypographyClassesAndStyles( | ||
| attributes, | ||
| isFluidFontSizeActive |
There was a problem hiding this comment.
Looking at use-color-props.js in the same directory as this file, it seems the other files have a function like getColorClassesAndStyles( attributes ) signature, and then provide a hook (useColorProps) that is responsible for doing its own settings look-up.
In a follow-up, we might also look to adding a hook like that to use-typography-props.js so that we don't need to do the check for fluid support in the Search block's edit function.
However, even if we were to do that, we'd still need to tell getTypographyClassesAndStyles about the state of fluid typography, so we'd still need the change to this signature in that case, I think.
All of which is a long way to say, the change to this function signature sounds good to me!
There was a problem hiding this comment.
Cheers for thinking about this. I will add a note to follow up on this if we don't decide to change it.
ramonjd
added a commit
that referenced
this pull request
Oct 11, 2022
- 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
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 <[email protected]> * 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 <[email protected]> Co-authored-by: ramonjd <[email protected]> * 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 <[email protected]> Co-authored-by: ramonjd <[email protected]> * 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 <[email protected]> Co-authored-by: Andrew Serong <[email protected]> Co-authored-by: Robert Anderson <[email protected]> Co-authored-by: Ben Dwyer <[email protected]>
|
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.
Parent issue:
What?
This PR ensures that we convert fluid font sizes to fluid values in the editor for search block block supports
Why?
Because search block skips serialization. Therefore, we have to make sure the edit component triggers the fluid conversion.
How?
By passing the getTypographyClassesAndStyles hook a flag to tell it whether to convert.
Testing Instructions
Check in post and site editors too please!