Fix split-pane tab clicks in minimal mode#91
Conversation
In minimal mode the window has no titlebar drag region, so AppKit relies on `mouseDownCanMoveWindow` on each NSView in the hit chain to decide whether a click is a window-drag intent. The default `NSHostingView` returned by `NSHostingController.view` reports `true` whenever its SwiftUI content appears opaque, so AppKit was treating clicks on Bonsplit pane tab bars as window-drag attempts and stealing the mouseUp before the SwiftUI tap gesture could fire — making split-pane tabs completely unclickable while terminal surfaces (which use a separate portal hosting path) kept working. The cmux app already overrides `mouseDownCanMoveWindow=false` on its top-level `MainWindowHostingView`, but Bonsplit's split layout creates its own nested `NSHostingController` instances inside `SinglePaneWrapper` and `SplitContainerView.makeHostingController`. Those inner hosting views were never covered. Fix: * Add `NonDraggableHostingView<Content>: NSHostingView<Content>` and `NonDraggableHostingController<Content>: NSHostingController<Content>` helpers in SplitNodeView.swift, both forcing `mouseDownCanMoveWindow=false`. * Use `NonDraggableHostingController` everywhere Bonsplit currently constructs an `NSHostingController` for pane content. * Override `mouseDownCanMoveWindow=false` on `PaneDragContainerView` and a new `SplitArrangedContainerView` (used to back the `NSSplitView` arranged subviews in `SplitContainerView`) so the entire pane container chain is non-draggable. Also fix a related minor hit-testing bug in `TabBarView.combinedMask`: the previous mask used a `Color.clear` region behind the split-button overlay, which silently blocks SwiftUI hit testing on tabs in that trailing area. Switch to a fully opaque mask plus an opaque backdrop on the buttons overlay so hit testing reaches every tab while the split-button area remains visually obscured. Adds DEBUG `dlog` instrumentation to TabBarBackgroundNSView and TabBarDragZoneView mouseDown handlers so future regressions can be diagnosed via `/tmp/cmux-debug*.log`.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces specialized, non-draggable hosting views to prevent unintended window-moving in split panes, and refactors tab-bar split-button hit-testing by replacing a transparent-area-based approach with an opaque trailing backdrop. Changes
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ 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 |
Greptile SummaryThis PR fixes split-pane tab clicks being silently consumed as window-drag events in minimal mode. The fix has two complementary parts: (1) Confidence Score: 5/5PR is safe to merge; all remaining findings are P2 style suggestions. The root cause is correctly identified and addressed with a defense-in-depth approach — mouseDownCanMoveWindow=false at every layer of the view hierarchy, plus the mask hit-testing fix. All three remaining comments are non-blocking P2 style items (dead code, a magic number, and an undocumented pattern assumption). No files require special attention; the P2 items in TabBarView.swift and SplitNodeView.swift are minor cleanup suggestions. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppKit
participant Container as NSView / SplitArrangedContainerView
participant HostingView as NSHostingView / NonDraggableHostingView
participant SwiftUI
participant DragZone as TabBarDragZoneView
Note over User,DragZone: BEFORE — tab click stolen as window drag
User->>AppKit: mouseDown on pane tab bar
AppKit->>Container: mouseDownCanMoveWindow?
Container-->>AppKit: true (bare NSView default)
AppKit->>HostingView: mouseDownCanMoveWindow?
HostingView-->>AppKit: true (opaque SwiftUI content)
AppKit-->>User: window drag — mouseUp consumed
Note over SwiftUI: SwiftUI tap gesture never fires
Note over User,DragZone: AFTER — tap reaches SwiftUI
User->>AppKit: mouseDown on pane tab bar
AppKit->>Container: mouseDownCanMoveWindow?
Container-->>AppKit: false (SplitArrangedContainerView override)
AppKit->>HostingView: mouseDownCanMoveWindow?
HostingView-->>AppKit: false (NonDraggableHostingView override)
AppKit->>SwiftUI: deliver mouseDown
SwiftUI-->>User: tab selected ✓
Note over DragZone: Only receives events on truly empty space
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| HStack(spacing: 0) { | ||
| LinearGradient( | ||
| colors: [backdropColor.opacity(0), backdropColor], | ||
| startPoint: .leading, | ||
| endPoint: .trailing | ||
| ) | ||
| .frame(width: 24) | ||
| Rectangle().fill(backdropColor) | ||
| } | ||
| .frame(width: 114) |
There was a problem hiding this comment.
The .frame(width: 114) is a magic number derived from the old buttonClearWidth (90 pt) + fadeWidth (24 pt). If a new split button is added or the button layout changes, the backdrop will under-cover the button area without an obvious failure signal. A named constant or inline comment would make the coupling explicit.
| HStack(spacing: 0) { | |
| LinearGradient( | |
| colors: [backdropColor.opacity(0), backdropColor], | |
| startPoint: .leading, | |
| endPoint: .trailing | |
| ) | |
| .frame(width: 24) | |
| Rectangle().fill(backdropColor) | |
| } | |
| .frame(width: 114) | |
| .frame(width: 90 + 24) // buttonClearWidth (90) + fadeWidth (24) |
| final class NonDraggableHostingController<Content: View>: NSHostingController<Content> { | ||
| override func loadView() { | ||
| view = NonDraggableHostingView(rootView: rootView) | ||
| } | ||
| } |
There was a problem hiding this comment.
loadView() relies on undocumented NSHostingController internals
Overriding loadView() to substitute NonDraggableHostingView without calling super.loadView() works because NSHostingController forwards rootView writes to self.view cast as NSHostingView<Content> at runtime — but this is not a documented contract. This is a well-known and widely-used pattern in the AppKit/SwiftUI community, so it's unlikely to break, but a brief comment acknowledging the assumption (as done for the other overrides in this file) would help future maintainers.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
NonDraggableHostingView/NonDraggableHostingControllerhelpers and route Bonsplit's nested pane hosting controllers through them so AppKit cannot grab a window-drag from the innerNSHostingViewchain.mouseDownCanMoveWindow=falseonPaneDragContainerViewand a newSplitArrangedContainerView(used to backNSSplitViewarranged subviews) so the entire pane container chain is non-draggable.Color.clearregion inTabBarView.combinedMask(which silently blocks SwiftUI hit testing on tabs in the masked area) and switch to a fully opaque mask plus an opaque backdrop on the split-button overlay.dloginstrumentation inTabBarBackgroundNSView/TabBarDragZoneViewmouseDown handlers.Why
In minimal mode the host window has no titlebar drag region. AppKit then relies on
mouseDownCanMoveWindowwalking up the hit chain to decide whether a click is a window-drag intent. The defaultNSHostingView<AnyView>returned byNSHostingController.viewreportstruewhenever its SwiftUI content appears opaque, so AppKit was treating clicks on Bonsplit pane tab bars as window drags and stealing the mouseUp before the SwiftUI tap gesture could fire — making split-pane tabs completely unclickable while terminal surfaces (which use a separate portal hosting path) kept working.The cmux app already overrides
mouseDownCanMoveWindow=falseon its top-levelMainWindowHostingView, but Bonsplit's split layout creates its own nestedNSHostingControllerinstances insideSinglePaneWrapperandSplitContainerView.makeHostingController. Those inner hosting views were never covered.Test plan
TabBarDragZoneViewfor the first pane in minimal mode) still allows window dragging.Summary by cubic
Fixes unclickable split-pane tabs in minimal mode by making nested SwiftUI hosting views non‑draggable and fixing tab bar masking so clicks reach tabs. Prevents AppKit from treating tab clicks as window drags; tabs and split buttons now work as expected.
NonDraggableHostingViewandNonDraggableHostingControllerand used them for all pane hosts to forcemouseDownCanMoveWindow = false.mouseDownCanMoveWindow = falseonPaneDragContainerViewand newSplitArrangedContainerViewso the full pane container chain is non‑draggable.Color.clearinTabBarView.combinedMaskwith a fully opaque mask and added an opaque split-button backdrop to preserve tab hit testing while hiding scrolled tabs.dlogto tab bar background/drag zonemouseDownhandlers for easier diagnostics.Written for commit 9c03ca9. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
UI/UX Improvements