reppners / ngx-drag-drop

Angular directives using the native HTML Drag And Drop API
https://reppners.github.io/ngx-drag-drop/
BSD 3-Clause "New" or "Revised" License
300 stars 119 forks source link

Typed demo, move up within Fruits list doesn't work #17

Closed mtraynham closed 2 years ago

mtraynham commented 6 years ago

I think this is an issue with primitives and move operations within their own list. The order of operations is as follows:

The state essentially looks unchanged. I looked into the order of drag/drop events, and typically dragend is called after drop. Because the item dragged is a primitive, the indexOf would resolve to the newly dropped item.

This can be fixed by instead wrapping the primitive in a simple object, or by handling all functionality of moving inside the drop callback.

I also tried using a let i = index inside the angular template, and performing onDragged(index: number), but Angular resolves that to the newly dropped index, thus removing the newly dropped item.

reppners commented 6 years ago

Good catch! The demo component needs to remember the dragged item index on dragstart to eliminate the need for the indexOf operation on dragend.

mtraynham commented 6 years ago

That sounds like a good solution, but would you want to make that part of the ngx-drag-drop API? I know currently, you invoke the dndMoved/dndCopied/dndLinked functions with only the DragEvent, but those could be extended to include the initial index. It seems very useful in almost all cases to provide the initial index of the item, instead of looking up the item in the parent array for removal. Not entirely sure how that index would be calculated though, or if it would have to be provided by the user as an @Input.

reppners commented 6 years ago

I've also thought about it but didn't want to do any DOM queries or "guessing" what the index on dragstart is inside the dndDraggable directive.

So my suggestion would be

mtraynham commented 6 years ago

I can probably put that together, thanks for the feedback!

reppners commented 2 years ago

I've updated the demo to handle the case by using object identity. This breaks when using trackBy because of the natural order of drag and drop events but showcases how reordering items within a list can be done.