Skip to content

Fix sidebar workspace reorder by drag (regressed in #18837) - #20015

Merged
austinywang merged 2 commits into
mainfrom
fix-sidebar-workspace-reorder-drag
Oct 12, 2026
Merged

austinywang merged 2 commits into
mainfrom
fix-sidebar-workspace-reorder-drag

Conversation

@austinywang

@austinywang austinywang commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Dragging a workspace in the vertical sidebar no longer reordered it. Caused by #18837 (48ba209, "Drop a sidebar workspace onto a pane to merge its tabs").

Cause

#18837 added the Bonsplit tab drag type to every sidebar row drag, so pane drop targets could treat the row as a merge. AppKit sends a drag to the frontmost view that is registered for one of its types and contains the pointer. It does not consult hitTest (see CloudSurfaceDropGateView.swift). In SidebarWorkspaceTableContainerView, bonsplitDropView is a full-size view stacked above reorderDropView and registered for that Bonsplit type. After #18837 it received every row drag. BonsplitTabDragPayload.transfer returns nil for a sidebar reorder pasteboard (also #18837), so the overlay rejected the drag, and the reorder view beneath never saw it.

The other suspects don't touch this path. #18303 and #18693 change the Cloud tree outline, and #19593 is a Bonsplit submodule bump for tab-bar motion.

Fix

SidebarBonsplitTabWorkspaceDropView now forwards dragging callbacks for sidebar reorder drags (pasteboard has SidebarTabDragPayload's type) to the container's reorderDropView. Bonsplit tab drags from panes keep their existing handling. Merge-onto-pane from #18837 is unchanged: pane destinations still read the row's merge capability, and inside the sidebar the reorder wins.

Tests

SidebarWorkspaceReorderDropOverlayHitTestingTests.workspaceRowDragOverBonsplitOverlayStillReorders builds the real container and checks that the Bonsplit overlay is the frontmost registered destination for a row drag carrying both types. It then drives entered, updated, prepare, perform and conclude on that overlay and expects the reorder view's update and drop callbacks to fire. The test was committed before the fix (dc42c54). On that commit the overlay returns [] because the transfer is nil. Neither red nor green was run locally (local xcodebuild tests are not allowed), so hosted CI gives the green result.

Changelog

  • Fixed: Dragging a workspace in the sidebar reorders it again.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes dragging a workspace in the sidebar to reorder it, which regressed in #18837.

#18837 made every sidebar row drag carry the Bonsplit pane-merge type, so AppKit routed those drags to the frontmost registered destination—the full-size Bonsplit overlay—which rejected them, and the reorder view beneath never saw them. The Bonsplit overlay now forwards sidebar reorder drags to the reorder view, while dropping a row onto a pane still merges its tabs.

Written for commit 9610feb. Summary will update on new commits.

View guided diff Turn on auto-fix

austinywang and others added 2 commits October 12, 2026 14:33
Fails on main: since #18837 the row drag also carries the Bonsplit
pane-merge type, so AppKit routes it to the frontmost Bonsplit overlay,
which rejects it, and the reorder view beneath never sees the drag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#18837 exported the Bonsplit pane-merge capability on every sidebar row
drag. AppKit routes a drag to the frontmost view registered for one of its
types, regardless of hitTest, so the full-size Bonsplit tab overlay above
the reorder view took every row drag and rejected it. The overlay now
hands sidebar reorder drags to the reorder view beneath it; dropping a
row on a pane still merges its tabs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@austinywang
austinywang merged commit 016b572 into main Oct 12, 2026
17 of 19 checks passed
@austinywang
austinywang deleted the fix-sidebar-workspace-reorder-drag branch October 12, 2026 21:34
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 9610febfa7, merged 2026-10-12 21:34:07 UTC

  • Not verified at merge: backend migrations applied (not reported), ci-status (not reported), CI fast guards (in progress), plan (in progress)
  • Verified: Fast static checks, web-validation
  • Skipped by policy: web-build, web-database-tests, web-tests
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 12, 2026
@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b92d97c1-8c98-4c18-b31e-28ba39d1f3e0

📥 Commits

Reviewing files that changed from the base of the PR and between f1f9774 and 9610feb.


📒 Files selected for processing (3)
  • Sources/Sidebar/AppKitList/SidebarWorkspaceTableContainerView.swift
  • Sources/Sidebar/SidebarBonsplitTabWorkspaceDropView.swift
  • cmuxTests/SidebarWorkspaceReorderDropOverlayHitTestingTests.swift

 _________________________________________________________________________________________________________________________________________________________________
< Don't use wizard code you don't understand. Wizards can generate reams of code. Make sure you understand all of it before you incorporate it into your project. >
 -----------------------------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review (subagent), posted after merge:

  • Root cause confirmed: Drop a sidebar workspace onto a pane to merge its tabs #18837 put the Bonsplit tab type on sidebar row drags. AppKit routes drags by registered type and geometry, so the full-size Bonsplit overlay above the reorder view captured and rejected them. No other change since v0.65.0 touches this path.
  • Critical: forwarding draggingEnded(_:) to SidebarWorkspaceReorderDropView raises an unrecognized selector, because NSView doesn't implement draggingEnded: at runtime (verified). Fixed in Fix a crash when a sidebar workspace drag ends (from #20015) #20018.
  • Everything else checks out. Entered, updated, exited, prepare, perform and conclude forward correctly. Merge-onto-pane and pane tab drags into the sidebar are unchanged. The SwiftUI sidebar paths aren't affected.

austinywang added a commit that referenced this pull request Oct 12, 2026
* test: a sidebar row drag ending over the Bonsplit overlay does not raise

Fails on main: #20015 forwards draggingEnded: to the reorder view, which
does not implement it, so AppKit raises an unrecognized selector.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Fix a crash when a sidebar workspace drag ends (from #20015)

The Bonsplit overlay forwarded draggingEnded: to the reorder view, which
does not implement it. Swallow it for reorder drags, as AppKit did before
#18837 when the reorder view was the destination, and type the
destination as the reorder view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang austinywang mentioned this pull request Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant