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

--wp--style--block-gap custom property not set by blockGap property in theme.json #40381

Closed
mrwweb opened this issue Apr 15, 2022 · 8 comments
Closed
Labels
Needs Testing Needs further testing to be confirmed.

Comments

@mrwweb
Copy link

mrwweb commented Apr 15, 2022

Description

I have run into this issue in two "hybrid" themes: classic PHP themes with theme.json. I first noticed this recently when a new post had no space between paragraphs in the editor. I then found the same issue on a second site where some additional editor styles were covering up for the problem.

On both sites, I have defined styles.spacing.blockGap in my theme. However, in both the editor and front end, the --wp--style--block-gap custom property is not set.

There seems to be a second bug in that the front-end columns block has a 2em fallback value, but in the editor for block spacing, there is no fallback value for the custom property. It seems like the fallback should be its own custom property like this so it can be reused across multiple block styles:

--wp--style--block-gap-fallback: 2em;
--wp--style--block-gap: var( /* theme.json custom value */, var(--wp--style--block-gap-fallback));

This may somehow be related to #39122, but I can't quite piece it all together, and so I think this is worth a new issue.

Step-by-step reproduction instructions

Example theme.json:

{
  "$schema": "https://schemas.wp.org/trunk/theme.json",
  "version": 2,
  "styles": {
    "spacing": {
      "blockGap": "1.5em"
    }
  }
}

Observe that --wp--style--block-gap is not defined in editor or front-end.

Screenshots, screen recording, code snippet

Front-end columns style:

The custom property is undefined and there is a fallback value of 2 em

Backend block spacing style:

The custom property is undefined in a CSS rule and there is no fallback value

Environment info

WordPress 5.9.3
No Gutenberg Plugin
Custom themes
Different active plugins
Confirmed by two people (me and @cdils)

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

Yes

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

No

@ndiego ndiego added the Needs Testing Needs further testing to be confirmed. label Apr 18, 2022
@carolinan
Copy link
Contributor

In (6.0-beta2-53249) without Gutenberg:
I can confirm that for classic themes with theme.json, --wp--style--block-gap is defined.

In the code example for theme.json above, block gap is not enabled in settings, so it would be undefined.
https://developer.wordpress.org/block-editor/reference-guides/theme-json-reference/theme-json-living/#spacing

@mrwweb Mark, does that help solve the issue?

@cdils
Copy link

cdils commented Apr 25, 2022

I can confirm (5.9.3 with 0 plugins) that defining settings.spacing.blockGap sets a custom property for --wp--style--block-gap.

{
    "$schema": "https://schemas.wp.org/trunk/theme.json",
    "version": 2,
    "settings": {
        "spacing": {
            "blockGap": true
        }
    },
    "styles": {
        "spacing": {
            "blockGap": "1.5em"
        }
    }
}

That's interesting as I originally tested this (with settings.spacing.blockGap defined) and still did not get the expected outcome. After testing more and confirming the above code works, I'm attributing that phenomenon to caching.

What is the appropriate way to clear the cache for theme.json? I tried toggling the version numbers (1,2) for grins, but that didn't do anything.

@mrwweb
Copy link
Author

mrwweb commented Apr 25, 2022

In the code example for theme.json above, block gap is not enabled in settings, so it would be undefined.

@carolinan So to confirm: The styles.spacing.blockGap custom property is only output if settings.spacing.blockGap is true? That is unexpected and feels like a bug to me.

@justinkruit
Copy link

So I need to enable the settings per block, to have a bit of proper spacing between blocks in the Gutenberg editor? That just feels weird to me. When you don't use a theme.json you automatically get a nice gap between blocks, and I just want that back while not having all kinds of size settings for the user.

@cdils
Copy link

cdils commented May 21, 2022

On a related note, today I learned this from @carolinan's excellent FSE lessons :

The blockGap settings are disabled (false) by default. If you have enabled appearanceTools in the settings section of theme.json, you do not need to enable blockGap separately.

    "settings": {
        "appearanceTools": true
    }

I thought that was an interested tidbit!

According to developer.wordpress.org docs, blockGap is an undefined type with a default of null (as opposed to false).
This is a departure from how other properties in theme.json are typed.

If it was a boolean, it'd make sense more sense (to me) to enable it in settings.spacing.blockGap and then define it in styles.spacing.blockGap. I'm not sure what the reasoning is to have it as undefined.

@justinkruit Check this resource for definining spacing/margin/padding (so that your users don't have to) -> https://fullsiteediting.com/lessons/theme-json-layout-and-spacing-options/#h-the-block-editor-layout-control

@mrwweb
Copy link
Author

mrwweb commented Jul 12, 2022

Just reconfirming that this behavior is still present in WordPress 6.0. This is a problem because without --wp--style--block-gap defined in the editor, there is no space between elements by default in the editor when theme.json is in use. It should be possible to define a default gap without having to support editing that gap per block.

@andrewserong
Copy link
Contributor

@mrwweb, I think to resolve the issue described here, in the theme.json file you can set settings.spacing.blockGap to false. This will prevent the UI controls from being displayed in the editor, while still outputting blockGap styles stored in theme.json. Is that what you had in mind?

According to developer.wordpress.org docs, blockGap is an undefined type with a default of null (as opposed to false).
This is a departure from how other properties in theme.json are typed.

Yes, the behaviour of blockGap is a little unusual in that it's a boolean with a default of null, which disables both the block spacing editor UI and gap style output. This extra level is necessary, because the core theme.json shipped in the editor includes a base spacing value. The setting was originally introduced in #34491, and I believe it was introduced like this so as not to cause conflict with themes that already define their own vertical spacing between paragraphs, headings, and the like.

It seems the nuances between the blockGap's three settings null, true, and false wasn't ever really documented, so I've opened up a PR in #42447 to hopefully clarify it a bit better in the Theme JSON page of the handbook. Happy for feedback or reviews over on that PR if anyone gets the chance to take a look!

@mrwweb
Copy link
Author

mrwweb commented Jul 15, 2022

@andrewserong Thank you so much for looking into this and clarifying.

I can confirm that the following does correctly output the --wp--style--block-gap custom property in the editor and front end:

{
  "$schema": "https://schemas.wp.org/trunk/theme.json",
  "version": 2,
  "styles": {
    "spacing": {
      "blockGap": "1.5em"
    }
  },
  "settings": {
    "spacing": {
      "blockGap": false
    }
  }
}

After reading the reasoning, I can understand why it was built this way. That said, the fact that someone can set a style in theme.json that is dependent on a setting feels weird. The documentation updates in #42447 is definitely necessary (thank you!), and I also hope that maybe this situation can remain an outlier.

@mrwweb mrwweb closed this as completed Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Testing Needs further testing to be confirmed.
Projects
None yet
Development

No branches or pull requests

6 participants