diff --git a/Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Navigation/WorkspaceAdjacentSelection.swift b/Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Navigation/WorkspaceAdjacentSelection.swift new file mode 100644 index 000000000000..d68fd644e5f2 --- /dev/null +++ b/Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Navigation/WorkspaceAdjacentSelection.swift @@ -0,0 +1,27 @@ +public import Foundation + +/// Resolves the target of a next/previous workspace cycling step over the +/// rows the sidebar shows. +public enum WorkspaceAdjacentSelection { + /// The workspace to select when stepping from `current` by `step` + /// (+1 next, -1 previous), wrapping at the edges of `stops`. + /// + /// - Parameters: + /// - stops: Visible row workspace ids in sidebar order. + /// - current: The selected workspace id. + /// - fallbackStop: Stop standing in for `current` when it is not a + /// stop itself (a hidden member's group header). + /// - Returns: The target stop, or nil when there is no position to step + /// from. The target may equal `current` when it is the only stop. + public static func target( + stops: [UUID], + current: UUID, + fallbackStop: UUID? = nil, + step: Int + ) -> UUID? { + guard !stops.isEmpty else { return nil } + guard let index = stops.firstIndex(of: current) + ?? fallbackStop.flatMap({ stops.firstIndex(of: $0) }) else { return nil } + return stops[(index + step + stops.count) % stops.count] + } +} diff --git a/Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Navigation/WorkspaceAdjacentSelectionTests.swift b/Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Navigation/WorkspaceAdjacentSelectionTests.swift new file mode 100644 index 000000000000..24a6b96dc84c --- /dev/null +++ b/Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Navigation/WorkspaceAdjacentSelectionTests.swift @@ -0,0 +1,42 @@ +import Foundation +import Testing +@testable import CmuxWorkspaces + +@Suite("Workspace adjacent selection") +struct WorkspaceAdjacentSelectionTests { + let a = UUID() + let b = UUID() + let c = UUID() + let hidden = UUID() + + @Test func stepsForwardAndWrapsAtTheEnd() { + let stops = [a, b, c] + #expect(WorkspaceAdjacentSelection.target(stops: stops, current: a, step: 1) == b) + #expect(WorkspaceAdjacentSelection.target(stops: stops, current: c, step: 1) == a) + } + + @Test func stepsBackwardAndWrapsAtTheStart() { + let stops = [a, b, c] + #expect(WorkspaceAdjacentSelection.target(stops: stops, current: b, step: -1) == a) + #expect(WorkspaceAdjacentSelection.target(stops: stops, current: a, step: -1) == c) + } + + @Test func hiddenCurrentStepsFromItsFallbackStop() { + let stops = [a, b, c] + #expect(WorkspaceAdjacentSelection.target( + stops: stops, current: hidden, fallbackStop: b, step: 1 + ) == c) + #expect(WorkspaceAdjacentSelection.target( + stops: stops, current: hidden, fallbackStop: b, step: -1 + ) == a) + } + + @Test func singleStopReturnsItself() { + #expect(WorkspaceAdjacentSelection.target(stops: [a], current: a, step: 1) == a) + } + + @Test func unresolvableCurrentReturnsNil() { + #expect(WorkspaceAdjacentSelection.target(stops: [a], current: hidden, step: 1) == nil) + #expect(WorkspaceAdjacentSelection.target(stops: [], current: a, step: 1) == nil) + } +} diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 7447846e2d85..ca10c835372b 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -3614,39 +3614,51 @@ class TabManager: ObservableObject { } func selectNextTab() { - guard let currentId = selectedTabId, - let currentIndex = tabs.firstIndex(where: { $0.id == currentId }) else { return } - let nextIndex = (currentIndex + 1) % tabs.count + selectAdjacentTab(step: 1, debugLabel: "next") + } + + func selectPreviousTab() { + selectAdjacentTab(step: -1, debugLabel: "prev") + } + + /// Cycle selection through the rows the sidebar shows, wrapping at the + /// edges. Stops come from `SidebarWorkspaceRenderItem.renderItems`, so a + /// collapsed group is one stop (its header) and its hidden members are + /// skipped. Previously cycling visited every workspace in `tabs` order, + /// which expanded collapsed groups as a side effect. + private func selectAdjacentTab(step: Int, debugLabel: String) { + guard let currentId = selectedTabId else { return } + let groupsById = Dictionary( + workspaceGroups.map { ($0.id, $0) }, + uniquingKeysWith: { first, _ in first } + ) + let stops = SidebarWorkspaceRenderItem + .renderItems(tabs: tabs, groupsById: groupsById) + .map(\.rowWorkspaceId) + // A hidden selection (member of a collapsed group) steps from its header. + let fallbackStop = tabs.first(where: { $0.id == currentId })? + .groupId.flatMap { groupsById[$0]?.anchorWorkspaceId } + guard let target = WorkspaceAdjacentSelection.target( + stops: stops, + current: currentId, + fallbackStop: fallbackStop, + step: step + ) else { return } #if DEBUG - let nextId = tabs[nextIndex].id - debugPrepareWorkspaceSwitch("next", from: currentId, to: nextId) + if target != currentId { + debugPrepareWorkspaceSwitch(debugLabel, from: currentId, to: target) + } #endif activateWorkspaceCycleHotWindow() selectWorkspaceId( - tabs[nextIndex].id, + target, notificationDismissalContext: .explicitWorkspaceResume ) // Keyboard nav is an explicit "focus one workspace" gesture, so drop // any stale sidebar multi-selection (Shift-click range) so subsequent // batch actions don't operate on workspaces the user thought they // had unselected by moving on. - clearSidebarMultiSelection(except: tabs[nextIndex].id) - } - - func selectPreviousTab() { - guard let currentId = selectedTabId, - let currentIndex = tabs.firstIndex(where: { $0.id == currentId }) else { return } - let prevIndex = (currentIndex - 1 + tabs.count) % tabs.count -#if DEBUG - let prevId = tabs[prevIndex].id - debugPrepareWorkspaceSwitch("prev", from: currentId, to: prevId) -#endif - activateWorkspaceCycleHotWindow() - selectWorkspaceId( - tabs[prevIndex].id, - notificationDismissalContext: .explicitWorkspaceResume - ) - clearSidebarMultiSelection(except: tabs[prevIndex].id) + clearSidebarMultiSelection(except: target) } /// Reduce sidebar multi-selection to a single workspace (or clear if diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index d4f659424708..cc0ffd83fda1 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -2528,6 +2528,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources C0DE36230000000000000001 /* WorkspaceFinderDirectoryResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE36230000000000000002 /* WorkspaceFinderDirectoryResolver.swift */; }; F0ACC0DE000000000000000B /* WorkspaceForkAgentConversationAvailability.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ACC0DE000000000000000C /* WorkspaceForkAgentConversationAvailability.swift */; }; F0ACC0DE0000000000000005 /* WorkspaceForkConversationContextMenuTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ACC0DE0000000000000006 /* WorkspaceForkConversationContextMenuTests.swift */; }; + C9A57031C9A57031C9A57031 /* WorkspaceGroupKeyboardCycleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57032C9A57032C9A57032 /* WorkspaceGroupKeyboardCycleTests.swift */; }; C9A57203C9A57203C9A57203 /* WorkspaceGroupMenuSnapshot.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57204C9A57204C9A57204 /* WorkspaceGroupMenuSnapshot.swift */; }; C9A57206C9A57206C9A57206 /* WorkspaceGroupMoveToMenuState.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57205C9A57205C9A57205 /* WorkspaceGroupMoveToMenuState.swift */; }; C9A57401C9A57401C9A57401 /* WorkspaceGroupMoveToMenuStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57402C9A57402C9A57402 /* WorkspaceGroupMoveToMenuStateTests.swift */; }; @@ -5132,6 +5133,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = C0DE36230000000000000002 /* WorkspaceFinderDirectoryResolver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceFinderDirectoryResolver.swift; sourceTree = ""; }; F0ACC0DE000000000000000C /* WorkspaceForkAgentConversationAvailability.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceForkAgentConversationAvailability.swift; sourceTree = ""; }; F0ACC0DE0000000000000006 /* WorkspaceForkConversationContextMenuTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceForkConversationContextMenuTests.swift; sourceTree = ""; }; + C9A57032C9A57032C9A57032 /* WorkspaceGroupKeyboardCycleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupKeyboardCycleTests.swift; sourceTree = ""; }; C9A57204C9A57204C9A57204 /* WorkspaceGroupMenuSnapshot.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupMenuSnapshot.swift; sourceTree = ""; }; C9A57205C9A57205C9A57205 /* WorkspaceGroupMoveToMenuState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupMoveToMenuState.swift; sourceTree = ""; }; C9A57402C9A57402C9A57402 /* WorkspaceGroupMoveToMenuStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupMoveToMenuStateTests.swift; sourceTree = ""; }; @@ -7815,6 +7817,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = 604500100000000000000004 /* WindowTitleTemplateTests.swift */, D1FFC0DE000000000000C001 /* DiffCommentStoreTests.swift */, C9A57002C9A57002C9A57002 /* WorkspaceGroupTests.swift */, + C9A57032C9A57032C9A57032 /* WorkspaceGroupKeyboardCycleTests.swift */, 9164A0029164A0029164A002 /* WorkspaceGroupNumberedSelectionTests.swift */, C9A57402C9A57402C9A57402 /* WorkspaceGroupMoveToMenuStateTests.swift */, FEED49850000000000000002 /* FeedEventClassificationTests.swift */, @@ -10997,6 +11000,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = 281F3AEA52379AF45C5C1189 /* WorkspaceCustomSidebarPullRequestContextTests.swift in Sources */, 72DB1838A0F1B710BFB45DC1 /* WorkspaceEnvironmentTests.swift in Sources */, F0ACC0DE0000000000000005 /* WorkspaceForkConversationContextMenuTests.swift in Sources */, + C9A57031C9A57031C9A57031 /* WorkspaceGroupKeyboardCycleTests.swift in Sources */, C9A57401C9A57401C9A57401 /* WorkspaceGroupMoveToMenuStateTests.swift in Sources */, 9164A0019164A0019164A001 /* WorkspaceGroupNumberedSelectionTests.swift in Sources */, C9A57001C9A57001C9A57001 /* WorkspaceGroupTests.swift in Sources */, diff --git a/cmuxTests/WorkspaceGroupKeyboardCycleTests.swift b/cmuxTests/WorkspaceGroupKeyboardCycleTests.swift new file mode 100644 index 000000000000..8d0f946d5128 --- /dev/null +++ b/cmuxTests/WorkspaceGroupKeyboardCycleTests.swift @@ -0,0 +1,130 @@ +import Foundation +import Testing + +import CmuxFoundation +import CmuxSettings + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +@MainActor +@Suite("Workspace group keyboard cycling", .serialized) +struct WorkspaceGroupKeyboardCycleTests { + + private func makeTabManager() -> TabManager { + let suiteName = "cmux.workspace-group-keyboard-cycle-tests.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defaults.removePersistentDomain(forName: suiteName) + let manager = TabManager( + autoWelcomeIfNeeded: false, + settings: UserDefaultsSettingsClient(defaults: defaults), + closeTabWarningDefaults: defaults + ) + manager.addWorkspace(autoWelcomeIfNeeded: false) + return manager + } + + private func select(_ workspaceId: UUID, in manager: TabManager) throws { + manager.selectWorkspace(try #require(manager.tabs.first { $0.id == workspaceId })) + } + + /// One ungrouped workspace plus a group with one member. + private func makeGroupedFixture() throws -> (TabManager, UUID, UUID, UUID, UUID) { + let manager = makeTabManager() + let ungroupedId = try #require(manager.selectedTabId) + let member = manager.addTab(select: false) + let groupId = try #require(manager.createWorkspaceGroup( + name: "Grouped", + childWorkspaceIds: [member.id] + )) + let group = try #require(manager.workspaceGroups.first { $0.id == groupId }) + return (manager, ungroupedId, groupId, group.anchorWorkspaceId, member.id) + } + + @Test func collapsedGroupIsASingleStopAndStaysCollapsed() throws { + let (manager, ungroupedId, groupId, anchorId, _) = try makeGroupedFixture() + manager.setWorkspaceGroupCollapsed(groupId: groupId, isCollapsed: true) + try select(ungroupedId, in: manager) + + manager.selectNextTab() + #expect(manager.selectedTabId == anchorId) + + manager.selectNextTab() + #expect(manager.selectedTabId == ungroupedId) + + let group = try #require(manager.workspaceGroups.first { $0.id == groupId }) + #expect(group.isCollapsed) + } + + @Test func expandedGroupTraversesItsMembers() throws { + let (manager, ungroupedId, _, anchorId, memberId) = try makeGroupedFixture() + try select(ungroupedId, in: manager) + + manager.selectNextTab() + #expect(manager.selectedTabId == anchorId) + + manager.selectNextTab() + #expect(manager.selectedTabId == memberId) + + manager.selectNextTab() + #expect(manager.selectedTabId == ungroupedId) + } + + @Test func previousDirectionTraversesExpandedMembersInReverse() throws { + // Three visible stops, so previous is distinguishable from next. + let (manager, ungroupedId, _, anchorId, memberId) = try makeGroupedFixture() + try select(ungroupedId, in: manager) + + manager.selectPreviousTab() + #expect(manager.selectedTabId == memberId) + + manager.selectPreviousTab() + #expect(manager.selectedTabId == anchorId) + + manager.selectPreviousTab() + #expect(manager.selectedTabId == ungroupedId) + } + + @Test func previousDirectionSkipsHiddenMembersOfACollapsedGroup() throws { + let (manager, ungroupedId, groupId, anchorId, _) = try makeGroupedFixture() + manager.setWorkspaceGroupCollapsed(groupId: groupId, isCollapsed: true) + try select(ungroupedId, in: manager) + + manager.selectPreviousTab() + #expect(manager.selectedTabId == anchorId) + + manager.selectPreviousTab() + #expect(manager.selectedTabId == ungroupedId) + + let group = try #require(manager.workspaceGroups.first { $0.id == groupId }) + #expect(group.isCollapsed) + } + + @Test func hiddenSelectedMemberStepsRelativeToItsGroupHeader() throws { + let (manager, ungroupedId, groupId, _, memberId) = try makeGroupedFixture() + try select(memberId, in: manager) + // The direct setter preserves focus, so the selected row is hidden. + manager.setWorkspaceGroupCollapsed(groupId: groupId, isCollapsed: true) + #expect(manager.selectedTabId == memberId) + + manager.selectNextTab() + #expect(manager.selectedTabId == ungroupedId) + + let group = try #require(manager.workspaceGroups.first { $0.id == groupId }) + #expect(group.isCollapsed) + } + + @Test func singleVisibleStopKeepsSelection() throws { + let manager = makeTabManager() + let onlyId = try #require(manager.selectedTabId) + + manager.selectNextTab() + #expect(manager.selectedTabId == onlyId) + + manager.selectPreviousTab() + #expect(manager.selectedTabId == onlyId) + } +}