Size Change: +284 B (0%)

Total Size: 1.03 MB

_{compressed-size-action}

Pinging @jasmussen as this PR relates to your concerns here: #32610 (comment)

I was discussing this a little with @andrewserong and @aaronrobertshaw, and considering potential issues with the Button block behaving differently when you unlink the sides in the padding controls compared to other blocks. One thought is that we could get rid of the linking/unlinking behavior for Button; so, only display the axial controls with no option to link/unlink. This prevents any frustration that might be caused by expanding the box control and perceiving missing options/less granularity than they may see in the padding controls for other blocks.

I'm unsure if that's necessary, or if it's consistent with other use cases for axial controls. Gap, for example, seems like it would benefit from having both general and axial spacing.

Also pinging @mtias since this is a follow-up on your request here: #31774 (comment) :)

Thanks for working on this! Here's before:

And here's after:

On the one hand, it's nice to be able to control both sides on an axis, and is very probably what you want most of the time.

On the other hand, this now becomes impossible:

I would consider it an edgecase, but it's also always important to be aware of the opportunity cost: which creative designs are we making impossible by our changes, even if well-intended? Additionally, the axial split controls are surfaced after you untoggle the "link" button, meaning you've already made a choice to surface the advanced padding controls.

In this case, I tend to think that axial padding support is possibly something we can implement in a more visual way such as #33221. In that interface we might enable axial resizing when holding Alt and dragging. Taking that a step further, we might even support Alt + dragging on the input control to do the same in the 4-split input controls.

Axial controls feel the most potent when affecting interfaces that are already technically locked in a similar way, such as column blocks where the axes control the gap property between them. Or in cases where we are absolutely sure we want to limit the configuration. In this case, I'm not convinced. But it's also not a strong opinion, and I'm happy to defer.

Nice work!

On the one hand, it's nice to be able to control both sides on an axis, and is very probably what you want most of the time.
On the other hand, this now becomes impossible:

For context (description updated now), I added the padding support on Button very recently in #31774, at which time it wasn't possible to enable axial selections via the block support. This is meant to be a relatively fast follow-up to address a suggestion to limit the controls to axial sides until we see concrete requests for padding support on individual arbitrary sides (at which point we could easily go back).

My take on this is that controlling top/bottom or left/right padding as pairs is likely what users want most of the time, and is a little cumbersome using the four individual side controls. It's possible by holding down the Alt key as you point out, but not obvious, and seems a little bit like requiring a "power feature" to optimize the most common use case. The visual controls in #33221 look fantastic, but as proposed require a key combination to access and also seem to me like a bit of a power feature.

I do worry about making it impossible to achieve those edge cases with mismatched padding on axial sides -- maybe we get the best of both worlds by limiting the Inspector Controls to the axial sides, and allowing individual side padding via the visual controls in the future? This makes the common use case very easy via Inspector Controls but allows finer customization via the more advanced/"power feature".

Additionally, the axial split controls are surfaced after you untoggle the "link" button, meaning you've already made a choice to surface the advanced padding controls.

+1 What do you think about removing the linking/unlinking here? So specifically for Button, we display the axial controls by default with no link/unlink button. The obvious downside is we lose the ability to change general padding for all sides very quickly (you can obviously still individually update the two inputs). This feels less cumbersome to me than updating the four individual inputs.

+1 What do you think about removing the linking/unlinking here? So specifically for Button, we display the axial controls by default with no link/unlink button. The obvious downside is we lose the ability to change general padding for all sides very quickly (you can obviously still individually update the two inputs). This feels less cumbersome to me than updating the four individual inputs.

That's an interesting thought!

With that in mind, we might actually want to just try this PR as is, and if the use case I described comes back to haunt us, we can explore that.

👍 👍

@stacimc I haven't had a chance to test this properly yet, but nice work hooking into the axial controls! I think we might need to also update the global styles spacing panel to ensure that the filterValuesBySides function sets filteredValues correctly — at the moment it looks like the function expects each of the sides to be left, top, right, bottom, so if the custom sides are horizontal or vertical, we might need special handling in that function to pass those values along.

Here's where the function lives:

https://github.com/wordpress/gutenberg/blob/trunk/packages/edit-site/src/components/sidebar/spacing-panel.js#L36-L36

We might be able to borrow a bit of the logic from the axial-input-controls to keep it consistent?

I think we might need to also update the global styles spacing panel

Good catch, thank you! I will take a look at this :)

This code change is looking good to me @stacimc, and nice inclusion of the validity check. Looks like this just needs a rebase now that the ToolsPanel is in, but I’m happy to give this another look over and test once it’s ready!

This has been updated to include the changes from the new Dimensions panel (#32392). I've also made padding a default control for Button.

Ran through every check and this worked as designed. 👍

I think the notice in the console is standard for letting developers know when something is deprecated or not implemented as expected, so this seems like a good place to me.

I think the more visual representation of padding on the canvas that @jasmussen linked to could be followed up on after this initial round of tools work is completed, as we continue to refine the tools and the UI we are adding.

Thanks for this follow up! This is a much better default.

On the other hand, this now becomes impossible

@jasmussen I'm fine with this not being possible. It can always be added later if there's a strong need for it. Right now, allowing arbitrary values on Button feels like it will invariably produce not great results.