Skip to content
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

Transitions not working in combination with drag and drop #96

Open
EmielH opened this issue Aug 5, 2020 · 16 comments
Open

Transitions not working in combination with drag and drop #96

EmielH opened this issue Aug 5, 2020 · 16 comments
Assignees
Labels
bug with a workaround available This a bug but there is a reasonable workaround for it

Comments

@EmielH
Copy link

EmielH commented Aug 5, 2020

When you have a draggable list that has transitions, as soon as you drag an item, it is removed from the list. You can still drag it around, but it's impossible to get it back into the list. I'd like to have transitions for adding items to and deleting items from the draggable list.

How to reproduce

Go to the Svelte REPL and enter the following code:

<style>
	div {
		height: 1.5em;
		width: 10em;
		text-align: center;
		border: 1px solid black;
		margin: 0.2em;
		padding: 0.3em;
	}
</style>
<script>
	import {dndzone} from 'svelte-dnd-action';
	import {fade} from 'svelte/transition';
	function handleSort(e) {
		items = e.detail.items;
	}
	let items = [
		{id:1, title: 'I'},
		{id:2, title: 'Am'},
		{id:3, title: 'Yoda'}
	];
</script>
<section use:dndzone={{items}} on:consider={handleSort} on:finalize={handleSort}>
	{#each items as item(item.id)}
		<div transition:fade>
			{item.title}	
		</div>
	{/each}
</section>

(Code based on this article.)

See that the item disappears from the list as soon as you start dragging and that it is impossible to put it back in the list.

When starting to drag, this error appears in the console:

action.js:360 Uncaught (in promise) TypeError: Cannot read property 'hasOwnProperty' of undefined
    at configure (action.js:360)
    at update (action.js:380)
    at Object.update [as p] (PlayerList.svelte:13)
    at update (index.mjs:730)
    at flush (index.mjs:699)

(The error is from my personal project, not from the REPL, because the REPL console doesn't show the source files and line numbers as clearly.)

When you remove transition:fade from the item div, everything is working perfectly again.

My (totally uneducated) guess is that the transition removes the element from the DOM, while svelte-dnd-action does not know about this. Could this be the case? Is it possible to make this work?

@isaacHagoel
Copy link
Owner

Hi,
TLDR: here is a solution. enjoy!

When you add transition fade (or any other), you tell svelte to postpone the removal of the element from the DOM until the transition is done and then remove it. This might be fine when deleting an item but you don't want it to happen when the item is dragged out (because it should continue to exist). Unfortunately, Svelte doesn't let us do the transition conditionally.
One solution is to implement the transition ourselves and apply it only when appropriate.
Notice that in the example (see REPL above) I prevent any dragging or dropping until the transition is complete to prevent any unpredictable outcomes.

@EmielH
Copy link
Author

EmielH commented Aug 6, 2020

Thanks so much for your fast reply and your help! I'm going to check this out and play with it for a bit, it looks very useful. I did notice I can break it by clicking the add button twice in quick succession, but that's probably something that can be fixed.

Does svelte-dnd-action actually remove the item from the list while it's being dragged?

@isaacHagoel
Copy link
Owner

isaacHagoel commented Aug 6, 2020

Hi,
Nice catch. When doing an example implementation it is always tempting to simplify and cut corners :/
I fixed the REPL to support quick add and remove operations. It can be done more neatly but at least now the example is behaving correctly :)

Technically speaking, svelte-dnd-action does not remove items from the list, your consider and finalize handlers do upon its request.
The library passes an updated list of items to the handlers (with less, more or the same items but in different order) in the event objects and depends on your component doing the actual update.

Two more thoughts:

  1. I think only only svelte's fade-out causes the conflict. fade-in should look very strange when items are dragged into a container but should not break. I haven't tried it, just a thought.
  2. Even if Svelte didn't conflict with the library somehow, I don't think it is good UX to have the items fade in and out during drag operations and I assume that wasn't the intended outcome when u added the transition (as in, u only wanted it to apply to add/remove operations). Is my assumption correct?

@EmielH
Copy link
Author

EmielH commented Aug 7, 2020

Thanks for the clarification about what svelte-dnd-action does exactly. This is how I thought it would work, but the behaviour with transitions confused me. Since svelte-dnd-action does not actually remove items from the list, and my consider and finalize handlers don't do this either, I'm a bit surprised that the transition is triggered at all.

My use case (which answers your thought 2) is a list that the user can add items to, using an input field, and remove items from, using a delete button, and sort by drag and drop. Items will not enter or leave the list through drag and drop. Am I correct in assuming that the transition should not be triggered in this case?

I agree completely that items fading in and out during drag operations would be very weird, since the user is "holding" the item while dragging.

@isaacHagoel
Copy link
Owner

Thanks to digging in this I did find a bug in the library/ svelte (#101) though it is not what's causing the incompatibility with fade.

To your question, open the console in this REPL. Notice that the handler is fired on drag start and removes the dragged element from the list of items.
When it realised it stayed in the territory of the list it fires another event adding the shadow item to the list.
Is there anything further you think needs to be done here or can I close this issue?

@EmielH
Copy link
Author

EmielH commented Aug 8, 2020

Hi, thanks so much for the time you're spending on this issue. 🙂 The library is really useful and I'm not trying to nitpick, but I do think this is something that can be improved and I'm willing to help out if I can!

I believe that removing the item from the list on drag start (which basically assumes that every item that is dragged will leave the territory) is not entirely correct. It causes unintentional state changes even when not using transitions. One example that illustrates this clearly is when dragging the last item of a list:

I first made your REPL work correctly again by removing the transition. Then, when you start to drag Yoda (the last item in the list), you can see that the dropzone actually becomes smaller. This doesn't happen when dragging any of the other items.

When letting go of Yoda on its original position, it actually is outside of the dropzone. Because of this, svelte-dnd-action reverts all changes and replaces the original list. This does provide the intended behaviour but it feels to me like this is sort of by accident. It would be nice if the dropzone would only change size after the item is actually dragged away from the original list. This may also solve the transition problem.

I tried to solve this naively by removing the splice from row 317, but this just broke everything. 😅 I think I'm missing some other moving parts when removing just this line.

// removing the original element by removing its data entry
items.splice(currentIdx, 1);
dispatchConsiderEvent(originDropZone, items);

I would like to tinker around a bit to see if I can make this work, but I'm also curious about your thoughts.

@isaacHagoel
Copy link
Owner

Hi,
Thanks for wanting to improve things. I am all for that. Here are some of my thoughts:
When implementing the library I have considered which behaviour is better when an item leaves the list: leaving a placeholder there or letting the list shrink. I decided on the later because the first one gets really weird as u are moving the item on top of multiple lists before dropping. The fact that it happens with the last item is purely because of the list shape (a simple vertical list in this case) and the fact that it has no size of its own and no significant padding.

The item is removed from the list when drag starts and then adds itself to the list it is over if any (which may or may not be the original list (depending on the layout of the page, the drag speed etc). Even if it won't be removed it still needs to be replaced with the shadow item (placeholder) which might be immediately removed if the item is not over the list. In other words, the same number of mutations or more.

Svelte's built in fade out will not play nice no matter what as far as I can tell.

I am willing to be convinced otherwise on any of the points and if you'd like to experiment you're more than welcome.
I suggest reading action.js first and trying to get a feel for the different events ('drag entered', 'dragged left' etc.).

@kyythane
Copy link

Hello 👋

I seem to be running into a similar issue, replicated in this REPL. It seems that a fade out causes the same breakage if nested anywhere inside the element being dragged (this example is only 1 level deep, but I have seen the same issue nested 4 levels in).

@isaacHagoel
Copy link
Owner

Hi @kyythane
Unfortunately svelte transitions don't play nice with this library (and afaik they don't play well with svelte animations either).
It doesn't seem like there is much I can do about it (will be happy to be proven wrong about this).
Luckily, transitions are usually very easy to implement without the help of svelte (much easier than implementing drag and drop :)).
I have fixed your REPL with a very simple workaround. Please see here.

@EmielH
Copy link
Author

EmielH commented Aug 23, 2020

Unfortunately I haven't had time to look into this yet.

Transitions and animations can play well together. Take a look at this REPL. Type something in the input and press enter. It uses both a transition and an animation when adding an item.

But as I said, unfortunately I haven't had the time yet to investigate how to make that work with this library as well, if that is at all possible.

@isaacHagoel
Copy link
Owner

isaacHagoel commented Aug 23, 2020

@EmielH it gets hairy when state changes are involved. I was referring mainly to this:
sveltejs/svelte#4999
see this tweetstorm which is also linked from the PR for more details

@isaacHagoel isaacHagoel added the bug with a workaround available This a bug but there is a reasonable workaround for it label Sep 16, 2020
@SarcevicAntonio
Copy link

This is what blocks using crossfade, right?

@odinhb
Copy link

odinhb commented Nov 18, 2022

So I just ran into this issue when trying to use crossfade, and have been experimenting w/ the suggested workarounds in the svelte REPL.

Problem is, unlike the fade transitions, I can't seem to get the workaround to... work.

I'm just gonna leave this here: REPL, and resort to removing the crossfade transition.

My REPL showcases the neccessity of the crossfade transition, which is that non-drag-n-drop changes to the data can be pretty jarring without crossfade.

@dangodai
Copy link

dangodai commented Feb 5, 2025

Any workaround for crossfade in the last 2+ years, or is it just not compatible with this library? There is a small note about transitions in the documentation, but IMO (since I just spent 90 minutes figuring out the issue) a more explicit callout is warranted.

My use-case: Imagine a kanban board that updates live as other users make changes. Not very understandable without crossfade.

@isaacHagoel
Copy link
Owner

isaacHagoel commented Feb 5, 2025 via email

@dangodai
Copy link

dangodai commented Feb 5, 2025

I was able to wrap both the in and out (send and receive from crossfade) transitions in conditional functions that only run if the data source change was remote, and not local (i.e. not from svelte-dnd-action). Wrapping just the out/send transition didn't seem to be enough to fix the errors, so I did both transitions. Something along the lines of this:

let boardOrigin: 'local' | 'remote' = 'remote';

const conditionalSend = (node: any, params: CrossfadeParams & { key: any }): () => TransitionConfig => {
	if (boardOrigin === 'remote') {
		return send(node, params);
	}

	return () => { return {} };
};

There are still 2 remaining issues I can see to deal with

  1. Disabling new DnD events while a transition is running (should be easy enough)
  2. Handling remote data source updates while mid-DnD. I think the easiest option would be to simply cancel the current dragging (and don't run finalize), but I'm not sure if that is currently a possibility with this library? Any thoughts?

Edit: Actually the second issue isn't really related to transitions. I'll see if this topic of updating the data source mid-drag has been discussed elsewhere.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug with a workaround available This a bug but there is a reasonable workaround for it
Projects
None yet
Development

No branches or pull requests

6 participants