Alfred-Skyblue / vue-draggable-plus

Universal Drag-and-Drop Component Supporting both Vue 3 and Vue 2
https://vue-draggable-plus.pages.dev/en/
MIT License
2.72k stars 106 forks source link

Remove DOM manipulation and set list in onAdd and onRemove #78

Closed MartinMalinda closed 6 months ago

MartinMalinda commented 6 months ago

Should close https://github.com/Alfred-Skyblue/vue-draggable-plus/issues/77

Maybe I've removed too much stuff? I am not sure what are the checks for isRef? Isn't it safe to assume the list will always be a ref ? Should the lib work with non reactive arrays (why?)?

Alfred-Skyblue commented 6 months ago

If that's the case, we are changing the reference to the original array. Essentially, we are performing operations like adding, deleting, or moving elements within the original array. If we forcefully change the reference to the array, it could potentially lead to unnecessary issues, such as watch deep.

MartinMalinda commented 6 months ago

I'm very frequently doing list.value = [...newList.value] without issues. This triggers the setter on the ref and reactivity works just fine. I think what's important is not to break the child objects reactivity by cloning.

Besides, the code is already changing the reference to the original array on the most common operation, changing order: https://github.com/Alfred-Skyblue/vue-draggable-plus/blob/main/src/useDraggable.ts#L175

Alfred-Skyblue commented 6 months ago

I am not sure what are the checks for isRef? Isn't it safe to assume the list will always be a ref ? Should the lib work with non reactive arrays (why?)?

Because ref is unpacked in the directive, isRef is used in useDraggable

But at least with this approach we rely on native Vue rerender instead of DOM manipulation 🤔

Manual manipulation of the DOM is necessary to trigger the animation effect.

MartinMalinda commented 6 months ago

Thanks for the reply, I'll close this and proceed with forking.