Use blockGap between Columns blocks #34456
Conversation
|
Size Change: +3.41 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
I think instead of doing this adhoc, I'd prefer if we explore making the columns block use an actual "flex" container. It will probably be more work though. We're experimenting the same for social icons block here #33987 I think all these horizontal blocks should be using the "flex" layout and avoid adhoc styles added to them directly. (Buttons, Social Links, Navigation, Columns). That way all options and features added to these layouts would benefit all these blocks automatically. |
I agree, in the long haul — but perhaps this could be a stop-gap to start using blockGap sooner? |
|
I wouldn't mind it personally |
|
I'm as big of a gap fan as you'll find, so thanks so much for the PR! Testing wise, I'm seeing the same result before and after, so that's good:
It also does come with the benefit of tying the gap to the gap custom property, which makes it nicely theme.json'able or themable instead of hard-coded. To echo Riad, it'd be nice to actually have a flex container here. The biggest benefit of flex & gap to me is that it can let the parent container handle the display of items inside, relieving us of complex first-child/last-child rules. This one just replaces the left-margin value with the custom property. But it is a tiny step forward in that it does simplify styling the gap, so it might be worth it. It also seems like any existing theme CSS that customizes this gap would still work? Will this work just fine in themes that don't use theme.json? Should we use |
|
Actually I missed a small issue, it looks like we have some additional rules in the editor.scss file that need updating:
|
andrewserong
added
[Block] Columns
|
Good call out @jasmussen — I've pushed an update to address those. Here are screenshots for blockGap used in each TT1 Blocks and Twenty Twenty One (to show a block theme vs. older theme): TT1 Blocks Editor: TT1 Blocks Front-end: Twenty Twenty One editor: Twenty Twenty One Front-end: |
I don't personally think it's necessary to have a fallback, as Gutenberg will add a blockGap value with or without a theme.json present. |
|
Thanks for adding this in @richtabor, I think this is good pragmatic next step to getting the Columns block using the
There's a separate PR here that skips generating layout gap styles if folks opt-out of the global blockGap styling. If I'm reading it correctly, they can set the root level This will skip generating the
So, for that use case, it looks like it'd still be useful to have the |
|
Thanks for pointing that out @andrewserong — I updated the PR to use the original |
There was a problem hiding this comment.
This is looking great to me @richtabor, it tests nicely, picking up the block gap CSS variable in block based themes, and falling back to the 2em value in other themes I tested it with (e.g. TwentyTwenty, Maywood):
I really like the idea of going with this PR in the short-term, it'll also unlock us opting the Columns block in to the new blockGap support from #33991 once this PR lands. I think we get the best of both worlds in starting to use the gap CSS variable, but we also side-step dealing with the breaking change of refactoring the block's CSS to use flex or grid gap, which will be a more complex process since it could break existing themes' columns overrides.
LGTM — adds in the behaviour, and existing themes I've tested it with appear to be unaffected. There is a slight change with this one running TT1-blocks in that the blockGap value is by default set to 24px in Gutenberg instead of 2em, but I actually prefer the slightly tighter gap! Since it is a change, though, it might be good to get another review for a second opinion, too 🙂
|
Hi, @richtabor Can you rebase this PR, and I think we can merge it 👍 |
Done, tests are running. Thanks! |
Description
Swapped out the hard-coded values of
1emand2emto usecalc(var(--wp--style--block-gap) / 2)andvar(--wp--style--block-gap), to leverageblockGapsupport added in #33812.Closes #34453.
How has this been tested?
Tested with TT1 Blocks, TT1 and TT20.
Screenshots
Types of changes
Style changes in columns .scss
Checklist:
*.native.jsfiles for terms that need renaming or removal).