Hudson/drag n drop cleanup #116
Conversation
| const onGlobalPointerUp = (e: PointerEvent) => { | ||
| setTimeout(() => { | ||
| log.debug('Compositor: DragEnd', e) | ||
| activePointerCoords.current = undefined | ||
| isDraggingSelf.current = false | ||
| setDraggingNodeId(null) | ||
|
|
||
| dragPreviewEl.toggleAttribute('data-active', false) | ||
| wrapperEl.toggleAttribute('data-dragging', false) | ||
| wrapperEl.toggleAttribute('data-drop-target-ready', false) | ||
| wrapperEl.querySelectorAll('[data-item]').forEach((x) => { | ||
| x.toggleAttribute('data-drag-target-active', false) | ||
| x.toggleAttribute('data-layout-drop-target-active', false) | ||
| x.toggleAttribute('data-transform-drop-target-active', false) | ||
| x.toggleAttribute('data-transform-drop-self-active', false) | ||
| }) | ||
| }) | ||
|
|
||
| document.removeEventListener('pointermove', onGlobalPointerMove) | ||
| document.removeEventListener('pointerup', onGlobalPointerUp) | ||
| } | ||
|
|
||
| document.addEventListener('pointermove', onGlobalPointerMove) | ||
| document.addEventListener('pointerup', onGlobalPointerUp) |
There was a problem hiding this comment.
This is just something to consider, but I’m concerned we might risk running into memory leaks if we don’t wrap the layout and transform drag handlers in memoization. I know it’s tricky, but it could also make it easier to track changes across render cycles. Right now, it seems harder to know what lingering references we might not be cleaning up, which could lead to a memory leak. Again, this is totally up to you; I’m not enforcing anything.
There was a problem hiding this comment.
I totally agree with your concern. truth is, there's so much inside of this file that I'd like to rework, I forced myself to keep everything as much the same as I could for this PR, since it works and is tested.
Currently it's a pretty haphazard combination of React vs native JS, so I am particularly worried about unexpected behavior if we begin to rely on the actual React lifecycle (I've had to lift some variables out of the component scope as a workaround, so component renders are not firing reliably).
I'll keep a close eye on performance and make a ticket for optimizations if needed. Regardless, I think we need to plan for some cleanup effort the next time changes are needed in this file.
Convert drag events to pointer events. This leaves the existing behavior feeling much the same, except we now have control over the cursor iconography. The cursor maintains the same "hand" style throughout the lifecycle of a drag.
Also, updates the drag preview to disappear when hovering over an eligible drop target.
Gif demonstration:
https://xsolla.atlassian.net/browse/LSTREAM-473