List View: Experiment with drag in place #34022
Conversation
gwwar
added
[Type] Enhancement
|
Size Change: +2.98 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
|
Leaving a note to myself that the current drag implementation doesn't correctly commit a valid drag operation sometimes either. Likely need to debug what's going on in the Edit: found the issue in this branch, was calculating the wrong index when the item to be moved was still in the tree. current.drag.error.mp4 |
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
715a6c0 to
4fa425a
Compare
|
That first video looks so good! |
|
Leaving another note to myself that if we're skipping over items like in the following video, the translate value from Some other things to try:
ignore.mp4 |
|
Edit: okay I think I can work around this by keeping track of velocity as a motion value in onDrag, and also using a shared When breaking out of a group, the dragging interaction will stop forcefully. This is likely due to the fact that instead of a single list, we render multiple lists using branch.js . If we like the inline swapping, I'll likely want to refactor this to remove branch.js, and instead favor looping over ListViewBlock using a selector that gives us a flat representation: 5268ff8#diff-aa92c48095c35f17feb834dd919baf52274a73c7b1180ff5acce7b6087b126cfR2236-R2263 dragging.forced.to.stop.mp4We may also need to add a UI piece at the bottom of block containers to more easily allow drag interactions where we swap drag parents. |
|
This is excellent! |
|
Some quick videos of moving items out of a container group: Moving up out of a group feels somewhat natural. The group item, acts as the swappable: move.out.of.group.mp4Moving down out of a group needs more thought. When we break out of a container, it should actually remain at the same item position (just with a different parent). Maybe worth inserting an "End group" item on drag so we can swap easily in and out? Or maybe more padding to containers? Kapture.2021-08-13.at.13.55.56.mp4Implementation Notes: Due to how List View is implemented, when moving items in or out of groups, we need to use a Shared drag session since it's moving to a different branch parent (and thus uses a different component). This is somewhat experimental. So far there's a bug around |
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
0c963a3 to
d593485
Compare
I think it seems good at the moment. From what I can tell, it's not possible right now to do the opposite and drag an item to inner blocks. Is that something I'm not doing right or a work in progress? It could be worth addressing that first because it's a closely related interaction that requires clearly showing the nesting level. |
More of a work in progress, I can tidy that. In terms of interactions, @talldan it sounds like you don't mind keeping a simple up/down drag interaction in containers (skipping over invalid items), vs another gesture (say left or right) to move up or down a level? |
I think that mostly is the best solution, but there are cases where some other gesture will probably be required: Empty parent blocksThere needs to be a way to differentiate dropping a block after the parent vs dropping a block as an inner block of the parent. There are some options, dragging to the right when hovering underneath an empty group (how it works now), or dropping on top of the parent block. Nested structures where there's no sibling at the same level (#33678)This is hard to describe but it's something unsolved in the implementation in With a nested structure like this: If I want to drag and drop the image block into one of the first, second or third level groups, but after the paragraph, that requires some way for the user to indicate the depth. The same situation happens when a group block with inner blocks is at the end of List View and a user might want to drop a block after the group. |
|
@talldan So right now I'm testing drag right + up/down for picking a new parent, and left + up/down to break out of a parent. A straight up/down drag will swap with the next sibling if possible. It feels pretty natural so far, but I'll see if I hit any other corner cases. left.right.mp4The Navigation Block makes the need for alternate gestures pretty clear since each navigation link is nestable. This means that a simple up/down only interaction is pretty confusing since we can't guess the user's intent on if they mean to swap a sibling or nest it. |
Empty parent blocks✅ Here's what it looks like to drag left/right. empty.group.mp4Nested structures where there's no sibling at the same level
✅ This one is possible using two drags. @talldan were you thinking of this being possible with just one? Drag down + right to nest, then up/down on sibling to reorder.
✅ This one is covered with a simple up/down drag. ordering.mp4Navigation Example:Navigation is interesting since each link is nestable: nav.example.mp4 |
|
I think folks like the interaction scheme, some other things to work on next:
|
|
Re: performance, one idea I might try is instead of modifying a cloned tree as we drag, we can see what happens if we update state to let child items add 36px padding or not to simulate where we intend to drop an item |
|
Took this for a spin. This is really very nice:
There are some small things that I imagine you're aware of since the PR is in draft, for example when first clicking and holding to move, it didn't take. It'd be nice to also use the grabby hand cursor as drag indicator (made a codepen here). Also need to tweak the interaction to avoid the horizontal scrollbar. The moving left/right to drop in a nested context is generally feeling valid here. But it feels slightly less obvious since any item is able to move freely in all directions, matching that of the cursor. The alternative would be to restrict vertical movement to the current nesting level, kind of like how Google Keep does with lists:
I know we're a bit limited in emulating the full behavior on display here, I don't even think we want it with splitting and such. But just the restricting of the vertical axis might make the indent/outdent behavior more obvious. What do you think? |
|
For folks 👀. As a heads up performance is a blocker for the current prototype. I'm working on seeing if I can retain this interaction behavior, but avoid needing to swap items in place as we drag. The other swap-in-place drag demos are toy examples with like 5 items, so this isn't unexpected that I need to throw out parts of this branch in favor of another implementation approach. I'm reserving some of the finer grain details for styling/polish for after that's resolved.
@jasmussen 💯 that makes sense! Did you link to the right codepen? I'm seeing lorem ipsum text without any cursor styling. Also, do we have any thoughts as to whether to give this a more consistent UI drag hint, like the 6 dots?
So, constraining y-movement definitely makes sense in certain contexts (for example block children that aren't insertable in the root document like Button), but the wrinkle here is that our block relationships are more complex. I assume in Google keep all items are nestable under each other? Columns is a good one example. Here, a Columns block may only insert a Column block. A Column block may insert a lot of content blocks. Usually a user might want to move content like a paragraph from one column to another, or move it out of the columns into the root document. In this case constraining to the container is probably too restrictive. Likewise, I ruled out a pure left/right movement, since it leads to unpredictable behavior. For example if we pull left for this paragraph, did we mean to insert it above the Columns block, below it? Or something else altogether?
|
Ack, I had made a typo in the comment, so it broke. Fixed it, try again.
Yes, I think we should start without any such interface as the drag and drop is purely additive. And if the grabby hand isn't sufficient (which I suspect it will be), one idea is to replace the block icon with the 6 dot grab handle on hover.
Yep, it's definitely simpler in Keep where you only have a single nesting level and nesting is more of a visual "indentation" thing than actual nesting.
That is indeed the wrinkle. I picture in your example, pulling left wouldn't do anything at all — pulling left would only work if it was the last block in the column block, and then it would land below the columns block. And to be clear, I'm not entirely sure it would be a good experience — so if it turns out to be onerous to explore, we'll think of something — it's mostly an instinct that could live or die by prototype. |
👍 I think that aligns well with the idea for this interaction style. Mainly that someone will naturally try to move an item to align the icon under the parent they want. So this might be more around relaxing the requirements for making sure that some amount of y-movement has happened, since it's valid for certain cases. I'm curious if we do need to think of providing some hint of feedback when we detect a gesture (say let's move to another parent, but let folks know it's an invalid move). |
The padding approach wasn't as promising as I would like. Going to fire up the profiler, and 🔪 going through some obvious things. Already spotted |
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
2c3b2b7 to
685c224
Compare
|
I think I've gotten down when to switch back to global state correctly. (We should no longer see items flying back on drop/to the new item). Typing should also reflect block changes in the sidebar. fix.state.swapping.mp4 |
I'm always a fan of starting with the least we can. Not just to keep PRs as small as they can be, or even just to ship these sweet improvements sooner, but also in case we find some of the proposed changes unnecessary in practical use. However if we do find the need to give feedback to "droppable areas", I do think we could use the blue line to indicate spaces where the item will end up, just like we do in the canvas. By virtue of that blue line not showing up as a direct descendant of the Columns (plural) block — unless of course you're dragging a Column block (singular), can hopefully help imply what's up. |
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
7533add to
1878d4a
Compare
A pretty natural approach for this component would be windowing (eg only render the items we see) but we haven't used it yet since there are some accessibility concerns with that approach. (Can't search all items / not a good way of knowing when more results are available with a screen reader). I did try making a smarter item block update when dragging in 7efb06f. Try |
If folks think it's good enough for most posts we can maybe leave some of the performance tuning for follow ups as well. Any thoughts here @draganescu @talldan @mtias ? |
|
Thanks, I'm taking it through the wringer with this one: I can't decide if that experimental commit is better or worse than the one I tested yesterday. It feels like starting and ending drag operations is perhaps slightly more stuttery, but the actual drag operation itself is a bit faster. In both cases I personally think it's worth it.
These are issues when the list is chunked up in its resting state, correct? Could the chunking happen only when you're actually dragging? This is definitely beyond my skill to fully comprehend so forgive me if this is off base. |
There was a problem hiding this comment.
First of all, super kudos to you for taking this on. It's very complex and it's going to be a amazing improving once it's done.
I gave this a spin on my older Mac (2.9 GHz 6-Core Intel Core i9 from 2018).
Unfortunately the experience was less smooth than I'd hope even without CPU throttling. As you should be able to see from the video below I noticed:
- a "jankyness" when moving the dragged item over another item boundary.
- a noticeable delay in "committing" the drag change on mouseup.
Screen.Capture.on.2021-09-24.at.09-51-29.mov
This is on a Macbook. Many people will have far less powerful machines even today.
When I tried with CPU throttling it became unusable for me and crashed my browser.
My gut is that we need more perf tuning here.
I did a quick profile and to me it looks like we're having the entire tree of [sidebar] "block" nodes re-render even though only a single block is being dragged. Am I right?
I notice that each <ListViewBlock /> re-renders due to props changes:
draggingIdchanges on each render. Why is that? Is it not the ID of the block being dragged. Therefore it seems to be ripe for memoization ofListViewBlockviaReact.memo(). We can check (using the comparator 2nd argument) whether that ID really changes and if it doesn't we can skip the render for that "block".- Some of the changing props are new function references (eg:
onClick,dragStart...etc) on each render. Could these be memoized viauseCallbackto ensure unchanging function refs? Currently I think that would be hard because it's within amapbut with some restructuring perhaps we could get there?
You've probably considered all this but I thought I'd mention it.
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
2752f76 to
c8b101a
Compare
Do you have a scenario where you see this? It should only happen on the initial drag and drop. (it changes from null to the clientId, and back to null on drop). CleanShot.2021-09-24.at.09.53.55.mp4 |
|
With how much overhead each Framer Item has we may need to look into windowing next, where we limit the number of items we render (typically a little more than what's visible). For every person there's some point where rendering >X list items will cause performance issues. Items in list view are fixed height which lends itself well to the problem. The downsides to this is additional complexity and unsolved a11y challenges. (How do screenreaders know that there are items next)? |
@jasmussen That's an interesting thought to virtualize only on drag. I'll keep that in mind.
I also did a quick experiment with only setting the framer props on items ~20 away from the dragged item. It does help a bit during drag, but it can be unstable in some cases, so we may want to avoid this.
|
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
c8b101a to
243b90f
Compare
|
I have some early work for using windowing in #35230. I applied it to this branch in d5ace41 So as a proof of concept this helps quite a bit in terms of performance. The following video is of the performance test post that has more than 900 blocks. However the technique here also makes some of framer's constraint system work against us and I'm seeing some elements fly in as we scroll. This suggests that I might need to turn off framer props while scrolling. Not a blocker, but definitely more to work through if we decide to use the windowing technique. CleanShot.2021-10-04.at.09.12.57.mp4 |
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
243b90f to
d5ace41
Compare
…ctor call to getDraggedBlockClientIds
gwwar
force-pushed
the
try/list-view-drag-motion
branch
from
d5ace41 to
993b2ec
Compare
This PR explores alternative drag interactions using motion framer. Instead of using a drag chip, we instead allow items to drag in place. Follow up to: #33765
This PR is pretty large in size since I wasn't able to find any smaller chunks to safely land. If folks spot pieces we can break off, I'd be happy to break this into additional PRs.
Dragging.mp4
Testing Instructions
Verify in each environment that:
Performance
Using motion components comes with a performance cost. I did my best here to memo key items while dragging and avoid unneeded renders. When adding new blocks or removing blocks, we may see additional lag for updating items in list view. Note that some odd laggy behavior is also present in trunk, but render costs may be more pronounced with this change. One thing to consider would be list virtualization, though I do understand that there are a11y concerns to work with. I think it'd be worth investigating in a follow up.
UX interactions
For this first pass, folks have noted that we would like to see:
Do let us know if any interactions don't feel quite right. Dragging so far works roughly like this:
Drag And Drop E2E
Puppeteer recently added some drag and drop E2E support, but I was unable to get this working with this drag implementation. (It currently selects text across elements instead of initiating a drag) @talldan let me know if you have any other tips. If we're not able to reasonably fix this, I'd recommend dropping the test.