-
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: Update draggable chip to resemble the list view item when dragged #49820
Conversation
Size Change: +76 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
Nice work. Here's a GIF showing a semitransparent "row" from the list view being lifted up as you are dragging: One of the things about drag and drop is that for the longest time, both for blocks in the canvas and for blocks in the list view, we've discussed in various theads that ideally we could adopt a drag and drop method that is instant, like this: The existing behavior in the canvas of graying out the block you're dragging, collapsing it to a chip, then dropping it on a blue line — that's implemented in lieu of a more instant behavior such as the above. I understand that is a larger undertaking and refactor of drag and drop in general, that's why it hasn't happened yet. But it bears mentioning that to an extent, that's still something to explore. This is perhaps especially important for the list view, where we are already a couple of steps towards that: already now, the block being moved is taken out of the flow when moved, it's just collapsing into a blue line, so to speak. Instead of that, I could see it working a bit more like how Google Keep works: I know that's a bit of a mouthful, but that is context for why I think if we do go this route of showing the item being moved, we probably need to adopt a library for it, so we can do collapsing animations, and proper moving ancestors around to "make room" for the new location of the block being dropped. The work in this branch is impressive, but I'm not convinced without the more full behavior, that it's a benefit over what ships in trunk, where the chip is idential to what you'd see in the canvas. What do you think? |
My reply turned out rather long, so I'll just add a TL;DR at the beginning here: I think we'll need this styling change to unblock further UX fixes in the shorter-term (drag to parent levels), and we can explore evaluating libraries in the longer-term / in-tandem with incremental UX fixes. Thanks for taking a look @jasmussen, it's important to take a step back and see how all this fits into the bigger picture! As I see it we currently have two potential directions to go in, in order to overall improve the UX for drag and drop in the list view:
My gut feeling is that the former is a better use of time at the moment, for a few reasons:
Where does adding a custom draggable chip fit it?While as a standalone PR it mightn't be obvious why this particular change is necessary, I'll try to recap my current explorations into improving the overall drag and drop UX for the list view (tracked in #49563, roughly from top to bottom).
So, essentially the display of the drag chip is a blocker for allowing users to drag to parent levels, as it obscures the drop indicator line. In terms of whether or not it makes for a good UX to have the partially transparent list view item and the drop indicator at the same time — personally, I quite how it looks as an incremental step. And it's pretty consistent with another tree library, React Complex Tree, that @mtias shared recently. Longer-term, I do think it's worth exploring the idea of drop-in-place, and expanding and contracting list view rows. My suspicion is that we might wind up running into usability issues with expanding the whole list view on drop, as it moves the list view around, which some folks might find hard to work with. Interestingly React Complex Tree uses the drop indicator line, but doesn't hide the block to be moved, instead it simply greys it out, which could also be a fun thing to try. Hope that all helps for extra context, apologies that this comment is so long! |
This is pleasing to my eye. The chip doesn't hide the inserter line and it is comforting to know that I'm dragging the right thing. I haven't memorized all the block icons yet. 😄 One question: what's your take on how to indicate dragging a sequence of siblings? Would it be useful in a later exploration to show multiple stacked chips, or extending chip content in such a situation? |
Thanks for taking this for a spin!
Good catch!
Off the top of my head, in a later exploration, it could be fun to either show multiple stacked chips / extend them vertically so it approximates the section being dragged, like you mention. We could always cap it if it looks too long with multiple blocks, and/or fade the subsequent ones. For now, I'd probably leave it as just using the info for the first dragged block in this PR. |
Thinking ahead -- I suspect with the path this will eventually take (leaving the space where it was removed, providing space for the new spot) it's going to need to attach to the cursor in the position you started dragging it. With it attaching to the cursor at the side there's going to be a noticeable jump back and forth as you drag and drop. I also wonder if this will feel more natural even without these future steps in place? |
That's a great idea, I'll give that a try 👍 |
a72deaf
to
4a0a913
Compare
Update: I've managed to get it mostly working now by retaining the initial position — I've reworked the PR a bit to simplify things (it's quite a small changeset now!), and pass down the 2023-04-18.16.31.23.mp4Next up: I'll look into seeing if we can get the list view item to render above the drop indicator line. I think the culprit is that Draggable attaches to the element wrapper when using All this seems quite promising so far, thanks again for the suggestion @apeatling. I'll keep chipping away. |
Flaky tests detected in 162ddab. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5947965716
|
That last change inspired an idea that may be a big can of worms, but in the spirit of ideas being cheap I'll ask anyway: would it be feasible for the new chip and the drop indicator to be combined? So instead of the blue line, a space equal to the height of the chip appears, pushing the other blocks out of the way? If it's not trivial to try, then please ignore :) |
That's essentially what I'm asking in this comment, but you managed to do with much fewer words. Teach me this magic! As is, I don't mind this branch as a step forward, it might bring us a small step closer to what you are suggesting. But it also departs the list view from the look of drag and drop in the canvas, and it's not clear to me this is a net win overall. But I trust Andrew quite a bit, and if the best approach forward is to slowly iterate, I'm definitely one to support that. But all the better to have a compelling destination to aim for, and I would agree — instant "movement" with a perfectly honed dragging behavior seems appropriate for this. |
I share this same sentiment. We already have the block icon in the current chip, which validates the selection. I think we should focus on the movement element, which leads to better confidence on dropping the chip into place. Unless I'm missing something? |
Thanks for the feedback and encouragement, folks, much appreciated!
Yes, that's very much one of the end goals to explore, nicely articulated 👍! Getting there will be non-trivial, but I believe we can get there iteratively, which is my current objective. But even if we don't get all the way there, we'll still have improved the UX quite a bit along the way.
I think answering this question will hopefully help get to the heart of why I believe we need to take an iterative approach to reach our goal. Over in #49563 I've broken down some of the UX issues we need to resolve in order to improve the drag and drop UX, and the highest impact of those tasks is unfortunately not very obvious. But it's improving the behaviour of dragging to parent blocks, which I'm exploring over in #49742. That PR (which is still an early experiment) looks at the mouse cursor positioning, and explores allowing the cursor to drop to any point of the block hierarchy in deeply nested blocks. Fixing that up will unlock most of the subsequent explorations, and is a requirement before we're able to look at making space in the destination drop target. The connection here is that once we have the ability to accurately target all levels of the block hierarchy, we'll be in a better place to visually reflect that by showing the block moving in place. That's my hunch so far, at least 🙂 However, while working on that PR, I encountered the issue that the current drag chip obscures the drop target. This isn't an issue in the editor canvas, as blocks are spaced out much more than the list view items are. So, before we can improve dragging to parent levels of the block hierarchy, we first need to ensure that the user is able to see all levels of the block hierarchy, and this seems like a good step to getting there to me. All that said, while I believe I need to work iteratively here in order to safely move the UX forward without breaking anything, there's also no rush to merge this PR. I'm happy to leave this one open until the drag-to-block-parents PR (#49742) is in a good, reviewable shape. Thanks again for all the questions and ideas! |
Code update: I've opened up a separate PR (#49911) to update the |
I'm just struggling to identify how changing the chip to be larger/resemble the list view item is an improvement, if we don't have the "pushing other blocks out of the way" UX. They seem quite knit together.
Is it a matter of opacity, or size? This PR makes it bigger. Seems the "pushing blocks out of the way" would solve for the drop target being covered, in both the canvas and the list view (without requiring a chip design change). Perhaps an improvement in the chip design, for list view, would be a nice follow-up to try out though. |
Thanks for the feedback, Rich!
From my perspective, I think they're related, but my subjective feeling is that it's still a nice enhancement on its own. It's fairly similar to the visual behaviour of React Complex Tree, and it's less of an abstraction in terms of shape. That said, I definitely don't want to try to merge anything that folks don't feel confident about.
Opacity is probably the main issue, we could always try tweaking the opacity of the existing drag chip, too. For now, I'll leave this PR open, and continue on with my work on the drag to parent PR. We can then revisit what might make a good logical step to change the drag chip in a more holistic way. I think it'll become a bit clearer once I can demo things all together 🤞 |
Sounds good, agree. A lot of potential to improve on this front. 💪 |
4a0a913
to
51d787d
Compare
Update: I've given this a rebase, as I'd like to play around with some of the visuals here again now that the drag and drop UX has been improved in other PRs. I see it's a bit buggy after the rebase, so it isn't ready for reviews or feedback just yet. I'll experiment with this a little next week. |
A non-urgent update: I've fixed up some of the styling and bugs, and this is ready for design review / playing around with now, and to see what we think of it. Here is a screengrab: 2023-08-24.16.21.42.mp4To recap what this PR is exploring:
All up for discussion as to whether or not folks think this is an improvement over what's in trunk, of course! My personal preference is that I like a combination of both the transparent drag chip in this PR and keeping the drop indicator line — now that we no longer hide dragged items, this means that the list overall feels pretty solid and doesn't move around as you drag an item up and down the list. It's also pretty consistent with dragging items on a Github project board, where the dragged item is transparent, and there's a blue drop indicator line for the drop target. |
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.
I think this is an improvement compared to what is in trunk. Of course it can continue to be iterated on alongside the list of ongoing improvements you have in the main issue. Standalone it's an improvement as it is and will continue to improve as more of that work is done. ✅
What's the lift to have other blocks live/pushed out of the way when dragging? |
@andrewserong @richtabor we had an old exploration on drag displacement that felt pretty good: #34022 It was running into performance issues because it predated a lot of the improvements done to list view since then. I think there's a lot of merit to displacement and the clarity it affords for dropping something in place. Nesting is slightly trickier, but not impossible. |
Thanks for the link @mtias! That last screengrab in the PR in particular (#34022 (comment)) helps give an overview of how it might work combined with the windowing logic (at least visually). Part of my concern was how it might look with longer lists, but overall that does seem promising for displacing list view items 👍 I'm currently focusing on a few of the lower-hanging-fruit pieces of drag and drop in the editor canvas and the list view, but I'll add this to my list to hack on again. I'll likely open up a separate PR to see what's possible in the current state of the list view.
Something I like about some of the screengabs in #34022 is that while dragging, the dragged item becomes solid with a blue background — I think something like that will help with nesting as the left edge of the dragged block is much clearer visually, so it'll be easier for folks to see what nesting level they're going for. Let's see how we go! |
I've opened up an issue over in #56539 to capture the next steps to attempt this PR again, but with behaviour to move other list items out of the way, in order to create more of a WYSIWYG experience while dragging within the list view. Thanks again for the feedback and ideas, everyone! I'll close out this PR now, but will revisit the ideas in follow-up explorations. Please share any other ideas or suggestions over in #56539 that anyone would like considered or incorporated when we try this again 🙂 My first step will most likely be to look into #33684 (expand collapsed list view items when dragging over them) as we'll need that in place in order to do WYSIWYG dragging to nested items without showing a drop indicator line. |
I've started a proof of concept for displacing list view items over in #56625. Happy for any early feedback! |
What?
Part of #49563
This is an exploratory PR to look into customising the draggable chip that's used when dragging blocks within the list view. The goal is to make it feel like the user is dragging the real list item, rather than using an abstraction for the drag chip.
Note: 🚧 🎨 this is just an experiment at the moment, it'll need design feedback on what the chip should ideally look like. The opacity is added so that the drop indicator line isn't too obscured.
Why?
There are two main goals here:
To elaborate on the second point above: while working on dragging to all levels of the block hierarchy (#49742) it's become clear that when dragging to higher levels of the hierarchy, we need the chip not to obscure the drop indicator line, otherwise it's hard for users to see where they'll be dropping the dragged block.
Not covered
How?
BlockDraggable
component to accept an__experimentalDragComponent
and pass it down to theDraggable
component.Draggable
to be overridden, and forBlockDraggable
to accept anelementId
null
, and passBlockDraggable
the id of the list view row — this tellsDraggable
to make a clone of the list view item for draggingTesting Instructions
id
attribute in the Advanced panel. It should display the heading title and id attribute in a way that's pretty similar to the non-dragged version.Testing Instructions for Keyboard
Screenshots or screencast
Screengrab:
2023-08-24.16.21.42.mp4