From d617a08a8ac81cc194f94ce34bfb18844e282d59 Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Tue, 2 Jun 2026 20:56:34 -0700 Subject: [PATCH] fix: follow a worktree in the sidebar when you pin or unpin it --- .../sidebar/WorktreeContextMenu.tsx | 10 +- .../src/components/sidebar/WorktreeList.tsx | 23 +--- .../src/store/slices/worktree-helpers.ts | 6 + .../src/store/slices/worktrees.test.ts | 107 ++++++++++++++++++ src/renderer/src/store/slices/worktrees.ts | 25 ++++ 5 files changed, 149 insertions(+), 22 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index d1c850534..64c50fcba 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -210,6 +210,7 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ const defaultSelectedWorktrees = useMemo(() => [worktree], [worktree]) const effectiveSelectedWorktrees = selectedWorktrees ?? defaultSelectedWorktrees const updateWorktreeMeta = useAppStore((s) => s.updateWorktreeMeta) + const setWorktreesPinnedAndReveal = useAppStore((s) => s.setWorktreesPinnedAndReveal) const workspaceStatuses = useAppStore((s) => s.workspaceStatuses) const openModal = useAppStore((s) => s.openModal) const projectGroups = useAppStore((s) => s.projectGroups) @@ -219,8 +220,9 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ const deleteState = useAppStore((s) => s.deleteStateByWorktreeId[worktree.id]) const [menuOpen, setMenuOpen] = useState(false) const [menuPoint, setMenuPoint] = useState({ x: 0, y: 0 }) - const [contextWorktrees, setContextWorktrees] = - useState(effectiveSelectedWorktrees) + const [contextWorktrees, setContextWorktrees] = useState( + effectiveSelectedWorktrees + ) const [createGroupDialogOpen, setCreateGroupDialogOpen] = useState(false) const isDeleting = deleteState?.isDeleting ?? false const repoMap = useRepoMap() @@ -306,8 +308,8 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ }, [worktree.id, worktree.isUnread, updateWorktreeMeta]) const handleTogglePin = useCallback(() => { - updateWorktreeMeta(worktree.id, { isPinned: !worktree.isPinned }) - }, [worktree.id, worktree.isPinned, updateWorktreeMeta]) + setWorktreesPinnedAndReveal([worktree.id], !worktree.isPinned) + }, [worktree.id, worktree.isPinned, setWorktreesPinnedAndReveal]) const handleCreateGroupFromRepo = useCallback(() => { if (!repo) { diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index e597201a0..a9b58e21e 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -3497,6 +3497,7 @@ const WorktreeList = React.memo(function WorktreeList({ const activeModal = useAppStore((s) => s.activeModal) const pendingRevealWorktree = useAppStore((s) => s.pendingRevealWorktree) const revealWorktreeInSidebar = useAppStore((s) => s.revealWorktreeInSidebar) + const setWorktreesPinnedAndReveal = useAppStore((s) => s.setWorktreesPinnedAndReveal) const clearPendingRevealWorktreeId = useAppStore((s) => s.clearPendingRevealWorktreeId) const agentSendPopoverTargetMode = useAppStore((s) => s.agentSendPopoverTargetMode) // Why: agent-send eligibility only matters while the picker is open. When it @@ -4386,30 +4387,16 @@ const WorktreeList = React.memo(function WorktreeList({ const pinWorktree = useCallback( (worktreeId: string) => { - const current = worktreeMap.get(worktreeId) - if (!current || current.isPinned) { - return - } - void updateWorktreeMeta(worktreeId, { isPinned: true }) + setWorktreesPinnedAndReveal([worktreeId], true) }, - [updateWorktreeMeta, worktreeMap] + [setWorktreesPinnedAndReveal] ) const pinWorktrees = useCallback( (worktreeIds: readonly string[]) => { - const updates = new Map() - for (const worktreeId of worktreeIds) { - const current = worktreeMap.get(worktreeId) - if (!current || current.isPinned) { - continue - } - updates.set(worktreeId, { isPinned: true }) - } - if (updates.size > 0) { - void updateWorktreesMeta(updates) - } + setWorktreesPinnedAndReveal(worktreeIds, true) }, - [updateWorktreesMeta, worktreeMap] + [setWorktreesPinnedAndReveal] ) const reorderWorktrees = useCallback( diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 6d02daeda..cf738b382 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -128,6 +128,12 @@ export type WorktreeSlice = { updateWorktreesMeta: ( updatesByWorktreeId: ReadonlyMap> ) => Promise + /** + * Pin/unpin worktrees, then reveal the first changed one. The reveal is the + * point: pinning moves the row to the Pinned section (unpinning moves it + * back), so without it the viewport stays put and the user loses the row. + */ + setWorktreesPinnedAndReveal: (worktreeIds: readonly string[], isPinned: boolean) => void markWorktreeUnread: (worktreeId: string) => void observeTerminalGitHubPullRequestLink: (worktreeId: string, link: TerminalGitHubPRLink) => void /** Clear the worktree's unread dot. Called on user interaction with any diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 49c8f4c0e..59a49ce94 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -3049,3 +3049,110 @@ describe('markWorktreeVisited', () => { expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ 'repo1::/hidden': 100 }) }) }) + +describe('setWorktreesPinnedAndReveal', () => { + beforeEach(() => { + vi.clearAllMocks() + resetRemoteRuntimeMocks() + }) + + it('pins a worktree and reveals it so the viewport follows it into the Pinned section', () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a', isPinned: false }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [wt] }, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([wt.id], true) + + expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) + expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) + }) + + it('reveals on unpin so the viewport follows the row back to its status group', () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a', isPinned: true }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [wt] }, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([wt.id], false) + + expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(false) + expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) + }) + + it('skips a no-op toggle without requesting a reveal', () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a', isPinned: true }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [wt] }, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([wt.id], true) + + expect(reveal).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) + }) + + it('does nothing for an unknown worktree id', () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a', isPinned: false }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [wt] }, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal(['repo1::/missing'], true) + + expect(reveal).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(false) + }) + + it('does nothing for an empty id list', () => { + const store = createTestStore() + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [] }, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([], true) + + expect(reveal).not.toHaveBeenCalled() + }) + + it('reveals only the first newly-pinned worktree when pinning several at once', () => { + const store = createTestStore() + const alreadyPinned = makeWorktree({ + id: 'repo1::/a', + repoId: 'repo1', + path: '/a', + isPinned: true + }) + const first = makeWorktree({ id: 'repo1::/b', repoId: 'repo1', path: '/b', isPinned: false }) + const second = makeWorktree({ id: 'repo1::/c', repoId: 'repo1', path: '/c', isPinned: false }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [alreadyPinned, first, second] }, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([alreadyPinned.id, first.id, second.id], true) + + expect(reveal).toHaveBeenCalledTimes(1) + expect(reveal).toHaveBeenCalledWith(first.id, { behavior: 'smooth', highlight: true }) + // Every targeted row is pinned, not just the revealed one, and the + // already-pinned row is left untouched. + expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) + expect(store.getState().worktreesByRepo.repo1[1].isPinned).toBe(true) + expect(store.getState().worktreesByRepo.repo1[2].isPinned).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index f7692a577..43f87b4c3 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -1690,6 +1690,31 @@ export const createWorktreeSlice: StateCreator ) }, + setWorktreesPinnedAndReveal: (worktreeIds, isPinned) => { + // Skip worktrees already in the target state so a no-op toggle doesn't + // scroll the viewport away from where the user is. + const updates = new Map>() + let revealWorktreeId: string | null = null + for (const worktreeId of worktreeIds) { + const current = get().getKnownWorktreeById(worktreeId) + if (!current || current.isPinned === isPinned) { + continue + } + updates.set(worktreeId, { isPinned }) + if (revealWorktreeId === null) { + revealWorktreeId = worktreeId + } + } + if (revealWorktreeId === null) { + return + } + // updateWorktreesMeta applies its store update synchronously (only the + // persistence is async), so the reveal below resolves against a render + // where the row already sits in its new section. + void get().updateWorktreesMeta(updates) + get().revealWorktreeInSidebar(revealWorktreeId, { behavior: 'smooth', highlight: true }) + }, + markWorktreeUnread: (worktreeId) => { // Why: terminal attention should remain visible until the user engages // with the worktree. Interaction with a pane inside the worktree dismisses