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

FontSizePicker: new t-shirt sizes don't match the existing font sizes in some themes #45268

Closed
t-hamano opened this issue Oct 25, 2022 · 4 comments
Labels
[Feature] Typography Font and typography-related issues and PRs [Package] Components /packages/components [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed

Comments

@t-hamano
Copy link
Contributor

t-hamano commented Oct 25, 2022

Description

In #43074, the font size ToggleGroupControl now displays the t-shirt size as the button name. I'm concerned that this may cause confusion in many themes that define font size variations.

For example, in the following theme, the button text and tooltips seem to be at odds with each other:

Twenty Twenty or Twenty Twenty Two

tt0

That is, if you are already using your own t-shirt size or if the beginning is not an S (e.g., XS).

I understand that this is a change to improve design consistency, but I believe it will have a significant impact on existing consumers. Therefore, I would like to propose the following changes:

Option to force selectbox

If there are more than 6 font size variations, the select box will be displayed and the t-shirt size will not be displayed. Therefore, I believe it would be useful to have an option to force a select box without having to add the necessary variations to make it a select box.

Option to change button text

In addition to size, slug, and name, add properties to change the button text. For example, shortName.

{
	"size": "1.5rem",
	"slug": "md",
	"name": "Medium",
	"shortName": "M"
}

shortName seems to be customarily used in the Classic theme, as seen in these issues (I'm not sure when this property appeared, and it doesn't appear to be considered in the latest WordPress/Gutenberg.

@ramonjd, @jasmussen
If there is anything I am missing or misunderstanding, please let me know 🙏

Step-by-step reproduction instructions

Activate Twenty Twenty or Twenty Twenty Two and check the font size picker text and tooltip labels.

@t-hamano t-hamano added [Package] Components /packages/components [Feature] Typography Font and typography-related issues and PRs labels Oct 25, 2022
@jasmussen
Copy link
Contributor

The segmented control and the sequence from small to XL has the added purpose of encouraging consistency in slugs and font size definitions, so that switching themes the sizes can transfer. For when a theme needs more customizability than this, and isn't as concerned about theme switching, the font dropdown is arguably a better choice since it affords more space to provide context. At the moment, the dropdown appears when you have 6 or more presets, but perhaps it can be opted-into through other means.

In this particular case, I would think either that TT2 could be updated, or we could override the tooltip to match the t-shirt sizes and the sequential nature of the segmented control. What do you think?

@ddryo
Copy link
Contributor

ddryo commented Oct 25, 2022

The segmented control and the sequence from small to XL has the added purpose of encouraging consistency in slugs and font size definitions, so that switching themes the sizes can transfer.

This is well understood.
However, if that is important to you, you should have done it since ver. 5.0.

Until now, we could define any size we wanted using add_theme_support( 'editor-font-sizes' ), and we could specify the display name with the 'name' and 'shortName' option.

Based on that specification, I defined the font sizes in my theme. (In my theme, it is "XS, S, M, L, XL, XL" instead of "S, M, L, XL, XXL".)

It is clearly an unreasonable change to force the display name to "S, M, L, XL, XXL" now, ignoring the previous specifications.

Either express them numerically as up to 6.0, or provide an additional option to specify what kind of notation should be used.

Translated with www.DeepL.com/Translator (free version)

@talldan
Copy link
Contributor

talldan commented Oct 26, 2022

This seems like a similar issue to #44245

@t-hamano
Copy link
Contributor Author

Thank you all for your input!
I think what this issue is about is essentially the same as #44245. Therefore, let's close this issue and focus the discussion on #44245.

@t-hamano t-hamano added the [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed label Oct 26, 2022
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 [Package] Components /packages/components [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed
Projects
None yet
Development

No branches or pull requests

4 participants