fix(dragListener): prevent spurious DRAG_UPDATE after drag ends#290
Open
fix(dragListener): prevent spurious DRAG_UPDATE after drag ends#290
Conversation
`handleCameraChange` was emitting DRAG_UPDATE during drag-end cleanup because `finished` was set *after* `mouseupBinded` and `cleanupDragState`. `cleanupDragState` (and, previously, custom `onDrop` callbacks) calls `disableAutoPanning`, which synchronously fires `camera-change`, which `handleCameraChange` intercepts — still seeing `finished=false` — and re-emits DRAG_UPDATE on the still-live emitter. The original crash was `startDragCoords.length` (TypeError on undefined) when the spurious DRAG_UPDATE arrived after `startDragCoords` had been cleared in `onEnd`. That was patched with `?.`, but the root cause remained. Fix: set `finished = true` first in all three end-of-drag handlers (mouseup, mouseleave, cancel) so `handleCameraChange` sees the flag and skips the emit immediately. Also replace `\!startCoords?.length` with `\!startCoords || \!prevCoords` in GraphComponent.onDrag — explicit null-guard on both coordinate variables instead of using `.length` as a truthiness proxy. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures drag end is marked as finished before any cleanup so DRAG_UPDATE emissions are suppressed after drag termination, and tightens the drag guard condition in GraphComponent to avoid accessing unset coordinates. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The three end-of-drag handlers now share the same
finished = true;/started/cleanupDragState/cleanupsequence; consider extracting this into a small helper to ensure the ordering stays consistent if it needs to change again. - In
GraphComponent.onDragtheif (!startCoords || !prevCoords) return;guard now silently drops updates; if this path is truly unexpected, consider logging or asserting so future regressions are easier to detect rather than failing quietly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The three end-of-drag handlers now share the same `finished = true;` / `started` / `cleanupDragState` / `cleanup` sequence; consider extracting this into a small helper to ensure the ordering stays consistent if it needs to change again.
- In `GraphComponent.onDrag` the `if (!startCoords || !prevCoords) return;` guard now silently drops updates; if this path is truly unexpected, consider logging or asserting so future regressions are easier to detect rather than failing quietly.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Preview is ready. |
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
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.
Summary
finished = truebefore mouseupBinded/cleanupDragState in all three end-of-drag handlers (mouseup, mouseleave, cancel) in dragListener.ts!startCoords?.lengthwith!startCoords || !prevCoordsin GraphComponent.onDragRoot Cause
handleCameraChange guards on
started && !finished && lastMouseEventbefore emitting DRAG_UPDATE. finished was assigned after cleanup calls, leaving a window where it was still false. The original crash was startDragCoords.length (TypeError on undefined) — patched in #194 with optional chaining, but the underlying issue remained.Fix
Move
finished = trueto the top of each end-of-drag handler so handleCameraChange sees the flag and skips the DRAG_UPDATE emit entirely.Test plan
Summary by Sourcery
Ensure drag interactions are correctly finalized to avoid spurious drag updates after a drag ends.
Bug Fixes: