Repository navigation
Fix minimal-mode top-bar drag pass-through #3121
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -399,6 +399,12 @@ final class WindowBrowserHostViewTests: XCTestCase { | |
| } | ||
| } | ||
|
|
||
| private final class FakeTabBarBackgroundNSView: NSView { | ||
| override func hitTest(_ point: NSPoint) -> NSView? { | ||
| bounds.contains(point) ? self : nil | ||
| } | ||
| } | ||
|
|
||
| private final class PrimaryPageProbeView: NSView { | ||
| override func hitTest(_ point: NSPoint) -> NSView? { | ||
| bounds.contains(point) ? self : nil | ||
|
|
@@ -457,6 +463,57 @@ final class WindowBrowserHostViewTests: XCTestCase { | |
| return inspectorView.isDescendant(of: hit) && !(pageView === hit || pageView.isDescendant(of: hit)) | ||
| } | ||
|
|
||
| func testHostViewPassesThroughUnderlyingTabStripBelowTitlebarBand() { | ||
| let window = NSWindow( | ||
| contentRect: NSRect(x: 0, y: 0, width: 420, height: 260), | ||
| styleMask: [.titled, .closable], | ||
| backing: .buffered, | ||
| defer: false | ||
| ) | ||
| defer { window.orderOut(nil) } | ||
| guard let contentView = window.contentView else { | ||
| XCTFail("Expected content view") | ||
| return | ||
| } | ||
| guard let container = contentView.superview else { | ||
| XCTFail("Expected content container") | ||
| return | ||
| } | ||
|
|
||
| let tabStripHeight: CGFloat = 44 | ||
| let tabStrip = FakeTabBarBackgroundNSView( | ||
| frame: NSRect( | ||
| x: 0, | ||
| y: contentView.bounds.maxY - tabStripHeight, | ||
| width: contentView.bounds.width, | ||
| height: tabStripHeight | ||
| ) | ||
| ) | ||
| tabStrip.autoresizingMask = [.width, .minYMargin] | ||
| contentView.addSubview(tabStrip) | ||
|
|
||
| let hostFrame = container.convert(contentView.bounds, from: contentView) | ||
| let host = WindowBrowserHostView(frame: hostFrame) | ||
| host.autoresizingMask = [.width, .height] | ||
| let child = CapturingView(frame: host.bounds) | ||
| child.autoresizingMask = [.width, .height] | ||
| host.addSubview(child) | ||
| container.addSubview(host, positioned: .above, relativeTo: contentView) | ||
|
|
||
| let titlebarBandHeight = max(28, min(72, window.frame.height - window.contentLayoutRect.height)) | ||
| let pointInContent = NSPoint( | ||
| x: contentView.bounds.midX, | ||
| y: contentView.bounds.maxY - titlebarBandHeight - 8 | ||
| ) | ||
| let pointInWindow = contentView.convert(pointInContent, to: nil) | ||
| let pointInHost = host.convert(pointInWindow, from: nil) | ||
|
|
||
| XCTAssertNil( | ||
| host.hitTest(pointInHost), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This test invokes Prompt for AI agents |
||
| "Browser portal should defer to the minimal tab strip even just below the native titlebar interaction band" | ||
| ) | ||
|
Comment on lines
+511
to
+514
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The same pattern breaks the mirror test in |
||
| } | ||
|
|
||
| func testHostViewPassesThroughDividerWhenAdjacentPaneIsCollapsed() { | ||
| let window = NSWindow( | ||
| contentRect: NSRect(x: 0, y: 0, width: 300, height: 180), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2075,6 +2075,12 @@ final class WindowTerminalHostViewTests: XCTestCase { | |
| } | ||
| } | ||
|
|
||
| private final class FakeTabBarBackgroundNSView: NSView { | ||
| override func hitTest(_ point: NSPoint) -> NSView? { | ||
| bounds.contains(point) ? self : nil | ||
| } | ||
| } | ||
|
|
||
| private final class BonsplitMockSplitDelegate: NSObject, NSSplitViewDelegate {} | ||
|
|
||
| private func makeHostedTerminalView(frame: NSRect) -> GhosttySurfaceScrollView { | ||
|
|
@@ -2105,6 +2111,57 @@ final class WindowTerminalHostViewTests: XCTestCase { | |
| ) | ||
| } | ||
|
|
||
| func testHostViewPassesThroughUnderlyingTabStripBelowTitlebarBand() { | ||
| let window = NSWindow( | ||
| contentRect: NSRect(x: 0, y: 0, width: 420, height: 260), | ||
| styleMask: [.titled, .closable], | ||
| backing: .buffered, | ||
| defer: false | ||
| ) | ||
| defer { window.orderOut(nil) } | ||
| guard let contentView = window.contentView else { | ||
| XCTFail("Expected content view") | ||
| return | ||
| } | ||
| guard let container = contentView.superview else { | ||
| XCTFail("Expected content container") | ||
| return | ||
| } | ||
|
|
||
| let tabStripHeight: CGFloat = 44 | ||
| let tabStrip = FakeTabBarBackgroundNSView( | ||
| frame: NSRect( | ||
| x: 0, | ||
| y: contentView.bounds.maxY - tabStripHeight, | ||
| width: contentView.bounds.width, | ||
| height: tabStripHeight | ||
| ) | ||
| ) | ||
| tabStrip.autoresizingMask = [.width, .minYMargin] | ||
| contentView.addSubview(tabStrip) | ||
|
|
||
| let hostFrame = container.convert(contentView.bounds, from: contentView) | ||
| let host = WindowTerminalHostView(frame: hostFrame) | ||
| host.autoresizingMask = [.width, .height] | ||
| let child = CapturingView(frame: host.bounds) | ||
| child.autoresizingMask = [.width, .height] | ||
| host.addSubview(child) | ||
| container.addSubview(host, positioned: .above, relativeTo: contentView) | ||
|
|
||
| let titlebarBandHeight = max(28, min(72, window.frame.height - window.contentLayoutRect.height)) | ||
| let pointInContent = NSPoint( | ||
| x: contentView.bounds.midX, | ||
| y: contentView.bounds.maxY - titlebarBandHeight - 8 | ||
| ) | ||
| let pointInWindow = contentView.convert(pointInContent, to: nil) | ||
| let pointInHost = host.convert(pointInWindow, from: nil) | ||
|
|
||
| XCTAssertNil( | ||
| host.hitTest(pointInHost), | ||
| "Terminal portal should defer to the minimal tab strip even just below the native titlebar interaction band" | ||
|
Comment on lines
+2157
to
+2161
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ) | ||
| } | ||
|
Comment on lines
+2114
to
+2163
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify the regression test calls hitTest without arranging a pointer currentEvent,
# while WindowTerminalHostView gates the pass-through path on NSApp.currentEvent.
rg -n -C5 'testHostViewPassesThroughUnderlyingTabStripBelowTitlebarBand|host\.hitTest|NSApp\.currentEvent|let isPointerEvent|NSEvent\.mouseEvent|sendEvent' \
cmuxTests/TerminalAndGhosttyTests.swift Sources/TerminalWindowPortal.swiftRepository: manaflow-ai/cmux Length of output: 10461 🏁 Script executed: #!/bin/bash
# Get the complete hitTest method implementation
sed -n '134,200p' Sources/TerminalWindowPortal.swiftRepository: manaflow-ai/cmux Length of output: 2530 🏁 Script executed: #!/bin/bash
# Get the rest of the hitTest method to see non-pointer path
sed -n '134,220p' Sources/TerminalWindowPortal.swiftRepository: manaflow-ai/cmux Length of output: 3221 🏁 Script executed: #!/bin/bash
# Check other hitTest calls in the same test class to see if they all have the same issue
rg -n 'func test.*\(\)' cmuxTests/TerminalAndGhosttyTests.swift | grep -A1 "hitTest" | head -20Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: #!/bin/bash
# Find all test methods that call hitTest and check if they set up events
rg -B20 'host\.hitTest|splitView\.hitTest' cmuxTests/TerminalAndGhosttyTests.swift | grep -E '(func test|NSEvent\.mouseEvent|NSApp\.sendEvent|host\.hitTest)'Repository: manaflow-ai/cmux Length of output: 587 Set The test expects Use 🤖 Prompt for AI Agents |
||
|
|
||
| func testHostViewPassesThroughWhenNoTerminalSubviewIsHit() { | ||
| let host = WindowTerminalHostView(frame: NSRect(x: 0, y: 0, width: 200, height: 120)) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasBonsplitTabBarBackgroundidentifies the tab strip by checkingNSStringFromClass(type(of: view)).contains("TabBarBackgroundNSView"). This is a string match against an internal Bonsplit class name with no public API contract. If Bonsplit renames that view, the fallback silently stops working with no compile-time signal. The primaryBonsplitTabBarHitRegionRegistrypath is robust; consider whether the fallback is worth keeping, or add a comment explaining the known fragility.