diff --git a/Sources/Bonsplit/Internal/Views/TabBarItemGeometryRegistry.swift b/Sources/Bonsplit/Internal/Views/TabBarItemGeometryRegistry.swift index b5a442b3..e222bbdd 100644 --- a/Sources/Bonsplit/Internal/Views/TabBarItemGeometryRegistry.swift +++ b/Sources/Bonsplit/Internal/Views/TabBarItemGeometryRegistry.swift @@ -36,7 +36,7 @@ final class TabBarItemGeometryRegistry { private var lastObservedSelectedTabDocumentFrame: CGRect? private var pendingScrollIntent: ScrollIntent? private var expectedProgrammaticOffset: CGFloat? - private var trailingObscuredWidth: CGFloat = 0 + private(set) var trailingObscuredWidth: CGFloat = 0 deinit { if let scrollBoundsObserver { @@ -201,6 +201,24 @@ final class TabBarItemGeometryRegistry { return frames } + /// The on-screen part of each tab: clipped by the strip's scroll view, so + /// a tab scrolled past either edge contributes only what is visible. + func visibleFrames(for tabIds: [UUID], in targetView: NSView) -> [UUID: CGRect] { + var frames: [UUID: CGRect] = [:] + frames.reserveCapacity(tabIds.count) + for tabId in tabIds { + guard let itemView = itemViews.object(forKey: tabId as NSUUID), + itemView.window === targetView.window, + isVisibleInHierarchy(itemView) else { + continue + } + let visible = itemView.visibleRect + guard !visible.isEmpty else { continue } + frames[tabId] = itemView.convert(visible, to: targetView) + } + return frames + } + func geometryDidChange(for tabId: UUID) { if tabId == selectedTabId { if selectedTabFrameDidChange(tabId) { diff --git a/Sources/Bonsplit/Internal/Views/TabBarView.swift b/Sources/Bonsplit/Internal/Views/TabBarView.swift index 2c2fa781..63c2ff36 100644 --- a/Sources/Bonsplit/Internal/Views/TabBarView.swift +++ b/Sources/Bonsplit/Internal/Views/TabBarView.swift @@ -828,6 +828,12 @@ struct TabBarView: View { @AppStorage("workspacePresentationMode") private var presentationMode = "standard" @AppStorage("debugFadeColorStyle") private var fadeColorStyle = -1 @State private var isHoveringTabBar = false + /// One hovered tab for the whole strip, resolved from the pointer against + /// the registered tab frames. Per-tab `.onHover` never fires when a tab + /// slides under a stationary pointer (closing the tab to its left), so the + /// tab under the cursor showed no hover or close button until the mouse + /// moved, while the tab that slid away could keep its stale hover. + @State private var hoveredTabId: UUID? @State private var dropTargetIndex: Int? @State private var scrollOffset: CGFloat = 0 @State private var contentWidth: CGFloat = 0 @@ -1212,7 +1218,13 @@ struct TabBarView: View { } .background(dragAndHoverBackground) .overlay( - TabBarHoverTrackingView { updateTabBarHover($0) } + TabBarHoverTrackingView( + geometryRegistry: tabItemGeometryRegistry, + tabIds: pane.tabs.map(\.id), + isDraggingTab: splitViewController.tabDragSession != nil, + onHoverChanged: { updateTabBarHover($0) }, + onHoveredTabChanged: { updateHoveredTab($0) } + ) ) .overlay(tabDropDestination) .background { @@ -1263,6 +1275,12 @@ struct TabBarView: View { } } + private func updateHoveredTab(_ tabId: UUID?) { + withTransaction(Transaction(animation: nil)) { + hoveredTabId = tabId + } + } + @ViewBuilder private func tabItem( for tab: TabItem, @@ -1277,6 +1295,7 @@ struct TabBarView: View { TabItemView( tab: tab, isSelected: pane.selectedTabId == tab.id, + isHovered: hoveredTabId == tab.id, showsZoomIndicator: showsZoomIndicator, appearance: appearance, fillsWidth: tabFillsWidth, @@ -1890,24 +1909,83 @@ private final class SplitActionMouseDownNSView: NSView { } } +/// Resolves which tab the pointer is over from the strip's registered tab +/// frames. Pure so the stationary-pointer cases are unit testable. +struct TabBarHoveredTabResolver { + func hoveredTabId( + pointInView: NSPoint?, + barBounds: CGRect, + tabIds: [UUID], + frames: [UUID: CGRect], + trailingObscuredWidth: CGFloat = 0 + ) -> UUID? { + guard let pointInView, barBounds.insetBy(dx: -1, dy: -1).contains(pointInView) else { + return nil + } + // Tabs scrolled under the trailing action lane are masked out; the + // pointer there is over the split buttons, not a tab. + guard pointInView.x < barBounds.maxX - trailingObscuredWidth else { return nil } + return tabIds.first { frames[$0]?.contains(pointInView) == true } + } +} + private struct TabBarHoverTrackingView: NSViewRepresentable { + let geometryRegistry: TabBarItemGeometryRegistry + let tabIds: [UUID] + let isDraggingTab: Bool let onHoverChanged: (Bool) -> Void + let onHoveredTabChanged: (UUID?) -> Void func makeNSView(context: Context) -> HoverNSView { let view = HoverNSView() - view.onHoverChanged = onHoverChanged + update(view) return view } func updateNSView(_ nsView: HoverNSView, context: Context) { - nsView.onHoverChanged = onHoverChanged + update(nsView) + } + + static func dismantleNSView(_ nsView: HoverNSView, coordinator: ()) { + nsView.geometryRegistry?.unregisterObserver(nsView) + } + + private func update(_ view: HoverNSView) { + view.onHoverChanged = onHoverChanged + view.onHoveredTabChanged = onHoveredTabChanged + if view.geometryRegistry !== geometryRegistry { + view.geometryRegistry?.unregisterObserver(view) + view.geometryRegistry = geometryRegistry + geometryRegistry.registerObserver(view) + } + view.tabIds = tabIds + view.isDraggingTab = isDraggingTab } - final class HoverNSView: NSView { + final class HoverNSView: NSView, TabBarItemGeometryObserving { var onHoverChanged: ((Bool) -> Void)? + var onHoveredTabChanged: ((UUID?) -> Void)? + weak var geometryRegistry: TabBarItemGeometryRegistry? + var tabIds: [UUID] = [] { + didSet { + guard tabIds != oldValue else { return } + schedulePointerRecheck() + } + } private var trackingArea: NSTrackingArea? private var localMouseMonitor: Any? private var isHovering = false + private var hoveredTabId: UUID? + private var pointerRecheckScheduled = false + /// A tab drag gets no move events here, but its autoscroll still + /// changes geometry; resolving hover then would reveal the close + /// button on the drop target under the drag. + var isDraggingTab = false { + didSet { + guard isDraggingTab != oldValue else { return } + schedulePointerRecheck() + } + } deinit { removeLocalMouseMonitor() @@ -1920,10 +1998,10 @@ private struct TabBarHoverTrackingView: NSViewRepresentable { if let window { window.acceptsMouseMovedEvents = true installLocalMouseMonitorIfNeeded() - updateHoverFromCurrentMouseLocation() + schedulePointerRecheck() } else { removeLocalMouseMonitor() - emitHoverChanged(false) + emitHover(pointInView: nil) } } @@ -1953,6 +2031,27 @@ private struct TabBarHoverTrackingView: NSViewRepresentable { updateHover(from: event) } + /// Tabs were added, removed, resized, or scrolled: the pointer may now + /// sit over a different tab without having moved. + func tabBarItemGeometryDidChange() { + schedulePointerRecheck() + } + + /// Tab-set and geometry changes arrive inside SwiftUI view updates + /// (updateNSView, hit-region registration), where publishing hover + /// would modify TabBarView state mid-update. Coalesce them into one + /// recheck on the next main-queue turn, resolved against the state at + /// that time so a superseded change never publishes. + private func schedulePointerRecheck() { + guard !pointerRecheckScheduled else { return } + pointerRecheckScheduled = true + DispatchQueue.main.async { [weak self] in + guard let self else { return } + self.pointerRecheckScheduled = false + self.updateHoverFromCurrentMouseLocation() + } + } + private func installLocalMouseMonitorIfNeeded() { guard localMouseMonitor == nil else { return } localMouseMonitor = NSEvent.addLocalMonitorForEvents( @@ -1972,34 +2071,52 @@ private struct TabBarHoverTrackingView: NSViewRepresentable { private func updateHover(from event: NSEvent) { guard let window else { - emitHoverChanged(false) + emitHover(pointInView: nil) return } guard event.window == nil || event.window === window else { - emitHoverChanged(false) + emitHover(pointInView: nil) return } let pointInWindow = event.window === window ? event.locationInWindow : window.mouseLocationOutsideOfEventStream - let pointInView = convert(pointInWindow, from: nil) - emitHoverChanged(bounds.insetBy(dx: -1, dy: -1).contains(pointInView)) + emitHover(pointInView: convert(pointInWindow, from: nil)) } private func updateHoverFromCurrentMouseLocation() { - guard let window else { - emitHoverChanged(false) + guard let window, + NSWindow.windowNumber(at: NSEvent.mouseLocation, belowWindowWithWindowNumber: 0) + == window.windowNumber else { + emitHover(pointInView: nil) return } - let pointInView = convert(window.mouseLocationOutsideOfEventStream, from: nil) - emitHoverChanged(bounds.insetBy(dx: -1, dy: -1).contains(pointInView)) + emitHover(pointInView: convert(window.mouseLocationOutsideOfEventStream, from: nil)) } - private func emitHoverChanged(_ newValue: Bool) { - guard isHovering != newValue else { return } - isHovering = newValue - onHoverChanged?(newValue) + private func emitHover(pointInView: NSPoint?) { + let hovering = pointInView.map { bounds.insetBy(dx: -1, dy: -1).contains($0) } ?? false + if isHovering != hovering { + isHovering = hovering + onHoverChanged?(hovering) + } + let tabId: UUID? + if isDraggingTab { + tabId = nil + } else { + tabId = TabBarHoveredTabResolver().hoveredTabId( + pointInView: pointInView, + barBounds: bounds, + tabIds: tabIds, + frames: geometryRegistry?.visibleFrames(for: tabIds, in: self) ?? [:], + trailingObscuredWidth: geometryRegistry?.trailingObscuredWidth ?? 0 + ) + } + if hoveredTabId != tabId { + hoveredTabId = tabId + onHoveredTabChanged?(tabId) + } } } } diff --git a/Sources/Bonsplit/Internal/Views/TabItemView.swift b/Sources/Bonsplit/Internal/Views/TabItemView.swift index c873d0a1..18f0d376 100644 --- a/Sources/Bonsplit/Internal/Views/TabItemView.swift +++ b/Sources/Bonsplit/Internal/Views/TabItemView.swift @@ -266,6 +266,9 @@ enum TabItemStyling { struct TabItemView: View { let tab: TabItem let isSelected: Bool + /// Owned by the tab strip so hover follows the pointer when tabs move + /// under it (see `TabBarView.hoveredTabId`). + let isHovered: Bool let showsZoomIndicator: Bool let appearance: BonsplitConfiguration.Appearance /// When true, the tab drops its fixed maximum width and grows to fill the slack @@ -293,8 +296,7 @@ struct TabItemView: View { let onContextAction: (TabContextAction) -> Void let onMoveDestination: (String) -> Void - @State private var isHovered = false - @State private var isCloseHovered = false + @State private var closeButtonPointerInside = false @State private var isZoomHovered = false @State private var isAudioHovered = false @State private var showGlobeFallback = true @@ -368,13 +370,16 @@ struct TabItemView: View { onZoomToggle() } ) - .onHover { hovering in - withTransaction(Transaction(animation: nil)) { - isHovered = hovering - } - } .accessibilityElement(children: .combine) .accessibilityLabel(tab.title) + // The close button is only built while selected or hovered and is + // merged into this element, so VoiceOver needs a named action to + // close any tab. + .accessibilityActions { + if allowsClose && !tab.isPinned { + Button(closeTabAccessibilityName) { onClose(.closeButton) } + } + } .accessibilityValue(accessibilityValue) .accessibilityAddTraits(isSelected ? [.isButton, .isSelected] : .isButton) .safeHelp(tab.title) @@ -1018,11 +1023,25 @@ struct TabItemView: View { // MARK: - Close Button / Dirty Indicator + /// Close-button highlight. Gated by the strip-owned tab hover so a stale + /// button-level flag cannot keep highlighting after the tab moved away. + private var isCloseHovered: Bool { + closeButtonPointerInside && isHovered + } + + private var closeTabAccessibilityName: String { + Bundle.module.localizedString( + forKey: "tab.close.accessibilityLabel", + value: "Close Tab", + table: nil + ) + } + @ViewBuilder private var closeOrDirtyIndicator: some View { ZStack { // Dirty indicator (shown when dirty and not hovering, hidden for selected tab) - if (!isSelected && !isHovered && !isCloseHovered) && (tab.isDirty || tab.showsNotificationBadge) { + if (!isSelected && !isHovered) && (tab.isDirty || tab.showsNotificationBadge) { HStack(spacing: 2) { if tab.showsNotificationBadge { Circle() @@ -1039,14 +1058,14 @@ struct TabItemView: View { } if tab.isPinned { - if isSelected || isHovered || isCloseHovered || (!tab.isDirty && !tab.showsNotificationBadge) { + if isSelected || isHovered || (!tab.isDirty && !tab.showsNotificationBadge) { Image(systemName: "pin.fill") .font(.system(size: scaledCloseIconSize, weight: .semibold)) .foregroundStyle(TabBarColors.inactiveText(for: appearance)) .frame(width: accessorySlotSize, height: accessorySlotSize) .saturation(saturation) } - } else if allowsClose && (isSelected || isHovered || isCloseHovered) { + } else if allowsClose && (isSelected || isHovered) { // Close button (always visible on active tab, shown on hover for others) Button { onClose(.closeButton) @@ -1071,9 +1090,15 @@ struct TabItemView: View { .buttonStyle(.plain) .onHover { hovering in withTransaction(Transaction(animation: nil)) { - isCloseHovered = hovering + closeButtonPointerInside = hovering } } + // Pinning, allowsClose, or deselect can remove the button with + // the pointer still on it, and .onHover(false) never arrives. + .onDisappear { closeButtonPointerInside = false } + // The tab element's named "Close Tab" action covers this; + // merged into the tab it would be announced twice. + .accessibilityHidden(true) .saturation(saturation) } } diff --git a/Sources/Bonsplit/Resources/en.lproj/Localizable.strings b/Sources/Bonsplit/Resources/en.lproj/Localizable.strings index 8fcd21e9..0a3806dc 100644 --- a/Sources/Bonsplit/Resources/en.lproj/Localizable.strings +++ b/Sources/Bonsplit/Resources/en.lproj/Localizable.strings @@ -41,3 +41,4 @@ "tabContext.forkConversation.newWorkspace" = "New Workspace"; "tabContext.disconnectRemote" = "Disconnect SSH"; "tabContext.remoteConnectedAccessibility" = "Connected over SSH"; +"tab.close.accessibilityLabel" = "Close Tab"; diff --git a/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings b/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings index 2364aa0d..6f3979be 100644 --- a/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings +++ b/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings @@ -41,3 +41,4 @@ "tabContext.forkConversation.newWorkspace" = "新規ワークスペース"; "tabContext.disconnectRemote" = "SSH接続を解除"; "tabContext.remoteConnectedAccessibility" = "SSH接続中"; +"tab.close.accessibilityLabel" = "タブを閉じる"; diff --git a/Tests/BonsplitTests/TabBarHoveredTabResolverTests.swift b/Tests/BonsplitTests/TabBarHoveredTabResolverTests.swift new file mode 100644 index 00000000..01497b82 --- /dev/null +++ b/Tests/BonsplitTests/TabBarHoveredTabResolverTests.swift @@ -0,0 +1,60 @@ +import CoreGraphics +import Foundation +import Testing +@testable import Bonsplit + +@Suite("Tab strip hovered tab") +struct TabBarHoveredTabResolverTests { + private let resolver = TabBarHoveredTabResolver() + private let bounds = CGRect(x: 0, y: 0, width: 360, height: 34) + private let a = UUID() + private let b = UUID() + private let c = UUID() + + private func frames(_ ids: [UUID]) -> [UUID: CGRect] { + Dictionary(uniqueKeysWithValues: ids.enumerated().map { index, id in + (id, CGRect(x: CGFloat(index) * 100, y: 0, width: 100, height: 34)) + }) + } + + @Test("Closing the tab under a stationary pointer hovers the tab that slides in") + func stationaryPointerFollowsReflow() { + let pointer = CGPoint(x: 150, y: 17) + #expect(resolver.hoveredTabId(pointInView: pointer, barBounds: bounds, tabIds: [a, b, c], frames: frames([a, b, c])) == b) + + // b closed: c reflows into b's slot under the same pointer. + #expect(resolver.hoveredTabId(pointInView: pointer, barBounds: bounds, tabIds: [a, c], frames: frames([a, c])) == c) + } + + @Test("Only one tab can be hovered, and none past the last tab or outside the bar") + func singleHoverAndEmptySpace() { + #expect(resolver.hoveredTabId(pointInView: CGPoint(x: 330, y: 17), barBounds: bounds, tabIds: [a, b], frames: frames([a, b])) == nil) + #expect(resolver.hoveredTabId(pointInView: CGPoint(x: 50, y: 60), barBounds: bounds, tabIds: [a, b], frames: frames([a, b])) == nil) + #expect(resolver.hoveredTabId(pointInView: nil, barBounds: bounds, tabIds: [a, b], frames: frames([a, b])) == nil) + } + + @Test("A tab with no registered frame is never hovered") + func unregisteredTabIgnored() { + let pointer = CGPoint(x: 150, y: 17) + #expect(resolver.hoveredTabId(pointInView: pointer, barBounds: bounds, tabIds: [a, b], frames: frames([a])) == nil) + } + + @Test("A tab scrolled under the trailing action lane is not hovered from the lane") + func trailingActionLaneMasksTabs() { + let ids = [a, b, c, UUID()] + #expect(resolver.hoveredTabId( + pointInView: CGPoint(x: 330, y: 17), + barBounds: bounds, + tabIds: ids, + frames: frames(ids), + trailingObscuredWidth: 60 + ) == nil) + #expect(resolver.hoveredTabId( + pointInView: CGPoint(x: 290, y: 17), + barBounds: bounds, + tabIds: ids, + frames: frames(ids), + trailingObscuredWidth: 60 + ) == c) + } +}