[Buttons block]: Add gap and vertical margin support #34546
Conversation
stacimc
added
[Status] In Progress
|
Size Change: -386 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Great work here @stacimc and thanks for the detailed PR description, there sure are a lot of variables when it comes to styling and positioning Button(s) blocks!
I've left a comment about the legacy standalone Button block (when it's rendered without a Buttons container), and a comment about the 100% width for the vertical variation.
Another slight oddity for the vertical variation, which isn't really caused by your PR because it exists on trunk, is that we're subtracting the block gap from the width of the 25%, 50% and 75% buttons. I think this works great in the horizontal layout as you've described (we expect the editor to figure out the appropriate balance with the gap when we set widths), however with the vertical layout when adjusting the gap, it seems unexpected that the gap would affect the widths... would it be worth adding rules like the following, or do you think it'd get too messy?
.wp-block-buttons.is-vertical > .wp-block-button {
&.wp-block-button__width-25 {
width: 25%;
}
&.wp-block-button__width-50 {
width: 50%;
}
&.wp-block-button__width-75 {
width: 75%;
}
}
Other than that, I think this is looking really good, and even without refactoring the CSS all that much, by adding in the gap variable, you've been able to get rid of quite a bit of CSS which neatens it a fair bit. I'm particularly excited that we'll be able to tweak layouts like the following with gap support, though, so I think it's going to be a great feature:
Good point -- agreed, I would not expect gap to be accounted for in the widths of vertically oriented buttons. I'm not worried about adding in a few extra rules considering how much we're able to clean up with the rest of this PR :) Happy to add that in while I'm here! |
There was a problem hiding this comment.
Thanks for fixing up those issues @stacimc! That's testing nicely in TT1-Blocks now, and I tested in the site editor, too 👍
I did a bit more testing today and tried out themes without a theme.json to test the fallback values, and noticed that it looks like Sass needs to use the interpolation syntax #{} to inject the Sass variable when used within a var().
To test, I switched my site to use the Maywood theme, then duplicated my test Buttons block and then removed the gap value so that the block doesn't have access to any --wp--style--block-gap and will use the var's fallback value.
Here's a before / after with the interpolation added, so I think that's all that's needed to get this working on non-theme.json themes 🤞
| WIthout interpolation syntax | With interpolation syntax |
|---|---|
 |
 |
Other than that, these changes are all testing well for me! Assuming others are fine with the change, I think the final step will be to see where the discussion arrives at in regards to switching on the block gap and where we expose the control in the UI. Once we arrive at a conclusion, hopefully we'll then be able to get this and the gallery and columns blocks opt-in PRs merged.
|
Thanks for this, gap feels like such a win here. These two:
Do we still need the In either case, setting two buttons both to 50% width, I'd expect that to be 50% of the allotted space, i.e. if there's a 50% wide gap, then actually the buttons are 33% of the full container space. On a separate note, rows with multiple buttons is likely an edgecase, but it would be nice to upgrade the gap control to support axial configuration for the cases where you want a narrow horizontal gap and a tall vertical gap. |
| @@ -63,7 +63,6 @@ | |||
| .is-selected & { | |||
| height: auto; | |||
| overflow: visible; | |||
| margin-top: $grid-unit-20; | |||
There was a problem hiding this comment.
Do you know what this margin was for?
There was a problem hiding this comment.
It was added here, looks like it was applied to the LinkControl in the block toolbar. It's been refactored since then and I'm fairly confident this isn't getting used at all anymore, actually 🤔
I have tested adding/removing a button's link and the controls look good to me.
| } | ||
| } | ||
|
|
||
| // If the browser supports column-gap, use that instead of the margins above. | ||
| @supports ( column-gap: #{ $blocks-block__margin } ) { |
I was not able to get this to work without the
@jasmussen I think we're on the same page here and I'm just confused by the % gap, so making sure I understand: if I have say a 400px-wide row containing two buttons, each set to 50% width, and an explicitly 200px gap, then I would expect the buttons to each be 100px or 25% of the full container space. I would still expect this to be true if the gap was set as a percentage -- so if I set the gap to 50%, the gap itself is 50% of the container (200px), and then set my 2 buttons to 50% they would each take up 50% of the remaining allotted 200px, rather than being treated like a ratio to the gap. Is that what you're thinking?
Definitely ➕ ! I think the block support initially explored that but it was decided that we should move that to a follow up. I like the idea of being able to control these separately for buttons.
Thanks, I've updated the PR description to reflect this 👍 |
|
I quickly browsed through the above comments. |
stacimc
changed the title
I agree, this would be really useful. #34529 is tracking the addition of axial gap support that would allow separate row/column gap configuration. On a related note I think I'm going to have to do some more tinkering on this for backwards compatibility in non-blocks themes. The substantially smaller column spacing as compared to row spacing is one issue:
This PR as it currently stands will also shrink the space between the Buttons container and blocks above/below it. I'm afraid we might not be able to get away with simplifying all the margin code after all. |
stacimc
force-pushed
the
add/gap-block-support-to-buttons
branch
from
5848145
to
454e445
Compare
|
This has been updated to implement the row gap using The styling rules use What that means specifically is that on TT1 blocks, for example, on trunk we have a
Another problem caused by using
Using the margin block support, the user can control the margin applied to the container itself but not to the individual buttons. This would be easily remedied if we could just use |
|
This PR is at a bit of an impasse due to some tradeoffs with respect to backwards compatibility. @jasmussen I'm curious how you feel about some of the priorities here? One big tradeoff is the use of There's also the issue of axial gap support. Since that hasn't been implemented just yet (#34529), we get a bit of a styling regression on themes that opt into the block support, as there's no way to preserve existing styles where column gap is smaller than row gap. Finally the unified variable approach for |
|
Backwards compatibility with regards to sufficient browser support is important, and I do recall there being an issue with
I don't think backwards compatibility with themes is something that should block forward movement on features that otherwise vastly improve the user experience. It is important to note changes like these in changelogs and dev notes, and it's important to ticket the changes. And it's a balance of course, if the change affects every existing theme, it's one thing. If it affects a single or two themes, it's quite another. For example, TwentyTwentyOne did the very best it could with the Navigation block in the state it was in when the theme launched. It was frankly impressive. However it had to add a great deal of CSS to make things work, and that CSS is now technical debt. We had to move forward with the navigation block, despite the issues that were caused. This ticket details those issues, and there's now a patch to fix TT1: https://core.trac.wordpress.org/ticket/53220 Can we get a sense of how big the problem is? I do recognize the problem of providing blanket top/bottom block margins, often by targetting If the problem is widespread enough, it might be worth looking at creating some fallback rules and put them in https://github.com/WordPress/gutenberg/blob/trunk/packages/edit-post/src/classic.scss, so that the appearance is okay. What do you think? |
stacimc
force-pushed
the
add/gap-block-support-to-buttons
branch
from
cf4d1a1
to
9fd3a7d
Compare
Pushed! When I originally suggested an
Ah yes, this is caused by a bug where the variable would sometimes render in classic themes -- it was fixed in #35209 and I have now rebased just to pick that up and confirm that it's working. |
|
Cool, I think we should try this. Gap vastly simplifies the user experience, as well as the implementation, and the behavior is both predictable and nearly always desirable when spacing items out. This takes a big step towards that, and doesn't preclude us from revisiting and tweaking in the future, as themes start to embrace this, or in case issues get surfaced. And thank you for your work on this! |
|
Is this feature available in the recent 5.9 release? It seems that I am unable to use it. It might be caused by the astra theme or the astra gutenberg block plugin, and if so, I'd report it there. Any hints?
|
It is available in 5.9, however themes need to explicitly opt-in to the |
|
Thanks! Is there a way to enable it for me with a code snippet in the meantime? Do you also know how to properly set the margin after a button block? For me I always have to add a paragraph afterwards, as the following text, image, etc is sticked way to close. I know this is another issue, but I could not find any information why this happens. It seems to be an upstream issue or decision. This workaround fixes it in the frontend, but not in the editor. Maybe you got any hint? .wp-block-button {
margin-bottom: 0.5em;
} |
Unfortunately, no. The way that each theme implements support for Gutenberg features can be quite unique depending on how they're set up, so beyond what's in the handbook, it's probably best to contact the theme developers via their support forums if you're not building your own theme from scratch.
Usually the Button block would be inside a Buttons block, so I don't think you'd need to add a bottom margin on the individual button? Again, this sounds like it's likely to do with how the theme handles spacing. In blocks-based themes that opt-in to block gap and margin you can select the optional margin control to adjust the spacing between the Buttons block and the following paragraph for a one-off change (however ideally the theme would set its desired spacing at the global level):
If the theme you're using doesn't support that, the theme developers will be the best ones to let you know what flexibility they provide. If you think there's a bug in Gutenberg, though, and it doesn't look like an issue has been reported for it, feel free to create an issue! |
|
Doesn't seem like there's a way to add a blockGap value to the core/button block in the theme.json, is that still an open issue? I've seen a solution like this that would be useful so a user wouldn't have to adjust the spacing every time they add buttons - assuming the default blockGap is larger. |
|
Thanks for reporting @bradley2083! That's correct, the |
Fixes: #29098
Related/Depends on: #28365
Description
Makes use of the new Gap block support (#33991) in the Buttons block to allow control over the space between buttons. At the moment horizontal and vertical gaps are not handled separately. It also adds vertical margin support to allow adjusting the margin above and below the Buttons container.
As with Columns in #34456, this is intended as a stop-gap to get the Gap block support used quickly in Buttons, while we separately look into making the block use the
flexlayout. My reasoning is that this approach works with vertically oriented buttons, while flex layouts currently only support horizontal layouts.It also fixes a few pre-existing buttons bugs:
Major Tradeoffs
As we move from using top & bottom margins to gap for controlling vertical spacing, there will be some styling regressions in themes that are currently tweaking those margin values.
0, themes will need to tweak therow-gapvalue instead.In particular here are two known bugs:
margin: 0on the Buttons container and adds extra margin to the:first-childbutton. With this PR applied, this causes all space between the Buttons container and adjacent blocks to disappear (unless the other block has its own margin), and vertically offsets the first button. https://core.trac.wordpress.org/ticket/54250Notes
How has this been tested?
Manually. Some areas to focus:
theme.jsonScreenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).