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

Layout blockGap / spacing: Add support for gap set at the block level in theme.json and global styles #39789

Closed
andrewserong opened this issue Mar 27, 2022 · 6 comments · Fixed by #40875
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.

Comments

@andrewserong
Copy link
Contributor

Following on from #39601 where we removed the blockGap / block spacing controls in global styles (since the gap value is not currently supported at the block level in global styles), this issue proposes a way to add it back in!

As of #37360 where the CSS variable approach for blockGap was removed, we haven't had support for blockGap at the individual block level in global styles or theme.json. One possible path forward to re-instating it could be:

  • Every block that uses the layout support is server-rendered and currently attempts to grab the block gap value from the block's attributes, and if it doesn't exist, falls back to using the global blockGap CSS variable.
  • Can we add in logic in layout.php and in the JS rendering for the layout support so that if the block's attributes do contain a blockGap setting we then look-up theme.json to grab the blockGap value if one is set at the corresponding block name level?
  • If this approach works, also roll it out to the Gallery block's custom code that implements gap support.
@andrewserong andrewserong added [Type] Enhancement A suggestion for improvement. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Mar 27, 2022
@andrewserong andrewserong self-assigned this Mar 27, 2022
@ramonjd
Copy link
Member

ramonjd commented Mar 30, 2022

As of #37360 where the CSS variable approach for blockGap was removed, we haven't had support for blockGap at the individual block level in global styles or theme.json.

I've been testing and it looks like it's because blockGap is constrained to a top level style. I commented about that over in this PR,

Specifically, this line:

'spacing' => array(
     ....
    'blockGap' => 'top', // <- this will be skipped when adding to `$schema['styles']['blocks']` 
),

There's a comment in sanitize() too:

// Some styles are only meant to be available at the top-level (e.g.: blockGap),
// hence, the schema for blocks & elements should not have them.

Can we add in logic in layout.php and in the JS rendering for the layout support so that if the block's attributes do contain a blockGap setting we then look-up theme.json to grab the blockGap value if one is set at the corresponding block name level?

Previously, Gutenberg rendered a global style targeting the block class:

.wp-block-group { 
    --wp--style--block-gap: 341px;
 }

Just so I understand, would we try to reinstate that rule, or something like that rule? Or look up the global value and only set for the specific block if that block itself hasn't got one (I think the latter?)

@ramonjd
Copy link
Member

ramonjd commented Mar 30, 2022

Something like this?

#39870

@andrewserong
Copy link
Contributor Author

Just a quick drive-by comment while I'm AFK, thank you so much for picking up this issue @ramonjd! The idea in #39870 is exactly the kind of thing I had in mind — looks like a good direction to investigate to me 😀

In principle:

  • The global styles only renders out the block gap CSS variable at the root level
  • The Layout support is responsible for grabbing block-level values in theme.json and uses those values to compute the right $gap_value as you've done in that PR

@ndiego
Copy link
Member

ndiego commented Apr 19, 2022

@andrewserong is this still something we are trying to get resolved in 6.0. If not, I will remove from the project board. Thanks!

@ramonjd
Copy link
Member

ramonjd commented Apr 19, 2022

@andrewserong is this still something we are trying to get resolved in 6.0. If not, I will remove from the project board. Thanks!

I've removed this issue and the PR. Thanks!

@andrewserong
Copy link
Contributor Author

Implemented in #40875.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.
Projects
None yet
3 participants