-
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
List View: Reduce spacing for each nesting level, and between block icon and block title #49600
Conversation
…con and block title
Size Change: +3.87 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Just pinged a few folks for visibility on this exploration. I'm not too sure how well it'll hold up, as one of the issues I've run into with the slightly offset position of the block icon for each nesting level is that the drag and drop indicator line is no longer lined up nicely across nesting levels. I've nudged the positioning slightly here, but very happy for any ideas on how to improve it (or ideas about the nestling level spacing in general): 2023-04-05.17.21.44.mp4 |
Flaky tests detected in 3e2ab5f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4615985561
|
I think this is a huge improvement visually. 🙇 For humungous lists, would it eventually run into the same problem? I tried and it seems to hug the right side of the column nicely.
I can't find where the indicator line is to test it, but what if we were to increase the gap between the parent/sibling at the insertion point, maybe with animation too, to indicate the drop placeholder onhover. Does that make sense? |
Thanks for taking a look @ramonjd!
It looks like your screenshot is from trunk? At that level of nesting there should be a bit more breathing room at the right hand side. With really huge lists, since the nesting is currently capped at 8 levels, there should be a bit more room, without blocks ever getting that close to the line at the right (so the block settings button should always be visible) 🤔. Once we start nesting beyond 8 levels, it'll stay at the same level visually. The hope is that if the spacing is a little denser, then we'll be less likely to encounter the need for scrolling horizontally (as in #49508 (review)), though it'd still be good for it to scroll when/if it ultimately needs to.
I think that makes sense — do you mean adjusting the vertical spacing between two blocks when you're hovering between them? I'd love to have a play with that, too, but likely separate to this PR as it gets a bit complex. Currently the drop indicator is rendered as a popover, so we'd need some additional code that adjusted spacing within the list view when hovered, as the drop indicator isn't "really" in the DOM along with the rest of the list view. |
Ho! Sorry, I uploaded the wrong screen grab. Thanks for catching it. I updated it in the comment.
Exactly! You expressed it 100 x better 😄 |
Great! I'll have a hack around in a separate PR next week — will let you know how I go! |
Oh, great catch @apeatling! I'll fix it up. |
Update: in 08f782f I've resolved the spacing issue with the first level indent. The current state of this PR:
Here's how it's looking for a deeply nested list: I think this PR is probably ready for a final design review now. Thanks for the feedback! |
Thanks so much for the PR, the attention to detail on the list view is much appreciated, it's a crucial UI to get right! As I noted in #49508 (comment), I'm not convinced this is a great solution, however. It optimizes for having very deep levels of nesting, at the expense of things lining up for the cases with very little nesting. As we increasingly explore auto collapsing, contentOnly locked blocks and patterns, and even the wp-pattern block, these deep nesting levels are hopefully going to be more rare occurrences. That's not to say deep nesting shouldn't be fixed. But without a scrollbar or otherwise, even this improvement would eventually run into the edge. In that light, I'd focus on getting the on-hover customized scrollbar from #49508 working as intended. This is already in place in the site editor templates list, on not-so-tall screens: Resizing the list view would also be a small ergonomic improvemment that could be useful: |
Thanks for taking a look at this @jasmussen, and for the clarity, it's very much appreciated as always! To my eyes, I personally prefer optimising for deeply nested lists, because they seem to be much more common in the site editor where we have more complex layouts within nested header and footer template parts. But I'm happy to disagree and commit, and I do see that the tighter spacing in this PR breaks the nice visual alignment on I'll close out this PR for now and switch over to other list view tasks, I also really like the idea of being able to drag the size of the list view sidebar, that'd be a cool one to play with 👍 |
Just one more thought about this line:
The list view currently has a capped depth limit of 8 levels, so in practice, it seems the spacing in this PR avoids kicking over into a horizontal scrollbar when we reach the max depth limit. At that point, everything gets displayed at a single level, as on From my perspective, I find horizontal scrollbars quite frustrating to work with, especially for users using a mousewheel that only scrolls vertically, so they'd be good to avoid if we can. Still, I do like the idea of seeing if we can add a |
We might indeed want to come back to this one! As a personal opinion I think it might be useful to start with the other options first, see how far that takes us first. 🙏 |
I'm conflicted on this one :) For me it's easier to use List View with the reduced spacing, especially when there are many levels of nesting which is common in the site editor. I appreciate increasing the depth cap as well. But it cannot be denied: the alignment isn't as easy on the eye. A resize handle would help, but the cost is a reduced canvas footprint which seems like a pretty meaningful trade-off. I feel we should be shooting for maximum economy when it comes to the space that List View occupies. Increasing the width should be a last resort for users. |
All of those points are good and I agree — I think my main suggestion is that accepting this alignment tradeoff feels more appropriate to do once we've explored the other improvements on deck. Not just the scrollbar and resize handle, but also the hopefully across the board reduction in nesting levels we can get through the contentOnly editing model, pattern block, focus mode, etc. If all those successfully reduce the times you see deep nesting levels, we might not need to accept this trade-off, which very much optimizes for the present. |
Thanks for the continued discussion here! I very much agree that it's important to get the tradeoffs right. Let's keep this PR in the back of our minds as we make further improvements — it'll be an easy one to dust off if we decide we'd like to proceed with it a little further down the track 🙂 |
What?
Part of #49563
Reduce the margin for each nesting level of the list view, so that more nesting levels can fit within the sidebar before needing to scroll.
This idea came out of review discussion over in #49508 (review)
Why?
It's quite common, especially in the site editor, to have heavily nested lists of blocks. The goal with this PR is to see if contracting the use of space will make for an easier to use UX for deeply nested hierarchies.
How?
4px
(from8px
)Testing Instructions
Note: the styling rules for the list view currently max out at a nesting level of 8, so deeper than 8 will render at the same nesting level within the UI.
Some heavily nested block markup for testing
Testing Instructions for Keyboard
Screenshots or screencast