Skip to content

fix(sidebar): stop bubbled dragleave events cancelling an in-progress drag - #6679

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/sidebar-drag-drop-cleanup
Aug 13, 2026
Merged

fix(sidebar): stop bubbled dragleave events cancelling an in-progress drag#6679
waleedlatif1 merged 3 commits into
stagingfrom
fix/sidebar-drag-drop-cleanup

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Fix drag-and-drop in the workflow sidebar silently doing nothing, which showed up most often when a folder was expanded
  • Close folders that a drag spring-opened on its way past, instead of leaving them open
  • Surface reorder failures instead of swallowing them, and report partial failures distinctly

The drag bug

dragleave bubbles, so a container-level listener fires for every descendant the pointer leaves — and Chrome reports relatedTarget as null on dragleave (Firefox populates it). Two places read "no related node" as "the drag left the list" and cleared the drop indicator; handleDrop bails on a null indicator, so releasing just after an internal boundary crossing did nothing at all.

Rows nested inside an expanded folder sit behind three extra boundaries (content zone → spacing div → row wrapper), so an expanded folder made it dramatically more likely — which is why it read as "open folders break dragging".

Both sites (the container listener and isLeavingElement, which backs the root drop zone's onDragLeave) now go through one predicate that falls back to hit-testing the pointer against the element box when relatedTarget is null.

Spring-open revert

Hovering a collapsed folder mid-drag opens it after 400ms, but nothing ever closed it again, so dragging past a folder left it permanently expanded. Folders opened this way are now tracked and closed on drag end, keeping the drop destination and its ancestors open. Runs on drop, Esc-cancel, and release-outside alike.

Reorder failures

Failures were logger.error only, so a failed reorder looked like the sidebar spontaneously undoing the move. Now a toast. Promise.all also became Promise.allSettled: with all, one rejection abandoned the sibling request while it was still in flight and committing anyway, so the caller could not distinguish a total failure from a half-applied one.

This does not make the two reorder writes atomic — folders and workflows share one sortOrder index space but commit through separate endpoints. The proper fix is a single application operation writing both in one transaction, which needs a new contract, a migration of two legacy raw routes, and a merged optimistic-update handler. Filed separately rather than bundled here.

Cleanup carried along

  • Deleted a dead sibling cache: both its read and write were gated on !isDraggingRef.current, so it was written only at drop time and cleared by the next dragstart before anything read it — a 100% miss rate on every path
  • Replaced a duplicated comparator with the existing exported compareByOrder from the sidebar's own utils
  • Hoisted five closure-free callbacks to module scope, shortening the dependency chains that feed every row's handler factories
  • Destructured mutateAsync off the reorder mutations: the mutation objects take a new identity on each state transition, so depending on them re-created every row's handler object mid-drop
  • initDragOver now reads isDraggingRef rather than the state that lags it by a render, which also stabilises all six drop-zone factories for the whole drag

Type of Change

  • Bug fix

Testing

8 unit tests covering both dragleave paths, the spring-open revert (including "drop landed inside" and "user opened it themselves"), and the existing stranded-drag reset. Each fix was verified by reverting it individually and confirming exactly the matching test goes red.

Not verified in a browser — the diagnosis is from source plus tests reproducing the exact event shape.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 13, 2026 10:05pm

Request Review

@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches core sidebar reorder and drag state with multi-request reorder semantics still non-atomic; behavior is well-covered by new unit tests but not browser-verified per PR notes.

Overview
Fixes workflow sidebar drag-and-drop that silently did nothing after internal pointer crossings—most visible when folders were expanded. hasDragLeftElement now treats dragleave as a real exit only when the pointer is outside the element box, because Chrome’s null relatedTarget on bubbled internal boundaries used to clear the drop indicator and handleDrop would bail with no indicator.

Spring-open folders opened mid-drag (hover expand) are tracked and closed on drag end unless the drop landed in that folder or its ancestors; the hover timer is cleared synchronously on drag end to avoid a late expand racing the next drag.

Reorder commits use Promise.allSettled instead of all, throw ReorderFailedError for partial vs total failure, and show toasts (“Only some items moved” / “Failed to move items”) instead of only logging. Related cleanup: remove a dead sibling cache, hoist drag helpers, stable mutateAsync dependencies, and initDragOver reads isDraggingRef so auto-scroll and row handlers stay stable during a drag.

Adds unit tests for both dragleave paths, spring-open revert edge cases, and existing stranded-drag behavior.

Reviewed by Cursor Bugbot for commit 3250c57. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR hardens sidebar drag-and-drop lifecycle handling and makes reorder failures visible.

  • Distinguishes bubbled internal dragleave events from genuine list exits using pointer hit-testing when relatedTarget is unavailable.
  • Tracks and reverts folders spring-opened during a drag while preserving the destination path.
  • Waits for both reorder mutations, distinguishes partial failures, and displays an error toast.
  • Adds focused tests for dragleave handling, stranded drags, and spring-open cleanup.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.ts Centralizes dragleave containment, manages spring-open folder cleanup, and reports total versus partial reorder failures.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.test.tsx Adds regression coverage for internal and genuine dragleave events, drag cancellation, destination preservation, timer cleanup, and pre-expanded folders.

Sequence Diagram

sequenceDiagram
  participant User
  participant Sidebar
  participant FolderState
  participant ReorderAPI

  User->>Sidebar: Drag item across rows
  Sidebar->>Sidebar: Ignore internal dragleave boundaries
  User->>Sidebar: Hover collapsed folder
  Sidebar->>FolderState: Spring-open after delay
  User->>Sidebar: Drop or cancel
  alt Drop
    Sidebar->>Sidebar: Preserve destination and ancestors
    Sidebar->>ReorderAPI: Reorder folders and workflows
    ReorderAPI-->>Sidebar: Settled results
    Sidebar->>FolderState: Collapse other spring-opened folders
  else Cancel
    Sidebar->>FolderState: Collapse all spring-opened folders
  end
Loading

Reviews (2): Last reviewed commit: "fix(sidebar): disarm the spring-open tim..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 force-pushed the fix/sidebar-drag-drop-cleanup branch from bab5221 to 3250c57 Compare August 13, 2026 22:05
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3250c57. Configure here.

@waleedlatif1
waleedlatif1 merged commit 046302a into staging Aug 13, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/sidebar-drag-drop-cleanup branch August 13, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant