Try always showing the dimensions controls #50305
Conversation
tellthemachines
added
Needs Design Feedback
|
Size Change: +26 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
If I'm not wrong there's a still a difference between the DimensionsPanel and the other panels.
defaultControls.contentSize ?? DEFAULT_CONTROLS.contentSize vs using DEFAULT_CONTROLS only when the prop is not provided.
I'm fine with either approach personally but do you think we should be consistent across all the panels? (Fine to be in a follow-up though)
|
Just to help the conversation, here's a GIF of the trunk behavior, where child blocks of group, row, and stack do not have margin/padding/spacing controls shown by default:
And here's this branch, showing those same controls shown for those blocks by default: Is that correctly understood as the purpose of this PR? And are these dimension controls shown only for child blocks of a group/row/stack block? Is there another adjacent purpose that I'm missing? For me it makes sense to show those child dimension controls for children of row and stack, absolutely, because flex containers are so unique in that sense as far as fit/fill behaviors. But it doesn't seem valuable to show margin/padding controls for blocks inside a group, at least not by default. The way I see it, the group is an invisible container, it's just a wrapper — select some blocks and hit ⌘G, there, now you have a group. I'm aware it's also a div with flow layout, but design tool wise it's meant to be an invisible wrapper, and does not feel substantial enough to change the appearance of design tools for child blocks. Not a strong opinion, but I'd love to hear an opinion why this should be otherwise. |
|
The purpose of this PR is not specific to the group block or a variation of the group block. It just changes all the dimension controls by default for all blocks to be visible unless that particular block overrides it. We don't have a way to change the default controls per block variation (like suggested above). We can however change the default controls for a given block (and all its variations). The purpose of this PR is only the default value change it the block doesn't override it. |
|
Before #49389 these tools were already exposed so for me, we should just merge this PR as it resets to the previous state. |
If I remove that, none of the controls will display by default in the post editor, because The purpose of the initial PR was to get flex child controls to always display by default, and because these can apply to any child of a flex block, they aren't configurable via any individual block's block.json. So I came up with this solution. Dimensions are kind of unique in that they have this control that isn't configurable at an individual block level. Or we could change the logic to check if Another thing we can do, if showing all the controls by default isn't desirable, is hide some of them from the block.json settings. |
That seems useful. Padding, margin, to an extent even spacing, aren't items we want to see on a group by default. In general margin is also something that remains somewhat hard to grok, even with the visualizer. Happy to find some clean heuristic here, but keeping the group light-weight is the main motivation. |
Just noting that right now, any change to group also applies to row/stack |
If I'm not wrong, that's not the case because for most blocks defaultControls is undefined. But regardless, I'm fine with keeping your changes too but applying them to all the other panels. What I don't like is that the panels work differently for no valid reason.
Can you clarify more? Why can't we just say "childLayout: true" in the defaultControls of a given block? |
It may be undefined in the block settings, but in the dimensions hook here we're defining it to be an empty object if there are no block settings. This logic is different from what's used in color or borders, because dimensions is grabbing settings for both dimensions and block spacing. I guess we could change that to pass undefined if there are no settings from the block.
The idea is that these child controls should always display by default, because there's a much higher chance folks will want to change the size of blocks inside Row/Stack blocks than margins or padding or other dimensions. The child layout control gets applied to any block that's inside a Row, Stack or Navigation block (potentially this can be extended to any block with flex layout in the future). Because all blocks can have child controls, as long as they're inside one of those flex blocks, we'd have to set childLayout in defaultControls for every single block in existence, which wouldn't be practical especially because we can't control third party blocks. |
tellthemachines
force-pushed
the
try/visible-dimension-controls
branch
from
c10c0c9
to
d29c1fe
Compare
|
Flaky tests detected in 15fc72f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4911500463
|
|
So this is interesting. The e2e tests are failing since a bunch of blocks now have dimensions controls displaying by default, and the tests for e.g. the font size picker are targeting It's not ideal for the tests to have such a generic target, as they will easily break if the default display settings change for certain blocks - cc @kevin940726 But neither is a refactor of e2e selectors in scope for this PR 😅 so I'm pushing a change to the Paragraph block defaults (which will make the UI match trunk) so the tests can pass. |
|
Yeah, ideally selectors should be unique and strict not only to prevent accidental breaks but also to increase readability. I think it's perfectly fine to add the intermediate |
|
@kevin940726 I'd rather do it as a follow-up if that's ok! I want to go ahead and merge this to improve the experience in global styles now that #49428 is in, but if any blocks suddenly have an overwhelming amount of controls visible, I'm also happy to follow up with more custom settings to hide controls per block. |
|
Coming to this late and definitely missing additional context-- thanks for the patience and sorry for just commenting on this. Hiding the controls was done to offer a more curated, less intrusive experience where only what's modified is displayed. More context can be found here: #27331 This undoes that effort in a way that doesn't make a lot of sense to me. It's also a fairly large UX change. In reading back over the original discussion, the root problem seems to be that we need a way to better display the new global styles UI and there was a solution offered that wasn't agreed upon. Can we go back to that discussion in a separate PR or issue to find another solution and revert this change for 15.8? Between the impact on extenders and the reverting of a lot of prior design thought, I think we need to take a step back. |
|
I just want to mention that this PR didn't change what is shown or hidden by default compared to what was there before the initial discussion. Everything was shown by default (aside childLayout which the original PR explicitly wanted to show by default). The combination of both PRs did change how the framework "overrides" the default values though. Right now, we need to pass all the keys we want to override and not assume that any omitted key is hidden by default, which seems like a better behavior. (fallback to the default is a key is omitted). We can always tweak the default values (global) or override the ones we like for any block we like. |
What?
Sets the default display values for the controls in the Dimensions tool panel to true.
It's still possible to hide controls for individual blocks from their block.json settings.
Why?
Follow-up to this discussion. This has become more pressing to address because of the new global styles UI from #49428, where it's awkward to have the dimensions controls all hidden by default.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
The biggest panel I found was for the Group block when nested inside a Row or Stack:
Otherwise everything looks pretty civilised 😄