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

Font Size Picker Hint: Fallback to font size slug if name is undefined #45041

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

mikachan
Copy link
Member

@mikachan mikachan commented Oct 17, 2022

What?

This PR tweaks the headerHint function so that the font size preset slug is used when name is missing/undefined. I believe name is optional, so this may be a common occurrence.

Why?

When a theme sets custom font sizes but does not include a name for a preset, the font picker displays 'undefined' as the size label:

image

Addresses this comment: #44791 (comment).

How?

This changes the logic so that instead of always trying to use the name from the font size preset, it will attempt to use name if it exists, and if it doesn't, it will use slug instead. I believe the font size slug is required, so this should be a safe fallback.

Here's an example from the Canary variation in TT3, which defines the font sizes with no name keys. I've added a name to the first size to show how this is currently used.

theme.json
"typography": {
	"fontSizes": [
		{
			"name": "Smallish",
			"size": "0.75rem",
			"slug": "small"
		},
		{
			"size": "1.125rem",
			"slug": "medium"
		},
		{
			"size": "1.75rem",
			"slug": "large"
		},
		{
			"size": "2.25rem",
			"slug": "x-large"
		},
		{
			"size": "10rem",
			"slug": "xx-large"
		}
	]
}

Before:

Screen.Recording.2022-10-17.at.16.50.15.mov

After:

Screen.Recording.2022-10-17.at.16.52.21.mov

Testing Instructions

  1. Add the above theme.json fontSizes array to a theme
  2. Go to the Site Editor and use the editor to change the font size of a text element/block
  3. Toggle between the available sizes and notice the label/hint value next to the 'Size' label

(I've not worked in this area before, so I'm not sure if this is correct/the best solution!)

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 catching this. I think it was reverted in the washing machine while working on #44761

✅ Slugs appear where there are no names in the post and site editors for both the option buttons (<=5 presets) and the dropdown (with 5++)
✅ Also tested with fluid typography toggled on and undefined is gone!

This is an improvement so LGTM!

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Nice one, thanks for fixing this up @mikachan! This is testing nicely for me, too.

I wonder if one of the reasons we didn't catch this sooner is that it turns out for the presets that match ones provided in Gutenberg's base theme.json file, if we don't include .name in our theme's font sizes, then it picks up the .name from the base theme.json's preset. E.g. in the provided theme.json in the PR description, "Medium", "Large", "Extra Large" all display correctly because they exist in the base file. That was a TIL for me! 😀

Falling back to the slug sounds good to me, and because presets can only work if there is a slug, it sounds like we don't need to worry about selectedOption.slug being undefined, either.

LGTM! ✨

@andrewserong andrewserong added the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 17, 2022
@ramonjd ramonjd merged commit 4c7d3e4 into trunk Oct 17, 2022
@ramonjd ramonjd deleted the fix/font-size-picker-label branch October 17, 2022 23:59
@github-actions github-actions bot added this to the Gutenberg 14.4 milestone Oct 17, 2022
@michalczaplinski
Copy link
Contributor

I just cherry-picked this PR to the wp/6.1-rc-2 branch to get it included in the next release: 8a00f6a

@michalczaplinski michalczaplinski removed the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 18, 2022
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Oct 18, 2022
Package updates for bug and regression fixes:
- `@wordpress/block-directory: 3.15.8`
- `@wordpress/block-editor: 10.0.7`
- `@wordpress/block-library: 7.14.8`
- `@wordpress/components: 21.0.6`
- `@wordpress/customize-widgets: 3.14.8`
- `@wordpress/edit-post: 6.14.8`
- `@wordpress/edit-site: 4.14.10`
- `@wordpress/edit-widgets: 4.14.8`
- `@wordpress/editor: 12.16.7`
- `@wordpress/format-library: 3.15.7`
- `@wordpress/interface: 4.16.6`
- `@wordpress/list-reusable-blocks: 3.15.6`
- `@wordpress/nux: 5.15.6`
- `@wordpress/preferences: 2.9.6`
- `@wordpress/reusable-blocks: 3.15.7`
- `@wordpress/server-side-render: 3.15.6`
- `@wordpress/widgets: 2.15.7`

Original PRs from Gutenberg repository:
- [WordPress/gutenberg#45041 #45041 Font Size Picker Hint: Fallback to font size `slug` if `name` is undefined]
- [WordPress/gutenberg#45045 #45045 Add: Missing output escaping on some blocks]
- [WordPress/gutenberg#44999 #44999 Escape comment author URL]
- [WordPress/gutenberg#44972 #44972 Navigator: restore focus only once per location]
- [WordPress/gutenberg#44858 #44858 Spacing Sizes Control: Try improving layout spacing]
- [WordPress/gutenberg#44878 #44878 Fix: Inspector is usable on the top level block even if it is content locked]
- [WordPress/gutenberg#44809 #44809 Fix list outdents on Enter in quote block]
- [WordPress/gutenberg#44864 #44864 List v2: fix selection when creating paragraph from empty list item]
- [WordPress/gutenberg#44853 #44853 Fix overflowing patterns]
- [WordPress/gutenberg#45050 #45050 Fix visibility of nested Group block appender]
- [WordPress/gutenberg#44887 #44887 wp-env: Use case insensitive regex when checking WP version string]

Follow-up to [54257], [54335], [54383], [54483], [54486], [54490].

Props bernhard-reiter, audrasjb.
See #56467.
Built from https://develop.svn.wordpress.org/trunk@54632


git-svn-id: http://core.svn.wordpress.org/trunk@54184 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Oct 18, 2022
Package updates for bug and regression fixes:
- `@wordpress/block-directory: 3.15.8`
- `@wordpress/block-editor: 10.0.7`
- `@wordpress/block-library: 7.14.8`
- `@wordpress/components: 21.0.6`
- `@wordpress/customize-widgets: 3.14.8`
- `@wordpress/edit-post: 6.14.8`
- `@wordpress/edit-site: 4.14.10`
- `@wordpress/edit-widgets: 4.14.8`
- `@wordpress/editor: 12.16.7`
- `@wordpress/format-library: 3.15.7`
- `@wordpress/interface: 4.16.6`
- `@wordpress/list-reusable-blocks: 3.15.6`
- `@wordpress/nux: 5.15.6`
- `@wordpress/preferences: 2.9.6`
- `@wordpress/reusable-blocks: 3.15.7`
- `@wordpress/server-side-render: 3.15.6`
- `@wordpress/widgets: 2.15.7`

Original PRs from Gutenberg repository:
- [WordPress/gutenberg#45041 #45041 Font Size Picker Hint: Fallback to font size `slug` if `name` is undefined]
- [WordPress/gutenberg#45045 #45045 Add: Missing output escaping on some blocks]
- [WordPress/gutenberg#44999 #44999 Escape comment author URL]
- [WordPress/gutenberg#44972 #44972 Navigator: restore focus only once per location]
- [WordPress/gutenberg#44858 #44858 Spacing Sizes Control: Try improving layout spacing]
- [WordPress/gutenberg#44878 #44878 Fix: Inspector is usable on the top level block even if it is content locked]
- [WordPress/gutenberg#44809 #44809 Fix list outdents on Enter in quote block]
- [WordPress/gutenberg#44864 #44864 List v2: fix selection when creating paragraph from empty list item]
- [WordPress/gutenberg#44853 #44853 Fix overflowing patterns]
- [WordPress/gutenberg#45050 #45050 Fix visibility of nested Group block appender]
- [WordPress/gutenberg#44887 #44887 wp-env: Use case insensitive regex when checking WP version string]

Follow-up to [54257], [54335], [54383], [54483], [54486], [54490].

Props bernhard-reiter, audrasjb.
See #56467.
Built from https://develop.svn.wordpress.org/trunk@54632


git-svn-id: https://core.svn.wordpress.org/trunk@54184 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@ciampo
Copy link
Contributor

ciampo commented Oct 20, 2022

Heya! Would it be possible to add a CHANGELOG entry for this PR? Thank you 🙏

Ideally, when we work on a bug fix like this one, it would be great if we could also add a unit test to prevent future regressions

@ramonjd
Copy link
Member

ramonjd commented Oct 20, 2022

Heya! Would it be possible to add a CHANGELOG entry for this PR? Thank you 🙏

Ideally, when we work on a bug fix like this one, it would be great if we could also add a unit test to prevent future regressions

I have a PR up for the changelog here:

Can also add a unit test to it.

ramonjd added a commit that referenced this pull request Oct 21, 2022
* Update changelog for #45041 Fallback to font size slug if name is undefined

* Adding an aria-label to the header hint label
Adding unit tests to cover the label using `slug` if `name` isn't available.
ootwch pushed a commit to ootwch/wordpress-develop that referenced this pull request Nov 4, 2022
Package updates for bug and regression fixes:
- `@wordpress/block-directory: 3.15.8`
- `@wordpress/block-editor: 10.0.7`
- `@wordpress/block-library: 7.14.8`
- `@wordpress/components: 21.0.6`
- `@wordpress/customize-widgets: 3.14.8`
- `@wordpress/edit-post: 6.14.8`
- `@wordpress/edit-site: 4.14.10`
- `@wordpress/edit-widgets: 4.14.8`
- `@wordpress/editor: 12.16.7`
- `@wordpress/format-library: 3.15.7`
- `@wordpress/interface: 4.16.6`
- `@wordpress/list-reusable-blocks: 3.15.6`
- `@wordpress/nux: 5.15.6`
- `@wordpress/preferences: 2.9.6`
- `@wordpress/reusable-blocks: 3.15.7`
- `@wordpress/server-side-render: 3.15.6`
- `@wordpress/widgets: 2.15.7`

Original PRs from Gutenberg repository:
- [WordPress/gutenberg#45041 #45041 Font Size Picker Hint: Fallback to font size `slug` if `name` is undefined]
- [WordPress/gutenberg#45045 #45045 Add: Missing output escaping on some blocks]
- [WordPress/gutenberg#44999 #44999 Escape comment author URL]
- [WordPress/gutenberg#44972 #44972 Navigator: restore focus only once per location]
- [WordPress/gutenberg#44858 #44858 Spacing Sizes Control: Try improving layout spacing]
- [WordPress/gutenberg#44878 #44878 Fix: Inspector is usable on the top level block even if it is content locked]
- [WordPress/gutenberg#44809 #44809 Fix list outdents on Enter in quote block]
- [WordPress/gutenberg#44864 #44864 List v2: fix selection when creating paragraph from empty list item]
- [WordPress/gutenberg#44853 #44853 Fix overflowing patterns]
- [WordPress/gutenberg#45050 #45050 Fix visibility of nested Group block appender]
- [WordPress/gutenberg#44887 #44887 wp-env: Use case insensitive regex when checking WP version string]

Follow-up to [54257], [54335], [54383], [54483], [54486], [54490].

Props bernhard-reiter, audrasjb.
See #56467.

git-svn-id: https://develop.svn.wordpress.org/trunk@54632 602fd350-edb4-49c9-b593-d223f7449a82
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants