mcFrax / omicron-dnd

Fast JavaScript drag-and-drop library for desktop and mobile browsers.
MIT License
4 stars 0 forks source link

FloatEl disappeared #11

Closed jan-warchol closed 2 years ago

jan-warchol commented 2 years ago

Describe the bug Something crashed when I was dragging tasks in organizer - not sure why, but floatEl disappeared (or didn't render at all, I'm not sure). I found this in console:

Uncaught TypeError: Cannot read properties of null (reading 'appendChild')
    at Xi (omicron-dnd.js:1367:10)
    at Li (omicron-dnd.js:563:5)

which points to:

    // Note: that may be before or after floatEl. Maybe that is the problem?
    toEl.appendChild(placeholderEl);
mcFrax commented 2 years ago

That is a very interesting note, indeed. 🤔 The comment is no longer relevant though, as floatEl is created on the body instead of in the container. Anyway, that doesn't really matter.

Oh, I know what the race is: we get leaveContainer in preDrag state. This is because anyState_container_PointerLeave doesn't differentiate between states, really. startDrag assumes that toEl is set, but it can be null at that point. Perhaps it can also be a culprit for this broken newIndex in #10, though that is not very likely.

This can happen on desktop, where we tolerate some moves during preDrag. On mobile, where no preDrag is allowed, even if we get pointerLeave before pointerMove, the pointerMove will come too, and cancel the drag, so the issue would be mitigated.

The "proper" solution seems at first to be to start in a state where toEl is unset, and then call synthetic updateOnMove to enter the fromEl container. However, there are a few catches:

Given the above, it seems that some dedicated solution would be preferred, and it should either just allow to skip entering the fromEl in case the pointer moved outside during the preDrag, or enforce entering fromEl anyway.

mcFrax commented 2 years ago

Alternative, simpler solution (that hopefully works without weird issues): delay the releasePointerCapture call until dragStart. This way we don't get any pointerleave or pointerenter during preDrag, which mitigates the issue. In the worst case, we will get a whole package of events after the release, but that we can handle already.

jan-warchol commented 2 years ago

Okay. I'll let you know if I notice anything wrong.