Block spacing: add axial gap block support #35454
Conversation
ramonjd
added
[Type] Technical Prototype
|
Size Change: +299 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
4bf8a1c to
2b7d169
Compare
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
4ab1ff9 to
88e2021
Compare
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
d085c08 to
ce855fd
Compare
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
0d12eb2 to
97406be
Compare
ramonjd
changed the title
ramonjd
added
[Feature] Design Tools
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
97406be to
d1b3072
Compare
ramonjd
requested review from
ajitbohra,
ellatrix,
mkaz,
oandregal,
spacedmonkey,
tellthemachines and
TimothyBJacobs
as code owners
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
d1b3072 to
1162aba
Compare
There was a problem hiding this comment.
Thanks so much for picking up this exploration @ramonjd! Axial controls for blockGap are pretty challenging to reason about, but I like the way you've taken the grouped value and are using it to sync to the separate column / row values, it feels like a pragmatic step forward 👍
This is testing pretty well for me on the Buttons block, opting it in to the vertical and horizontal controls:
Something I noticed when running TwentyTwentyTwo theme is that it sets in its global styles the spacing of 1.5rem in its theme.json which gets rendered out on the server to --wp--style--block-gap as a single CSS variable. I think this means that we might need additional logic in WP_Theme_JSON_Gutenberg for rendering out the body spacing variables (I think this is where it's set), to also generate the other two variables when a string value is set?
I imagine this would be similar logic to what you've included for the inline variable rendering for individual blocks?
Without it, the spacing between paragraphs, for example, doesn't get applied because it can't find the row variable:
I was also wondering if it'd be worth storing the value of the variable as a single value when the row/column values are the same (E.g. 1rem instead of 1rem 1rem)?
But overall, I'm pretty excited about the progress you're making here — it demonstrates that it'll be possible to use axial controls while also not being totally locked to CSS gap as margin can still use the individual row-gap value on its own 👍
And it could be pretty cool to see what sorts of patterns the axial gap might enable in WP 6.0 — perhaps a tile-based Query Loop pattern that is tucked in closely horizontally, but has a larger vertical gap between blog post cards?
Happy to do more testing next week! 😃
packages/edit-site/src/components/global-styles/use-global-styles-output.js
Outdated
Show resolved
Hide resolved
|
I'm very grateful for review @andrewserong 🙇
Great find, and thanks for the advice. I'll take a look at this.
It would look nicer, and probably be less confusing. 😄 Is there any benefit otherwise? |
It might just improve neatness. Originally, when I first looked at the PR I thought we might need the single value instead of storing both values in the variable for cases where the CSS variable is used on a margin property (e.g. In terms of the API we're effectively creating via the CSS variables, I suppose for other blocks that might want to opt-in to block gap support, the idea here is that if you're using the main CSS variable, then you should expect it to behave like the CSS |
Makes sense. I was genuinely asking in case I'd missed something. 😆 Not a difficult change to make, and I think it would also be good to maintain the current CSS gap behaviour as well. Thanks! |
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
2 times, most recently
from
9b597b9 to
542bb75
Compare
If I understand correctly, we need to automatically set the new CSS vars So if a particular theme sets : "spacing": {
"blockGap": "2em" // or "blockGap": "1.5rem 2em"
},We'd need to also set the following: body {
...
--wp--style--block-gap: 2em; // or 1.5rem 2em if --wp--style--block-gap sets both
--wp--style--block-row-gap: 2em; // or 1.5rem if --wp--style--block-gap sets both
--wp--style--block-column-gap: 2rem;
}I'm just trying to work out where the best place in include this logic and how. Looking at WP_Theme_JSON_Gutenberg and where the styles are generated from the properties, there don't seem to any other special cases and I don't want to start a precedent for custom logic in that particular class. Maybe a series of parser methods external to that class. Not sure yet. 🤞 @oandregal will have some advice. :) I'm just making notes for now. I'll think about this and come back with something a little more nuanced. Thank you! |
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
9df1405 to
bf58722
Compare
After a very cursory glance, maybe we could implement something along the lines of PRESETS_METADATA, with a collection of bootstrapping instructions for Just an example, I have no idea which properties would be appropriate. Still learning and looking. const STYLES_METADATA = array(
array(
'path' => array( 'spacing', 'blockGap' ),
'css_vars' => array( '--wp--style--block-row-gap', '--wp--style--block-column-gap' ),
'value_func' => 'gutenberg_render_blockgap_style',
),
); |
updating php unit tests adding axial tests replacing block usages of block gap var with equivalent axial var Update packages/block-library/src/button/style.scss Co-authored-by: Andrew Serong <[email protected]>
…e columns gap value. Where the row and column values are the same, use a single value instead of shorthand in the block-gap CSS var.
ramonjd
force-pushed
the
try/add-split-axial-gap-block-support
branch
from
bf58722 to
a742255
Compare
I'm trying some things over at WordPress/wordpress-develop#2038 Still early days and lots of questions. When I gather my notes and thoughts, I'll probably write up an issue to get feedback on them. 🙇 |
|
Hey, haven't been able to dig deep into this PR and WordPress/wordpress-develop#2038 My main question is whether this is targeted for WordPress 5.9 or 6.0? If it's 6.0 we need to do a few things: 1) do not port it to wordpress-develop just yet, and 2) we can't modify code that lives in If it's 5.9, I can spend some time digging into this and help fitting it into the current codebase. |
|
Thanks for your thoughts @oandregal !
That's a good question. There is at least one bug that might benefit from a solution around axial gap support. I'm not sure about the severity, and therefore the need to target 5.9 or not. It appears to be something that might raise eyebrows if we do not fix it. In that issue, @youknowriad also proposed a way to resolve a conflict between theme.json and block-level block spacing settings. I'd imagine the code for that idea would similarly need to find a home.
Thanks very much! I'd love your advice on how to do this. For this PR specifically, assuming the concept behind it sound, I was thinking it would helpful to be able to filter style properties after we add them to the |
|
The solution to this bug #36521 means the removal of the CSS variables (at least for everything but the root level of theme.json) which would definitely impact this PR. We should probably focus on that related issue first before introducing new CSS variables which would suffer from the same issues. |
|
Closing this as we're not using CSS vars for block gap styles. See #37360 |
Description
Gap support for blocks was introduced in #33991.
It added a CSS var,
--wp--style--block-gap, to store the value.This PR splits gap support values into row and column values, and refactors the value of
--wp--style--block-gapto employ shorthand syntax:-wp--style--block-gap: 99px 1px;.To house row and columns values, we're adding two CSS vars in addition to
--wp--style--block-gap:--wp--style--block-row-gap--wp--style--block-column-gapMore CSS vars? Well, yes.
Splitting values means we can specify gap values for individual properties, offering us more control over vertical and horizontal spacing, and allows us to align other spacing definitions, such as margin, to the gap values –
margin-left: var(--wp--style--block-column-gap, 2em);– see for example the Columns Block styles.scss.
The persistence of
--wp--style--block-gapseems tautological, yet, aside from retaining backwards compatibility, it grants us convenient access to the gap shorthand value.Fixes #34529
Stealing ideas from #35301
Related #34546
Nov-09-2021.12-22-04.mp4
How has this been tested?
Test scenarios:
margin-left: var(--wp--style--block-column-gap, 2em);--wp--style--block-gapshould updates values in--wp--style--block-[row|column]-gapand vice versa.navigation/block.jsonblockGapsupport to"blockGap": [ "vertical", "horizontal" ],to test axial gap support.--wp--style--block-gapCSS var should be a single value, e.g.,101px--wp--style--block-gapCSS var should use shorthand syntax value, e.g.,101px 101%--wp--style--block-gapto determine spacing should not be impacted when axial controls are turned on.blockGapvalues (See Example block code below )and make sure that block gap value is filled in the supports control in the sidebar.Example block code
Run the unit tests:
npm run test-unit-php /var/www/html/wp-content/plugins/gutenberg/phpunit/block-supports/spacing-test.phpChecklist:
*.native.jsfiles for terms that need renaming or removal).