feat(nav): back/forward traverses Tasks page visits (#1207)
Widens worktree nav history entries to string | 'tasks' so the titlebar back/forward buttons and Cmd/Ctrl+Alt+Arrow shortcut work from the Tasks page. openTaskPage records a 'tasks' entry; closeTaskPage rewinds the index when parked on one; goBack/goForward dispatches Tasks entries via a separate view activator to bypass openTaskPage side effects. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
GitHub
parent
8aff317fc0
commit
bdf5f66ffd
|
|
@ -0,0 +1,138 @@
|
|||
# Back/Forward support for the Tasks page
|
||||
|
||||
## Goal
|
||||
|
||||
Make the titlebar back/forward buttons and the `Cmd/Ctrl+Alt+←/→` shortcut
|
||||
traverse Tasks visits in addition to worktree activations. Today both are
|
||||
no-ops outside `activeView === 'terminal'` (see `App.tsx:616`, `App.tsx:900`).
|
||||
|
||||
## Current shape
|
||||
|
||||
- History lives in `src/renderer/src/store/slices/worktree-nav-history.ts`.
|
||||
- Entries are `string[]` (worktree IDs), recorded from
|
||||
`worktree-activation.ts:96` via `recordWorktreeVisit`.
|
||||
- `goBackWorktree`/`goForwardWorktree` skip dead worktrees via
|
||||
`findPrev/NextLiveWorktreeHistoryIndex`, gated by an `activator` ref and
|
||||
`isNavigatingHistory` to prevent re-recording.
|
||||
- UI (pre-change): titlebar buttons hidden unless
|
||||
`activeView === 'terminal'`; shortcut ignored outside terminal view.
|
||||
|
||||
## Minimal implementation (downscoped)
|
||||
|
||||
1. **History entry type** → `string | 'tasks'`.
|
||||
- `findPrev/NextLiveWorktreeHistoryIndex` must short-circuit on `'tasks'`
|
||||
before calling `findWorktreeById` (which takes a worktree id, not a
|
||||
view tag) — Tasks entries are unconditionally live.
|
||||
- Keep `recordWorktreeVisit(worktreeId: string)` signature to avoid churn
|
||||
in `terminals-hydration.test.ts`. Add a sibling
|
||||
`recordViewVisit(entry: 'tasks')` that shares the dedupe/truncate/cap
|
||||
logic.
|
||||
- Keep existing names (`worktreeNavHistory`, `goBackWorktree`,
|
||||
`canGoBackWorktreeHistory`). They're now slight misnomers since
|
||||
entries may be `'tasks'`, but renaming touches ~20 call sites for no
|
||||
behavioral win. Add a one-line comment at the top of the slice
|
||||
stating the entry type and the intentional keeping of the name, so
|
||||
future readers don't assume it's worktree-only.
|
||||
|
||||
2. **Record Tasks visits** → call `recordViewVisit('tasks')` from
|
||||
`openTaskPage` (`ui.ts:189`). No `isNavigatingHistory` guard needed:
|
||||
back-to-Tasks routes through `setActiveView('tasks')` per step 3, which
|
||||
never touches `openTaskPage`. The slice's existing adjacent-entry dedupe
|
||||
covers any other re-entry. Note: `openTaskPage` is called with varying
|
||||
`taskPageData` (e.g. `{ taskSource: 'github' }` vs `'linear'` from
|
||||
`SidebarNav.tsx:88,102`); all collapse to a single `'tasks'` entry and
|
||||
dedupe against the current entry, so toggling presets produces no extra
|
||||
history entries. This is consistent with the out-of-scope "no per-entry
|
||||
snapshotting" decision.
|
||||
|
||||
3. **Dispatch in goBack/goForward** → based on entry kind:
|
||||
- worktreeId → existing `activator(worktreeId)` path. When current entry
|
||||
is `'tasks'`, no extra view handling is needed:
|
||||
`activateAndRevealWorktree` at `worktree-activation.ts:82-84` already
|
||||
switches `activeView` back to `'terminal'`.
|
||||
- `'tasks'` → `setActiveView('tasks')` (not `openTaskPage` — avoids
|
||||
mutating `previousViewBeforeTasks` and the SWR prefetch).
|
||||
|
||||
4. **Fix Esc-close history desync.** `closeTaskPage` (`ui.ts:210`) currently
|
||||
sets `activeView` to `previousViewBeforeTasks` without touching the
|
||||
history index, so after `A → Tasks → Esc` the index still points at the
|
||||
`'tasks'` entry and Back becomes a visual no-op (activator re-activates A)
|
||||
while Forward re-opens Tasks. Fix: in `closeTaskPage`, if the current
|
||||
history entry is `'tasks'`, move the index to the previous live entry
|
||||
(same scan as `findPrevLiveWorktreeHistoryIndex`). Sub-case: if there is
|
||||
no previous live entry (e.g. user's first action was open Tasks, then
|
||||
Esc — history `['tasks']` at index 0), leave the index unchanged at 0.
|
||||
Accept the minor cost (Back becomes a visual no-op until a real visit
|
||||
records a new entry) rather than setting to -1, which would lose the
|
||||
only forward target. Guard with `isNavigatingHistory` is unnecessary
|
||||
here — `closeTaskPage` is never invoked from the history path.
|
||||
|
||||
5. **Unhide UI on Tasks** (allowlist, not denylist — `activeView` union is
|
||||
`'terminal' | 'settings' | 'tasks'`, but an allowlist won't silently
|
||||
include future views):
|
||||
- Replace the `activeView !== 'terminal'` early-return at `App.tsx:616`
|
||||
with `activeView !== 'terminal' && activeView !== 'tasks'`. Update the
|
||||
adjacent comment (`App.tsx:613-615`) — it currently claims the
|
||||
shortcut is a no-op outside terminal because the buttons are hidden;
|
||||
replace with: back/forward traverse worktree + Tasks visits, so the
|
||||
shortcut is active whenever the button cluster is (terminal or
|
||||
Tasks); still suppressed elsewhere (Settings).
|
||||
- Change the titlebar cluster guard at `App.tsx:900` to
|
||||
`activeView === 'terminal' || activeView === 'tasks'`. Update the
|
||||
adjacent comment (`App.tsx:896-899`) — drop the "terminal view only"
|
||||
framing; explain the cluster is shown wherever the history shortcut
|
||||
is live, and hidden in Settings to keep that view modal-ish.
|
||||
|
||||
6. **Tests**: extend `worktree-nav-history.test.ts` for mixed entries:
|
||||
- `A → Tasks → B`, back lands on Tasks, back again on A.
|
||||
- Tasks dedupe against current entry.
|
||||
- Dead worktree between Tasks entries is skipped.
|
||||
- `A → Tasks → closeTaskPage()`: index moves back to A; subsequent Back
|
||||
is a no-op, Forward re-opens Tasks.
|
||||
- `Tasks (only entry) → closeTaskPage()`: index stays at 0, Back is a
|
||||
visual no-op.
|
||||
|
||||
## Explicitly out of scope
|
||||
|
||||
- **Settings in history.** Stays modal-ish; closes via
|
||||
`previousViewBeforeSettings`.
|
||||
- **Per-entry `taskPageData` snapshotting.** Back-to-Tasks opens Tasks with
|
||||
default filters/source. Revisit if users complain.
|
||||
- **Session persistence of history.** Keep in-memory only (it already is).
|
||||
|
||||
## Edge cases handled
|
||||
|
||||
- Worktree deletion mid-session → skipped by existing live-check; Tasks
|
||||
entries always live.
|
||||
- Dedupe semantics → same adjacency rule works for `'tasks'`.
|
||||
- Activator failure on worktree → unchanged (`result !== false` gate). Tasks
|
||||
dispatch can't fail.
|
||||
- Editable-target guard at `App.tsx:600` still fires first → typing in the
|
||||
Tasks search box + `Cmd+Alt+←` remains a no-op.
|
||||
- `MAX_HISTORY = 50` → Tasks entries consume slots; worst-case effective
|
||||
worktree depth halves to ~25. Acceptable.
|
||||
|
||||
## Known residual quirks (accepted)
|
||||
|
||||
1. **Prefetch loss on back-to-Tasks.** Routing through `setActiveView('tasks')`
|
||||
skips the SWR prefetch at `ui.ts:201-208`; back-to-Tasks is ~300–800ms
|
||||
slower than a fresh open via the sidebar. Not a regression vs today
|
||||
(back-to-Tasks isn't possible at all today).
|
||||
|
||||
2. **Titlebar layout shift.** Revealing the button cluster on Tasks changes
|
||||
the titlebar — needs a visual check that nothing Tasks-specific collides.
|
||||
|
||||
## Files touched
|
||||
|
||||
- `src/renderer/src/store/slices/worktree-nav-history.ts` — entry type,
|
||||
`recordViewVisit`, dispatch in `goBack/goForwardWorktree`.
|
||||
- `src/renderer/src/store/slices/worktree-nav-history.test.ts` — new cases.
|
||||
- `src/renderer/src/store/slices/ui.ts` — `openTaskPage` records via
|
||||
`recordViewVisit`; `closeTaskPage` rewinds history index when the current
|
||||
entry is `'tasks'`.
|
||||
- `src/renderer/src/App.tsx` — widen the two `activeView === 'terminal'`
|
||||
guards to include `'tasks'`, and update the adjacent "why" comments at
|
||||
`App.tsx:613-615` and `App.tsx:896-899` to reflect the new invariant.
|
||||
|
||||
Estimated diff: ~30 lines of slice logic, 1 call site in `ui.ts`, 2 guards
|
||||
in `App.tsx`, plus tests.
|
||||
|
|
@ -610,10 +610,10 @@ function App(): React.JSX.Element {
|
|||
(isMac ? e.metaKey && !e.ctrlKey : e.ctrlKey && !e.metaKey) &&
|
||||
(e.code === 'ArrowLeft' || e.code === 'ArrowRight')
|
||||
) {
|
||||
// Why: hidden buttons in non-terminal views mean the shortcut must be
|
||||
// a no-op there too — navigating worktree history from Settings or
|
||||
// Tasks is not a meaningful action.
|
||||
if (activeView !== 'terminal') {
|
||||
// Why: Back/Forward traverse mixed worktree + Tasks visits, so the
|
||||
// shortcut is active wherever the titlebar button cluster is (terminal
|
||||
// or tasks). Still suppressed in Settings to keep that view modal-ish.
|
||||
if (activeView !== 'terminal' && activeView !== 'tasks') {
|
||||
return
|
||||
}
|
||||
dispatchClearModifierHints()
|
||||
|
|
@ -893,11 +893,10 @@ function App(): React.JSX.Element {
|
|||
</Popover>
|
||||
) : null}
|
||||
</div>
|
||||
{/* Why: Back/Forward navigate worktree-activation history. Only
|
||||
meaningful while viewing a worktree (terminal view); hidden in
|
||||
Settings/Tasks/Landing to keep the titlebar compact and the
|
||||
semantics unambiguous. */}
|
||||
{activeView === 'terminal' && (
|
||||
{/* Why: Back/Forward traverse mixed worktree + Tasks history, so the
|
||||
cluster is shown wherever the history shortcut is live (terminal or
|
||||
tasks). Hidden in Settings to keep that view modal-ish. */}
|
||||
{(activeView === 'terminal' || activeView === 'tasks') && (
|
||||
<div className="ml-auto mr-3 flex items-center">
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
|
|
|
|||
|
|
@ -3,7 +3,10 @@ import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-t
|
|||
import { buildSetupRunnerCommand } from './setup-runner'
|
||||
import { useAppStore } from '@/store'
|
||||
import { findWorktreeById } from '@/store/slices/worktree-helpers'
|
||||
import { setWorktreeNavActivator } from '@/store/slices/worktree-nav-history'
|
||||
import {
|
||||
setWorktreeNavActivator,
|
||||
setWorktreeNavViewActivator
|
||||
} from '@/store/slices/worktree-nav-history'
|
||||
|
||||
// Why: issue commands can originate from two sources with different shapes —
|
||||
// (1) a repo-level runner script generated by main (WorktreeSetupLaunch), or
|
||||
|
|
@ -211,3 +214,11 @@ export function ensureWorktreeHasInitialTerminal(
|
|||
// @/store, which activation already imports from. Registering the activator
|
||||
// at module init here lets the slice call back without importing this file.
|
||||
setWorktreeNavActivator(activateAndRevealWorktree)
|
||||
|
||||
// Why: Tasks entries in the nav history dispatch via setActiveView('tasks')
|
||||
// (not openTaskPage) — see the 'tasks' branch in navigateToIndex. Going this
|
||||
// route avoids mutating previousViewBeforeTasks and skips the SWR prefetch
|
||||
// (an accepted residual from the design doc; see "Known residual quirks").
|
||||
setWorktreeNavViewActivator((entry) => {
|
||||
useAppStore.getState().setActiveView(entry)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
/* eslint-disable max-lines */
|
||||
import type { StateCreator } from 'zustand'
|
||||
import type { AppState } from '../types'
|
||||
import { findPrevLiveWorktreeHistoryIndex } from './worktree-nav-history'
|
||||
import type {
|
||||
ChangelogData,
|
||||
PersistedUIState,
|
||||
|
|
@ -198,6 +199,13 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
|
|||
taskPageData: {},
|
||||
newWorkspaceDraft: null,
|
||||
openTaskPage: (data = {}) => {
|
||||
// Why: record a Tasks visit in the shared back/forward history so the
|
||||
// titlebar Back/Forward buttons can return to Tasks. All task-source
|
||||
// variants (github/linear presets) collapse to a single 'tasks' entry;
|
||||
// the slice's adjacent-entry dedupe drops re-opens. No isNavigatingHistory
|
||||
// guard needed — back-to-Tasks routes through setActiveView('tasks') and
|
||||
// never re-enters openTaskPage.
|
||||
get().recordViewVisit('tasks')
|
||||
set((state) => ({
|
||||
activeView: 'tasks',
|
||||
previousViewBeforeTasks:
|
||||
|
|
@ -219,10 +227,30 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
|
|||
}
|
||||
},
|
||||
closeTaskPage: () =>
|
||||
set((state) => ({
|
||||
activeView: state.previousViewBeforeTasks,
|
||||
taskPageData: {}
|
||||
})),
|
||||
set((state) => {
|
||||
// Why: Esc-close from Tasks must rewind the history index if we're
|
||||
// currently parked on a 'tasks' entry. Without this, A → Tasks → Esc
|
||||
// leaves the index at the 'tasks' entry, making Back a visual no-op
|
||||
// (activator re-activates A) and Forward re-opens Tasks. If there is no
|
||||
// earlier live entry (e.g. history is just ['tasks']), leave the index
|
||||
// at 0 — setting it to -1 would lose the only forward target, while the
|
||||
// resulting Back visual no-op self-heals as soon as a real visit records
|
||||
// a new entry. closeTaskPage never runs from the history-nav path, so no
|
||||
// isNavigatingHistory guard is needed.
|
||||
const currentEntry = state.worktreeNavHistory[state.worktreeNavHistoryIndex]
|
||||
let nextHistoryIndex = state.worktreeNavHistoryIndex
|
||||
if (currentEntry === 'tasks') {
|
||||
const prev = findPrevLiveWorktreeHistoryIndex(state)
|
||||
if (prev !== null) {
|
||||
nextHistoryIndex = prev
|
||||
}
|
||||
}
|
||||
return {
|
||||
activeView: state.previousViewBeforeTasks,
|
||||
taskPageData: {},
|
||||
worktreeNavHistoryIndex: nextHistoryIndex
|
||||
}
|
||||
}),
|
||||
setNewWorkspaceDraft: (draft) => set({ newWorkspaceDraft: draft }),
|
||||
clearNewWorkspaceDraft: () => set({ newWorkspaceDraft: null }),
|
||||
openSettingsPage: () =>
|
||||
|
|
|
|||
|
|
@ -6,7 +6,9 @@ import {
|
|||
canGoBackWorktreeHistory,
|
||||
canGoForwardWorktreeHistory,
|
||||
createWorktreeNavHistorySlice,
|
||||
setWorktreeNavActivator
|
||||
findPrevLiveWorktreeHistoryIndex,
|
||||
setWorktreeNavActivator,
|
||||
setWorktreeNavViewActivator
|
||||
} from './worktree-nav-history'
|
||||
|
||||
type MinimalState = Pick<
|
||||
|
|
@ -15,6 +17,7 @@ type MinimalState = Pick<
|
|||
| 'worktreeNavHistoryIndex'
|
||||
| 'isNavigatingHistory'
|
||||
| 'recordWorktreeVisit'
|
||||
| 'recordViewVisit'
|
||||
| 'goBackWorktree'
|
||||
| 'goForwardWorktree'
|
||||
| 'worktreesByRepo'
|
||||
|
|
@ -182,6 +185,104 @@ describe('worktree-nav-history slice: goBack / goForward', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('worktree-nav-history slice: Tasks entries', () => {
|
||||
afterEach(() => {
|
||||
setWorktreeNavActivator(null)
|
||||
setWorktreeNavViewActivator(null)
|
||||
})
|
||||
|
||||
it('A -> Tasks -> B, back lands on Tasks then A', () => {
|
||||
const store = createHistoryStore(['a', 'b'])
|
||||
const activated: string[] = []
|
||||
const viewed: string[] = []
|
||||
setWorktreeNavActivator((id) => {
|
||||
activated.push(id as string)
|
||||
return { primaryTabId: null }
|
||||
})
|
||||
setWorktreeNavViewActivator((v) => {
|
||||
viewed.push(v)
|
||||
})
|
||||
|
||||
store.getState().recordWorktreeVisit('a')
|
||||
store.getState().recordViewVisit('tasks')
|
||||
store.getState().recordWorktreeVisit('b')
|
||||
|
||||
expect(store.getState().worktreeNavHistory).toEqual(['a', 'tasks', 'b'])
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(2)
|
||||
|
||||
store.getState().goBackWorktree()
|
||||
expect(viewed).toEqual(['tasks'])
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(1)
|
||||
|
||||
store.getState().goBackWorktree()
|
||||
expect(activated).toEqual(['a'])
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(0)
|
||||
})
|
||||
|
||||
it('dedupes Tasks against the current Tasks entry', () => {
|
||||
const store = createHistoryStore(['a'])
|
||||
store.getState().recordWorktreeVisit('a')
|
||||
store.getState().recordViewVisit('tasks')
|
||||
store.getState().recordViewVisit('tasks')
|
||||
store.getState().recordViewVisit('tasks')
|
||||
|
||||
expect(store.getState().worktreeNavHistory).toEqual(['a', 'tasks'])
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(1)
|
||||
})
|
||||
|
||||
it('skips a dead worktree between two Tasks entries', () => {
|
||||
// 'b' is deleted; history is [tasks, b, tasks].
|
||||
const store = createHistoryStore([])
|
||||
const viewed: string[] = []
|
||||
setWorktreeNavViewActivator((v) => {
|
||||
viewed.push(v)
|
||||
})
|
||||
|
||||
store.setState({
|
||||
worktreeNavHistory: ['tasks', 'b', 'tasks'],
|
||||
worktreeNavHistoryIndex: 2
|
||||
})
|
||||
|
||||
store.getState().goBackWorktree()
|
||||
expect(viewed).toEqual(['tasks'])
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(0)
|
||||
})
|
||||
|
||||
it('closeTaskPage-style rewind: A -> Tasks, rewind moves index to A', () => {
|
||||
const store = createHistoryStore(['a'])
|
||||
store.getState().recordWorktreeVisit('a')
|
||||
store.getState().recordViewVisit('tasks')
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(1)
|
||||
|
||||
// Simulate closeTaskPage's rewind logic.
|
||||
const prev = findPrevLiveWorktreeHistoryIndex(store.getState() as AppState)
|
||||
expect(prev).toBe(0)
|
||||
store.setState({ worktreeNavHistoryIndex: prev ?? store.getState().worktreeNavHistoryIndex })
|
||||
|
||||
// Forward re-opens Tasks.
|
||||
setWorktreeNavActivator(() => ({ primaryTabId: null }))
|
||||
const viewed: string[] = []
|
||||
setWorktreeNavViewActivator((v) => {
|
||||
viewed.push(v)
|
||||
})
|
||||
|
||||
store.getState().goForwardWorktree()
|
||||
expect(viewed).toEqual(['tasks'])
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(1)
|
||||
})
|
||||
|
||||
it('closeTaskPage-style rewind with only-Tasks history leaves index at 0', () => {
|
||||
const store = createHistoryStore([])
|
||||
store.getState().recordViewVisit('tasks')
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(0)
|
||||
|
||||
const prev = findPrevLiveWorktreeHistoryIndex(store.getState() as AppState)
|
||||
expect(prev).toBeNull()
|
||||
// closeTaskPage leaves the index unchanged when there's no prior live entry.
|
||||
expect(store.getState().worktreeNavHistoryIndex).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('worktree-nav-history selectors', () => {
|
||||
it('reports back availability only when a live prior entry exists', () => {
|
||||
const store = createHistoryStore(['c'])
|
||||
|
|
|
|||
|
|
@ -8,9 +8,16 @@ import { findWorktreeById } from './worktree-helpers'
|
|||
// linear skip-deleted scan in goBack/goForward stays trivially cheap.
|
||||
const MAX_HISTORY = 50
|
||||
|
||||
// Why: entries are worktree IDs OR the sentinel 'tasks' for visits to the
|
||||
// Tasks page. The slice, selector, and action names retain the
|
||||
// "worktree"/"WorktreeHistory" prefix for call-site stability — renaming
|
||||
// across ~20 sites would churn for no behavior win. Tasks entries are
|
||||
// always live (never skipped by findPrev/NextLiveWorktreeHistoryIndex).
|
||||
export type WorktreeNavHistoryEntry = string | 'tasks'
|
||||
|
||||
export type WorktreeNavHistorySlice = {
|
||||
// Linear history, oldest -> newest.
|
||||
worktreeNavHistory: string[]
|
||||
worktreeNavHistory: WorktreeNavHistoryEntry[]
|
||||
// Index into worktreeNavHistory; points at the currently-active entry.
|
||||
// -1 means empty (no worktree ever activated this session).
|
||||
worktreeNavHistoryIndex: number
|
||||
|
|
@ -21,26 +28,76 @@ export type WorktreeNavHistorySlice = {
|
|||
isNavigatingHistory: boolean
|
||||
|
||||
recordWorktreeVisit: (worktreeId: string) => void
|
||||
recordViewVisit: (entry: 'tasks') => void
|
||||
goBackWorktree: () => void
|
||||
goForwardWorktree: () => void
|
||||
}
|
||||
|
||||
type ActivateFn = (worktreeId: string) => unknown
|
||||
type ViewActivateFn = (entry: 'tasks') => void
|
||||
|
||||
// Why: the slice must call activateAndRevealWorktree from goBack/goForward, but
|
||||
// importing it directly would create a cycle (activation imports the store).
|
||||
// Install the reference at module init via setWorktreeNavActivator and keep
|
||||
// the slice itself unaware of the activation module.
|
||||
let activator: ActivateFn | null = null
|
||||
let viewActivator: ViewActivateFn | null = null
|
||||
|
||||
export function setWorktreeNavActivator(fn: ActivateFn | null): void {
|
||||
activator = fn
|
||||
}
|
||||
|
||||
// Why: installed by App-level init so the slice can dispatch 'tasks' entries
|
||||
// to setActiveView('tasks') without importing the UI slice directly (the UI
|
||||
// slice already transitively depends on this module via the store creator).
|
||||
export function setWorktreeNavViewActivator(fn: ViewActivateFn | null): void {
|
||||
viewActivator = fn
|
||||
}
|
||||
|
||||
// Why: Tasks entries short-circuit as live unconditionally — findWorktreeById
|
||||
// takes a worktree id and would always return undefined for the 'tasks'
|
||||
// sentinel, which would incorrectly treat Tasks entries as dead.
|
||||
function isLiveEntry(entry: WorktreeNavHistoryEntry, state: AppState): boolean {
|
||||
if (entry === 'tasks') {
|
||||
return true
|
||||
}
|
||||
return findWorktreeById(state.worktreesByRepo, entry) !== undefined
|
||||
}
|
||||
|
||||
function appendHistoryEntry(
|
||||
s: { worktreeNavHistory: WorktreeNavHistoryEntry[]; worktreeNavHistoryIndex: number },
|
||||
entry: WorktreeNavHistoryEntry
|
||||
): { worktreeNavHistory: WorktreeNavHistoryEntry[]; worktreeNavHistoryIndex: number } {
|
||||
// Why: re-visiting the same entry must not pollute history. The de-dup
|
||||
// applies only to the current entry so that A -> B -> A remains a valid
|
||||
// stack (user left B, returned to A). Same rule covers Tasks re-opens
|
||||
// with different taskPageData — all collapse to a single 'tasks' entry.
|
||||
if (s.worktreeNavHistory[s.worktreeNavHistoryIndex] === entry) {
|
||||
return s
|
||||
}
|
||||
|
||||
// Truncate any forward entries, then append and advance the index.
|
||||
const truncated = s.worktreeNavHistory.slice(0, s.worktreeNavHistoryIndex + 1)
|
||||
truncated.push(entry)
|
||||
let nextIndex = s.worktreeNavHistoryIndex + 1
|
||||
|
||||
// Why: cap eviction drops the oldest entries. The index must shift left
|
||||
// by the same count so it still points at the just-appended current entry.
|
||||
if (truncated.length > MAX_HISTORY) {
|
||||
const evict = truncated.length - MAX_HISTORY
|
||||
truncated.splice(0, evict)
|
||||
nextIndex = Math.max(0, nextIndex - evict)
|
||||
}
|
||||
|
||||
return {
|
||||
worktreeNavHistory: truncated,
|
||||
worktreeNavHistoryIndex: nextIndex
|
||||
}
|
||||
}
|
||||
|
||||
export function findPrevLiveWorktreeHistoryIndex(state: AppState): number | null {
|
||||
for (let i = state.worktreeNavHistoryIndex - 1; i >= 0; i--) {
|
||||
const id = state.worktreeNavHistory[i]
|
||||
if (findWorktreeById(state.worktreesByRepo, id)) {
|
||||
if (isLiveEntry(state.worktreeNavHistory[i], state)) {
|
||||
return i
|
||||
}
|
||||
}
|
||||
|
|
@ -49,8 +106,7 @@ export function findPrevLiveWorktreeHistoryIndex(state: AppState): number | null
|
|||
|
||||
export function findNextLiveWorktreeHistoryIndex(state: AppState): number | null {
|
||||
for (let i = state.worktreeNavHistoryIndex + 1; i < state.worktreeNavHistory.length; i++) {
|
||||
const id = state.worktreeNavHistory[i]
|
||||
if (findWorktreeById(state.worktreesByRepo, id)) {
|
||||
if (isLiveEntry(state.worktreeNavHistory[i], state)) {
|
||||
return i
|
||||
}
|
||||
}
|
||||
|
|
@ -76,95 +132,90 @@ export const createWorktreeNavHistorySlice: StateCreator<
|
|||
isNavigatingHistory: false,
|
||||
|
||||
recordWorktreeVisit: (worktreeId) => {
|
||||
set((s) => {
|
||||
// Why: re-activating the same worktree must not pollute history. The
|
||||
// de-dup applies only to the current entry so that A -> B -> A remains
|
||||
// a valid stack (user left B, returned to A via the sidebar).
|
||||
if (s.worktreeNavHistory[s.worktreeNavHistoryIndex] === worktreeId) {
|
||||
return s
|
||||
}
|
||||
set((s) => appendHistoryEntry(s, worktreeId))
|
||||
},
|
||||
|
||||
// Truncate any forward entries, then append and advance the index.
|
||||
const truncated = s.worktreeNavHistory.slice(0, s.worktreeNavHistoryIndex + 1)
|
||||
truncated.push(worktreeId)
|
||||
let nextIndex = s.worktreeNavHistoryIndex + 1
|
||||
|
||||
// Why: cap eviction drops the oldest entries. The index must shift left
|
||||
// by the same count so it still points at the just-appended current entry.
|
||||
if (truncated.length > MAX_HISTORY) {
|
||||
const evict = truncated.length - MAX_HISTORY
|
||||
truncated.splice(0, evict)
|
||||
nextIndex = Math.max(0, nextIndex - evict)
|
||||
}
|
||||
|
||||
return {
|
||||
worktreeNavHistory: truncated,
|
||||
worktreeNavHistoryIndex: nextIndex
|
||||
}
|
||||
})
|
||||
recordViewVisit: (entry) => {
|
||||
set((s) => appendHistoryEntry(s, entry))
|
||||
},
|
||||
|
||||
goBackWorktree: () => {
|
||||
const state = get()
|
||||
if (state.worktreeNavHistoryIndex <= 0) {
|
||||
return
|
||||
}
|
||||
const targetIndex = findPrevLiveWorktreeHistoryIndex(state)
|
||||
if (targetIndex === null) {
|
||||
return
|
||||
}
|
||||
if (!activator) {
|
||||
// Why: a silent no-op here would mean the back/forward chord simply
|
||||
// does nothing with no diagnostic. The activator is registered at
|
||||
// module init by worktree-activation.ts, so a missing activator means
|
||||
// either test setup forgot to install one or the production import
|
||||
// graph regressed.
|
||||
console.warn('goBackWorktree called before worktree activator was registered')
|
||||
return
|
||||
}
|
||||
const targetId = state.worktreeNavHistory[targetIndex]
|
||||
// Why: capture-and-restore (not force false) so re-entrant navigation
|
||||
// (e.g. a store subscriber synchronously triggers another goBack) does
|
||||
// not race on the boolean — the outer call's `finally` restores its own
|
||||
// prior value rather than clobbering state set by an inner call.
|
||||
const prevNavigating = get().isNavigatingHistory
|
||||
set({ isNavigatingHistory: true })
|
||||
try {
|
||||
// Why: activateAndRevealWorktree returns `ActivateAndRevealResult | false`;
|
||||
// `false` is the only observable failure signal. Advance the index only on
|
||||
// success so the slice stays consistent with what the user actually sees.
|
||||
const result = activator(targetId)
|
||||
if (result !== false) {
|
||||
set({ worktreeNavHistoryIndex: targetIndex })
|
||||
}
|
||||
} finally {
|
||||
set({ isNavigatingHistory: prevNavigating })
|
||||
}
|
||||
navigateToIndex(get, set, 'back')
|
||||
},
|
||||
|
||||
goForwardWorktree: () => {
|
||||
const state = get()
|
||||
navigateToIndex(get, set, 'forward')
|
||||
}
|
||||
})
|
||||
|
||||
function navigateToIndex(
|
||||
get: () => AppState,
|
||||
set: (partial: Partial<AppState>) => void,
|
||||
direction: 'back' | 'forward'
|
||||
): void {
|
||||
const state = get()
|
||||
if (direction === 'back') {
|
||||
if (state.worktreeNavHistoryIndex <= 0) {
|
||||
return
|
||||
}
|
||||
} else {
|
||||
if (state.worktreeNavHistoryIndex >= state.worktreeNavHistory.length - 1) {
|
||||
return
|
||||
}
|
||||
const targetIndex = findNextLiveWorktreeHistoryIndex(state)
|
||||
if (targetIndex === null) {
|
||||
return
|
||||
}
|
||||
if (!activator) {
|
||||
console.warn('goForwardWorktree called before worktree activator was registered')
|
||||
return
|
||||
}
|
||||
const targetId = state.worktreeNavHistory[targetIndex]
|
||||
const prevNavigating = get().isNavigatingHistory
|
||||
set({ isNavigatingHistory: true })
|
||||
try {
|
||||
const result = activator(targetId)
|
||||
if (result !== false) {
|
||||
set({ worktreeNavHistoryIndex: targetIndex })
|
||||
}
|
||||
} finally {
|
||||
set({ isNavigatingHistory: prevNavigating })
|
||||
}
|
||||
}
|
||||
})
|
||||
const targetIndex =
|
||||
direction === 'back'
|
||||
? findPrevLiveWorktreeHistoryIndex(state)
|
||||
: findNextLiveWorktreeHistoryIndex(state)
|
||||
if (targetIndex === null) {
|
||||
return
|
||||
}
|
||||
const targetEntry = state.worktreeNavHistory[targetIndex]
|
||||
|
||||
// Why: capture-and-restore (not force false) so re-entrant navigation
|
||||
// (e.g. a store subscriber synchronously triggers another goBack) does
|
||||
// not race on the boolean — the outer call's `finally` restores its own
|
||||
// prior value rather than clobbering state set by an inner call.
|
||||
const prevNavigating = get().isNavigatingHistory
|
||||
set({ isNavigatingHistory: true } as Partial<AppState>)
|
||||
try {
|
||||
if (targetEntry === 'tasks') {
|
||||
if (!viewActivator) {
|
||||
// Why: a silent no-op would mean the back/forward chord lands on a
|
||||
// Tasks history entry and appears broken. See setWorktreeNavActivator
|
||||
// rationale above.
|
||||
console.warn(
|
||||
`go${direction === 'back' ? 'Back' : 'Forward'}Worktree: view activator not registered`
|
||||
)
|
||||
return
|
||||
}
|
||||
// Why: dispatch via setActiveView (installed as viewActivator) rather
|
||||
// than openTaskPage so we don't mutate previousViewBeforeTasks or fire
|
||||
// the SWR prefetch on back-to-Tasks. activateAndRevealWorktree on the
|
||||
// other branch already switches activeView back to 'terminal'.
|
||||
viewActivator('tasks')
|
||||
set({ worktreeNavHistoryIndex: targetIndex } as Partial<AppState>)
|
||||
} else {
|
||||
if (!activator) {
|
||||
// Why: a silent no-op here would mean the back/forward chord simply
|
||||
// does nothing with no diagnostic. The activator is registered at
|
||||
// module init by worktree-activation.ts, so a missing activator means
|
||||
// either test setup forgot to install one or the production import
|
||||
// graph regressed.
|
||||
console.warn(
|
||||
`go${direction === 'back' ? 'Back' : 'Forward'}Worktree called before worktree activator was registered`
|
||||
)
|
||||
return
|
||||
}
|
||||
// Why: activateAndRevealWorktree returns `ActivateAndRevealResult | false`;
|
||||
// `false` is the only observable failure signal. Advance the index only on
|
||||
// success so the slice stays consistent with what the user actually sees.
|
||||
const result = activator(targetEntry)
|
||||
if (result !== false) {
|
||||
set({ worktreeNavHistoryIndex: targetIndex } as Partial<AppState>)
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
set({ isNavigatingHistory: prevNavigating } as Partial<AppState>)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue