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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport fix for fluid typography/convert font size number to pixels GB PR 44807 #3428

Conversation

hellofromtonya
Copy link
Contributor

Backport of Gutenberg PR 44807's PHP code WordPress/gutenberg#44807

Converts incoming raw font size values that are numbers to a "value + pixel" string.

So 30 will be treated as '30px' for the purposes of fluid font size calculations 馃

Trac ticket: https://core.trac.wordpress.org/ticket/56467


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Copy link
Contributor

@dream-encode dream-encode left a comment

Choose a reason for hiding this comment

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

@hellofromtonya This looks great! Do you want to merge this, or shall I?

ramonjd added a commit to WordPress/gutenberg that referenced this pull request Oct 11, 2022
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
Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. 鉂わ笍

I'm in the middle of bringing these changes back to Gutenberg (and then back to Core), since a few things have changed in the meantime.

Here's the PR: WordPress/gutenberg#44847

I think we need to support floats since they might be added in theme.json or (no, because the schema specifies a string) via the UI as a custom sizes.

Maybe we could wait on this PR until WordPress/gutenberg#44847 is tested and merged since it will have the extra tests you have added as well.

What do folks think?

cc @noisysocks @andrewserong @tellthemachines

@@ -263,10 +263,24 @@ function wp_typography_get_preset_inline_style_value( $style_value, $css_propert
* `null` on failure.
*/
function wp_get_typography_value_and_unit( $raw_value, $options = array() ) {
if ( ! is_string( $raw_value ) && ! is_int( $raw_value ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

For backwards compatibility think we need to support numbers here, that is, 10 and 10.2

@andrewserong
Copy link

To try to consolidate things (and avoid potential merge conflicts), I've rolled the changes in this PR and WordPress/gutenberg#44847 into #3431 which seeks to backport all the current / recently merged PHP changes for fluid typography (excluding changes to the block-library directory, which I believe will be handled as part of the npm packages updates).

andrewserong pushed a commit to WordPress/gutenberg that referenced this pull request Oct 11, 2022
* 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
ramonjd added a commit to WordPress/gutenberg that referenced this pull request Oct 11, 2022
* 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
ockham pushed a commit to WordPress/gutenberg 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>
@ramonjd
Copy link
Member

ramonjd commented Oct 12, 2022

I think we can close this one now that #3431 has landed?

@ockham
Copy link
Contributor

ockham commented Oct 12, 2022

I think we can close this one now that #3431 has landed?

Yeah, should be fine 馃憤

@ockham ockham closed this Oct 12, 2022
@hellofromtonya hellofromtonya deleted the fix/fluid-typography/convert-font-size-number-to-pixels branch October 12, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5 participants