Skip to content

Fix minimal tab drag edge hit testing #126

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
May 20, 2026
Merged

Fix minimal tab drag edge hit testing #126

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
6 changes: 1 addition & 5 deletions Sources/Bonsplit/Internal/Styling/TabBarColors.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,7 @@ enum TabBarColors {
return activeTabBackground
}
if appearance.usesSharedBackdrop {
let semanticBackground = semanticTabBarBackgroundColor(for: appearance) ?? custom
let overlayColor = semanticBackground.isBonsplitLightColor
? NSColor.black.withAlphaComponent(0.06)
: NSColor.white.withAlphaComponent(0.08)
return Color(nsColor: overlayColor)
return .clear
}
let adjusted = custom.isBonsplitLightColor
? custom.bonsplitDarken(by: 0.065)
Expand Down
33 changes: 30 additions & 3 deletions Sources/Bonsplit/Internal/Views/TabBarView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,25 @@ public enum BonsplitTabItemHitRegionRegistry {
}
}

enum BonsplitTabItemHitTesting {
static let horizontalSlop: CGFloat = 2
static let verticalSlop: CGFloat = 6

static func containsTabLaneHit(
localPoint: NSPoint,
tabFrames: [CGRect],
bounds: NSRect
) -> Bool {
guard bounds.insetBy(dx: 0, dy: -verticalSlop).contains(localPoint) else {
return false
}
return tabFrames.contains { frame in
localPoint.x >= frame.minX - horizontalSlop
&& localPoint.x <= frame.maxX + horizontalSlop
}
Comment on lines +109 to +115

@greptile-apps greptile-apps Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Symmetric vertical expansion hits both edges

bounds.insetBy(dx: 0, dy: -verticalSlop) expands the accepted y-range by verticalSlop (6 pt) on both the top edge (toward the titlebar — the intended fix) and the bottom edge (toward app content). Any point within a tab's x-lane that lies up to 6 pt below the tab bar's bottom edge will be claimed as a tab hit. The old per-tab insetBy(dx: -2, dy: -2) also over-extended downward, but only by 2 pt. The new code triples that slop, and shouldCaptureHit only guards this with bounds.contains(point) on the drag-zone view, so if that view's bounds extend past the tab bar's visual bottom (e.g. it fills the full content view), the downward expansion starts suppressing window-drag events for touches in the app content area. The test only verifies the upward direction.

}
}

private struct TabItemHitRegionView: NSViewRepresentable {
func makeNSView(context: Context) -> RegionNSView {
RegionNSView()
Expand Down Expand Up @@ -150,7 +169,12 @@ private struct TabItemHitRegionView: NSViewRepresentable {
}

nonisolated func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool {
hitBounds.insetBy(dx: -2, dy: -2).contains(localPoint)
hitBounds
.insetBy(
dx: -BonsplitTabItemHitTesting.horizontalSlop,
dy: -BonsplitTabItemHitTesting.verticalSlop
)
.contains(localPoint)
}

override func hitTest(_ point: NSPoint) -> NSView? {
Expand Down Expand Up @@ -2464,8 +2488,11 @@ private struct TabBarDragAndHoverView: NSViewRepresentable {
}

nonisolated func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool {
let paddedFrames = tabFrames.map { $0.insetBy(dx: -2, dy: -2) }
return paddedFrames.contains { $0.contains(localPoint) }
BonsplitTabItemHitTesting.containsTabLaneHit(
localPoint: localPoint,
tabFrames: tabFrames,
bounds: bounds
)
}

override func updateTrackingAreas() {
Expand Down
40 changes: 40 additions & 0 deletions Tests/BonsplitTests/BonsplitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,28 @@ final class BonsplitTests: XCTestCase {
XCTAssertLessThan(hoverAlpha, 0.12)
}

func testSharedBackdropActiveTabBackgroundIsClear() {
let appearance = BonsplitConfiguration.Appearance(
chromeColors: .init(
backgroundHex: "#272822",
tabBarBackgroundHex: "#00000000",
splitButtonBackdropHex: "#00000000",
paneBackgroundHex: "#00000000"
),
usesSharedBackdrop: true
)
let active = NSColor(TabBarColors.activeTabBackground(for: appearance)).usingColorSpace(.sRGB)!

var alpha: CGFloat = 1
active.getRed(nil, green: nil, blue: nil, alpha: &alpha)
Comment on lines +1277 to +1280

@greptile-apps greptile-apps Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Force-unwrapping usingColorSpace(.sRGB) will crash the test rather than fail it cleanly if the color-space conversion ever returns nil. Color.clear resolves to NSColor.clear (calibrated gray-space), and while conversion to sRGB works today, a guarded call makes the failure mode explicit and the intent clearer.

Suggested change
let active = NSColor(TabBarColors.activeTabBackground(for: appearance)).usingColorSpace(.sRGB)!
var alpha: CGFloat = 1
active.getRed(nil, green: nil, blue: nil, alpha: &alpha)
let active = try XCTUnwrap(
NSColor(TabBarColors.activeTabBackground(for: appearance)).usingColorSpace(.sRGB),
"Could not convert activeTabBackground color to sRGB"
)
var alpha: CGFloat = 1
active.getRed(nil, green: nil, blue: nil, alpha: &alpha)


XCTAssertLessThan(
alpha,
0.01,
"Shared-backdrop selected tabs should rely on the active indicator instead of a hover-like fill"
)
}

func testSplitActionPressedStateUsesHigherContrast() {
let appearance = BonsplitConfiguration.Appearance(
chromeColors: .init(backgroundHex: "#272822")
Expand Down Expand Up @@ -1714,6 +1736,15 @@ final class BonsplitTests: XCTestCase {
"A short-titled tab must own the full configured visible tab width so minimal-mode drags do not become window drags"
)

let topEdgeTabPoint = hostingView.convert(
NSPoint(x: appearance.tabMinWidth - 20, y: appearance.tabBarHeight + 3),
to: nil
)
XCTAssertTrue(
BonsplitTabItemHitRegionRegistry.containsWindowPoint(topEdgeTabPoint, in: window),
"A tab's horizontal lane must own near-titlebar-edge drags so minimal-mode top-edge tab drags do not become window drags"
)

let emptyChromePoint = hostingView.convert(
NSPoint(x: appearance.tabMinWidth + 30, y: verticalCenter),
to: nil
Expand All @@ -1722,6 +1753,15 @@ final class BonsplitTests: XCTestCase {
BonsplitTabItemHitRegionRegistry.containsWindowPoint(emptyChromePoint, in: window),
"Empty tab-strip chrome after the configured tab remains available for app-window dragging"
)

let topEdgeEmptyChromePoint = hostingView.convert(
NSPoint(x: appearance.tabMinWidth + 30, y: appearance.tabBarHeight + 3),
to: nil
)
XCTAssertFalse(
BonsplitTabItemHitRegionRegistry.containsWindowPoint(topEdgeEmptyChromePoint, in: window),
"Near-titlebar-edge empty chrome after the tab should remain available for app-window dragging"
)
}

@MainActor
Expand Down
Loading