WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.18k stars 4.06k forks source link

Undefined spacing presets in theme.json cause zero spacing when are in pattern markup #60129

Open miksansegundo opened 4 months ago

miksansegundo commented 4 months ago

Related to https://github.com/WordPress/gutenberg/issues/39371

What problem does this address?

Developers have a lot of flexibility when creating a spacing scale for a theme. This is great.

For instance, TT4 uses a 10-60 scale, and TT3 uses a 30-80 scale.

However, when a spacing preset is in a pattern markup (maybe copied from a public library or another theme) but is not defined in the theme.json of the active theme, the result is zero spacing.

The following is an example of the preset --wp--preset--spacing--20 being used in a pattern while it is not defined:

image

The editor only creates CSS variables for the presets defined in theme.json, resulting in zero spacing.

Screenshot 2567-03-22 at 18 20 12

What is your proposed solution?

We could avoid this issue by defaulting to the variable --wp--style--block-gap when a spacing preset is not defined.

Screenshot 2567-03-22 at 18 20 59

Here is a possible implementation for the default 20-80 spacing scale:

:body {
  --wp--preset--spacing--20: var(--wp--style--block-gap);
  --wp--preset--spacing--30: var(--wp--style--block-gap);
  --wp--preset--spacing--40: var(--wp--style--block-gap); 
  --wp--preset--spacing--50: var(--wp--style--block-gap);
  --wp--preset--spacing--60: var(--wp--style--block-gap);
  --wp--preset--spacing--70: var(--wp--style--block-gap);    
  --wp--preset--spacing--80: var(--wp--style--block-gap);  
}

For reference: What is blockGap and how can I use it?

cc: @richtabor @annezazu

annezazu commented 4 months ago

@WordPress/block-themers for thoughts from you all!

ndiego commented 4 months ago

I like the idea but I think this is a bit more tricky than it looks. The spacing preset slugs can actually be anything. For example, you could specify small, medium, and large instead of using numbers. Therefore, the spacing prests in a third-party pattern could be anything.

miksansegundo commented 4 months ago

Therefore, the spacing presets in a third-party pattern could be anything.

That's fine.

This idea is to include the CSS variables for the 20-80 presets in a CSS hierarchy level that theme presets will overwrite.

In other words, Patterns using 20-80 presets will work on any theme because that's the default WordPress spacing scale, while patterns using any other custom presets will only work in a theme that defines those presets in its theme.json.

carolinan commented 3 months ago

There are other presets like the default colors that are printed even if a theme opts-out of them. But the default spacing preset is completely replaced if the theme adjusts it. I don't remember why that decision was made.

Could the editor add a fallback value to the CSS var()? The pattern creator could add a fallback value, but that is a "new" practice that would need to be established.

madhusudhand commented 3 months ago

Here is a possible implementation for the default 20-80 spacing scale: :body { --wp--preset--spacing--20: var(--wp--style--block-gap); --wp--preset--spacing--30: var(--wp--style--block-gap); --wp--preset--spacing--40: var(--wp--style--block-gap); --wp--preset--spacing--50: var(--wp--style--block-gap); --wp--preset--spacing--60: var(--wp--style--block-gap); --wp--preset--spacing--70: var(--wp--style--block-gap);
--wp--preset--spacing--80: var(--wp--style--block-gap);
}

This requires to check the existence of the spacing presets, before adding fallback.

Could the editor add a fallback value to the CSS var()?

IMO, this is the better approach and solves variables with any naming pattern, by taking native CSS variable fallback approach.

gap: var(--wp--preset--spacing--50, --wp--style--block-gap)

What if block gap is also disabled?

This is still an issue. May be block gap variable could also add a default fallback value?

--wp--style--block-gap: var(value-from-theme-json, editor-defined-default-fallback-value)
simison commented 3 months ago

However, when a spacing preset is in a pattern markup (maybe copied from a public library or another theme) but is not defined in the theme.json of the active theme, the result is zero spacing.

Just to share another example where the issue came up. New Block hooks API allows injecting blocks, or groups of blocks in templates.

In Jetpack plugin we're adding a group of blocks (group, paragraphs, and a subscription form) at the end of each post; so essentially a pattern. We had to use pixel values for spacing to make the pattern work reliably across themes.

We had issues with some chosen spacing variables missing, as well too much size variety between themes that the pattern didn't look good anymore in some themes.

miksansegundo commented 1 month ago

I believe this issue will improve thanks to Theme.json version 3 because settings.spacing.defaultSpacingSizes will be true by default, adding the default spacing presets.

The issue will remain for themes that set defaultSpacingSizes as false. I'll test it to confirm if that will just "hide" them from the editor or "not define" the default spacing sizes.

cc: @scruffian @ajlende related to https://github.com/WordPress/gutenberg/pull/61842