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

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

Open
miksansegundo opened this issue Mar 22, 2024 · 7 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Enhancement A suggestion for improvement.

Comments

@miksansegundo
Copy link
Contributor

miksansegundo commented Mar 22, 2024

Related to #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

@miksansegundo miksansegundo added the [Type] Enhancement A suggestion for improvement. label Mar 22, 2024
@miksansegundo miksansegundo changed the title Missing presets Not defined spacing presets in theme.json results in zero spacing when exist in a pattern markup Mar 22, 2024
@miksansegundo miksansegundo changed the title Not defined spacing presets in theme.json results in zero spacing when exist in a pattern markup Undefined spacing presets in theme.json cause zero spacing when are in pattern markup Mar 22, 2024
@jordesign jordesign added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Mar 24, 2024
@annezazu
Copy link
Contributor

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

@ndiego
Copy link
Member

ndiego commented Mar 26, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

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.

  • What if block gap is also disabled?

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
Copy link
Member

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
Copy link
Member

simison commented Apr 5, 2024

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
Copy link
Contributor Author

miksansegundo commented Jun 20, 2024

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 #61842

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

7 participants