-
Notifications
You must be signed in to change notification settings - Fork 36
Handle hidden-titlebar Bonsplit chrome overlap #27
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
base: main
Are you sure you want to change the base?
Changes from all commits
780dd77
a559813
743de85
1ae8ee4
29a36a1
d807a44
fb9fcc2
b136cad
06841fd
31c3810
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 |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import AppKit | ||
| import SwiftUI | ||
|
|
||
| final class BonsplitHostingView<Content: View>: NSHostingView<Content> { | ||
| private let zeroSafeAreaLayoutGuide = NSLayoutGuide() | ||
|
|
||
| override var safeAreaInsets: NSEdgeInsets { NSEdgeInsetsZero } | ||
| override var safeAreaRect: NSRect { bounds } | ||
| override var safeAreaLayoutGuide: NSLayoutGuide { zeroSafeAreaLayoutGuide } | ||
| override var mouseDownCanMoveWindow: Bool { false } | ||
| override var intrinsicContentSize: NSSize { | ||
| NSSize(width: NSView.noIntrinsicMetric, height: NSView.noIntrinsicMetric) | ||
| } | ||
|
|
||
| required init(rootView: Content) { | ||
| super.init(rootView: rootView) | ||
| addLayoutGuide(zeroSafeAreaLayoutGuide) | ||
| NSLayoutConstraint.activate([ | ||
| zeroSafeAreaLayoutGuide.leadingAnchor.constraint(equalTo: leadingAnchor), | ||
| zeroSafeAreaLayoutGuide.trailingAnchor.constraint(equalTo: trailingAnchor), | ||
| zeroSafeAreaLayoutGuide.topAnchor.constraint(equalTo: topAnchor), | ||
| zeroSafeAreaLayoutGuide.bottomAnchor.constraint(equalTo: bottomAnchor), | ||
| ]) | ||
| } | ||
|
|
||
| @available(*, unavailable) | ||
| required init?(coder: NSCoder) { | ||
| fatalError("init(coder:) has not been implemented") | ||
| } | ||
| } | ||
|
|
||
| final class BonsplitHostingController<Content: View>: NSViewController { | ||
| private let hostingView: BonsplitHostingView<Content> | ||
|
|
||
| var rootView: Content { | ||
| get { hostingView.rootView } | ||
| set { hostingView.rootView = newValue } | ||
| } | ||
|
|
||
| init(rootView: Content) { | ||
| hostingView = BonsplitHostingView(rootView: rootView) | ||
| super.init(nibName: nil, bundle: nil) | ||
| } | ||
|
|
||
| @available(*, unavailable) | ||
| required init?(coder: NSCoder) { | ||
| fatalError("init(coder:) has not been implemented") | ||
| } | ||
|
|
||
| override func loadView() { | ||
| view = hostingView | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,159 @@ import SwiftUI | |
| import UniformTypeIdentifiers | ||
| import AppKit | ||
|
|
||
| private final class TabBarInteractionContainerView: NSView { | ||
| private var eventMonitor: Any? | ||
| private weak var monitoredWindow: NSWindow? | ||
| private var previousWindowMovableState: Bool? | ||
|
|
||
| override var mouseDownCanMoveWindow: Bool { false } | ||
| override func isAccessibilityElement() -> Bool { true } | ||
| override func accessibilityRole() -> NSAccessibility.Role? { .group } | ||
|
|
||
| deinit { | ||
| removeEventMonitor() | ||
| restoreWindowDraggingIfNeeded() | ||
| } | ||
|
|
||
| override func viewDidMoveToWindow() { | ||
| super.viewDidMoveToWindow() | ||
|
|
||
| if window !== monitoredWindow { | ||
| removeEventMonitor() | ||
| monitoredWindow = window | ||
| installEventMonitor() | ||
| } | ||
|
|
||
| if window == nil { | ||
|
Comment on lines
+24
to
+28
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.
Useful? React with 👍 / 👎. |
||
| restoreWindowDraggingIfNeeded() | ||
| } | ||
|
Comment on lines
+19
to
+30
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. Restore the old window before swapping If Possible fix override func viewDidMoveToWindow() {
super.viewDidMoveToWindow()
if window !== monitoredWindow {
+ let previousWindow = monitoredWindow
+ restoreWindowDraggingIfNeeded(on: previousWindow)
removeEventMonitor()
monitoredWindow = window
- installEventMonitor()
- }
-
- if window == nil {
- restoreWindowDraggingIfNeeded()
+ if window != nil {
+ installEventMonitor()
+ }
}
}
@@
- private func restoreWindowDraggingIfNeeded() {
+ private func restoreWindowDraggingIfNeeded(on window: NSWindow? = nil) {
guard let previousWindowMovableState else { return }
- monitoredWindow?.isMovable = previousWindowMovableState
+ (window ?? monitoredWindow)?.isMovable = previousWindowMovableState
self.previousWindowMovableState = nil
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| private func installEventMonitor() { | ||
| guard eventMonitor == nil else { return } | ||
| eventMonitor = NSEvent.addLocalMonitorForEvents(matching: [.leftMouseDown, .leftMouseDragged, .leftMouseUp]) { [weak self] event in | ||
| self?.handleLocalMouseEvent(event) ?? event | ||
| } | ||
| } | ||
|
|
||
| private func removeEventMonitor() { | ||
| guard let eventMonitor else { return } | ||
| NSEvent.removeMonitor(eventMonitor) | ||
| self.eventMonitor = nil | ||
| } | ||
|
|
||
| private func handleLocalMouseEvent(_ event: NSEvent) -> NSEvent? { | ||
| guard let window = monitoredWindow else { | ||
| return event | ||
| } | ||
|
|
||
| if event.window !== window { | ||
| if event.type == .leftMouseUp { | ||
| restoreWindowDraggingIfNeeded() | ||
| } | ||
| return event | ||
| } | ||
|
|
||
| let point = convert(event.locationInWindow, from: nil) | ||
| switch event.type { | ||
| case .leftMouseDown: | ||
| if bounds.contains(point), !hitTestRoutesToWindowDragRegion(at: point) { | ||
| suppressWindowDraggingIfNeeded(window: window) | ||
|
Comment on lines
+61
to
+62
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.
Useful? React with 👍 / 👎. |
||
| } | ||
| case .leftMouseDragged: | ||
| if previousWindowMovableState == nil, bounds.contains(point), !hitTestRoutesToWindowDragRegion(at: point) { | ||
| suppressWindowDraggingIfNeeded(window: window) | ||
| } | ||
| case .leftMouseUp: | ||
| restoreWindowDraggingIfNeeded() | ||
| default: | ||
| break | ||
| } | ||
|
|
||
| return event | ||
| } | ||
|
|
||
| private func hitTestRoutesToWindowDragRegion(at point: NSPoint) -> Bool { | ||
| guard let hitView = super.hitTest(point) else { return false } | ||
| var current: NSView? = hitView | ||
| while let view = current { | ||
| if view is TabBarWindowDragRegionView { | ||
| return true | ||
| } | ||
| current = view.superview | ||
| } | ||
| return descendantWindowDragRegionContains(point: point, in: self) | ||
| } | ||
|
|
||
| private func descendantWindowDragRegionContains(point: NSPoint, in view: NSView) -> Bool { | ||
| if let dragRegionView = view as? TabBarWindowDragRegionView { | ||
| let pointInDragRegion = dragRegionView.convert(point, from: self) | ||
| if dragRegionView.bounds.contains(pointInDragRegion) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| for subview in view.subviews { | ||
| if descendantWindowDragRegionContains(point: point, in: subview) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| private func suppressWindowDraggingIfNeeded(window: NSWindow) { | ||
| guard previousWindowMovableState == nil else { return } | ||
| previousWindowMovableState = window.isMovable | ||
| if window.isMovable { | ||
| window.isMovable = false | ||
| } | ||
| } | ||
|
|
||
| private func restoreWindowDraggingIfNeeded() { | ||
| guard let previousWindowMovableState else { return } | ||
| monitoredWindow?.isMovable = previousWindowMovableState | ||
| self.previousWindowMovableState = nil | ||
| } | ||
| } | ||
|
|
||
| private struct TabBarHostingWrapper<Content: View>: NSViewRepresentable { | ||
| let content: Content | ||
|
|
||
| func makeCoordinator() -> Coordinator { | ||
| Coordinator() | ||
| } | ||
|
|
||
| func makeNSView(context: Context) -> NSView { | ||
| let containerView = TabBarInteractionContainerView() | ||
| containerView.setAccessibilityElement(true) | ||
| containerView.setAccessibilityIdentifier("paneTabBar") | ||
|
|
||
| let hostingView = BonsplitHostingView(rootView: content) | ||
| hostingView.translatesAutoresizingMaskIntoConstraints = false | ||
| hostingView.setAccessibilityElement(false) | ||
| containerView.addSubview(hostingView) | ||
|
|
||
| NSLayoutConstraint.activate([ | ||
| hostingView.topAnchor.constraint(equalTo: containerView.topAnchor), | ||
| hostingView.leadingAnchor.constraint(equalTo: containerView.leadingAnchor), | ||
| hostingView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor), | ||
| hostingView.bottomAnchor.constraint(equalTo: containerView.bottomAnchor), | ||
| ]) | ||
|
|
||
| context.coordinator.hostingView = hostingView | ||
| return containerView | ||
| } | ||
|
|
||
| func updateNSView(_ nsView: NSView, context: Context) { | ||
| context.coordinator.hostingView?.rootView = content | ||
|
Comment on lines
+128
to
+150
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: # Find BonsplitHostingView definition
fd -t f "\.swift$" | xargs rg -l "BonsplitHostingView" | head -20Repository: manaflow-ai/bonsplit Length of output: 177 🏁 Script executed: # Search for BonsplitHostingView implementation
rg -A 20 "class BonsplitHostingView|struct BonsplitHostingView" --type swiftRepository: manaflow-ai/bonsplit Length of output: 2319 🏁 Script executed: # Check TabBarView's environment requirements
rg "@Environment.*BonsplitController|@Environment.*SplitViewController" --type swift -B 2 -A 2Repository: manaflow-ai/bonsplit Length of output: 2716 🏁 Script executed: # Check how environments are provided at the view hierarchy level
rg "\.environment.*BonsplitController|\.environment.*SplitViewController" --type swift -B 2 -A 2Repository: manaflow-ai/bonsplit Length of output: 46 🏁 Script executed: # Get full PaneContainerView struct definition
cat -n Sources/Bonsplit/Internal/Views/PaneContainerView.swift | head -100Repository: manaflow-ai/bonsplit Length of output: 4054 🏁 Script executed: # Search for PaneContainerView instantiation
rg "PaneContainerView" --type swift -B 3 -A 3 | head -100Repository: manaflow-ai/bonsplit Length of output: 3058 🏁 Script executed: # Check where environments are set on views
rg "\.environment\(" --type swift | head -50Repository: manaflow-ai/bonsplit Length of output: 894 🏁 Script executed: # Get full PaneContainerView.swift file
wc -l Sources/Bonsplit/Internal/Views/PaneContainerView.swiftRepository: manaflow-ai/bonsplit Length of output: 123 🏁 Script executed: # Read the complete PaneContainerView.swift
cat -n Sources/Bonsplit/Internal/Views/PaneContainerView.swiftRepository: manaflow-ai/bonsplit Length of output: 31387 🏁 Script executed: # Check the test setup at lines 603-605 and surrounding context
sed -n '600,610p' Tests/BonsplitTests/BonsplitTests.swiftRepository: manaflow-ai/bonsplit Length of output: 406 🏁 Script executed: # Search for where BonsplitHostingView is used with environment modifiers
rg "BonsplitHostingView" --type swift -B 5 -A 10Repository: manaflow-ai/bonsplit Length of output: 6504 🏁 Script executed: # Check if there's any environment setup in NSViewRepresentable context handling
rg "Context.*environment|\.environment" Sources/Bonsplit/Internal/Views/PaneContainerView.swiftRepository: manaflow-ai/bonsplit Length of output: 116 🏁 Script executed: # Check BonsplitHostingController to see if it handles environments
fd -t f "BonsplitHostingController" --type swift | xargs cat 2>/dev/null || rg "BonsplitHostingController" --type swift -A 20Repository: manaflow-ai/bonsplit Length of output: 236 🏁 Script executed: # Get more context around the test setup
sed -n '590,620p' Tests/BonsplitTests/BonsplitTests.swiftRepository: manaflow-ai/bonsplit Length of output: 1447 🏁 Script executed: # Search for all usages of environment in BonsplitTests
rg "\.environment\(" Tests/BonsplitTests/BonsplitTests.swift -B 2 -A 2 | head -80Repository: manaflow-ai/bonsplit Length of output: 914 🏁 Script executed: # Check if TabBarView is ever wrapped with .environment before passing to TabBarHostingWrapper
rg "TabBarView" --type swift -B 3 -A 3Repository: manaflow-ai/bonsplit Length of output: 3038 🏁 Script executed: # Look for NSHostingView or environment propagation patterns in the codebase
rg "NSHostingView|NSViewRepresentable" --type swift -B 2 -A 8 | head -100Repository: manaflow-ai/bonsplit Length of output: 7121 Apply environments to TabBarView before wrapping in NSHostingView.
Fix needed in PaneContainerView.swift, around line 326-331TabBarHostingWrapper(
content: TabBarView(
pane: pane,
isFocused: isFocused,
showSplitButtons: showSplitButtons
)
.environment(bonsplitController)
.environment(controller)
)🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| final class Coordinator { | ||
| var hostingView: BonsplitHostingView<Content>? | ||
| } | ||
| } | ||
|
|
||
| /// Drop zone positions for creating splits | ||
| public enum DropZone: Equatable { | ||
| case center | ||
|
|
@@ -170,10 +323,12 @@ struct PaneContainerView<Content: View, EmptyContent: View>: View { | |
| var body: some View { | ||
| VStack(spacing: 0) { | ||
| // Tab bar | ||
| TabBarView( | ||
| pane: pane, | ||
| isFocused: isFocused, | ||
| showSplitButtons: showSplitButtons | ||
| TabBarHostingWrapper( | ||
| content: TabBarView( | ||
| pane: pane, | ||
| isFocused: isFocused, | ||
| showSplitButtons: showSplitButtons | ||
| ) | ||
| ) | ||
|
|
||
| // Content area with drop zones | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
P1: When the view detaches from its window, you clear
monitoredWindowbefore restoringisMovableand still install a new event monitor. This can leave the old window stuck with dragging disabled and creates a detached monitor.Prompt for AI agents