diff --git a/doc-onevcat/shelf-view.md b/doc-onevcat/shelf-view.md index bdc3a458..87cf36f1 100644 --- a/doc-onevcat/shelf-view.md +++ b/doc-onevcat/shelf-view.md @@ -221,10 +221,15 @@ Mirror Prowl's normal-mode tab close behavior: ### Closing the Last Tab in a Book -Closing the last tab does **not** remove the book from the shelf. Instead, the -book remains, its spine stays in place, and the open area shows an **empty -terminal placeholder UI** (consistent with normal-mode behavior). The user can -add a tab back via the bottom controls (after opening the book). +Closing the last tab **retires the book from the Shelf**. Its spine disappears; +if the closed book was the one currently open, Shelf auto-advances to the next +remaining book (in Shelf order). The user can bring the book back by clicking +its worktree in the left navigation, which re-opens it and re-adds its spine +with the standard spine-flow animation. + +(Earlier drafts of this doc proposed keeping the book on the shelf with an +empty-terminal placeholder. Reversed: a lingering empty book felt unnatural and +doubled as dead weight. See the Implementation Decisions Journal for the switch.) ### Removing a Book from the Shelf @@ -403,12 +408,21 @@ Shelf-originated taps additionally pass the same animation to `store.send(_, animation:)` so the TCA-side mutation carries the transaction along. -### Close-last-tab behavior - -The empty-book state falls out naturally: `ShelfOpenBookView` already -renders `EmptyTerminalPaneView(message: "No terminals open")` when -`selectedTabId == nil`. The spine remains on the shelf, with its bottom -controls still enabling new tab / split to recover. +### Close-last-tab behavior (revised) + +Reversed from the original decision. Closing the last tab now removes the +book from the Shelf entirely. The implementation: + +- `TerminalClient.Event.tabClosed` gained a `remainingTabs: Int` payload so + AppFeature can detect the last-tab case. When it sees `remainingTabs == 0`, + it dispatches `.repositories(.markWorktreeClosed(id))`. +- The `markWorktreeClosed` reducer handler removes the ID from + `openedWorktreeIDs`, and — only when Shelf is active and the closed + worktree was the open book — auto-advances selection to the next + remaining book (via `shelfBookSelectionEffect`). In normal view, the + selection is left alone so the user's current context isn't disturbed. +- When the closed book was the last book on the Shelf, selection is kept + as-is; `ShelfView` falls through to its "No book selected" empty state. ### Opened-worktrees set diff --git a/supacode/Clients/Terminal/TerminalClient.swift b/supacode/Clients/Terminal/TerminalClient.swift index d7af6afe..751787c0 100644 --- a/supacode/Clients/Terminal/TerminalClient.swift +++ b/supacode/Clients/Terminal/TerminalClient.swift @@ -50,7 +50,7 @@ struct TerminalClient { case notificationReceived(worktreeID: Worktree.ID, title: String, body: String) case notificationIndicatorChanged(count: Int) case tabCreated(worktreeID: Worktree.ID) - case tabClosed(worktreeID: Worktree.ID) + case tabClosed(worktreeID: Worktree.ID, remainingTabs: Int) case focusChanged(worktreeID: Worktree.ID, surfaceID: UUID) case taskStatusChanged(worktreeID: Worktree.ID, status: WorktreeTaskStatus) case runScriptStatusChanged(worktreeID: Worktree.ID, isRunning: Bool) diff --git a/supacode/Features/App/Reducer/AppFeature.swift b/supacode/Features/App/Reducer/AppFeature.swift index 671588a2..0da91a68 100644 --- a/supacode/Features/App/Reducer/AppFeature.swift +++ b/supacode/Features/App/Reducer/AppFeature.swift @@ -1031,6 +1031,12 @@ struct AppFeature { // worktree; other restored worktrees only surface here. return .send(.repositories(.markWorktreeOpened(worktreeID))) + case .terminalEvent(.tabClosed(let worktreeID, let remainingTabs)): + // Closing the last tab retires the book from the Shelf. Other + // closes are routine and need no Reducer-side bookkeeping. + guard remainingTabs == 0 else { return .none } + return .send(.repositories(.markWorktreeClosed(worktreeID))) + case .terminalEvent: return .none } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index 1e2bf558..7d3e32d9 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -284,6 +284,7 @@ struct RepositoriesFeature { case selectPreviousShelfBook case selectShelfBook(Int) case markWorktreeOpened(Worktree.ID) + case markWorktreeClosed(Worktree.ID) case setSidebarSelectedWorktreeIDs(Set) case selectRepository(Repository.ID?) case selectWorktree(Worktree.ID?, focusTerminal: Bool = false) @@ -682,6 +683,24 @@ struct RepositoriesFeature { state.openedWorktreeIDs.insert(worktreeID) return .none + case .markWorktreeClosed(let worktreeID): + // Closing the last tab of a book retires the book from the + // Shelf. If this book was the one currently open on the + // Shelf, move focus to the next remaining book (by Shelf + // order) so the user lands on something useful rather than + // an empty-Shelf placeholder. + state.openedWorktreeIDs.remove(worktreeID) + guard state.isShelfActive, + state.selectedTerminalWorktree?.id == worktreeID + else { + return .none + } + let remaining = state.orderedShelfBooks() + guard let next = remaining.first else { + return .none + } + return shelfBookSelectionEffect(for: next) + case .toggleShelf: if state.isShelfActive { state.isShelfActive = false diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 3d4d4610..3f481688 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -231,8 +231,10 @@ final class WorktreeTerminalManager { state.onTabCreated = { [weak self] in self?.emit(.tabCreated(worktreeID: worktree.id)) } - state.onTabClosed = { [weak self] in - self?.emit(.tabClosed(worktreeID: worktree.id)) + state.onTabClosed = { [weak self, weak state] in + guard let self else { return } + let remaining = state?.tabManager.tabs.count ?? 0 + emit(.tabClosed(worktreeID: worktree.id, remainingTabs: remaining)) } state.onFocusChanged = { [weak self] surfaceID in self?.emit(.focusChanged(worktreeID: worktree.id, surfaceID: surfaceID)) diff --git a/supacodeTests/ShelfFeatureTests.swift b/supacodeTests/ShelfFeatureTests.swift index fb9d5ceb..d9ea5d54 100644 --- a/supacodeTests/ShelfFeatureTests.swift +++ b/supacodeTests/ShelfFeatureTests.swift @@ -434,6 +434,120 @@ struct ShelfFeatureTests { await store.finish() } + @Test(.dependencies) func markWorktreeClosedRemovesFromOpenedSet() async { + let rootURL = URL(fileURLWithPath: "/tmp/repo") + let wt1 = Worktree( + id: "/tmp/repo/wt1", + name: "wt1", + detail: "", + workingDirectory: URL(fileURLWithPath: "/tmp/repo/wt1"), + repositoryRootURL: rootURL + ) + let repo = Repository( + id: rootURL.path(percentEncoded: false), + rootURL: rootURL, + name: "repo", + worktrees: IdentifiedArray(uniqueElements: [wt1]) + ) + var state = RepositoriesFeature.State(repositories: [repo]) + state.openedWorktreeIDs = [wt1.id] + state.selection = nil // Not currently selected, no auto-next needed. + let store = TestStore(initialState: state) { + RepositoriesFeature() + } + + await store.send(.markWorktreeClosed(wt1.id)) { + $0.openedWorktreeIDs = [] + } + await store.finish() + } + + @Test(.dependencies) func markWorktreeClosedAutoAdvancesToNextBookWhenOpen() async { + let rootURL = URL(fileURLWithPath: "/tmp/repo") + let wt1 = Worktree( + id: "/tmp/repo/wt1", + name: "wt1", + detail: "", + workingDirectory: URL(fileURLWithPath: "/tmp/repo/wt1"), + repositoryRootURL: rootURL + ) + let wt2 = Worktree( + id: "/tmp/repo/wt2", + name: "wt2", + detail: "", + workingDirectory: URL(fileURLWithPath: "/tmp/repo/wt2"), + repositoryRootURL: rootURL + ) + let repo = Repository( + id: rootURL.path(percentEncoded: false), + rootURL: rootURL, + name: "repo", + worktrees: IdentifiedArray(uniqueElements: [wt1, wt2]) + ) + var state = RepositoriesFeature.State(repositories: [repo]) + state.repositoryRoots = [rootURL] + state.repositoryOrderIDs = [repo.id] + state.selection = .worktree(wt1.id) + state.isShelfActive = true + state.openedWorktreeIDs = [wt1.id, wt2.id] + let store = TestStore(initialState: state) { + RepositoriesFeature() + } + + // Close wt1 (the open book on the Shelf). wt2 is the only remaining + // book → reducer should auto-select wt2 so the user lands on + // content rather than an empty-Shelf placeholder. + await store.send(.markWorktreeClosed(wt1.id)) { + $0.openedWorktreeIDs = [wt2.id] + } + await store.receive(\.selectWorktree) { + $0.selection = .worktree(wt2.id) + $0.sidebarSelectedWorktreeIDs = [wt2.id] + $0.pendingTerminalFocusWorktreeIDs = [wt2.id] + $0.openedWorktreeIDs = [wt2.id] + } + await store.receive(\.delegate.selectedWorktreeChanged) + await store.finish() + } + + @Test(.dependencies) func markWorktreeClosedLeavesSelectionAloneInNormalView() async { + let rootURL = URL(fileURLWithPath: "/tmp/repo") + let wt1 = Worktree( + id: "/tmp/repo/wt1", + name: "wt1", + detail: "", + workingDirectory: URL(fileURLWithPath: "/tmp/repo/wt1"), + repositoryRootURL: rootURL + ) + let wt2 = Worktree( + id: "/tmp/repo/wt2", + name: "wt2", + detail: "", + workingDirectory: URL(fileURLWithPath: "/tmp/repo/wt2"), + repositoryRootURL: rootURL + ) + let repo = Repository( + id: rootURL.path(percentEncoded: false), + rootURL: rootURL, + name: "repo", + worktrees: IdentifiedArray(uniqueElements: [wt1, wt2]) + ) + var state = RepositoriesFeature.State(repositories: [repo]) + state.selection = .worktree(wt1.id) + state.isShelfActive = false // Normal view. + state.openedWorktreeIDs = [wt1.id, wt2.id] + let store = TestStore(initialState: state) { + RepositoriesFeature() + } + + // In normal view, removing from the opened set must not also steal + // selection away from the user — they are actively on wt1. + await store.send(.markWorktreeClosed(wt1.id)) { + $0.openedWorktreeIDs = [wt2.id] + } + await store.finish() + } + @Test(.dependencies) func markWorktreeOpenedAddsToOpenedSet() async { let rootURL = URL(fileURLWithPath: "/tmp/repo") let wt1 = Worktree(