fix(visx-drag): reset drag state on pointercancel - #2030
Open
ankit1324 wants to merge 1 commit into
Open
Conversation
useDrag only exposed dragStart/dragMove/dragEnd, wired to pointerdown/ pointermove/pointerup. The browser fires pointercancel instead of pointerup when it takes over an in-progress gesture, most commonly on touch when scroll arbitration wins, so dragEnd never ran: isDragging stayed true and dx/dy stayed stale until a whole new drag completed. Add a dragCancel handler that ends the drag, wire it to onPointerCancel in Drag's capture rect, and add an optional onDragCancel callback that falls back to onDragEnd so existing consumers still get an end-of-drag signal. Wire the same handler through visx-brush, whose overlay, selection, handles and corners each bind dragEnd to pointerup and were left in the same stuck state. Closes airbnb#2029
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2029
Problem
useDragexposes onlydragStart/dragMove/dragEnd, wired toonPointerDown/onPointerMove/onPointerUp. Per the Pointer Events spec the browser firespointercancelinstead ofpointerupwhen it can no longer generate events for an in-progress pointer — most commonly on touch, when the browser's own gesture arbitration decides the drag is really a page scroll.When that happens
dragEndnever runs, soisDraggingstaystrueanddx/dystay stale until the user starts and completes a whole new drag. NoonDragEndfires either, so consumers holding their own "currently dragging" state stay stuck too.Changes
useDrag: newdragCancelhandler that ends the drag, plus an optionalonDragCancelcallback. WhenonDragCancelisn't provided it falls back toonDragEnd, so existing consumers keep getting an end-of-drag signal and nothing changes for them.Drag: wiresonPointerCancelon thecaptureDragArearect.visx-brush:BaseBrush,BrushSelection,BrushHandleandBrushCornereach binddragEndtoonPointerUpon their own elements, so they were left in the same stuck state that the issue calls out forBaseBrush. Each now also bindsdragCanceltoonPointerCancel, andBrushOverlaygained the matching prop (it spreads ontoBar, so no other change was needed).This mirrors the fix
@vueuse/coremade for the identical gap inuseDraggable(vueuse/vueuse#5550), referenced in the issue.Tests
Two new cases in
packages/visx-drag/test/useDrag.test.tsx:isDragging(the reported bug);onDragCancelis invoked on cancel, and consumers that only passonDragEndstill get called.Both are genuine regression tests — with the cancel path removed they fail with
expected true to be false(the stuckisDragging) andexpected "vi.fn()" to be called 1 times, but got 0 times.yarn vitest run packages/visx-drag packages/visx-brush→ 6 files, 15 tests passing.yarn build(babel +tsc --build) clean, and lint on both packages is back to the repo's pre-existing baseline.