-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix splitter hitbox overlap and terminal scrollbar width resync #1950
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
1590194
1073aad
8396885
d77ae21
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 |
|---|---|---|
|
|
@@ -5034,10 +5034,12 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| // Origin-only frame churn is common while the surrounding split layout | ||
| // settles. Reapplying the side-docked inspector at the same size fights | ||
| // WebKit's own dock layout and shows up as visible flicker. | ||
| if !isHostedInspectorSideDockActive() && | ||
| !isHostedInspectorDividerDragActive && | ||
| !hasStoredHostedInspectorWidthPreference { | ||
| captureHostedInspectorPreferredWidthFromCurrentLayout(reason: "host.layout.sameSize") | ||
| if !isHostedInspectorDividerDragActive { | ||
| if hasStoredHostedInspectorWidthPreference { | ||
| reapplyHostedInspectorDividerToStoredWidthIfNeeded(reason: "host.layout.sameSize") | ||
| } else if !isHostedInspectorSideDockActive() { | ||
| captureHostedInspectorPreferredWidthFromCurrentLayout(reason: "host.layout.sameSize") | ||
| } | ||
| } | ||
| updateHostedInspectorDockControlAvailabilityIfNeeded(reason: "host.layout.sameSize") | ||
| notifyGeometryChangedIfNeeded() | ||
|
|
@@ -5049,7 +5051,9 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| lastHostedInspectorLayoutBoundsSize = bounds.size | ||
| if isHostedInspectorSideDockActive() { | ||
| layoutHostedInspectorSideDockIfNeeded(reason: "host.layout.sideDock") | ||
| } else if !hasStoredHostedInspectorWidthPreference { | ||
| } else if hasStoredHostedInspectorWidthPreference { | ||
| reapplyHostedInspectorDividerToStoredWidthIfNeeded(reason: "host.layout") | ||
| } else { | ||
| captureHostedInspectorPreferredWidthFromCurrentLayout(reason: "host.layout") | ||
| } | ||
| updateHostedInspectorDockControlAvailabilityIfNeeded(reason: "host.layout") | ||
|
|
@@ -5127,26 +5131,24 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| return nil | ||
| } | ||
| if let hostedInspectorHit { | ||
| let isSideDockHit = isHostedInspectorSideDockHit(hostedInspectorHit) | ||
| if let nativeHit = nativeHostedInspectorHit(at: point, hostedInspectorHit: hostedInspectorHit) { | ||
| #if DEBUG | ||
| debugLogHitTest(stage: "hitTest.hostedInspectorNative", point: point, passThrough: false, hitView: nativeHit) | ||
| #endif | ||
| if !isSideDockHit || | ||
| (nativeHit !== hostedInspectorHit.inspectorView && | ||
| !hostedInspectorHit.inspectorView.isDescendant(of: nativeHit)) { | ||
| if nativeHit !== hostedInspectorHit.inspectorView && | ||
| !hostedInspectorHit.inspectorView.isDescendant(of: nativeHit) { | ||
| return nativeHit | ||
| } | ||
| } | ||
| #if DEBUG | ||
| debugLogHitTest( | ||
| stage: isSideDockHit ? "hitTest.hostedInspectorManual" : "hitTest.hostedInspectorFallback", | ||
| stage: "hitTest.hostedInspectorManual", | ||
| point: point, | ||
| passThrough: false, | ||
| hitView: hostedInspectorHit.inspectorView | ||
| hitView: self | ||
| ) | ||
| #endif | ||
| return isSideDockHit ? self : hostedInspectorHit.inspectorView | ||
| return self | ||
| } | ||
|
Comment on lines
+5138
to
5152
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🌐 Web query:
💡 Result: Yes, when NSView.hitTest(_:) returns a container view instead of a descendant view, the rightMouseDown(with:) and otherMouseDown(with:) events are dispatched to that container view. Citations:
🏁 Script executed: # First, verify the file exists and examine the code around lines 5138-5152
wc -l Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 102 🏁 Script executed: # Read the specific section mentioned in the review
sed -n '5130,5160p' Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 1177 🏁 Script executed: # Find HostContainerView class to check mouse event handlers
rg -A 20 "class HostContainerView" Sources/Panels/Repository: manaflow-ai/cmux Length of output: 1697 🏁 Script executed: # Get the full HostContainerView class implementation with all methods
rg -A 200 "^\s*final class HostContainerView: NSView" Sources/Panels/BrowserPanelView.swift | head -300Repository: manaflow-ai/cmux Length of output: 10329 🏁 Script executed: # More specific search for mouse event handlers in HostContainerView
rg "(mouseDown|rightMouseDown|otherMouseDown|mouseMoved)" Sources/Panels/BrowserPanelView.swift | grep -A 2 -B 2 "HostContainerView\|func.*[mM]ouse"Repository: manaflow-ai/cmux Length of output: 511 🏁 Script executed: # Search for all methods in HostContainerView to understand its structure
ast-grep --pattern 'final class HostContainerView {
$$$
}' | head -100Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: # Search for mouse event handlers specifically in HostContainerView
rg -n "override func (mouseDown|rightMouseDown|otherMouseDown|mouseMoved)" Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 236 🏁 Script executed: # Get the full HostContainerView class boundaries to understand its scope
rg -n "^\s*final class HostContainerView|^ }" Sources/Panels/BrowserPanelView.swift | head -20Repository: manaflow-ai/cmux Length of output: 255 🏁 Script executed: # Find the exact line where HostContainerView is defined and where it ends
rg -n "final class HostContainerView" Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 108 🏁 Script executed: # Now find the closing brace of HostContainerView class by searching for its extent
sed -n '4705,4750p' Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 2276 🏁 Script executed: # Get a better view - search for where HostContainerView class definition starts
grep -n "final class HostContainerView" Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 108 🏁 Script executed: # Now that we know HostContainerView likely starts around line 4705+, let's find mouse handlers around that area
sed -n '5150,5180p' Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 1354 🏁 Script executed: # Search for rightMouseDown and otherMouseDown in entire HostContainerView class
# We know it starts at line 4287, so let's search after that line
rg -n "override func (rightMouseDown|otherMouseDown)" Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: # Verify the mouseMoved handler around line 5115 is in HostContainerView
sed -n '5110,5125p' Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 699 🏁 Script executed: # Get a complete picture - show all override methods in HostContainerView starting from line 4287
sed -n '4287,6392p' Sources/Panels/BrowserPanelView.swift | grep -n "override func" | head -20Repository: manaflow-ai/cmux Length of output: 922 🏁 Script executed: # Final confirmation - view the exact hitTest logic one more time to ensure understanding
sed -n '5115,5160p' Sources/Panels/BrowserPanelView.swiftRepository: manaflow-ai/cmux Length of output: 1923 Gate divider hits to left-button events only.
Add an event-type check before accepting the divider hit: Suggested adjustment- if let hostedInspectorHit {
- if let nativeHit = nativeHostedInspectorHit(at: point, hostedInspectorHit: hostedInspectorHit) {
+ if let hostedInspectorHit {
+ let nativeHit = nativeHostedInspectorHit(at: point, hostedInspectorHit: hostedInspectorHit)
+ if let nativeHit {
`#if` DEBUG
debugLogHitTest(stage: "hitTest.hostedInspectorNative", point: point, passThrough: false, hitView: nativeHit)
`#endif`
if nativeHit !== hostedInspectorHit.inspectorView &&
!hostedInspectorHit.inspectorView.isDescendant(of: nativeHit) {
return nativeHit
}
}
+ let shouldUseManualDividerHit: Bool = {
+ switch NSApp.currentEvent?.type {
+ case .leftMouseDown, .leftMouseDragged, .leftMouseUp:
+ return true
+ default:
+ return false
+ }
+ }()
+ guard shouldUseManualDividerHit else {
+ return nativeHit ?? super.hitTest(point)
+ }
`#if` DEBUG
debugLogHitTest(
stage: "hitTest.hostedInspectorManual",🤖 Prompt for AI Agents |
||
| let hit = super.hitTest(point) | ||
| #if DEBUG | ||
|
|
@@ -5157,8 +5159,7 @@ struct WebViewRepresentable: NSViewRepresentable { | |
|
|
||
| override func mouseDown(with event: NSEvent) { | ||
| let point = convert(event.locationInWindow, from: nil) | ||
| guard let hostedInspectorHit = hostedInspectorDividerHit(at: point), | ||
| isHostedInspectorSideDockHit(hostedInspectorHit) else { | ||
| guard let hostedInspectorHit = hostedInspectorDividerHit(at: point) else { | ||
| super.mouseDown(with: event) | ||
| return | ||
| } | ||
|
|
@@ -5255,7 +5256,7 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| ) | ||
| ) | ||
| #endif | ||
| layoutHostedInspectorSideDockIfNeeded(reason: "drag.end") | ||
| reapplyHostedInspectorDividerToStoredWidthIfNeeded(reason: "drag.end") | ||
| } | ||
| super.mouseUp(with: event) | ||
| } | ||
|
|
@@ -5270,7 +5271,7 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| // Pass through a narrow leading-edge band so the shared sidebar divider | ||
| // handle can receive hover/click even when WKWebView is attached here. | ||
| // Keeping this deterministic avoids flicker from dynamic left-edge scans. | ||
| guard point.x >= 0, point.x <= SidebarResizeInteraction.hitWidthPerSide else { | ||
| guard point.x >= 0, point.x <= SidebarResizeInteraction.contentSideHitWidth else { | ||
| return false | ||
| } | ||
| guard let window, let contentView = window.contentView else { | ||
|
|
@@ -5492,9 +5493,9 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| guard let self else { return } | ||
| self.hostedInspectorReapplyWorkItem = nil | ||
| _ = self.promoteHostedInspectorSideDockFromCurrentLayoutIfNeeded() | ||
| if self.isHostedInspectorSideDockActive() { | ||
| if self.hasStoredHostedInspectorWidthPreference { | ||
| self.reapplyHostedInspectorDividerToStoredWidthIfNeeded(reason: reason) | ||
| } else if !self.hasStoredHostedInspectorWidthPreference { | ||
| } else { | ||
| self.captureHostedInspectorPreferredWidthFromCurrentLayout(reason: reason) | ||
| } | ||
| } | ||
|
|
@@ -5555,7 +5556,6 @@ struct WebViewRepresentable: NSViewRepresentable { | |
| private func reapplyHostedInspectorDividerToStoredWidthIfNeeded(reason: String) { | ||
| guard !isApplyingHostedInspectorLayout else { return } | ||
| guard let hit = hostedInspectorDividerCandidate() else { return } | ||
| guard isHostedInspectorSideDockHit(hit) else { return } | ||
| guard let preferredWidth = resolvedPreferredHostedInspectorWidth(in: hit.containerView.bounds) else { | ||
| return | ||
| } | ||
|
|
||
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.
#if DEBUGguard missing on public wrapperGhosttySurfaceScrollView.debugPendingSurfaceSize()callssurfaceView.debugPendingSurfaceSize(), which is defined only inside a#if DEBUGblock onGhosttyNSView. In a release build, theGhosttyNSViewmethod is stripped, so this wrapper will fail to compile with "Value of type 'GhosttyNSView' has no member 'debugPendingSurfaceSize'".