-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Dimensions Panel: add padding tool as default for blocks where this is common setting #34026
Conversation
Happy to take feedback on which blocks should have the padding setting exposed by default - @jasmussen do you have any thoughts on this? |
This is the best possible challenge to have, so nice to have this new tools panel 👌 — and thanks for the followups on it. I find myself very often using the padding control on the Cover block, it seems nice to add it there by default. For tagline, title and verse, I think there's a slightly larger question to settle: if the block has a ToolsPanel, should it ever be empty or should it always have at least one design tool inside? The argument being that if we hide it by default, we might as well not have the panel in the first place. On the flipside, I don't see myself using padding on any of those three blocks very often, but it's nice to have access to it when needed, and so it's fine that a fully collapsed dimensions panel exists. It seems worth going with the 2nd approach for now: being okay with a completely empty dimensions panel where you have to add the control. |
Have added to the Cover block as default now as well. |
… might be a common setting
f14f926
to
e884007
Compare
Note: Cover block currently shows two dimension panels, PR for that issue here #34008 |
I was just looking at blocks with margin support. The Site Title and Site Tagline are the only ones with margin support so far. I was thinking that would be convenient to be able to control the margin on such headings. e.g,: It also has the added benefit of surfacing the controls in order to introduce the concept of "Dimensions", it being a new concept in the controls and all. Here's a teensy PR just in case folks agree: #34026 Happy to can it. |
This is working well for me @glendaviesnz If you like we can move the default padding over to #34008 to unblock the others in this PR? What do you think? |
Actually, I'm talking nonsense. The duplicate Dimensions panel will still be there, so let's leave it in. Sorry for the confusion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests well for me. ✨
✅ Column block displays padding control by default
✅ Cover block displays padding control by default
✅ Group block displays padding control by default
This PR adds the default option to 3 of the 7 core blocks that currently support the padding option:
- Button
- Group
- Columns
It appears the button block actually has the padding control in the defaults via #33859.
From a code perspective this one looks good. In terms of which blocks show padding by default the selection here make sense to me.
I'd suggest if there are refinements around the ToolsPanel
behaviour that should be discussed in a dedicated issue and updated in a separate PR.
There is an alternate approach to resolving the two dimensions panels in #34065. It leverages work around allowing for ad hoc controls to be injected into block support panels. #34063
I could be a little confused by the semantics around "empty" here. One could argue the panel isn't empty if it does house a control, even if it isn't currently being displayed. Such as the case where the block supports padding but it doesn't need it to be prominent. To me, this is similar to the original collapsible panels. If the panel was collapsed, it isn't displaying any controls but I would not call it empty. I take it here we are referring to if a panel is visually empty? Default controls can be hidden by toggling them off via the My understanding was the goal of the ToolsPanel was to assist in lowering the prominence of controls that didn't need "everyday" type of attention from users. This allowed blocks to adopt more block supports and for the exposure of new controls to be managed. I think that being able to have the "empty" visual state as it does now facilitates the above goals. Happy to discuss this further in a separate issue if needed.
@ramonjd I think this introduction will also be helped by increasing the consistency between the different block support panels i.e. adopting the |
That's a good point. Maybe I was remaining paranoid that, given that it's a significant UX change, it might need some watering down. There's also the fine idea of replacing the ellipsis icon with a Cheers! |
Description
Currently blocks that support the
{spacing: { padding: true}}
do not show the option by default in the Dimensions panel. Now there is an__experimentalDefaultControls
there are some blocks that could benefit from exposing this option by default to make it more obvious to users.This PR adds the default option to 3 of the 7 core blocks that currently support the padding option:
These seem to be the blocks where users are most likely to be looking for an option to modify padding.
The other four blocks that currently support padding, but don't seem as likely to need padding adjustments always visible are:
Testing
Check out this PR to local dev env
Add a Button, Group, Column block and check that padding displays by default in the Dimensions panel
Screenshots
Before:
After: