diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 73726d09a..6c3dcad28 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -269,7 +269,7 @@ import { buildSidebarHostOptions } from './sidebar-host-options' import { HostSectionHeaderMenu } from './HostSectionHeaderMenu' import { ProjectHeaderActions } from './ProjectHeaderActions' import { translate } from '@/i18n/i18n' -import { folderWorkspaceKey, parseWorkspaceKey } from '../../../../shared/workspace-scope' +import { folderWorkspaceKey, getActiveSidebarWorkspaceId } from '../../../../shared/workspace-scope' import { getHostDisplayLabelOverrides } from '../../../../shared/host-setting-overrides' import { isConfirmedStaleFolderPathStatus, @@ -407,22 +407,6 @@ function getWorktreeOptionId(rowKey: string): string { return `worktree-list-option-${encodeURIComponent(rowKey)}` } -// Why: folder workspaces are tracked by the scoped active key, while older -// worktree-only paths still read activeWorktreeId. -function getActiveSidebarWorkspaceId( - activeWorkspaceKey: string | null, - activeWorktreeId: string | null -): string | null { - const scope = activeWorkspaceKey ? parseWorkspaceKey(activeWorkspaceKey) : null - if (scope?.type === 'folder') { - return folderWorkspaceKey(scope.folderWorkspaceId) - } - if (scope?.type === 'worktree') { - return scope.worktreeId - } - return activeWorktreeId -} - function getMountedWorktreeOptions(worktreeId: string, root?: ParentNode | null): HTMLElement[] { const scope = root ?? document const result: HTMLElement[] = [] diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index e43a5700c..3b1485624 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -6686,12 +6686,13 @@ describe('setWorktreesPinnedAndReveal', () => { resetRemoteRuntimeMocks() }) - it('pins a worktree and reveals it so the viewport follows it into the Pinned section', () => { + it('pins the focused 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] }, + activeWorktreeId: wt.id, revealWorktreeInSidebar: reveal } as Partial) @@ -6701,12 +6702,13 @@ describe('setWorktreesPinnedAndReveal', () => { expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) }) - it('reveals on unpin so the viewport follows the row back to its status group', () => { + it('reveals on unpin of the focused worktree so the viewport follows it 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] }, + activeWorktreeId: wt.id, revealWorktreeInSidebar: reveal } as Partial) @@ -6716,6 +6718,41 @@ describe('setWorktreesPinnedAndReveal', () => { expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) }) + it('does not scroll when unpinning an unfocused worktree, but still unpins it', () => { + const store = createTestStore() + const focused = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a', isPinned: false }) + const pinned = makeWorktree({ id: 'repo1::/b', repoId: 'repo1', path: '/b', isPinned: true }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [focused, pinned] }, + activeWorktreeId: focused.id, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([pinned.id], false) + + // The row unpins, but the viewport stays put because it isn't focused. + expect(store.getState().worktreesByRepo.repo1[1].isPinned).toBe(false) + expect(reveal).not.toHaveBeenCalled() + }) + + it('does not scroll when pinning an unfocused worktree, but still pins it', () => { + const store = createTestStore() + const focused = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a', isPinned: false }) + const other = makeWorktree({ id: 'repo1::/b', repoId: 'repo1', path: '/b', isPinned: false }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [focused, other] }, + activeWorktreeId: focused.id, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([other.id], true) + + expect(store.getState().worktreesByRepo.repo1[1].isPinned).toBe(true) + expect(reveal).not.toHaveBeenCalled() + }) + 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 }) @@ -6759,7 +6796,7 @@ describe('setWorktreesPinnedAndReveal', () => { expect(reveal).not.toHaveBeenCalled() }) - it('reveals only the first newly-pinned worktree when pinning several at once', () => { + it('pins several at once and reveals the focused row even when it is not first', () => { const store = createTestStore() const alreadyPinned = makeWorktree({ id: 'repo1::/a', @@ -6768,23 +6805,49 @@ describe('setWorktreesPinnedAndReveal', () => { 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 focused = makeWorktree({ id: 'repo1::/c', repoId: 'repo1', path: '/c', isPinned: false }) const reveal = vi.fn() store.setState({ - worktreesByRepo: { repo1: [alreadyPinned, first, second] }, + worktreesByRepo: { repo1: [alreadyPinned, first, focused] }, + activeWorktreeId: focused.id, revealWorktreeInSidebar: reveal } as Partial) - store.getState().setWorktreesPinnedAndReveal([alreadyPinned.id, first.id, second.id], true) + store.getState().setWorktreesPinnedAndReveal([alreadyPinned.id, first.id, focused.id], true) + // Only the focused row is revealed, not the first-changed one. expect(reveal).toHaveBeenCalledTimes(1) - expect(reveal).toHaveBeenCalledWith(first.id, { behavior: 'smooth', highlight: true }) + expect(reveal).toHaveBeenCalledWith(focused.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) }) + + it('pins several at once without scrolling when none of them are focused', () => { + const store = createTestStore() + 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 elsewhere = makeWorktree({ + id: 'repo1::/z', + repoId: 'repo1', + path: '/z', + isPinned: false + }) + const reveal = vi.fn() + store.setState({ + worktreesByRepo: { repo1: [first, second, elsewhere] }, + activeWorktreeId: elsewhere.id, + revealWorktreeInSidebar: reveal + } as Partial) + + store.getState().setWorktreesPinnedAndReveal([first.id, second.id], true) + + expect(reveal).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) + expect(store.getState().worktreesByRepo.repo1[1].isPinned).toBe(true) + }) }) describe('migrateWorktreeIdentity', () => { diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index aca7efcd3..5541ad0ef 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -73,6 +73,7 @@ import { import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { folderWorkspaceKey, + getActiveSidebarWorkspaceId, isWorkspaceKey, parseWorkspaceKey, worktreeWorkspaceKey @@ -4128,33 +4129,44 @@ export const createWorktreeSlice: StateCreator }, setWorktreesPinnedAndReveal: (worktreeIds, isPinned) => { + // Only follow a toggled row with the viewport when it's the focused + // worktree; pinning/unpinning an unfocused card shouldn't yank the user's + // scroll to a row they aren't looking at. + const activeSidebarWorktreeId = getActiveSidebarWorkspaceId( + get().activeWorkspaceKey, + get().activeWorktreeId + ) // 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 didChange = false let revealWorktreeId: string | null = null for (const worktreeId of worktreeIds) { const current = get().getKnownWorktreeById(worktreeId) if (!current || current.isPinned === isPinned) { continue } + didChange = true const workspaceScope = parseWorkspaceKey(worktreeId) if (workspaceScope?.type === 'folder') { void get().updateWorktreeMeta(worktreeId, { isPinned }) } else { updates.set(worktreeId, { isPinned }) } - if (revealWorktreeId === null) { + if (revealWorktreeId === null && worktreeId === activeSidebarWorktreeId) { revealWorktreeId = worktreeId } } - if (revealWorktreeId === null) { + if (!didChange) { return } // updateWorktreesMeta applies its store update synchronously (only the // persistence is async), so the reveal below resolves against a render // where the shortcut row already exists. void get().updateWorktreesMeta(updates) - get().revealWorktreeInSidebar(revealWorktreeId, { behavior: 'smooth', highlight: true }) + if (revealWorktreeId !== null) { + get().revealWorktreeInSidebar(revealWorktreeId, { behavior: 'smooth', highlight: true }) + } }, markWorktreeUnread: (worktreeId) => { diff --git a/src/shared/workspace-scope.ts b/src/shared/workspace-scope.ts index 289657226..6aed11b6e 100644 --- a/src/shared/workspace-scope.ts +++ b/src/shared/workspace-scope.ts @@ -29,3 +29,19 @@ export function parseWorkspaceKey(value: string): WorkspaceScope | null { export function isWorkspaceKey(value: string): value is WorkspaceKey { return parseWorkspaceKey(value) !== null } + +// Why: folder workspaces are tracked by the scoped active key, while older +// worktree-only paths still read activeWorktreeId. +export function getActiveSidebarWorkspaceId( + activeWorkspaceKey: string | null, + activeWorktreeId: string | null +): string | null { + const scope = activeWorkspaceKey ? parseWorkspaceKey(activeWorkspaceKey) : null + if (scope?.type === 'folder') { + return folderWorkspaceKey(scope.folderWorkspaceId) + } + if (scope?.type === 'worktree') { + return scope.worktreeId + } + return activeWorktreeId +}