diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 1799a8618ed3..6348551f4816 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -9571,25 +9571,80 @@ private final class SidebarTabItemSettingsStore: ObservableObject { } } -struct SidebarTabItemPresentationSnapshot: Equatable { - let tabId: UUID - let unreadCount: Int - let latestNotificationText: String? - let showsModifierShortcutHints: Bool +/// Transient sidebar drag/drop state, owned by `VerticalTabsSidebar` and passed +/// by reference into rows and drop delegates. `@Observable` gives per-property +/// tracking: writing `draggedTabId` or `dropIndicator` during drag invalidates +/// only the views that read those properties (the dragged row's opacity and the +/// drop-indicator overlays), never the sidebar body or the `LazyVStack` itself. +/// That invariant is what prevents the layout-invalidation loop that caused +/// https://github.com/manaflow-ai/cmux/issues/2586. +@MainActor +@Observable +final class SidebarDragState { + var draggedTabId: UUID? + var dropIndicator: SidebarDropIndicator? + + init() {} } -struct SidebarTabItemPresentationResolutionPolicy { +/// Per-row drop-indicator visibility, computed by the parent from value +/// inputs only. Takes UUIDs (not `Tab` objects or `SidebarDragState`) so it's +/// trivially unit-testable and the row's view subtree never reads the +/// `@Observable` store directly. Same predicate that used to live inside +/// `SidebarTabDropIndicatorOverlay`. +enum SidebarTabDropIndicatorPredicate { + static func topVisible( + forTabId tabId: UUID, + draggedTabId: UUID?, + dropIndicator: SidebarDropIndicator?, + tabIds: [UUID] + ) -> Bool { + guard draggedTabId != nil, let indicator = dropIndicator else { return false } + if indicator.tabId == tabId && indicator.edge == .top { + return true + } + guard indicator.edge == .bottom, + let currentIndex = tabIds.firstIndex(of: tabId), + currentIndex > 0 + else { + return false + } + return tabIds[currentIndex - 1] == indicator.tabId + } + + /// Convenience used by `SidebarEmptyArea`: the empty area's "top" indicator + /// (drawn above the empty space below all rows) is visible when the drop + /// indicator targets nothing (end-of-list) or the bottom edge of the last + /// row. + static func emptyAreaTopVisible( + draggedTabId: UUID?, + dropIndicator: SidebarDropIndicator?, + lastTabId: UUID? + ) -> Bool { + guard draggedTabId != nil, let indicator = dropIndicator else { return false } + if indicator.tabId == nil { + return true + } + guard indicator.edge == .bottom, let lastTabId else { return false } + return indicator.tabId == lastTabId + } +} + +/// Freezes `showsModifierShortcutHints` for the row whose context menu is open, +/// so pressing/releasing the modifier key while the menu is up does not flip +/// the underlying row's shortcut badges (which would be visible around the +/// open context menu). All other rows transition live. +enum SidebarShortcutHintFreezePolicy { static func resolved( - live: SidebarTabItemPresentationSnapshot, - frozen: SidebarTabItemPresentationSnapshot? - ) -> SidebarTabItemPresentationSnapshot { - guard let frozen, frozen.tabId == live.tabId else { return live } - return SidebarTabItemPresentationSnapshot( - tabId: live.tabId, - unreadCount: live.unreadCount, - latestNotificationText: live.latestNotificationText, - showsModifierShortcutHints: frozen.showsModifierShortcutHints - ) + live: Bool, + currentTabId: UUID, + frozenTabId: UUID?, + frozenValue: Bool + ) -> Bool { + if frozenTabId == currentTabId { + return frozenValue + } + return live } } @@ -9611,9 +9666,13 @@ struct VerticalTabsSidebar: View { @StateObject private var dragFailsafeMonitor = SidebarDragFailsafeMonitor() @StateObject private var tabItemSettingsStore = SidebarTabItemSettingsStore() @ObservedObject private var keyboardShortcutSettingsObserver = KeyboardShortcutSettingsObserver.shared - @State private var draggedTabId: UUID? - @State private var dropIndicator: SidebarDropIndicator? - @State private var frozenTabItemPresentation: SidebarTabItemPresentationSnapshot? + @State private var dragState = SidebarDragState() + // Freezes `showsModifierShortcutHints` for the workspace whose context menu + // is open. Set on the row's contextMenu.onAppear and cleared on + // .onDisappear so modifier-key transitions don't flip the badges on the + // row sitting behind the open menu. See `SidebarShortcutHintFreezePolicy`. + @State private var frozenShortcutHintsTabId: UUID? + @State private var frozenShortcutHintsValue: Bool = false @State private var terminalScrollBarVisibilityGeneration: UInt64 = 0 @State private var laidOutWorkspaceRowIds: Set = [] @State private var pendingSelectedWorkspaceScrollId: UUID? @@ -9634,6 +9693,52 @@ struct VerticalTabsSidebar: View { MinimalModeChromeMetrics.titlebarHeight } + /// Adapter binding for unmigrated consumers (extension sidebar drop + /// delegates, bonsplit overlays) that still expect @Binding. Reads + /// flow through `dragState.draggedTabId` so @Observable per-property + /// tracking still applies to whoever calls the binding's get. + private var draggedTabIdBinding: Binding { + Binding( + get: { dragState.draggedTabId }, + set: { dragState.draggedTabId = $0 } + ) + } + + /// Adapter binding mirroring `draggedTabIdBinding`. See its doc comment. + private var dropIndicatorBinding: Binding { + Binding( + get: { dragState.dropIndicator }, + set: { dragState.dropIndicator = $0 } + ) + } + + /// Computed in the parent so `SidebarEmptyArea` can render its top-edge + /// indicator from a value snapshot without holding a `SidebarDragState` + /// reference (snapshot-boundary rule). Delegates to a pure predicate so + /// the logic is unit-testable in isolation from view state. + private func emptyAreaTopDropIndicatorVisible() -> Bool { + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: dragState.draggedTabId, + dropIndicator: dragState.dropIndicator, + lastTabId: tabManager.tabs.last?.id + ) + } + + /// Constructs the drop delegate for the empty area in the parent scope, + /// so the child view receives a closure-bundle-equivalent value rather + /// than an `@Observable` store. + private func emptyAreaTabDropDelegate() -> SidebarTabDropDelegate { + SidebarTabDropDelegate( + targetTabId: nil, + tabManager: tabManager, + dragState: dragState, + selectedTabIds: $selectedTabIds, + lastSidebarSelectionIndex: $lastSidebarSelectionIndex, + targetRowHeight: nil, + dragAutoScrollController: dragAutoScrollController + ) + } + private var sidebarTopScrimHeight: CGFloat { SidebarWorkspaceListMetrics.topScrimHeight } @@ -9709,6 +9814,10 @@ struct VerticalTabsSidebar: View { private struct WorkspaceListRenderContext { let tabs: [Workspace] + /// Stored snapshot of `tabs.map(\.id)` so per-row predicates that need + /// it (e.g. `SidebarTabDropIndicatorPredicate.topVisible`) don't pay + /// O(n) per row. + let tabIds: [UUID] let workspaceCount: Int let canCloseWorkspace: Bool let workspaceNumberShortcut: StoredShortcut @@ -9720,9 +9829,7 @@ struct VerticalTabsSidebar: View { let allSelectedRemoteContextMenuTargetsDisconnected: Bool let workspaceTerminalScrollBarHiddenById: [UUID: Bool] - var workspaceIds: [UUID] { - tabs.map(\.id) - } + var workspaceIds: [UUID] { tabIds } } var body: some View { @@ -9750,6 +9857,7 @@ struct VerticalTabsSidebar: View { selectedRemoteContextMenuTargets.allSatisfy { $0.remoteConnectionState == .disconnected } let renderContext = WorkspaceListRenderContext( tabs: tabs, + tabIds: tabs.map(\.id), workspaceCount: workspaceCount, canCloseWorkspace: canCloseWorkspace, workspaceNumberShortcut: workspaceNumberShortcut, @@ -9784,8 +9892,8 @@ struct VerticalTabsSidebar: View { ) .onAppear { modifierKeyMonitor.start() - draggedTabId = nil - dropIndicator = nil + dragState.draggedTabId = nil + dragState.dropIndicator = nil SidebarDragLifecycleNotification.postStateDidChange( tabId: nil, reason: "sidebar_appear" @@ -9795,14 +9903,14 @@ struct VerticalTabsSidebar: View { modifierKeyMonitor.stop() dragAutoScrollController.stop() dragFailsafeMonitor.stop() - draggedTabId = nil - dropIndicator = nil + dragState.draggedTabId = nil + dragState.dropIndicator = nil SidebarDragLifecycleNotification.postStateDidChange( tabId: nil, reason: "sidebar_disappear" ) } - .onChange(of: draggedTabId) { newDraggedTabId in + .onChange(of: dragState.draggedTabId) { newDraggedTabId in SidebarDragLifecycleNotification.postStateDidChange( tabId: newDraggedTabId, reason: "drag_state_change" @@ -9818,20 +9926,21 @@ struct VerticalTabsSidebar: View { } dragFailsafeMonitor.stop() dragAutoScrollController.stop() - dropIndicator = nil - } - .onChange(of: tabs.map(\.id)) { tabIds in - guard let frozenTabItemPresentation, - !tabIds.contains(frozenTabItemPresentation.tabId) else { return } - self.frozenTabItemPresentation = nil + dragState.dropIndicator = nil } .onReceive(NotificationCenter.default.publisher(for: SidebarDragLifecycleNotification.requestClear)) { notification in - guard draggedTabId != nil else { return } + guard dragState.draggedTabId != nil || dragState.dropIndicator != nil else { return } let reason = SidebarDragLifecycleNotification.reason(from: notification) #if DEBUG - cmuxDebugLog("sidebar.dragClear tab=\(debugShortSidebarTabId(draggedTabId)) reason=\(reason)") + cmuxDebugLog("sidebar.dragClear tab=\(debugShortSidebarTabId(dragState.draggedTabId)) reason=\(reason)") #endif - draggedTabId = nil + dragState.draggedTabId = nil + dragState.dropIndicator = nil + } + .onChange(of: tabManager.tabs.map(\.id)) { tabIds in + guard let frozenTabId = frozenShortcutHintsTabId, + !tabIds.contains(frozenTabId) else { return } + frozenShortcutHintsTabId = nil } .onReceive( NotificationCenter.default.publisher(for: Workspace.terminalScrollBarHiddenDidChangeNotification) @@ -9890,19 +9999,18 @@ struct VerticalTabsSidebar: View { .background(TitlebarDoubleClickMonitorView()) } .overlay(alignment: .top) { - if draggedTabId != nil, let firstWorkspaceId = renderContext.workspaceIds.first { + if dragState.draggedTabId != nil, let firstWorkspaceId = renderContext.workspaceIds.first { Color.clear .contentShape(Rectangle()) .frame(height: scrollInsets.top + 8) .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: SidebarTabDropDelegate( targetTabId: firstWorkspaceId, tabManager: tabManager, - draggedTabId: $draggedTabId, + dragState: dragState, selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, targetRowHeight: nil, - dragAutoScrollController: dragAutoScrollController, - dropIndicator: $dropIndicator + dragAutoScrollController: dragAutoScrollController )) } } @@ -9994,8 +10102,9 @@ struct VerticalTabsSidebar: View { selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, dragAutoScrollController: dragAutoScrollController, - draggedTabId: $draggedTabId, - dropIndicator: $dropIndicator + topDropIndicatorVisible: emptyAreaTopDropIndicatorVisible(), + tabDropDelegate: emptyAreaTabDropDelegate(), + bonsplitDropIndicator: dropIndicatorBinding ) .frame(maxWidth: .infinity, minHeight: 48) } @@ -10254,8 +10363,8 @@ struct VerticalTabsSidebar: View { rowSpacing: tabRowSpacing, orderedRows: dropRows, dragAutoScrollController: dragAutoScrollController, - draggedTabId: $draggedTabId, - dropIndicator: $dropIndicator, + draggedTabId: draggedTabIdBinding, + dropIndicator: dropIndicatorBinding, onNewTab: onNewTab, onMove: { move in handleExtensionSidebarMutation(.moveWorkspace(move)) @@ -10345,20 +10454,20 @@ struct VerticalTabsSidebar: View { .buttonStyle(.plain) .frame(maxWidth: .infinity) .safeHelp(row.title) - .opacity(draggedTabId == row.workspaceId ? 0.55 : 1) + .opacity(dragState.draggedTabId == row.workspaceId ? 0.55 : 1) .onDrag { - draggedTabId = row.workspaceId - dropIndicator = nil + dragState.draggedTabId = row.workspaceId + dragState.dropIndicator = nil return SidebarTabDragPayload.provider(for: row.workspaceId) } .internalOnlyTabDrag() .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: ExtensionSidebarBrowserStackDropDelegate( targetWorkspaceId: row.workspaceId, orderedRows: dropRows, - draggedTabId: $draggedTabId, + draggedTabId: draggedTabIdBinding, targetRowHeight: targetRowHeight, dragAutoScrollController: dragAutoScrollController, - dropIndicator: $dropIndicator, + dropIndicator: dropIndicatorBinding, onMove: { move in handleExtensionSidebarMutation(.moveWorkspace(move)) } @@ -10424,20 +10533,20 @@ struct VerticalTabsSidebar: View { .contentShape(Rectangle()) } .buttonStyle(.plain) - .opacity(draggedTabId == row.workspaceId ? 0.55 : 1) + .opacity(dragState.draggedTabId == row.workspaceId ? 0.55 : 1) .onDrag { - draggedTabId = row.workspaceId - dropIndicator = nil + dragState.draggedTabId = row.workspaceId + dragState.dropIndicator = nil return SidebarTabDragPayload.provider(for: row.workspaceId) } .internalOnlyTabDrag() .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: ExtensionSidebarBrowserStackDropDelegate( targetWorkspaceId: row.workspaceId, orderedRows: dropRows, - draggedTabId: $draggedTabId, + draggedTabId: draggedTabIdBinding, targetRowHeight: targetRowHeight, dragAutoScrollController: dragAutoScrollController, - dropIndicator: $dropIndicator, + dropIndicator: dropIndicatorBinding, onMove: { move in handleExtensionSidebarMutation(.moveWorkspace(move)) } @@ -10468,7 +10577,7 @@ struct VerticalTabsSidebar: View { row: CmuxExtensionSidebarRenderRow, edge: SidebarDropEdge ) -> some View { - if dropIndicator == SidebarDropIndicator(tabId: row.workspaceId, edge: edge) { + if dragState.dropIndicator == SidebarDropIndicator(tabId: row.workspaceId, edge: edge) { Rectangle() .fill(cmuxAccentColor()) .frame(height: 2) @@ -10751,8 +10860,9 @@ struct VerticalTabsSidebar: View { selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, dragAutoScrollController: dragAutoScrollController, - draggedTabId: $draggedTabId, - dropIndicator: $dropIndicator + topDropIndicatorVisible: emptyAreaTopDropIndicatorVisible(), + tabDropDelegate: emptyAreaTabDropDelegate(), + bonsplitDropIndicator: dropIndicatorBinding ) .frame(maxWidth: .infinity, maxHeight: .infinity) } @@ -10760,9 +10870,11 @@ struct VerticalTabsSidebar: View { } private func workspaceRows(renderContext: WorkspaceListRenderContext) -> some View { - // Workspaces are bounded, so prefer a non-lazy stack here. - // LazyVStack + drag-state invalidations can recurse through layout. - VStack(spacing: tabRowSpacing) { + // LazyVStack is safe here because `dragState` is @Observable: + // drag mutations at 60fps invalidate only the rows/overlays that + // read them, never this sidebar body. See SidebarDragState and + // https://github.com/manaflow-ai/cmux/issues/2586. + LazyVStack(spacing: tabRowSpacing) { ForEach(renderContext.tabs, id: \.id) { tab in workspaceRow(tab, renderContext: renderContext) } @@ -10808,7 +10920,7 @@ struct VerticalTabsSidebar: View { }, selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, - dropIndicator: $dropIndicator, + dropIndicator: dropIndicatorBinding, updateAutoscroll: { dragAutoScrollController.updateFromDragLocation() }, @@ -10869,19 +10981,57 @@ struct VerticalTabsSidebar: View { return trimmed.isEmpty ? nil : trimmed }() let liveShowsModifierShortcutHints = modifierKeyMonitor.isModifierPressed - let livePresentation = SidebarTabItemPresentationSnapshot( - tabId: tab.id, - unreadCount: liveUnreadCount, - latestNotificationText: liveLatestNotificationText, - showsModifierShortcutHints: liveShowsModifierShortcutHints + let resolvedShowsModifierShortcutHints = SidebarShortcutHintFreezePolicy.resolved( + live: liveShowsModifierShortcutHints, + currentTabId: tab.id, + frozenTabId: frozenShortcutHintsTabId, + frozenValue: frozenShortcutHintsValue ) - let frozenPresentation = frozenTabItemPresentation?.tabId == tab.id - ? frozenTabItemPresentation - : nil - let resolvedPresentation = SidebarTabItemPresentationResolutionPolicy.resolved( - live: livePresentation, - frozen: frozenPresentation + let onContextMenuAppear: () -> Void = { [tabId = tab.id, snapshot = resolvedShowsModifierShortcutHints] in + frozenShortcutHintsTabId = tabId + frozenShortcutHintsValue = snapshot + } + let onContextMenuDisappear: () -> Void = { [tabId = tab.id] in + if frozenShortcutHintsTabId == tabId { + frozenShortcutHintsTabId = nil + } + } + + // Per-row drag/drop snapshots. Reading `dragState` here in the parent + // is intentional: the parent owns the @Observable store, and these + // value snapshots are what get passed to the row. The row's + // Equatable conformance ignores closures, so rows whose snapshot is + // unchanged skip re-render when drag state moves. + let isBeingDragged = dragState.draggedTabId == tab.id + let topDropIndicatorVisible = SidebarTabDropIndicatorPredicate.topVisible( + forTabId: tab.id, + draggedTabId: dragState.draggedTabId, + dropIndicator: dragState.dropIndicator, + tabIds: renderContext.tabIds ) + let onDragStart: () -> NSItemProvider = { [tabId = tab.id] in + #if DEBUG + cmuxDebugLog("sidebar.onDrag tab=\(tabId.uuidString.prefix(5))") + #endif + dragState.draggedTabId = tabId + dragState.dropIndicator = nil + return SidebarTabDragPayload.provider(for: tabId) + } + let tabDropDelegateFactory: (CGFloat) -> SidebarTabDropDelegate = { [ + tabId = tab.id, + selectedTabIds = $selectedTabIds, + lastSidebarSelectionIndex = $lastSidebarSelectionIndex + ] rowHeight in + SidebarTabDropDelegate( + targetTabId: tabId, + tabManager: tabManager, + dragState: dragState, + selectedTabIds: selectedTabIds, + lastSidebarSelectionIndex: lastSidebarSelectionIndex, + targetRowHeight: rowHeight, + dragAutoScrollController: dragAutoScrollController + ) + } return TabItemView( tabManager: tabManager, @@ -10896,16 +11046,18 @@ struct VerticalTabsSidebar: View { workspaceShortcutModifierSymbol: renderContext.workspaceNumberShortcut.numberedDigitHintPrefix, canCloseWorkspace: renderContext.canCloseWorkspace, accessibilityWorkspaceCount: renderContext.workspaceCount, - unreadCount: resolvedPresentation.unreadCount, - latestNotificationText: resolvedPresentation.latestNotificationText, + unreadCount: liveUnreadCount, + latestNotificationText: liveLatestNotificationText, rowSpacing: tabRowSpacing, setSelectionToTabs: { selection = .tabs }, selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, - showsModifierShortcutHints: resolvedPresentation.showsModifierShortcutHints, + showsModifierShortcutHints: resolvedShowsModifierShortcutHints, dragAutoScrollController: dragAutoScrollController, - draggedTabId: $draggedTabId, - dropIndicator: $dropIndicator, + isBeingDragged: isBeingDragged, + topDropIndicatorVisible: topDropIndicatorVisible, + onDragStart: onDragStart, + tabDropDelegateFactory: tabDropDelegateFactory, contextMenuWorkspaceIds: contextMenuWorkspaceIds, remoteContextMenuWorkspaceIds: remoteContextMenuWorkspaceIds, allRemoteContextMenuTargetsConnecting: allRemoteContextMenuTargetsConnecting, @@ -10913,8 +11065,8 @@ struct VerticalTabsSidebar: View { allContextMenuWorkspacesHideTerminalScrollBar: allContextMenuWorkspacesHideTerminalScrollBar, contextMenuPinState: contextMenuPinState, settings: renderContext.tabItemSettings, - livePresentation: livePresentation, - frozenPresentation: $frozenTabItemPresentation + onContextMenuAppear: onContextMenuAppear, + onContextMenuDisappear: onContextMenuDisappear ) .equatable() .id(tab.id) @@ -13125,8 +13277,11 @@ private struct SidebarEmptyArea: View { @Binding var selectedTabIds: Set @Binding var lastSidebarSelectionIndex: Int? let dragAutoScrollController: SidebarDragAutoScrollController - @Binding var draggedTabId: UUID? - @Binding var dropIndicator: SidebarDropIndicator? + // Value snapshot + closure bundles instead of an @Observable store + // reference (snapshot-boundary rule). + let topDropIndicatorVisible: Bool + let tabDropDelegate: SidebarTabDropDelegate + let bonsplitDropIndicator: Binding var body: some View { Color.clear @@ -13140,19 +13295,18 @@ private struct SidebarEmptyArea: View { } selection = .tabs } - .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: SidebarTabDropDelegate( - targetTabId: nil, - tabManager: tabManager, - draggedTabId: $draggedTabId, - selectedTabIds: $selectedTabIds, - lastSidebarSelectionIndex: $lastSidebarSelectionIndex, - targetRowHeight: nil, - dragAutoScrollController: dragAutoScrollController, - dropIndicator: $dropIndicator - )) - .overlay { SidebarBonsplitTabNewWorkspaceDropOverlay(tabManager: tabManager, selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, dropIndicator: $dropIndicator).frame(maxWidth: .infinity, maxHeight: .infinity) } + .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegate) + .overlay { + SidebarBonsplitTabNewWorkspaceDropOverlay( + tabManager: tabManager, + selectedTabIds: $selectedTabIds, + lastSidebarSelectionIndex: $lastSidebarSelectionIndex, + dropIndicator: bonsplitDropIndicator + ) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } .overlay(alignment: .top) { - if shouldShowTopDropIndicator { + if topDropIndicatorVisible { Rectangle() .fill(cmuxAccentColor()) .frame(height: 2) @@ -13161,15 +13315,6 @@ private struct SidebarEmptyArea: View { } } } - - private var shouldShowTopDropIndicator: Bool { - guard let indicator = dropIndicator else { return false } - if indicator.tabId == nil { - return true - } - guard indicator.edge == .bottom, let lastTabId = tabManager.tabs.last?.id else { return false } - return indicator.tabId == lastTabId - } } private struct ExtensionSidebarBrowserStackEmptyArea: View { @@ -13363,6 +13508,8 @@ private struct TabItemView: View, Equatable { lhs.allRemoteContextMenuTargetsDisconnected == rhs.allRemoteContextMenuTargetsDisconnected && lhs.allContextMenuWorkspacesHideTerminalScrollBar == rhs.allContextMenuWorkspacesHideTerminalScrollBar && lhs.contextMenuPinState == rhs.contextMenuPinState && + lhs.isBeingDragged == rhs.isBeingDragged && + lhs.topDropIndicatorVisible == rhs.topDropIndicatorVisible && lhs.settings == rhs.settings } @@ -13387,8 +13534,19 @@ private struct TabItemView: View, Equatable { @Binding var lastSidebarSelectionIndex: Int? let showsModifierShortcutHints: Bool let dragAutoScrollController: SidebarDragAutoScrollController - @Binding var draggedTabId: UUID? - @Binding var dropIndicator: SidebarDropIndicator? + // Row receives precomputed drag/drop snapshot values + action closures + // instead of an `@Observable` store reference. This keeps TabItemView in + // compliance with the snapshot-boundary rule for views under a LazyVStack + // (see CLAUDE.md). When drag state changes, the parent recomputes these + // per-row snapshots and `==` skips re-render for rows whose snapshot is + // unchanged. + let isBeingDragged: Bool + let topDropIndicatorVisible: Bool + let onDragStart: () -> NSItemProvider + /// Factory invoked from `body` with the row's measured `rowHeight`. Closure + /// captures the parent's `dragState`, so TabItemView itself never holds an + /// `@Observable` store reference (snapshot-boundary rule). + let tabDropDelegateFactory: (CGFloat) -> SidebarTabDropDelegate let contextMenuWorkspaceIds: [UUID] let remoteContextMenuWorkspaceIds: [UUID] let allRemoteContextMenuTargetsConnecting: Bool @@ -13396,8 +13554,12 @@ private struct TabItemView: View, Equatable { let allContextMenuWorkspacesHideTerminalScrollBar: Bool let contextMenuPinState: WorkspaceActionDispatcher.PinState? let settings: SidebarTabItemSettingsSnapshot - let livePresentation: SidebarTabItemPresentationSnapshot - @Binding var frozenPresentation: SidebarTabItemPresentationSnapshot? + /// Called from this row's contextMenu.onAppear so the parent can freeze + /// `showsModifierShortcutHints` to the value it last passed in. Prevents + /// modifier-key transitions from flipping the badges on the row sitting + /// behind the open context menu. + let onContextMenuAppear: () -> Void + let onContextMenuDisappear: () -> Void @State private var workspaceSnapshotStorage: SidebarWorkspaceSnapshotBuilder.Snapshot? @StateObject private var contextMenuState = SidebarTabItemContextMenuState() @State private var rowInteractionState = SidebarWorkspaceRowInteractionState() @@ -13409,10 +13571,6 @@ private struct TabItemView: View, Equatable { selectedTabIds.contains(tab.id) } - private var isBeingDragged: Bool { - draggedTabId == tab.id - } - private var sidebarShortcutHintXOffset: Double { settings.sidebarShortcutHintXOffset } @@ -13624,6 +13782,18 @@ private struct TabItemView: View, Equatable { } } + private var rowHeightProbe: some View { + GeometryReader { proxy in + Color.clear + .onAppear { + rowHeight = max(proxy.size.height, 1) + } + .onChange(of: proxy.size.height) { newHeight in + rowHeight = max(newHeight, 1) + } + } + } + @ViewBuilder private var remoteWorkspaceSection: some View { let workspaceSnapshot = self.workspaceSnapshot @@ -13998,17 +14168,7 @@ private struct TabItemView: View, Equatable { } .shortcutHintVisibilityAnimation(value: showsWorkspaceShortcutHint) .padding(.horizontal, 6) - .background { - GeometryReader { proxy in - Color.clear - .onAppear { - rowHeight = max(proxy.size.height, 1) - } - .onChange(of: proxy.size.height) { newHeight in - rowHeight = max(newHeight, 1) - } - } - } + .background { rowHeightProbe } .contentShape(Rectangle()) .opacity(isBeingDragged ? 0.6 : 1) .overlay { @@ -14023,7 +14183,7 @@ private struct TabItemView: View, Equatable { } } .overlay(alignment: .top) { - if showsCenteredTopDropIndicator { + if topDropIndicatorVisible { Rectangle() .fill(cmuxAccentColor()) .frame(height: 2) @@ -14084,25 +14244,9 @@ private struct TabItemView: View, Equatable { .onChange(of: settings) { _ in refreshWorkspaceSnapshot(force: true) } - .onDrag { - #if DEBUG - cmuxDebugLog("sidebar.onDrag tab=\(tab.id.uuidString.prefix(5))") - #endif - draggedTabId = tab.id - dropIndicator = nil - return SidebarTabDragPayload.provider(for: tab.id) - } + .onDrag(onDragStart) .internalOnlyTabDrag() - .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: SidebarTabDropDelegate( - targetTabId: tab.id, - tabManager: tabManager, - draggedTabId: $draggedTabId, - selectedTabIds: $selectedTabIds, - lastSidebarSelectionIndex: $lastSidebarSelectionIndex, - targetRowHeight: rowHeight, - dragAutoScrollController: dragAutoScrollController, - dropIndicator: $dropIndicator - )) + .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegateFactory(rowHeight)) .onDrop(of: BonsplitTabDragPayload.dropContentTypes, delegate: SidebarBonsplitTabDropDelegate( targetWorkspaceId: tab.id, tabManager: tabManager, @@ -14128,11 +14272,11 @@ private struct TabItemView: View, Equatable { rowInteractionState.contextMenuDidAppear() contextMenuState.hasDeferredWorkspaceObservationInvalidation = false contextMenuState.pendingWorkspaceSnapshot = nil - frozenPresentation = livePresentation + onContextMenuAppear() } .onDisappear { rowInteractionState.contextMenuDidDisappear() - frozenPresentation = nil + onContextMenuDisappear() flushDeferredWorkspaceObservationInvalidation() } } @@ -14488,21 +14632,6 @@ private struct TabItemView: View, Equatable { ) ?? NSColor(hex: hex) ?? .gray } - private var showsCenteredTopDropIndicator: Bool { - guard let indicator = dropIndicator else { return false } - if indicator.tabId == tab.id && indicator.edge == .top { - return true - } - - guard indicator.edge == .bottom, - let currentIndex = tabManager.tabs.firstIndex(where: { $0.id == tab.id }), - currentIndex > 0 - else { - return false - } - return tabManager.tabs[currentIndex - 1].id == indicator.tabId - } - private var accessibilityTitle: String { String(localized: "accessibility.workspacePosition", defaultValue: "\(workspaceSnapshot.title), workspace \(index + 1) of \(accessibilityWorkspaceCount)") } @@ -15805,19 +15934,19 @@ private struct SidebarBonsplitTabDropDelegate: DropDelegate { } } +@MainActor private struct SidebarTabDropDelegate: DropDelegate { let targetTabId: UUID? let tabManager: TabManager - @Binding var draggedTabId: UUID? + let dragState: SidebarDragState @Binding var selectedTabIds: Set @Binding var lastSidebarSelectionIndex: Int? let targetRowHeight: CGFloat? let dragAutoScrollController: SidebarDragAutoScrollController - @Binding var dropIndicator: SidebarDropIndicator? func validateDrop(info: DropInfo) -> Bool { let hasType = info.hasItemsConforming(to: [SidebarTabDragPayload.typeIdentifier]) - let hasDrag = draggedTabId != nil + let hasDrag = dragState.draggedTabId != nil #if DEBUG cmuxDebugLog("sidebar.validateDrop target=\(targetTabId?.uuidString.prefix(5) ?? "end") hasType=\(hasType) hasDrag=\(hasDrag)") #endif @@ -15836,8 +15965,8 @@ private struct SidebarTabDropDelegate: DropDelegate { #if DEBUG cmuxDebugLog("sidebar.dropExited target=\(targetTabId?.uuidString.prefix(5) ?? "end")") #endif - if dropIndicator?.tabId == targetTabId { - dropIndicator = nil + if dragState.dropIndicator?.tabId == targetTabId { + dragState.dropIndicator = nil } } @@ -15847,7 +15976,7 @@ private struct SidebarTabDropDelegate: DropDelegate { #if DEBUG cmuxDebugLog( "sidebar.dropUpdated target=\(targetTabId?.uuidString.prefix(5) ?? "end") " + - "indicator=\(debugIndicator(dropIndicator))" + "indicator=\(debugIndicator(dragState.dropIndicator))" ) #endif return DropProposal(operation: .move) @@ -15855,14 +15984,14 @@ private struct SidebarTabDropDelegate: DropDelegate { func performDrop(info: DropInfo) -> Bool { defer { - draggedTabId = nil - dropIndicator = nil + dragState.draggedTabId = nil + dragState.dropIndicator = nil dragAutoScrollController.stop() } #if DEBUG cmuxDebugLog("sidebar.drop target=\(targetTabId?.uuidString.prefix(5) ?? "end")") #endif - guard let draggedTabId else { + guard let draggedTabId = dragState.draggedTabId else { #if DEBUG cmuxDebugLog("sidebar.drop.abort reason=missingDraggedTab") #endif @@ -15878,14 +16007,14 @@ private struct SidebarTabDropDelegate: DropDelegate { guard let targetIndex = SidebarDropPlanner.targetIndex( draggedTabId: draggedTabId, targetTabId: targetTabId, - indicator: dropIndicator, + indicator: dragState.dropIndicator, tabIds: tabIds, pinnedTabIds: Set(tabManager.tabs.filter(\.isPinned).map(\.id)) ) else { #if DEBUG cmuxDebugLog( "sidebar.drop.abort reason=noTargetIndex tab=\(draggedTabId.uuidString.prefix(5)) " + - "target=\(targetTabId?.uuidString.prefix(5) ?? "end") indicator=\(debugIndicator(dropIndicator))" + "target=\(targetTabId?.uuidString.prefix(5) ?? "end") indicator=\(debugIndicator(dragState.dropIndicator))" ) #endif return false @@ -15917,15 +16046,15 @@ private struct SidebarTabDropDelegate: DropDelegate { let tabIds = tabManager.tabs.map(\.id) let pinnedTabIds = Set(tabManager.tabs.filter(\.isPinned).map(\.id)) let nextIndicator = SidebarDropPlanner.indicator( - draggedTabId: draggedTabId, + draggedTabId: dragState.draggedTabId, targetTabId: targetTabId, tabIds: tabIds, pinnedTabIds: pinnedTabIds, pointerY: targetTabId == nil ? nil : info.location.y, targetHeight: targetRowHeight ) - guard dropIndicator != nextIndicator else { return } - dropIndicator = nextIndicator + guard dragState.dropIndicator != nextIndicator else { return } + dragState.dropIndicator = nextIndicator } private func syncSidebarSelection(preferredSelectedTabId: UUID? = nil) { diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index de0aae8e75b8..7ceec9d32063 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -97,6 +97,7 @@ EA1F00000000000000000003 /* SidebarDirectoryText.swift in Sources */ = {isa = PBXBuildFile; fileRef = EA1F00000000000000000004 /* SidebarDirectoryText.swift */; }; D7AB34300000000000000003 /* SidebarBonsplitTabWorkspaceDropOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */; }; D7AB34300000000000000005 /* SidebarWorkspaceDropPlannerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */; }; + D7AB34300000000000000105 /* SidebarTabDropIndicatorPredicateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000106 /* SidebarTabDropIndicatorPredicateTests.swift */; }; D7AB34400000000000000001 /* WorkspaceSurfaceConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34400000000000000002 /* WorkspaceSurfaceConfig.swift */; }; D7AB34400000000000000003 /* GhosttyTerminalViewVisibilityPolicyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34400000000000000004 /* GhosttyTerminalViewVisibilityPolicyTests.swift */; }; F4350A110000000000000001 /* AppBundleIconPersistencePolicy.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4350A130000000000000001 /* AppBundleIconPersistencePolicy.swift */; }; @@ -625,6 +626,7 @@ 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketSecurityTests.swift; sourceTree = ""; }; 51D800000000000000000002 /* SidebarIdentifierFormattingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarIdentifierFormattingTests.swift; sourceTree = ""; }; D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarWorkspaceDropPlannerTests.swift; sourceTree = ""; }; + D7AB34300000000000000106 /* SidebarTabDropIndicatorPredicateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarTabDropIndicatorPredicateTests.swift; sourceTree = ""; }; D7AB34400000000000000004 /* GhosttyTerminalViewVisibilityPolicyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GhosttyTerminalViewVisibilityPolicyTests.swift; sourceTree = ""; }; 9C1BEA3D2E6F49709A71C021 /* TerminalControllerSocketWriteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketWriteTests.swift; sourceTree = ""; }; E7E000000000000000000004 /* CmuxEventBusTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CmuxEventBusTests.swift; sourceTree = ""; }; @@ -1669,6 +1671,7 @@ C7A509000000000000000001 /* CmuxTopSnapshotScopeTests.swift */, C7A50C000000000000000001 /* CmuxTopProcessCPUTests.swift */, D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */, + D7AB34300000000000000106 /* SidebarTabDropIndicatorPredicateTests.swift */, C3677001000000000000002 /* CmuxSSHURLRequestTests.swift */, 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */, 9C1BEA3D2E6F49709A71C021 /* TerminalControllerSocketWriteTests.swift */, @@ -2468,6 +2471,7 @@ C7A509000000000000000002 /* CmuxTopSnapshotScopeTests.swift in Sources */, C7A50C000000000000000002 /* CmuxTopProcessCPUTests.swift in Sources */, D7AB34300000000000000005 /* SidebarWorkspaceDropPlannerTests.swift in Sources */, + D7AB34300000000000000105 /* SidebarTabDropIndicatorPredicateTests.swift in Sources */, C3677001000000000000001 /* CmuxSSHURLRequestTests.swift in Sources */, 8C4BBF2DEF6DF93F395A9EE7 /* TerminalControllerSocketSecurityTests.swift in Sources */, 9C1BEA3D2E6F49709A71C020 /* TerminalControllerSocketWriteTests.swift in Sources */, diff --git a/cmuxTests/SidebarTabDropIndicatorPredicateTests.swift b/cmuxTests/SidebarTabDropIndicatorPredicateTests.swift new file mode 100644 index 000000000000..b25ed1b75154 --- /dev/null +++ b/cmuxTests/SidebarTabDropIndicatorPredicateTests.swift @@ -0,0 +1,320 @@ +import XCTest + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +/// Tests for `SidebarTabDropIndicatorPredicate.topVisible(forTabId:draggedTabId:dropIndicator:tabIds:)`. +/// +/// This predicate is the snapshot the parent computes for each sidebar row to +/// decide whether to draw the drop-line indicator above it. Lifting it out of +/// the row view subtree (per the snapshot-boundary rule) makes it a pure +/// function — these tests cover the resulting branches end-to-end. +final class SidebarTabDropIndicatorPredicateTopVisibleTests: XCTestCase { + func testReturnsFalseWhenNoDragInProgress() { + let rowId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: rowId, + draggedTabId: nil, + dropIndicator: SidebarDropIndicator(tabId: rowId, edge: .top), + tabIds: [rowId] + ), + "An indicator value alone shouldn't trigger the overlay; a drag must be in flight." + ) + } + + func testReturnsFalseWhenNoIndicator() { + let rowId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: rowId, + draggedTabId: rowId, + dropIndicator: nil, + tabIds: [rowId] + ) + ) + } + + func testReturnsTrueWhenIndicatorTargetsThisRowTopEdge() { + let rowId = UUID() + let draggedId = UUID() + XCTAssertTrue( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: rowId, + draggedTabId: draggedId, + dropIndicator: SidebarDropIndicator(tabId: rowId, edge: .top), + tabIds: [rowId, draggedId] + ) + ) + } + + func testReturnsFalseWhenIndicatorTargetsThisRowBottomEdge() { + let rowId = UUID() + let draggedId = UUID() + // A .bottom indicator on this row paints the indicator above the *next* + // row, not above this one. + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: rowId, + draggedTabId: draggedId, + dropIndicator: SidebarDropIndicator(tabId: rowId, edge: .bottom), + tabIds: [rowId, draggedId] + ) + ) + } + + func testReturnsTrueWhenIndicatorTargetsPreviousRowBottomEdge() { + let firstId = UUID() + let middleId = UUID() + let draggedId = UUID() + // The visual indicator for "insert between row 0 and row 1" is drawn + // above row 1, even though the indicator semantically points at row 0 + // with .bottom. + XCTAssertTrue( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: middleId, + draggedTabId: draggedId, + dropIndicator: SidebarDropIndicator(tabId: firstId, edge: .bottom), + tabIds: [firstId, middleId, draggedId] + ) + ) + } + + func testReturnsFalseWhenIndicatorTargetsUnrelatedRow() { + let rowId = UUID() + let otherId = UUID() + let draggedId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: rowId, + draggedTabId: draggedId, + dropIndicator: SidebarDropIndicator(tabId: otherId, edge: .top), + tabIds: [rowId, otherId, draggedId] + ) + ) + } + + func testReturnsFalseForFirstRowWithBottomIndicatorAboveIt() { + // The first row has no previous neighbor — a .bottom indicator from a + // hypothetical previous row can't apply. + let firstId = UUID() + let draggedId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: firstId, + draggedTabId: draggedId, + dropIndicator: SidebarDropIndicator(tabId: UUID(), edge: .bottom), + tabIds: [firstId, draggedId] + ) + ) + } + + func testReturnsFalseWhenRowIsNotInTabsList() { + // Defensive: if the row id isn't in tabIds (stale snapshot), the + // predicate should return false rather than crashing on the lookup. + let strayId = UUID() + let draggedId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.topVisible( + forTabId: strayId, + draggedTabId: draggedId, + dropIndicator: SidebarDropIndicator(tabId: UUID(), edge: .bottom), + tabIds: [UUID(), draggedId] + ) + ) + } +} + +/// Tests for `SidebarTabDropIndicatorPredicate.emptyAreaTopVisible(...)`. +/// The "empty area" sits below the workspace list and shows an indicator when +/// the drop will append at the end of the list. +final class SidebarTabDropIndicatorPredicateEmptyAreaTests: XCTestCase { + func testReturnsFalseWhenNoDragInProgress() { + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: nil, + dropIndicator: SidebarDropIndicator(tabId: nil, edge: .top), + lastTabId: UUID() + ) + ) + } + + func testReturnsFalseWhenNoIndicator() { + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: UUID(), + dropIndicator: nil, + lastTabId: UUID() + ) + ) + } + + func testReturnsTrueWhenIndicatorTargetsEndOfList() { + // tabId == nil means "after the last row" — the empty area shows the + // indicator regardless of which row was last. + XCTAssertTrue( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: UUID(), + dropIndicator: SidebarDropIndicator(tabId: nil, edge: .top), + lastTabId: UUID() + ) + ) + } + + func testReturnsTrueWhenIndicatorTargetsLastRowBottomEdge() { + let lastId = UUID() + XCTAssertTrue( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: UUID(), + dropIndicator: SidebarDropIndicator(tabId: lastId, edge: .bottom), + lastTabId: lastId + ) + ) + } + + func testReturnsFalseWhenIndicatorTargetsLastRowTopEdge() { + // A .top indicator on the last row draws the line *above* the last + // row, not below — so the empty area below it should stay clear. + let lastId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: UUID(), + dropIndicator: SidebarDropIndicator(tabId: lastId, edge: .top), + lastTabId: lastId + ) + ) + } + + func testReturnsFalseWhenIndicatorTargetsNonLastRowBottomEdge() { + let middleId = UUID() + let lastId = UUID() + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: UUID(), + dropIndicator: SidebarDropIndicator(tabId: middleId, edge: .bottom), + lastTabId: lastId + ) + ) + } + + func testReturnsFalseWhenListIsEmpty() { + XCTAssertFalse( + SidebarTabDropIndicatorPredicate.emptyAreaTopVisible( + draggedTabId: UUID(), + dropIndicator: SidebarDropIndicator(tabId: UUID(), edge: .bottom), + lastTabId: nil + ) + ) + } +} + +/// Tests for `SidebarDragState` (the @MainActor @Observable bag that owns +/// the per-window drag transient state). +@MainActor +final class SidebarDragStateTests: XCTestCase { + func testInitialStateIsCleared() { + let state = SidebarDragState() + XCTAssertNil(state.draggedTabId) + XCTAssertNil(state.dropIndicator) + } + + func testIndependentMutationOfEachProperty() { + // Per-property invariant the PR depends on: writes to one field must + // not silently disturb the other. Verifies the @Observable container + // doesn't enforce coupled updates. + let state = SidebarDragState() + let tabId = UUID() + let indicator = SidebarDropIndicator(tabId: tabId, edge: .top) + + state.draggedTabId = tabId + XCTAssertEqual(state.draggedTabId, tabId) + XCTAssertNil(state.dropIndicator) + + state.dropIndicator = indicator + XCTAssertEqual(state.draggedTabId, tabId) + XCTAssertEqual(state.dropIndicator, indicator) + + state.draggedTabId = nil + XCTAssertNil(state.draggedTabId) + XCTAssertEqual(state.dropIndicator, indicator) + } + + func testClearingBothLeavesStateIdle() { + // Mirror the `requestClear` notification handler: both fields go to + // nil and the state is back to its initial shape. + let state = SidebarDragState() + state.draggedTabId = UUID() + state.dropIndicator = SidebarDropIndicator(tabId: UUID(), edge: .bottom) + + state.draggedTabId = nil + state.dropIndicator = nil + + XCTAssertNil(state.draggedTabId) + XCTAssertNil(state.dropIndicator) + } +} + +/// Covers the freeze policy that holds `showsModifierShortcutHints` stable +/// for the row whose context menu is open. Without it, pressing/releasing +/// the modifier key while a context menu is up would flip badges on the row +/// sitting behind the menu (visual regression flagged on the lazy-sidebar PR). +final class SidebarShortcutHintFreezePolicyTests: XCTestCase { + func testReturnsLiveWhenNoRowIsFrozen() { + let rowId = UUID() + XCTAssertTrue( + SidebarShortcutHintFreezePolicy.resolved( + live: true, + currentTabId: rowId, + frozenTabId: nil, + frozenValue: false + ) + ) + XCTAssertFalse( + SidebarShortcutHintFreezePolicy.resolved( + live: false, + currentTabId: rowId, + frozenTabId: nil, + frozenValue: true + ) + ) + } + + func testReturnsFrozenWhenCurrentTabMatchesFrozenTab() { + let rowId = UUID() + XCTAssertFalse( + SidebarShortcutHintFreezePolicy.resolved( + live: true, + currentTabId: rowId, + frozenTabId: rowId, + frozenValue: false + ), + "When this row is frozen, the modifier flipping live should not surface." + ) + XCTAssertTrue( + SidebarShortcutHintFreezePolicy.resolved( + live: false, + currentTabId: rowId, + frozenTabId: rowId, + frozenValue: true + ), + "Frozen-true must remain true even after the modifier is released." + ) + } + + func testReturnsLiveForRowsOtherThanTheFrozenOne() { + let frozenRow = UUID() + let otherRow = UUID() + XCTAssertTrue( + SidebarShortcutHintFreezePolicy.resolved( + live: true, + currentTabId: otherRow, + frozenTabId: frozenRow, + frozenValue: false + ), + "Freeze is per-row; only the row whose menu is open should be pinned." + ) + } +} diff --git a/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift b/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift index 3dc064cadb9c..3bf9715051d4 100644 --- a/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift +++ b/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift @@ -208,73 +208,6 @@ final class SidebarSelectedWorkspaceScrollPolicyTests: XCTestCase { } } -final class SidebarTabItemPresentationResolutionPolicyTests: XCTestCase { - func testFrozenContextMenuPresentationDoesNotSuppressLiveNotificationState() { - let tabId = UUID() - let frozen = SidebarTabItemPresentationSnapshot( - tabId: tabId, - unreadCount: 0, - latestNotificationText: nil, - showsModifierShortcutHints: true - ) - let live = SidebarTabItemPresentationSnapshot( - tabId: tabId, - unreadCount: 1, - latestNotificationText: "done", - showsModifierShortcutHints: false - ) - - let resolved = SidebarTabItemPresentationResolutionPolicy.resolved( - live: live, - frozen: frozen - ) - - XCTAssertEqual(resolved.unreadCount, 1) - XCTAssertEqual(resolved.latestNotificationText, "done") - XCTAssertTrue(resolved.showsModifierShortcutHints) - } - - func testNoFrozenPresentationUsesLiveSnapshot() { - let live = SidebarTabItemPresentationSnapshot( - tabId: UUID(), - unreadCount: 2, - latestNotificationText: "live", - showsModifierShortcutHints: true - ) - - let resolved = SidebarTabItemPresentationResolutionPolicy.resolved( - live: live, - frozen: nil - ) - - XCTAssertEqual(resolved, live) - } - - func testNonMatchingTabIdUsesLiveShortcutHints() { - let frozen = SidebarTabItemPresentationSnapshot( - tabId: UUID(), - unreadCount: 0, - latestNotificationText: nil, - showsModifierShortcutHints: true - ) - let live = SidebarTabItemPresentationSnapshot( - tabId: UUID(), - unreadCount: 1, - latestNotificationText: "done", - showsModifierShortcutHints: false - ) - - let resolved = SidebarTabItemPresentationResolutionPolicy.resolved( - live: live, - frozen: frozen - ) - - XCTAssertEqual(resolved.unreadCount, 1) - XCTAssertEqual(resolved.latestNotificationText, "done") - XCTAssertFalse(resolved.showsModifierShortcutHints) - } -} - final class SidebarWorkspaceRowInteractionStateTests: XCTestCase { func testHoverRevealIsIndependentFromStaleContextMenuVisibility() { var state = SidebarWorkspaceRowInteractionState()