Skip to content

Fix split-pane tab clicks in minimal mode #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
austinywang merged 1 commit into from
Apr 7, 2026
Merged

Fix split-pane tab clicks in minimal mode #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions Sources/Bonsplit/Internal/Views/SplitContainerView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ struct SplitContainerView<Content: View, EmptyContent: View>: NSViewRepresentabl

// Keep arranged subviews stable (always 2) to avoid transient "collapse" flashes when
// replacing pane<->split content. We swap the hosted content within these containers.
let firstContainer = NSView()
// The containers use `SplitArrangedContainerView` (rather than bare NSView) so they
// override `mouseDownCanMoveWindow=false` — see `NonDraggableHostingView` in
// SplitNodeView.swift for the regression this guards against.
let firstContainer = SplitArrangedContainerView()
firstContainer.wantsLayer = true
firstContainer.layer?.backgroundColor = NSColor.clear.cgColor
firstContainer.layer?.isOpaque = false
Expand All @@ -128,7 +131,7 @@ struct SplitContainerView<Content: View, EmptyContent: View>: NSViewRepresentabl
splitView.addArrangedSubview(firstContainer)
context.coordinator.firstHostingController = firstController

let secondContainer = NSView()
let secondContainer = SplitArrangedContainerView()
secondContainer.wantsLayer = true
secondContainer.layer?.backgroundColor = NSColor.clear.cgColor
secondContainer.layer?.isOpaque = false
Expand Down Expand Up @@ -356,8 +359,8 @@ struct SplitContainerView<Content: View, EmptyContent: View>: NSViewRepresentabl

// MARK: - Helpers

private func makeHostingController(for node: SplitNode) -> NSHostingController<AnyView> {
let hostingController = NSHostingController(rootView: AnyView(makeView(for: node)))
private func makeHostingController(for node: SplitNode) -> NonDraggableHostingController<AnyView> {
let hostingController = NonDraggableHostingController(rootView: AnyView(makeView(for: node)))
if #available(macOS 13.0, *) {
// NSSplitView owns pane geometry. Keep NSHostingController from publishing
// intrinsic-size constraints that force a minimum pane width.
Expand All @@ -379,7 +382,7 @@ struct SplitContainerView<Content: View, EmptyContent: View>: NSViewRepresentabl
return hostingController
}

private func installHostingController(_ hostingController: NSHostingController<AnyView>, into container: NSView) {
private func installHostingController(_ hostingController: NonDraggableHostingController<AnyView>, into container: NSView) {
let hostedView = hostingController.view
hostedView.frame = container.bounds
hostedView.autoresizingMask = [.width, .height]
Expand All @@ -392,7 +395,7 @@ struct SplitContainerView<Content: View, EmptyContent: View>: NSViewRepresentabl
in container: NSView,
node: SplitNode,
nodeTypeChanged: Bool,
controller: inout NSHostingController<AnyView>?
controller: inout NonDraggableHostingController<AnyView>?
) {
// Historically we recreated the NSHostingController when the child node type changed
// (pane <-> split) to force a full detach/reattach of native AppKit subviews.
Expand Down Expand Up @@ -469,8 +472,8 @@ struct SplitContainerView<Content: View, EmptyContent: View>: NSViewRepresentabl
var firstNodeType: SplitNode.NodeType
var secondNodeType: SplitNode.NodeType
/// Retain hosting controllers so SwiftUI content stays alive
var firstHostingController: NSHostingController<AnyView>?
var secondHostingController: NSHostingController<AnyView>?
var firstHostingController: NonDraggableHostingController<AnyView>?
var secondHostingController: NonDraggableHostingController<AnyView>?

init(
splitState: SplitState,
Expand Down
41 changes: 39 additions & 2 deletions Sources/Bonsplit/Internal/Views/SplitNodeView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,46 @@ struct SplitNodeView<Content: View, EmptyContent: View>: View {
}
}

/// NSHostingView subclass that refuses to act as a window-drag handle.
///
/// Bonsplit nests SwiftUI panes inside their own `NSHostingController` instances
/// (via `SinglePaneWrapper` and `SplitContainerView.makeHostingController`).
/// The default `NSHostingView` returned by `NSHostingController.view` inherits the
/// AppKit default of `mouseDownCanMoveWindow == true` when the view appears opaque.
/// In `presentationMode == "minimal"` (where the window has no titlebar drag region)
/// AppKit was treating clicks on pane tab bars as window-drag intents and stealing the
/// mouseUp before the SwiftUI tap gesture could fire — making split pane tabs
/// completely unclickable. Routing clicks through this subclass keeps the entire pane
/// hosting chain non-draggable so SwiftUI gesture recognizers receive every click.
final class NonDraggableHostingView<Content: View>: NSHostingView<Content> {
override var mouseDownCanMoveWindow: Bool { false }
}

/// NSHostingController whose view is a `NonDraggableHostingView`. See the comment on
/// `NonDraggableHostingView` for the rationale — this exists so call sites can keep using
/// the controller-based lifecycle (root-view swapping, sizing options, etc.) without
/// having to construct and reparent the hosting view manually.
final class NonDraggableHostingController<Content: View>: NSHostingController<Content> {
override func loadView() {
view = NonDraggableHostingView(rootView: rootView)
}
}
Comment on lines +66 to +70
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 7, 2026

Choose a reason for hiding this comment

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

P2 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!


/// Container NSView for a pane inside SinglePaneWrapper.
class PaneDragContainerView: NSView {
override var isOpaque: Bool { false }
// Mirror the override on `NonDraggableHostingView` so AppKit cannot grab a window
// drag from this container either, even before the click reaches the inner hosting
// view. See `NonDraggableHostingView` for the full rationale.
override var mouseDownCanMoveWindow: Bool { false }
}

/// Bare container used by `SplitContainerView` to back NSSplitView arranged subviews.
/// Like `PaneDragContainerView`, this exists purely to suppress AppKit window-drag
/// intent so split-pane tab clicks are not consumed by drag detection in minimal mode.
final class SplitArrangedContainerView: NSView {
override var isOpaque: Bool { false }
override var mouseDownCanMoveWindow: Bool { false }
}

/// Wrapper that uses NSHostingController for proper AppKit layout constraints
Expand All @@ -68,7 +105,7 @@ struct SinglePaneWrapper<Content: View, EmptyContent: View>: NSViewRepresentable
showSplitButtons: showSplitButtons,
contentViewLifecycle: contentViewLifecycle
)
let hostingController = NSHostingController(rootView: paneView)
let hostingController = NonDraggableHostingController(rootView: paneView)
hostingController.view.translatesAutoresizingMaskIntoConstraints = false

let containerView = PaneDragContainerView()
Expand Down Expand Up @@ -116,6 +153,6 @@ struct SinglePaneWrapper<Content: View, EmptyContent: View>: NSViewRepresentable
}

class Coordinator {
var hostingController: NSHostingController<PaneContainerView<Content, EmptyContent>>?
var hostingController: NonDraggableHostingController<PaneContainerView<Content, EmptyContent>>?
}
}
72 changes: 50 additions & 22 deletions Sources/Bonsplit/Internal/Views/TabBarView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,18 +183,40 @@ struct TabBarView: View {
}
.frame(height: TabBarMetrics.barHeight)
.mask(combinedMask)
// Buttons float on top. No backdrop color needed because
// the mask hides scroll content and the tab bar's own
// background shows through naturally.
// Split buttons sit on top of the tab strip in their own opaque backdrop.
// The backdrop visually obscures any tabs that scroll under the buttons,
// and (critically) does not break hit testing on tabs outside the backdrop —
// unlike the prior approach of using a `Color.clear` region in `combinedMask`,
// which silently blocked SwiftUI hit tests in the masked-out area and let
// tab clicks fall through to `TabBarDragAndHoverView` (which performs a
// window drag in minimal mode).
.overlay(alignment: .trailing) {
if showSplitButtons {
let shouldShow = presentationMode != "minimal" || isHoveringTabBar
splitButtons
.saturation(tabBarSaturation)
.padding(.bottom, 1)
.opacity(shouldShow ? 1 : 0)
.allowsHitTesting(shouldShow)
.animation(.easeInOut(duration: 0.14), value: shouldShow)
let backdropColor = Color(nsColor: Self.buttonBackdropColor(
for: appearance,
focused: isFocused,
style: fadeColorStyle
))
ZStack(alignment: .trailing) {
HStack(spacing: 0) {
LinearGradient(
colors: [backdropColor.opacity(0), backdropColor],
startPoint: .leading,
endPoint: .trailing
)
.frame(width: 24)
Rectangle().fill(backdropColor)
}
.frame(width: 114)
Comment on lines +202 to +211
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 7, 2026

Choose a reason for hiding this comment

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

P2 Hardcoded 114pt backdrop width

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.

Suggested change
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)


splitButtons
.saturation(tabBarSaturation)
}
.padding(.bottom, 1)
.opacity(shouldShow ? 1 : 0)
.allowsHitTesting(shouldShow)
.animation(.easeInOut(duration: 0.14), value: shouldShow)
}
}
}
Expand Down Expand Up @@ -571,32 +593,31 @@ struct TabBarView: View {
}

// MARK: - Combined Mask (scroll fades + button area)
//
// IMPORTANT: SwiftUI's `.mask()` with `Color.clear` regions blocks hit testing on the
// masked content in those regions. Previously this mask used a 90pt clear region at the
// trailing edge to hide tabs under the split buttons; that caused clicks on tabs in that
// 90pt area to fall through the masked ScrollView to the `TabBarDragAndHoverView`
// background, which (in minimal mode) interpreted the click as a window drag instead
// of a tab tap. Keep the entire mask opaque so hit testing works on every tab; the split
// buttons' opaque backdrop (rendered in the splitButtons overlay) handles the visual
// obscuring of tabs underneath.

@ViewBuilder
private var combinedMask: some View {
let fadeWidth: CGFloat = 24
let shouldShowButtons = showSplitButtons && (presentationMode != "minimal" || isHoveringTabBar)
let buttonClearWidth: CGFloat = shouldShowButtons ? 90 : 0
let buttonFadeWidth: CGFloat = shouldShowButtons ? fadeWidth : 0

HStack(spacing: 0) {
// Left scroll fade
LinearGradient(colors: [.clear, .black], startPoint: .leading, endPoint: .trailing)
.frame(width: canScrollLeft ? fadeWidth : 0)

// Visible content area
// Visible content area (always opaque so hit testing reaches the tabs)
Rectangle().fill(Color.black)

// Right: either scroll fade or button area fade
// Right scroll fade only when scroll content actually overflows.
LinearGradient(colors: [.black, .clear], startPoint: .leading, endPoint: .trailing)
.frame(width: canScrollRight || shouldShowButtons ? fadeWidth : 0)

// Button clear area (content hidden here)
if shouldShowButtons {
Color.clear.frame(width: buttonClearWidth)
}
.frame(width: canScrollRight ? fadeWidth : 0)
}
.animation(.easeInOut(duration: 0.14), value: shouldShowButtons)
}

// MARK: - Fade Overlays
Expand Down Expand Up @@ -713,6 +734,9 @@ private struct TabBarDragAndHoverView: NSViewRepresentable {
}

override func mouseDown(with event: NSEvent) {
#if DEBUG
dlog("tab.bar.bg.mouseDown isMinimal=\(isMinimalMode ? 1 : 0) clickCount=\(event.clickCount)")
#endif
guard isMinimalMode, let window else {
super.mouseDown(with: event)
return
Expand Down Expand Up @@ -760,6 +784,10 @@ private struct TabBarDragZoneView: NSViewRepresentable {
}

override func mouseDown(with event: NSEvent) {
#if DEBUG
let isMinimal = UserDefaults.standard.string(forKey: "workspacePresentationMode") == "minimal"
dlog("tab.bar.dragZone.mouseDown isMinimal=\(isMinimal ? 1 : 0) clickCount=\(event.clickCount)")
#endif
guard let window = self.window else {
super.mouseDown(with: event)
return
Expand Down
Loading