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 displays incorrect font-sizes in incorrect order. #55744

Closed
tp-cast-ai opened this issue Oct 31, 2023 · 12 comments
Closed

Font Size Picker displays incorrect font-sizes in incorrect order. #55744

tp-cast-ai opened this issue Oct 31, 2023 · 12 comments
Labels
[Feature] Typography Font and typography-related issues and PRs Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@tp-cast-ai
Copy link

tp-cast-ai commented Oct 31, 2023

Description

Font Size Picker seems to be broken since v16.9.0 release. See the issue below.

Step-by-step reproduction instructions

  1. Add these font sizes to theme.json:
"fontSizes": [
	{
		"size": "0.5rem",
		"slug": "caption",
		"name": "Caption"
	},
	{
		"size": "1rem",
		"slug": "x-small",
		"name": "XS"
	},
	{
		"size": "2rem",
		"slug": "small",
		"name": "S"
	},
	{
		"size": "3rem",
		"slug": "medium",
		"name": "M"
	},
	{
		"size": "4rem",
		"slug": "large",
		"name": "L"
	},
	{
		"size": "5rem",
		"slug": "x-large",
		"name": "XL"
	},
	{
		"size": "6rem",
		"slug": "xx-large",
		"name": "XXL"
	}
]
  1. Try selecting a font size with Gutenberg plugin disabled (or with Gutenberg plugin v16.8.1), you will see Font Size Picker displayed correctly:
    image

  2. Enable Gutenberg 16.9.0, Font Size Picker displays nonsense - order is incorrect, default sizes displays incorrect values (in pixels, however it still works fine on the frontend)
    image

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@t-hamano
Copy link
Contributor

t-hamano commented Nov 1, 2023

Thanks for the report. I was also able to confirm the same problem.

@t-hamano t-hamano added [Type] Regression Related to a regression in the latest release [Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended labels Nov 1, 2023
@t-hamano
Copy link
Contributor

t-hamano commented Nov 1, 2023

I identified that this issue was caused by #55219, which decided to merge all origins.

However, regarding font size, I think it should not be merged with the default origin, but should be completely replaced by the theme.json definition.

Also, I found that the same problem occurs with SpacingControl. Here is an example theme.json:

Details
{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 2,
	"settings": {
		"appearanceTools": true,
		"layout": {
			"contentSize": "840px",
			"wideSize": "1100px"
		},
		"spacing": {
			"spacingSizes": [
				{ "name": "XXXS", "size": "1em", "slug": "xxxs" },
				{ "name": "XXS", "size": "2em", "slug": "xxs" },
				{ "name": "XS", "size": "3em", "slug": "xs" },
				{ "name": "S", "size": "4em", "slug": "s" },
				{ "name": "M", "size": "5em", "slug": "m" },
				{ "name": "L", "size": "6em", "slug": "l" },
				{ "name": "XL", "size": "7em", "slug": "xl" },
				{ "name": "XXL", "size": "8em", "slug": "xxl" },
				{ "name": "XXXL", "size": "9em", "slug": "xxxl" },
				{ "name": "XXXXL", "size": "10em", "slug": "xxxxk" }
			]
		}
	}
}
Expected Actual
Only the space sizes defined in theme.json and the default, zero, are displayed. expected Both the space sizes defined in theme.json and the default space size (0, 1, 2...) are displayed. actual

cc: @matiasbenedetto, @pbking

@iamtakashi
Copy link

I’ve noticed this too. I’m glad the cause has been identified.

@hanneslsm
Copy link

For spacing, this seems to be the same issue: #55636

@t-hamano
Copy link
Contributor

t-hamano commented Nov 2, 2023

I'm searching for what solution would be best, but haven't found an answer yet. Therefore, I give it the "Needs Technical Feedback" label.

@t-hamano
Copy link
Contributor

I discovered that a similar problem also occurs with inline highlighting.

WP6.4.1 with Gutenberg trunk enabled (Unexpected behavior) WP6.4.1 with Gutenberg trunk disabled (Expected behavior)
gb_enabled gb_disabled

In TT4, the default palette is displayed even though it is disabled. If you temporarily revert the changes made in ##55219 on the current trunk, this issue will not occur.

@graylaurenm
Copy link
Contributor

Since the suggestion on #55636 is to add settings.spacing.defaultScale perhaps doing a similar settings.typography.defaultFontSizes here would make sense.

@t-hamano
Copy link
Contributor

I think this issue a high priority because I believe this behavior affects many users. So I'll add it to the WP6.5 Editor Tasks board.

@annezazu annezazu moved this to ❓ Triage in WordPress 6.5 Editor Tasks Jan 5, 2024
@annezazu annezazu moved this from ❓ Triage to 🗣️ In Discussion / Needs Decision in WordPress 6.5 Editor Tasks Jan 5, 2024
@gaambo
Copy link
Contributor

gaambo commented Jan 25, 2024

I've got the same problem.
Is there someway I can help with this issue - reproduction steps, testing, development?
Actually there is a defaultFontSizes property in theme.json (see #52200 and theme.json reference) but setting it to false does not provide the expected result.

@gaambo
Copy link
Contributor

gaambo commented Jan 25, 2024

An interesting aspect:

  1. Add a font size to theme.json > settings > typography > fontSizes that has the same slug as a core default font-size:
{
          "fluid": false,
          "name": "Groß",
          "size": "1.375rem",
          "slug": "large"
},
  1. In the font-size picker the core font-size with it's value is displayed and all core-sizes are displayed before the defined ones, therefore appearing out of order.
    grafik
  2. The correct font-size (1.375rem) is applied in the frontend

@ramonjd ramonjd moved this from 🗣️ In Discussion / Needs Decision to 🎯 Needs Core Commit in WordPress 6.5 Editor Tasks Feb 1, 2024
@ramonjd ramonjd moved this from 🎯 Needs Core Commit to 🗣️ In Discussion / Needs Decision in WordPress 6.5 Editor Tasks Feb 1, 2024
@annezazu annezazu moved this from 🗣️ In Discussion / Needs Decision to 📥 Todo in WordPress 6.5 Editor Tasks Feb 8, 2024
@getdave
Copy link
Contributor

getdave commented Feb 13, 2024

Noting that @ajlende has a PR open for this #58409

@colorful-tones colorful-tones moved this from 📥 Todo to 🏗️ In Progress in WordPress 6.5 Editor Tasks Feb 13, 2024
@t-hamano
Copy link
Contributor

This issue has been fixed in #58951. #58951 has been given a backport label, so it should be reflected in WP6.5 Beta2.

@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.5 Editor Tasks Feb 14, 2024
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 Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
No open projects
Status: Done
Development

No branches or pull requests

7 participants