search

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
299 stars 118 forks source link

DndDropEvent.index may be -1 #40

Closed sebek64 closed 5 years ago

sebek64 commented 5 years ago

Describe the bug When drag and drop is done rapidly, DndDropEvent.index is -1.

To Reproduce https://ngx-drag-drop-issue-template-hja8ga.stackblitz.io

Steps to reproduce the behavior: Try to rapidly drag an element. Sometimes, console shows

dropped {
  "event": {
    "isTrusted": true
  },
  "dropEffect": "move",
  "isExternal": false,
  "data": "myDragData",
  "index": -1
}

Expected behavior We should never get index === -1 (or if so, it should be clearly documented).

Desktop (please complete the following information):

Additional context The issue is present in 2.0.0-rc.2 and probably also in 1.0.4.

reppners commented 5 years ago

Thanks for the detailed report! Looking into it 🤔

reppners commented 5 years ago

So.. how rapid do I need to perform the drag and drop for this to happen? I wasn't able to reproduce on my machine though I'm quite sure this can happen if inserting the placeholder takes too long for whatever reason.

There's two options:

  1. If the placeholder is not inserted do not propagate the drop event.
  2. Propagate with index === -1 and document that it can happen.

I think I'd prefer option 1 but I'm open to suggestions and feedback.

sebek64 commented 5 years ago

Well, I'm not sure how rapid the drag must be. I've tried as fast as I can, and for a fresh Chrome instance, I was able to hit it on the fourth attempt on an AMD Ryzen 5.

My current workaround for this issue is to drag "to the end of the list". I think the solution 1 might be a bit better, since it doesn't confuse any new users of the library. It doesn't support my current workaround, but it is not a problem, at least for me.

reppners commented 5 years ago

Not yet released. I'll ping in this issue when its contained in next release.