diff --git a/Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+CloseSelection.swift b/Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+CloseSelection.swift new file mode 100644 index 000000000000..6a81491e8667 --- /dev/null +++ b/Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+CloseSelection.swift @@ -0,0 +1,35 @@ +public import Foundation + +// Selection hand-off for the workspace close path. +extension WorkspacesModel { + /// The workspace that takes selection after the workspace previously at + /// `closedIndex` was removed from `tabs`. + /// + /// Keeps the user's focused *row position* stable where possible: if a + /// workspace still occupies the closed workspace's slot, that one is + /// focused (it moved up into the slot); otherwise the closed workspace was + /// last, so focus falls back to the new last workspace. + /// + /// The slot's occupant is resolved to a workspace the sidebar actually + /// renders: `tabs` stores every workspace, but a non-anchor member of a + /// *collapsed* group has no row of its own, and selecting one would fire + /// `expandWorkspaceGroupForSelectionIfNeeded` and unfold a group the user + /// deliberately collapsed. Such a member resolves to its group's anchor — + /// the row the sidebar draws for it — matching how + /// `toggleWorkspaceGroupCollapsed` moves focus to the anchor when a + /// collapse would hide the selected member. + /// + /// Callers must have already removed the closed workspace from `tabs`. + /// Returns nil when no workspace remains. + public func selectionTargetAfterClose(closedIndex: Int) -> UUID? { + guard !tabs.isEmpty else { return nil } + let newIndex = min(closedIndex, max(0, tabs.count - 1)) + let candidate = tabs[newIndex] + guard let groupId = candidate.groupId, + let group = workspaceGroups.first(where: { $0.id == groupId }), + group.isCollapsed else { + return candidate.id + } + return group.anchorWorkspaceId + } +} diff --git a/Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCloseSelectionTests.swift b/Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCloseSelectionTests.swift new file mode 100644 index 000000000000..fb43a8eed853 --- /dev/null +++ b/Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCloseSelectionTests.swift @@ -0,0 +1,125 @@ +import Foundation +import Testing +@testable import CmuxWorkspaces + +/// Close-path selection hand-off. The sidebar renders group anchors and +/// ungrouped workspaces as rows; a non-anchor member of a *collapsed* group has +/// no row of its own. Selecting such a member fires the selection auto-expand +/// (`expandWorkspaceGroupForSelectionIfNeeded`) and unfolds a group the user +/// deliberately collapsed, so the close path must never land there. +@MainActor +struct WorkspaceCloseSelectionTests { + private func makeWorld() -> ( + model: WorkspacesModel, + host: StubGroupHost, + groups: WorkspaceGroupCoordinator + ) { + let model = WorkspacesModel() + let host = StubGroupHost(model: model) + let groups = WorkspaceGroupCoordinator(model: model) + groups.attach(host: host) + return (model, host, groups) + } + + /// Builds `[anchor, m1, m2, outside]`: one group run followed by a single + /// ungrouped workspace, which is the ordering invariant's normal shape + /// (groups above ungrouped-unpinned). + /// + /// The coordinator holds its host weakly, so callers must keep the returned + /// `host` alive for the duration of the test. + private func makeGroupAboveLoneWorkspace() throws -> ( + model: WorkspacesModel, + host: StubGroupHost, + groups: WorkspaceGroupCoordinator, + groupId: UUID, + anchorId: UUID, + m2: CoordinatorStubTab, + outside: CoordinatorStubTab + ) { + let (model, host, groups) = makeWorld() + let m1 = CoordinatorStubTab() + let m2 = CoordinatorStubTab() + let outside = CoordinatorStubTab() + model.tabs = [m1, m2, outside] + let groupId = try #require( + groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [m1.id, m2.id]) + ) + let anchorId = try #require( + model.workspaceGroups.first(where: { $0.id == groupId })?.anchorWorkspaceId + ) + #expect(model.tabs.map(\.id) == [anchorId, m1.id, m2.id, outside.id]) + return (model, host, groups, groupId, anchorId, m2, outside) + } + + /// Closing the LAST workspace walks selection *backwards*, straight into the + /// tail of the preceding group's member run. When that group is collapsed + /// the tail member is invisible, so selection must land on the group's + /// anchor (its visible header row) instead. + @Test + func closingLastWorkspaceSelectsAnchorNotHiddenMemberOfCollapsedGroup() throws { + let world = try makeGroupAboveLoneWorkspace() + world.groups.setWorkspaceGroupCollapsed(groupId: world.groupId, isCollapsed: true) + + let index = try #require(world.model.tabs.firstIndex(where: { $0.id == world.outside.id })) + world.model.selectedTabId = world.outside.id + world.model.tabs.remove(at: index) + + let target = try #require(world.model.selectionTargetAfterClose(closedIndex: index)) + #expect(target == world.anchorId) + #expect(target != world.m2.id) + } + + /// End-to-end consequence of the rule above: after the close path applies + /// its selection and the selection side-effect chain runs, the group the + /// user collapsed is still collapsed. + @Test + func closingLastWorkspaceKeepsPrecedingGroupCollapsed() throws { + let world = try makeGroupAboveLoneWorkspace() + world.groups.setWorkspaceGroupCollapsed(groupId: world.groupId, isCollapsed: true) + + let index = try #require(world.model.tabs.firstIndex(where: { $0.id == world.outside.id })) + world.model.selectedTabId = world.outside.id + world.model.tabs.remove(at: index) + + world.model.selectedTabId = world.model.selectionTargetAfterClose(closedIndex: index) + // The model has no host attached here, so drive the selection + // side-effect the window's `selectedTabId` didSet would have run. + world.model.expandWorkspaceGroupForSelectionIfNeeded() + + #expect(world.model.workspaceGroups.first(where: { $0.id == world.groupId })?.isCollapsed == true) + } + + /// Guard against over-correcting: an EXPANDED group's members do have their + /// own sidebar rows, so selection keeps landing on the adjacent member and + /// is not redirected to the anchor. + @Test + func closingLastWorkspaceSelectsAdjacentMemberOfExpandedGroup() throws { + let world = try makeGroupAboveLoneWorkspace() + + let index = try #require(world.model.tabs.firstIndex(where: { $0.id == world.outside.id })) + world.model.selectedTabId = world.outside.id + world.model.tabs.remove(at: index) + + let target = try #require(world.model.selectionTargetAfterClose(closedIndex: index)) + #expect(target == world.m2.id) + } + + /// Closing a non-last workspace still walks *forwards* into the workspace + /// that moved up into the vacated slot. + @Test + func closingNonLastWorkspaceSelectsTheWorkspaceThatMovedUp() throws { + let (model, host, _) = makeWorld() + _ = host + let a = CoordinatorStubTab() + let b = CoordinatorStubTab() + let c = CoordinatorStubTab() + model.tabs = [a, b, c] + + let index = try #require(model.tabs.firstIndex(where: { $0.id == b.id })) + model.selectedTabId = b.id + model.tabs.remove(at: index) + + let target = try #require(model.selectionTargetAfterClose(closedIndex: index)) + #expect(target == c.id) + } +} diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index b538fc20f460..9f7916d3aa67 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -2462,12 +2462,11 @@ class TabManager: ObservableObject { // fixup. let promotedAnchorIds = workspaces.promoteAnchorOrRemoveGroupsAnchoredBy(closedWorkspaceId: workspace.id) - if selectedTabId == workspace.id { - // Keep the "focused index" stable when possible: - // - If we closed workspace i and there is still a workspace at index i, focus it (the one that moved up). - // - Otherwise (we closed the last workspace), focus the new last workspace (i-1). - let newIndex = min(index, max(0, tabs.count - 1)) - selectedTabId = tabs[newIndex].id + if selectedTabId == workspace.id, + let nextSelectedId = workspaces.selectionTargetAfterClose(closedIndex: index) { + // Keep the "focused row position" stable when possible; see + // WorkspacesModel.selectionTargetAfterClose for the rule. + selectedTabId = nextSelectedId } // A promoted anchor's resolved display title switches from its own