Fix first-attempt Dock pane drops - #7536
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPointer-up drag routing now consults active pane-drop state, pane drop targets maintain that state during drag lifecycles, Dock moves can preserve an emptied source workspace, and new tests cover unfocused Dock drop flows and the external-drop path. ChangesPointer-up routing and Dock drop handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WindowInputRoutingContext
participant WindowTerminalHostView
participant PaneDropRoutingSession
participant DragOverlayRoutingPolicy
WindowTerminalHostView->>WindowInputRoutingContext: allowsTerminalPortalDragRouting(.pointerUp)
WindowInputRoutingContext-->>WindowTerminalHostView: true
WindowTerminalHostView->>PaneDropRoutingSession: hasActiveDropDrag
PaneDropRoutingSession-->>WindowTerminalHostView: active or inactive
WindowTerminalHostView->>DragOverlayRoutingPolicy: shouldPassThroughTerminalPortalHitTesting(..., hasActiveDropDrag)
DragOverlayRoutingPolicy-->>WindowTerminalHostView: pass through or block
Possibly related PRs
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR fixes first-attempt Dock pane drops by sharing drag state between hover and mouse-up routing. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (9): Last reviewed commit: "Reject remote tmux Dock transfer for #75..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/DragOverlayRoutingPolicy.swift (2)
379-396: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
.pointerUpterminal routing is missing the sidebar-reorder check present in.pointerDrag.For
.pointerDrag,shouldPassThroughTerminalPortalHitTestingdelegates toshouldPassThroughPortalHitTesting, which includeshasSidebarReorder. The new.pointerUpbranch is hand-rolled ashasBonsplitTabTransfer(pasteboardTypes) || hasFileDropPayload(pasteboardTypes)and omits the sidebar-tab-reorder check entirely. Per the PR objective ("allowing pane, file-preview, and sidebar drag payloads to reach SwiftUI drop targets at drop time"), sidebar drag payloads dropped on a terminal portal would fail to route correctly on mouse-up, unlike during drag/hover.🐛 Proposed fix to include sidebar-reorder payloads in the drop-time path
case .pointerUp: - return hasBonsplitTabTransfer(pasteboardTypes) || hasFileDropPayload(pasteboardTypes) + return hasBonsplitTabTransfer(pasteboardTypes) + || hasSidebarTabReorder(pasteboardTypes) + || hasFileDropPayload(pasteboardTypes)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/DragOverlayRoutingPolicy.swift` around lines 379 - 396, The .pointerUp path in shouldPassThroughTerminalPortalHitTesting is missing the sidebar reorder payload check that .pointerDrag already gets through shouldPassThroughPortalHitTesting. Update the .pointerUp branch in DragOverlayRoutingPolicy to include the same sidebar-reorder condition alongside hasBonsplitTabTransfer and hasFileDropPayload, so drop-time routing matches the drag/hover behavior for sidebar-tab drags.
363-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
.pointerDrag/.pointerUpcase bodies — merge to avoid divergence.The two cases are byte-for-byte identical. Beyond DRY, keeping them separate is exactly what let the sibling
shouldPassThroughTerminalPortalHitTestingfunction's.pointerUpbranch silently diverge and drop the sidebar-reorder check (see other comment). Merging the cases here removes one such divergence opportunity.♻️ Proposed consolidation
switch routingContext.eventKind { - case .pointerDrag: - return hasTabTransfer - || hasFilePreviewTransfer(pasteboardTypes) - || hasSidebarReorder - case .pointerUp: - return hasTabTransfer - || hasFilePreviewTransfer(pasteboardTypes) - || hasSidebarReorder + case .pointerDrag, .pointerUp: + return hasTabTransfer + || hasFilePreviewTransfer(pasteboardTypes) + || hasSidebarReorder case .pointerHover: return hasTabTransfer || hasSidebarReorder🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/DragOverlayRoutingPolicy.swift` around lines 363 - 374, Merge the identical `.pointerDrag` and `.pointerUp` branches in the routing switch so they share one body, and keep the combined logic in the same decision point inside the event-kind handling. Use the surrounding `routingContext.eventKind` switch in `DragOverlayRoutingPolicy` to consolidate the `hasTabTransfer || hasFilePreviewTransfer(pasteboardTypes) || hasSidebarReorder` check, reducing the chance of future divergence between drag and up handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/WindowInputRoutingContext.swift`:
- Around line 93-104: The drag-routing logic currently treats `.pointerUp` as
eligible in `allowsBrowserPortalDragRouting` and
`allowsTerminalPortalDragRouting`, which lets
`TerminalWindowPortal.performHitTest`, `BrowserWindowPortal.hitTest`, and
`DragOverlayRoutingPolicy` read drag pasteboard types during ordinary mouse-up
events. Update `WindowInputRoutingContext` so `.pointerUp` is not routed through
the drag-pasteboard path unless there is an explicit active-drag signal, either
by removing `.pointerUp` from the drag-routing checks or by adding a separate
guard used by the hit-test/routing methods before touching `NSPasteboard(name:
.drag).types`.
---
Outside diff comments:
In `@Sources/DragOverlayRoutingPolicy.swift`:
- Around line 379-396: The .pointerUp path in
shouldPassThroughTerminalPortalHitTesting is missing the sidebar reorder payload
check that .pointerDrag already gets through shouldPassThroughPortalHitTesting.
Update the .pointerUp branch in DragOverlayRoutingPolicy to include the same
sidebar-reorder condition alongside hasBonsplitTabTransfer and
hasFileDropPayload, so drop-time routing matches the drag/hover behavior for
sidebar-tab drags.
- Around line 363-374: Merge the identical `.pointerDrag` and `.pointerUp`
branches in the routing switch so they share one body, and keep the combined
logic in the same decision point inside the event-kind handling. Use the
surrounding `routingContext.eventKind` switch in `DragOverlayRoutingPolicy` to
consolidate the `hasTabTransfer || hasFilePreviewTransfer(pasteboardTypes) ||
hasSidebarReorder` check, reducing the chance of future divergence between drag
and up handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d30e8e80-d6f9-4f02-b933-5e772858283d
📒 Files selected for processing (4)
Sources/DragOverlayRoutingPolicy.swiftSources/WindowInputRoutingContext.swiftcmux.xcodeproj/project.pbxprojcmuxTests/DockPaneDropUnfocusedRoutingTests.swift
austinywang
force-pushed
the
issue-7529-sidebar-dock-drop-unfocused
branch
from
July 7, 2026 22:56
847ee17 to
11d0900
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/PaneDropRoutingSupport.swift`:
- Around line 53-81: PaneDropRoutingSession can remain active if
PaneDropTargetView is torn down mid-drag because draggingExited(_:) and
performDragOperation(_:) may never run. Add a view-lifecycle cleanup path in
PaneDropTargetView such as willMove(toSuperview:), viewDidMoveToWindow, or
deinit that calls PaneDropRoutingSession.clearActiveDropDrag(nil) or otherwise
removes the current sequence, so the active drop state is always reset even when
the view disappears.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d29f088-6b75-4133-92ec-cf983611a431
📒 Files selected for processing (7)
Sources/DragOverlayRoutingPolicy.swiftSources/PaneDropRoutingSupport.swiftSources/TerminalPaneDropTargetView.swiftSources/TerminalWindowPortal.swiftSources/WindowInputRoutingContext.swiftcmux.xcodeproj/project.pbxprojcmuxTests/DockPaneDropUnfocusedRoutingTests.swift
austinywang
enabled auto-merge
July 8, 2026 03:06
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 002b2d9. Configure here.
| guard sourceWorkspace.panels.isEmpty else { return } | ||
| sourceWorkspace.detachRemoteTmuxMirrorKeptOpenLocallyIfNeeded() | ||
| _ = sourceWorkspace.createReplacementTerminalPanel() | ||
| sourceWorkspace.scheduleTerminalGeometryReconcile() |
There was a problem hiding this comment.
Preserve workspace skips focus reconcile
Medium Severity
When the last main panel is moved into its own Dock and the source workspace is preserved via preserveEmptySourceWorkspaceAfterSurfaceMove, a replacement terminal is created but the workspace never focuses or selects that tab and does not schedule focus reconciliation, unlike other empty-workspace replacement paths in Workspace.
Reviewed by Cursor Bugbot for commit 002b2d9. Configure here.
Contributor
Author
There was a problem hiding this comment.
Fixed in a314102. The replacement helper now centralizes replacement tab selection, and the Dock-preserve caller opts out of AppKit focus reassertion so the destination Dock keeps keyboard ownership.
— Claude Code
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/TerminalPaneDropTargetView.swift (1)
115-146: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFail closed for ineligible Dock drops
AtSources/TerminalPaneDropTargetView.swift:115-146,canMoveSurfaceIntoDock(...) == falsefalls through toworkspace.performPortalPaneDrop(...), and that path will resolve the Dock pane ID against the workspace by falling back to the focused/first pane. Reject here instead so non-movable Dock drops can’t be routed to the wrong workspace pane.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/TerminalPaneDropTargetView.swift` around lines 115 - 146, The Dock-drop handling in TerminalPaneDropTargetView should fail closed when canMoveSurfaceIntoDock(sourceTabId:destinationDock:) is false instead of falling through to workspace routing. Update the drop decision around the existing PaneDragTransfer.decode and dock.performPortalPaneDrop path so ineligible Dock drops are rejected early, preventing the fallback workspace pane resolution from sending the drop to the wrong pane.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Workspace.swift`:
- Around line 3466-3473: The remote-tmux session-ended path in
handleRemoteTmuxSessionEndedKeepingWorkspaceOpenIfNeeded() creates a replacement
terminal panel but does not perform the same post-close focus and selection
handling used in the didCloseTab empty-workspace path. After
createReplacementTerminalPanel() is called, mirror the existing empty-workspace
logic by focusing the pane, selecting the newly created tab, and applying tab
selection so the workspace ends on an active terminal instead of an unselected
pane.
---
Outside diff comments:
In `@Sources/TerminalPaneDropTargetView.swift`:
- Around line 115-146: The Dock-drop handling in TerminalPaneDropTargetView
should fail closed when canMoveSurfaceIntoDock(sourceTabId:destinationDock:) is
false instead of falling through to workspace routing. Update the drop decision
around the existing PaneDragTransfer.decode and dock.performPortalPaneDrop path
so ineligible Dock drops are rejected early, preventing the fallback workspace
pane resolution from sending the drop to the wrong pane.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 488ff58f-70de-4e5f-a2dd-4f8c570092ed
📒 Files selected for processing (8)
Sources/AppDelegate+DockSurfaceMove.swiftSources/BrowserPaneDropTargetView.swiftSources/TerminalPaneDropTargetView.swiftSources/TerminalWindowPortal.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/DockPaneDropUnfocusedRoutingTests.swiftcmuxTests/WindowDockLifecycleTests.swift
This was referenced Jul 9, 2026
5 of 6 tasks
This branch was previously deployed
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.
Fixes #7529
Summary
Testing
python3 scripts/swift_file_length_budget.py./scripts/lint-pbxproj-test-wiring.shscripts/check-pbxproj.shgit diff --check./scripts/reload.sh --tag issue-7529-dock-dropafter the final pushed commitDemo Video
Review Trigger
Checklist
cmuxTests.github/*.tsvedits and no submodule editsSummary by CodeRabbit
Note
Medium Risk
Touches drag hit-testing, Dock surface moves, and workspace empty-state preservation; wrong gating could break drops or leave workspaces in an inconsistent state, but behavior is heavily regression-tested.
Overview
Fixes first-attempt Dock pane drops (including unfocused windows) by tracking an active pane drop session on
WindowTerminalHostViewand routing terminal portal mouse-up through the same path as hover, gated on that session (and sidebar reorder drags) instead of stale pasteboard state. Raw Finder file URL mouse-up no longer uses terminal portal pass-through; tab/file-preview/sidebar payloads still do when a drop was accepted.Dock moves use shared
canMoveSurfaceIntoDock(blocks remote tmux mirror panels); moving the last main panel into the same workspace/window Dock is allowed and preserves the emptied workspace via a replacement terminal instead of tearing it down. Browser/terminal pane targets call the same eligibility check for Dock routing.Adds
DockPaneDropUnfocusedRoutingTestsand updates window Dock lifecycle expectations.Reviewed by Cursor Bugbot for commit 002b2d9. Bugbot is set up for automated code reviews on this repo. Configure here.