[WIP] LayoutPanel: Update to use ToolsPanel and ToolsPanelItems #45833
Conversation
|
|
andrewserong
added
[Feature] Layout
|
Size Change: +403 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
andrewserong
force-pushed
the
try/update-layout-panel-to-use-tools-panel
branch
from
475324a
to
288af0b
Compare
|
One the potentially challenging design issues for this one: The Justification and Orientation controls bump up against each other, as the Justification controls extend slightly past the gutter that's meant to exist between these two controls:
|
andrewserong
force-pushed
the
try/update-layout-panel-to-use-tools-panel
branch
from
288af0b
to
c91060a
Compare
|
@jasmussen and @jameskoster 👋 just a heads-up that I'm working on this one now. It seems to be going pretty well so far, but there are a couple of styling issues to address. The main issue is currently with the spacing between the Justification and Orientation controls — they just manage to fit in next to each other, but it's a bit of a squeeze. There's still a bit more to work on for this PR (I haven't tested it out across blocks, we'll need to tweak defaults, etc), but just thought I'd let you know that work is under way! |
|
Thanks for working on this. One initial observation: I don't think "Content size" should be a dedicated control. It's intrinsically linked to the "Content layout" option, and should be presented automatically when the latter is set to true. When I toggle the content layout option to true, the Justification control appears, is that intended? I couldn't really work out why that happened and my expectation is that it would remain toggled off until explicitly enabled via the tools menu. As you mentioned, when there are four options the Justification and Orientation controls are breaking the grid:
Probably best to place them on separate rows for now because we'll likely need to revisit these controls – they do not allow Stacks to be have right/center justification with space-between spacing. |
|
Thanks for the testing and feedback @jameskoster! I've made a few updates:
I had to move things around a fair bit while doing the updates, so I very likely might have broken something in the process. I'll do some more testing and follow-up tomorrow, so no rush on taking another look at this just yet 🙂 |
|
I realize I'm a bit late to this, but I think showing these two on a single row may be worth breaking the 2 column grid for:
I think there may be a separate exploration worth having, of making the orientation + justification controls count as a single control 🤔 |
Yeah I think that makes sense. But as with #44723 if we're going to consider the full layout scope, we need to consider things like container height and width. It may be that the entire layout panel is a single control, like in Figma which I personally find to be very intuitive. We can potentially find inspiration there: layout.mp4
|
|
Intriguing! Some good bits there worth noodling on. Some things would need refinement, but at a more high level, I wonder if it would make sense to think of this in terms of such a boolan "having layout" toggle, with layout being flex and no layout being block 🤔 |
|
Yep that's exactly what I was thinking :) |
|
It would be nice to still have a single "Layout" panel, though, so I wonder if there was a way to have these |
|
Perhaps the options here could be revised to represent flex/flow:
And depending on which option you select determines which controls are available in the Layout panel. That would also solve the awkward duplication we have currently between this variation switcher and the orientation control. |
|
Love the ideation, folks! I'm very happy seeing all the attention to exploring our Layout panel options 😀
Very much so. I suppose at the moment, my main focus is to try to figure out what makes for a good logical next step in this PR, where the UX feels decently improved, but without overhauling how we conceptualise layout just yet. Still, it's super valuable taking a look at the direction we'd like to go in, to confidence check that we're taking steps in the right direction 👍 At the moment, the layout concept is more of an So, I think I lean a bit more toward thinking of the different layout types as adjacent rather than additive, if that makes sense? E.g. we can switch between layouts, with flow as default, rather than thinking about adding a layout per se — but this might just be because I've been working closely with it for so long, so I might have a familiarity bias here 🙂 One technical note is that I don't think we have a way for the plus icon in the ToolsPanel menu to do anything other than display the current menu items available, so having that button perform an action would be a departure from the other instances in the sidebar and could be a challenge figuring out the right accessibility for it.
If we group all the controls together, does that then defeat the purpose of switching over to the ToolsPanel? I think the panel is most useful for granular control over the display and resetting of separate settings. The change in this PR is probably going to be most valuable if we have seldom used controls that we'd like users to be able to reach for only when needed (the sticky position support being the more pressing use case).
One of the other things to keep in mind with this PR is that the changes affect all blocks that opt in to layout, and not just the Group block. So while there's duplication of the control for Row and Stack, there is no duplication in Buttons and Social Icons blocks. Anyhow, thanks again for all the explorations! If we're veering off track from the current task of the conversion to ToolsPanel, I'm happy for us to continue the discussion in separate issues if need be. For the moment, what do you think the best next step is for this PR design-wise? Is the main thing to address what we do about justification and orientation controls? |
|
The layout concept has many threads, and the current implementation is not perfect – particularly how the controls switch based on whether the block is flow or flex. So my instinct is that this needs a bit more design consideration before making any changes to the layout panel in the short term, for the reasons above, and because the migration seems to require some tangential effort – grouping certain controls, presentation of options like "Inner blocks use content width", etc. |
Understood, thanks for clarifying! I'll pause work on this PR for now, then. In terms of the sticky position support over in #44723, do you think it's still worth pursuing for now, or does that also need more design-time to figure out how it'll work inside the panel? From my perspective, sticky position will be used a bit too infrequently for it to be displayed prominently in the existing panel without the ToolsPanel for it to be hidden by default, but happy to go with your or @jasmussen's lead 🙂 |
There was a problem hiding this comment.
Thanks for working on this! Just catching up on the discussion now and adding a few thoughts:
- When adding a block with "constrained" type I was hoping that the controls for custom content and wide size would no longer display by default. In the multiple discussions around the non-intuitiveness of layout controls 😅 one point that came up a bit was that it generally makes more sense for users to be able to set a global default than a custom size for each block. Given these controls may not get much use, it would be nice to separate them logically from the "use content width" toggle and have them hidden by default.
- It feels a bit weird to have "allow to wrap" control display by default for the horizontal flex blocks, whereas for vertical it's "orientation". Perhaps we could display orientation and justification by default for both?
I wonder if it would make sense to think of this in terms of such a boolan "having layout" toggle, with layout being flex and no layout being block
I'm not sure this will be possible at this point. We already have two non-flex layout types ("flow" and "constrained"), "position" will be another and in the future we also want to implement "grid".
the current implementation is not perfect – particularly how the controls switch based on whether the block is flow or flex
I'm inclined to consider this a feature 😄 because they are different layout types with different features, so it makes sense that they have different controls. Having the same-looking control potentially have different effects for different layout types would be quite confusing.
So my instinct is that this needs a bit more design consideration before making any changes to the layout panel in the short term
My biggest concern here is that we're considering this change as an essential part of the work towards stabilising the layout support, which we should do ASAP because the feature is starting to see some real-world use, and the more extenders come to depend on it, the less flexibility we have to make changes going forward.
Could we perhaps turn this around and not consider the migration to ToolsPanel essential, as it is mostly a cosmetic rearrangement of the controls, and focus instead on stabilising the API for extenders?
| // Only show the inherit toggle if it's supported, | ||
| // a default theme layout is set (e.g. one that provides `contentSize` and/or `wideSize` values), | ||
| // and either the default / flow or the constrained layout type is in use, as the toggle switches from one to the other. | ||
| const showInheritToggle = !! ( |
There was a problem hiding this comment.
Could we keep this logic in the parent layout panel, so as not to duplicate it across two layout types?
There was a problem hiding this comment.
Could we keep this logic in the parent layout panel, so as not to duplicate it across two layout types?
Good question! I couldn't work out a way to combine this toggle with the content size controls without moving it down to the layout type level. If we keep them separate (as you suggest in your other comment), then we can happily move it back to the parent.
Let me know if you can think of a better way to go about it, though! (I'll hold off on implementing further changes until we hear more from designers)
That one needs a little more work. It came up in a conversation yesterday with Matias and Joen, and consensus was to take another look at fixed & sticky together to make sure we're not painting ourselves into a corner with the design. It may still be that we implement Sticky on its own first. Hopefully we'll have an update soon :)
Sorry I was referring to the UX. I understand the need for different sets of controls, but we need to present them in ways that feel intuitive and the current experience is a bit lacklustre in that regard. For instance it's not really clear why switching from Group to Stack results in totally different Layout controls. Or that 'Orientation' and block type (Row / Stack) are the same thing presented two different ways. |
|
Thanks for the feedback and updates, folks! I think it was a useful exploration this PR, in digging up these ideas and insights into the controls. For now, I think let's park this PR,particularly with the discussion over in #46032 that looks at other options for where the sticky/fixed position control could go 👍
Treating the ToolsPanel migration as non-essential sounds like a good idea to me, as that'll free up other more impactful work if we can workaround the absence of the ToolsPanel like in #46032 |
|
Since we're pursuing a different direction for the Position UI now that is decoupled from Layout, and this PR is a little old, I'll close out this experiment. We can always open a new PR and borrow ideas from here if and when we'd like to revisit using the ToolsPanel for Layout. |
🚧 🚧 🚧 🚧 This is still a draft, it isn't quite ready for testing yet 🚧 🚧 🚧 🚧
What?
Fixes #44560
Update the
LayoutPanelwhich appears in the inspector controls to use theToolsPanelso that controls can be conditionally displayed and reset.Why?
As additional controls are added to the layout panel, some of which might be used infrequently, it will be helpful if we can hide some of them by default. A good example is sticky position support (as raised in #30121 and being worked on in #44723), which will likely be a useful feature that will only be used in particular circumstances.
How?
layoutgroup to the inspector controlsToolsPanelItemTo-do
ToolsPanelItemTesting Instructions
TBD
Screenshots or screencast