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

Should particular 'fluid' setting of a font size preset work if the global typography.fluid setting isn't set or it's false? #64766

Closed
2 tasks done
matiasbenedetto opened this issue Aug 23, 2024 · 4 comments · Fixed by #64790
Assignees
Labels
[Feature] Typography Font and typography-related issues and PRs Needs Decision Needs a decision to be actionable or relevant [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Question Questions about the design or development of the editor.

Comments

@matiasbenedetto
Copy link
Contributor

Description

Should particular fluid settings of a font size preset work if settings.typography.fluid is false or not set in theme.json?

Let's consider these settings as example:

{
	"settings": {
		"typography": {
			"fontSizes": [
				{
					"fluid": {
						"max": "60px",
						"min": "30px"
					},
					"name": "Large",
					"size": "59px",
					"slug": "large"
				},
				{
					"name": "Small",
					"size": "1rem",
					"slug": "small"
				}
			]
		}
	},
	"version": 3,
	"$schema": "https://schemas.wp.org/trunk/theme.json"
}

In this case, the 'Large' font size preset fluid settings are ignored by the styles renderer because settings.typography.fluid isn't set. The same would happen if settings.typography.fluid is set to false. Fluid styles are only set if settings.typography.fluid is true.

Expected

If the particular font size preset has fluid settings defined, it should render the fluid styles because more specific settings take precedence over more general settings. In the case mentioned before, the global fluid setting is not even defined, so the more particular font size preset fluid settings should be used. What do you think?

If we think that the particular font size preset fluid settings should be respected if they are set explicitely we can close this another related issue because it won't be valid anymore: # 64765 Font size presets inconsistent with the global fluid typography setting
.

cc. @WordPress/gutenberg-design @WordPress/block-themers

Step-by-step reproduction instructions

  1. Use the theme.json settings of the example.
  2. Check that the 'Large' font size preset is not rendered as a fluid font size in CSS.

Screenshots, screen recording, code snippet

No response

Environment info

  • Gutenberg trunk
  • WordPress 6.6.1

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
@matiasbenedetto matiasbenedetto added [Type] Bug An existing feature does not function as intended [Type] Question Questions about the design or development of the editor. Needs Decision Needs a decision to be actionable or relevant [Feature] Typography Font and typography-related issues and PRs labels Aug 23, 2024
@matiasbenedetto matiasbenedetto changed the title Should particular 'fluid' setting of a font size preset work if the global typography.fluid setting is not set or is false? Should particular 'fluid' setting of a font size preset work if the global typography.fluid setting isn't set or it's false? Aug 23, 2024
@t-hamano
Copy link
Contributor

t-hamano commented Aug 24, 2024

Thanks for the suggestion.

In my opinion, we need first to resolve the inconsistency between the actual specifications of theme.json and the global styles UI. In other words, if settings.typography.fluid is not enabled, hide the fluid typography setting in the global styles UI.

After that, we can consider whether to respect the fluid setting for specific font size presets.

Personally, it seems contradictory to me that we can disable the fluid setting for particular font size presets, but there is no way to enable it.

If we decide to respect the fluid settings of a specific font size preset, we will always be able to show the fluid typography settings in the global styles UI. However, this will change the theme.json spec, so a bump in the API version might be required.

Pinging @ramonjd: Because I think you were mainly working on this feature.

@ramonjd
Copy link
Member

ramonjd commented Aug 26, 2024

Should particular fluid settings of a font size preset work if settings.typography.fluid is false or not set in theme.json?

The original intention was to provide a way for individual font sizes to opt-out of fluid typography if it was turned on globally.

So yes, why not the opposite: individual font size presets can opt-in if fluid typography is not activated globally?

I don't think it's a huge thing to implement.

fluid typography settings in the global styles UI

I'm not sure about this, is it new? I can't seem to find any fluid font size-specific UI.

@t-hamano
Copy link
Contributor

@ramonjd Thanks for the feedback!

fluid typography settings in the global styles UI

Font size preset UI was added in #63057. This allows us to add font sizes with fluid setting from the global styles.

Site Editor > Global Styles > Typography > Font Size Presets:

image

Currently, we can enable fluid settings even though settings.typography.fluid is not defined, so this issue was reported in #64765. Regarding this problem, this shouldn't be difficult to fix.

@ramonjd
Copy link
Member

ramonjd commented Aug 26, 2024

Font size preset UI was added in #63057. This allows us to add font sizes with fluid setting from the global styles.

Oh, thanks! I totally missed that 😄

I threw up a quick PR for this specific issue:

I'm not sure, however, if it plays well with the UI, or what's expected here right now, so I'd appreciate some sanity checks.

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 Decision Needs a decision to be actionable or relevant [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants