diff --git a/docs/new-worktree-sidebar-reveal.md b/docs/new-worktree-sidebar-reveal.md new file mode 100644 index 00000000000..75abb7bd647 --- /dev/null +++ b/docs/new-worktree-sidebar-reveal.md @@ -0,0 +1,79 @@ +# New Worktree Sidebar Reveal + +## Problem + +Issue https://github.com/stablyai/orca-internal/issues/350 asks that newly created worktrees jump into view in the left sidebar list with no scroll animation. + +Current behavior: + +- `activateAndRevealWorktree(...)` always calls `state.revealWorktreeInSidebar(worktreeId)` with no options. +- `revealWorktreeInSidebar` defaults `behavior` to `'smooth'` (`ui.ts`). +- `WorktreeList` forwards that behavior to `virtualizer.scrollToIndex(..., { behavior })`. + +Result: off-screen targets animate by default, including freshly created worktrees. + +## Root Cause + +`activateAndRevealWorktree` conflates two intents: + +1. activate existing worktree navigation (smooth reveal is fine); +2. activate a just-created worktree (must jump immediately). + +Created-worktree callers cannot currently express reveal intent, so they inherit `'smooth'`. + +## Scope and Non-goals + +- Add an opt-in reveal behavior at activation call sites. +- Apply `'auto'` only where the worktree is newly created/added in that flow. +- Preserve existing behavior for normal worktree navigation (clicks, keyboard, history nav, palette selection of existing worktrees, port/status-driven activation) unless a caller opts in. +- Do not change direct raw reveal paths in IPC handlers that intentionally call `store.revealWorktreeInSidebar(...)` outside `activateAndRevealWorktree` (terminal/editor/mobile focus paths remain smooth). +- Do not change sorting/grouping/filter UI, list virtualization strategy, or sidebar styling. + +## Design + +1. Extend `activateAndRevealWorktree` options: + - `sidebarRevealBehavior?: PendingSidebarWorktreeReveal['behavior']`. +2. In `activateAndRevealWorktree`, call: + - `state.revealWorktreeInSidebar(worktreeId, { behavior: opts.sidebarRevealBehavior })` when provided; + - otherwise keep `state.revealWorktreeInSidebar(worktreeId)` so default behavior stays unchanged. +3. Pass `sidebarRevealBehavior: 'auto'` only from created/added-worktree flows: + - `useComposerState` full-create path; + - `useComposerState` quick-create path; + - `useIpcEvents` `onActivateWorktree` only when the event corresponds to a newly created worktree; + - `launch-work-item-direct`; + - folder add/create flows that activate a newly-added synthetic folder worktree (`AddRepoCreateStep` folder branch, `NonGitFolderDialog`, and `repos` slice `addNonGitFolder` path). +4. Keep existing navigation activations smooth, including: + - `AddRepoDialog` / `ProjectAddedDialog` “open primary worktree” actions (these can target pre-existing worktrees, not guaranteed newly created); + - all existing `activateAndRevealWorktree(...)` callers that do not opt in. +5. Keep `WorktreeList` reveal effect unchanged; it already honors `pendingRevealWorktree.behavior`. + +## Correctness Notes and Edge Cases + +- Repo-filter clearing remains unchanged: `activateAndRevealWorktree` only clears `filterRepoIds` when target repo is excluded. +- Other visibility constraints are not auto-cleared. If the target exists but is hidden by other sidebar state, `resolvePendingSidebarReveal(...)` keeps the reveal pending. +- The reveal effect uncollapses lineage/group containers before scroll; behavior changes only animation mode, not visibility resolution. +- Pending reveal is a single store slot (`pendingRevealWorktree`). Concurrent reveal requests are last-writer-wins; this change should not alter that behavior. +- If activation cannot resolve the worktree (`getKnownWorktreeById` miss), behavior remains unchanged (`false`, no reveal queued). +- `ui:activateWorktree` is an overloaded IPC used by both creation and non-creation activation paths. The renderer must choose `'auto'` only for create cases (for example, worktree absent before fetch and present after fetch), and keep default smooth reveal for existing-worktree activations. +- Multi-window consistency remains per renderer window store; each window applies its own reveal behavior locally. +- This change is renderer-only; it does not add main-process coordination and does not make reveal ordering transactional across concurrent async creators. + +## Tests + +Add/adjust focused tests in `worktree-activation` coverage: + +- explicit `sidebarRevealBehavior: 'auto'` is forwarded to `revealWorktreeInSidebar(worktreeId, { behavior: 'auto' })`; +- no option still calls `revealWorktreeInSidebar(worktreeId)` (store default remains smooth). + +Add call-site regression tests (recommended, small): + +- one composer create path passes `'auto'`; +- one non-created navigation path stays default (no behavior option). + +## Rollout + +1. Add `sidebarRevealBehavior` option in `activateAndRevealWorktree`. +2. Update created-worktree callers to pass `'auto'`. +3. Add tests above. +4. Run targeted Vitest tests, then `pnpm typecheck` and `pnpm lint`. +5. Validate in Electron: with sidebar overflow, create a worktree and verify the list jumps to it without smooth animation. diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 1cfe10cf9b7..0b18f5f3f09 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -1409,7 +1409,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { skipRestoreFocusRef.current = true // Why: selecting a project or repo group is a sidebar navigation action; // it should reveal the grouping row without activating an arbitrary workspace. - revealSidebarRow(result.rowKey, { highlight: true }) + revealSidebarRow(result.rowKey, { behavior: 'smooth', highlight: true }) recordFeatureInteraction('cmd-j') closeModal() setSelectedItemId('') diff --git a/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx b/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx index 33de6b6eb1d..5ec3354ec22 100644 --- a/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx +++ b/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx @@ -82,7 +82,10 @@ const NonGitFolderDialog = React.memo(function NonGitFolderDialog() { onboarding, hadProjectBeforeAdd ) - activateAndRevealWorktree(folderWorktree.id, startup ? { startup } : undefined) + activateAndRevealWorktree(folderWorktree.id, { + sidebarRevealBehavior: 'auto', + ...(startup ? { startup } : {}) + }) } } catch (err) { // This code path calls addRemote directly (not through the store), diff --git a/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx b/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx index 66409526770..f3580472439 100644 --- a/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx +++ b/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx @@ -145,7 +145,9 @@ describe('ProjectAddedDialog', () => { expect(markup).toBe('') expect(mocks.state.fetchWorktrees).toHaveBeenCalledWith('repo-1') - expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('repo-1::folder') + expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('repo-1::folder', { + sidebarRevealBehavior: 'auto' + }) expect(mocks.state.closeModal).toHaveBeenCalledTimes(1) expect(mocks.finishProjectAddWithDefaultCheckout).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx b/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx index 34f53ffd6ac..6532a39cae8 100644 --- a/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx +++ b/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx @@ -81,7 +81,7 @@ export default function ProjectAddedDialog(): null { } const folderWorktree = useAppStore.getState().worktreesByRepo[repoId]?.[0] if (folderWorktree) { - activateAndRevealWorktree(folderWorktree.id) + activateAndRevealWorktree(folderWorktree.id, { sidebarRevealBehavior: 'auto' }) } closeModal() })() diff --git a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx index 033cb286dfa..7bfda38d231 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx @@ -36,7 +36,7 @@ function revealCompactAgentCard(agentListRoot: HTMLElement | null): void { if (!(sidebarElement instanceof HTMLElement) || !worktreeOptionElement) { return } - revealElementInScrollContainer(sidebarElement, worktreeOptionElement) + revealElementInScrollContainer(sidebarElement, worktreeOptionElement, 'auto') } type Props = { @@ -238,7 +238,7 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ // Why: defer the reveal scroll out of the expand commit. Running it inline // forces a synchronous sidebar layout that blocks the animation's opening // frames (a visible jump); next-frame keeps the open smooth and the - // instant reveal still lands before the height transition finishes. + // ScrollBehavior 'auto' still lands before the height transition finishes. const handle = requestAnimationFrame(() => { revealCompactAgentCard(compactAgentListRootRef.current) }) diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index a1542323e62..e9403b41b11 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -100,7 +100,6 @@ import { type RenderRow } from './worktree-list-virtual-rows' import { - getElementCenteringScrollPadding, revealElementInScrollContainer, WORKTREE_SIDEBAR_REVEAL_TOP_INSET } from './worktree-sidebar-reveal' @@ -288,7 +287,6 @@ import { import { getFolderWorkspaceCardPrDisplay } from './folder-workspace-card-pr-display' export { - getCenteringScrollPadding, getScrollTopToRevealBounds, WORKTREE_SIDEBAR_REVEAL_TOP_INSET } from './worktree-sidebar-reveal' @@ -472,15 +470,31 @@ function markSidebarWorktreeActiveImmediately(worktreeId: string, primaryRowKey? } } +function revealMountedWorktreeElement( + container: HTMLElement, + worktreeId: string, + behavior: ScrollBehavior, + optionId?: string +): HTMLElement | null { + const element = optionId + ? document.getElementById(optionId) + : getMountedWorktreeOptions(worktreeId, container)[0] + if (!element || !container.contains(element)) { + return null + } + return revealElementInScrollContainer(container, element, behavior) ? element : null +} + function revealMountedSidebarRowElement( container: HTMLElement, - rowKey: string + rowKey: string, + behavior: ScrollBehavior ): HTMLElement | null { const element = document.getElementById(getWorktreeOptionId(rowKey)) if (!element || !container.contains(element)) { return null } - return revealElementInScrollContainer(container, element) ? element : null + return revealElementInScrollContainer(container, element, behavior) ? element : null } function getRenderRowSidebarKey(row: RenderRow): string | null { @@ -1322,11 +1336,6 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp const [worktreeDragState, setWorktreeDragState] = useState( WORKTREE_ROW_DRAG_INITIAL_STATE ) - const [centeringScrollPadding, setCenteringScrollPadding] = useState<{ - targetKey: string - start: number - end: number - } | null>(null) const [pendingRevealRetryTick, setPendingRevealRetryTick] = useState(0) const [documentVisibilityRevision, setDocumentVisibilityRevision] = useState(0) const [highlightedRevealRowKey, setHighlightedRevealRowKey] = useState(null) @@ -1976,8 +1985,6 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp ), overscan: 10, gap: 6, - paddingStart: centeringScrollPadding?.start ?? 0, - paddingEnd: centeringScrollPadding?.end ?? 0, // Why: the active sticky group header is rendered inside the virtual list, // so TanStack's scroll math needs the same top inset as the exact DOM reveal. scrollPaddingStart: WORKTREE_SIDEBAR_REVEAL_TOP_INSET, @@ -2128,35 +2135,14 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp clearPendingRevealWorktreeId() } } - const optionId = getRenderRowOptionId(targetRow, pendingRevealWorktree.worktreeId) - const centeringTargetKey = `worktree:${optionId ?? pendingRevealWorktree.worktreeId}` - if (centeringScrollPadding && centeringScrollPadding.targetKey !== centeringTargetKey) { - setCenteringScrollPadding(null) - retryExactRevealOnNextFrame() - return - } - const option = optionId - ? document.getElementById(optionId) - : getMountedWorktreeOptions(pendingRevealWorktree.worktreeId, container)[0] - const mountedOption = container && option && container.contains(option) ? option : null - if (container && mountedOption) { - const additionalPadding = getElementCenteringScrollPadding(container, mountedOption) - if (additionalPadding && (additionalPadding.start > 0 || additionalPadding.end > 0)) { - // Why: the first and last virtual rows need temporary boundary space - // or the browser clamps their centered position to the list edge. - setCenteringScrollPadding({ - targetKey: centeringTargetKey, - start: (centeringScrollPadding?.start ?? 0) + additionalPadding.start, - end: (centeringScrollPadding?.end ?? 0) + additionalPadding.end - }) - retryExactRevealOnNextFrame() - return - } - } - const revealedOption = - container && mountedOption && revealElementInScrollContainer(container, mountedOption) - ? mountedOption - : null + const revealedOption = container + ? revealMountedWorktreeElement( + container, + pendingRevealWorktree.worktreeId, + pendingRevealWorktree.behavior, + getRenderRowOptionId(targetRow, pendingRevealWorktree.worktreeId) + ) + : null if (revealedOption) { if (pendingRevealWorktree.highlight) { const revealedRowKey = @@ -2228,7 +2214,6 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp pendingRevealRetryTick, flashRevealedRow, setRenamingWorktreeId, - centeringScrollPadding, schedulePendingRevealFrame, cancelPendingRevealFrames ]) @@ -2312,16 +2297,12 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp } const container = scrollRef.current - // Why: only clear stale boundary padding here. While a worktree reveal is - // still pending, clearing would wipe the padding it just accumulated and - // its boundary target could exhaust retries without ever centering. - if (centeringScrollPadding && !pendingRevealWorktree) { - setCenteringScrollPadding(null) - retryExactRevealOnNextFrame() - return - } const revealedElement = container - ? revealMountedSidebarRowElement(container, pendingRevealSidebarRow.rowKey) + ? revealMountedSidebarRowElement( + container, + pendingRevealSidebarRow.rowKey, + pendingRevealSidebarRow.behavior + ) : null if (revealedElement) { if (pendingRevealSidebarRow.highlight) { @@ -2345,7 +2326,6 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp } }, [ pendingRevealSidebarRow, - pendingRevealWorktree, repoMap, projectGroups, projectGrouping, @@ -2357,7 +2337,6 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp pendingRevealRetryTick, flashRevealedRow, clearPendingRevealSidebarRow, - centeringScrollPadding, schedulePendingRevealFrame, cancelPendingRevealFrames ]) @@ -6681,6 +6660,7 @@ const WorktreeList = React.memo(function WorktreeList({ { target: { type: 'sidebar-row' } } > revealSidebarRow(detail.target.rowKey, { + behavior: 'smooth', highlight: sidebarDetail.highlight !== false }) return @@ -6702,6 +6682,7 @@ const WorktreeList = React.memo(function WorktreeList({ clearFilters() } revealWorktreeInSidebar(currentSidebarWorktreeId, { + behavior: 'smooth', highlight: true, beginRename: (detail as { beginRename?: boolean } | undefined)?.beginRename === true }) diff --git a/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts b/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts index 83caaaa6d15..51a5ad755b5 100644 --- a/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts +++ b/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts @@ -213,7 +213,9 @@ describe('useCreateRepo default-checkout handoff', () => { kind: 'git' }) expect(mocks.fetchWorktrees).toHaveBeenCalledWith(repo.id) - expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith(worktree.id) + expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith(worktree.id, { + sidebarRevealBehavior: 'auto' + }) expect(mocks.markOnboardingProjectAdded).toHaveBeenCalledWith('addedFolder') expect(closeModal).toHaveBeenCalled() expect(mocks.onGitRepoReady).not.toHaveBeenCalled() diff --git a/src/renderer/src/components/sidebar/useCreateRepo.ts b/src/renderer/src/components/sidebar/useCreateRepo.ts index eb42f347e6f..abf35db7e66 100644 --- a/src/renderer/src/components/sidebar/useCreateRepo.ts +++ b/src/renderer/src/components/sidebar/useCreateRepo.ts @@ -190,7 +190,7 @@ export function useCreateRepo( } const folderWorktree = useAppStore.getState().worktreesByRepo[repo.id]?.[0] if (folderWorktree) { - activateAndRevealWorktree(folderWorktree.id) + activateAndRevealWorktree(folderWorktree.id, { sidebarRevealBehavior: 'auto' }) } await markOnboardingProjectAdded('addedFolder') closeModal() diff --git a/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts b/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts index f9a5a5f62ce..924a4edc88a 100644 --- a/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { countRecordKeysByReference, - getCenteringScrollPadding, getScrollTopToRevealBounds, resolvePendingSidebarReveal, WORKTREE_SIDEBAR_REVEAL_TOP_INSET, @@ -44,11 +43,8 @@ const makeImportedCardRow = (): Extract ({ scrollTop, clientHeight, scrollHeight }) as HTMLElement +const makeScrollContainer = (scrollTop: number, clientHeight: number): HTMLElement => + ({ scrollTop, clientHeight }) as HTMLElement describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => { it('counts record keys once per object reference', () => { @@ -125,64 +121,64 @@ describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => { }) describe('getScrollTopToRevealBounds', () => { - it('requests leading space when the first target would be clamped above center', () => { - const container = makeScrollContainer(0, 400) - - expect( - getCenteringScrollPadding( - container, - { - start: 34, - end: 74 - }, - WORKTREE_SIDEBAR_REVEAL_TOP_INSET - ) - ).toEqual({ start: 163, end: 0 }) - }) - - it('centers a fully visible target', () => { + it('treats the sticky header as occluding the viewport top', () => { const container = makeScrollContainer(100, 400) expect( getScrollTopToRevealBounds( container, { - start: 300, - end: 400 - }, - WORKTREE_SIDEBAR_REVEAL_TOP_INSET - ) - ).toBe(133) - }) - - it('centers within the area below the sticky header', () => { - const container = makeScrollContainer(100, 400) - - expect( - getScrollTopToRevealBounds( - container, - { - start: 300, - end: 400 + start: 100, + end: 216 }, GROUP_HEADER_ROW_HEIGHT ) - ).toBe(136) + ).toBe(72) }) - it('requests trailing space when the last target would be clamped below center', () => { - const container = makeScrollContainer(100, 400, 500) + it('includes extra reveal clearance for the highlight ring', () => { + const container = makeScrollContainer(100, 400) expect( - getCenteringScrollPadding( + getScrollTopToRevealBounds( container, { - start: 430, - end: 470 + start: 100, + end: 216 }, WORKTREE_SIDEBAR_REVEAL_TOP_INSET ) - ).toEqual({ start: 0, end: 133 }) + ).toBe(66) + }) + + it('does not scroll when the bounds are below the sticky header', () => { + const container = makeScrollContainer(100, 400) + + expect( + getScrollTopToRevealBounds( + container, + { + start: 128, + end: 244 + }, + GROUP_HEADER_ROW_HEIGHT + ) + ).toBeNull() + }) + + it('keeps the viewport bottom independent of the sticky header inset', () => { + const container = makeScrollContainer(100, 400) + + expect( + getScrollTopToRevealBounds( + container, + { + start: 430, + end: 520 + }, + GROUP_HEADER_ROW_HEIGHT + ) + ).toBe(120) }) }) diff --git a/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts b/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts index 850fc45704d..933261f20a4 100644 --- a/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts +++ b/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts @@ -8,17 +8,15 @@ describe('getScrollTopToRevealBounds', () => { clientHeight }) as HTMLElement - it('centers a mounted current workspace card that starts above the viewport', () => { - // Raw result may be negative; boundary reveals convert the deficit into - // temporary paddingStart instead of clamping (see getCenteringScrollPadding). - expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 60, end: 120 })).toBe(-10) + it('scrolls upward to reveal a mounted current workspace card above the viewport', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 60, end: 120 })).toBe(60) }) - it('centers a mounted current workspace card that starts below the viewport', () => { - expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 250, end: 340 })).toBe(195) + it('scrolls downward to reveal a mounted current workspace card below the viewport', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 250, end: 340 })).toBe(140) }) - it('recenters a card that is already fully visible', () => { - expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 125, end: 260 })).toBe(92.5) + it('does not scroll when the current workspace card is already fully visible', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 125, end: 260 })).toBeNull() }) }) diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts b/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts index 765efc2eabc..38b158532af 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts @@ -9,11 +9,6 @@ type SidebarRevealBounds = { end: number } -type SidebarCenteringScrollPadding = { - start: number - end: number -} - function getElementScrollBounds(container: HTMLElement, element: Element): SidebarRevealBounds { const containerRect = container.getBoundingClientRect() const elementRect = element.getBoundingClientRect() @@ -27,40 +22,24 @@ export function getScrollTopToRevealBounds( container: HTMLElement, bounds: SidebarRevealBounds, topInset = 0 -): number { +): number | null { const viewportTopInset = Math.max(0, Math.min(container.clientHeight, topInset)) - // Why: the sticky header reduces the usable viewport, so center within the - // visible area below it instead of behind it. - const targetCenter = bounds.start + (bounds.end - bounds.start) / 2 - const viewportCenterOffset = (viewportTopInset + container.clientHeight) / 2 - return targetCenter - viewportCenterOffset -} - -export function getCenteringScrollPadding( - container: HTMLElement, - bounds: SidebarRevealBounds, - topInset = 0 -): SidebarCenteringScrollPadding { - const desiredScrollTop = getScrollTopToRevealBounds(container, bounds, topInset) - const maxScrollTop = Math.max(0, container.scrollHeight - container.clientHeight) - return { - start: Math.ceil(Math.max(0, -desiredScrollTop)), - end: Math.ceil(Math.max(0, desiredScrollTop - maxScrollTop)) + const viewportTop = container.scrollTop + viewportTopInset + const viewportBottom = container.scrollTop + container.clientHeight + if (bounds.start < viewportTop) { + return bounds.start - viewportTopInset } + if (bounds.end > viewportBottom) { + return bounds.end - container.clientHeight + } + return null } -export function getElementCenteringScrollPadding( +export function revealElementInScrollContainer( container: HTMLElement, element: Element, - topInset = WORKTREE_SIDEBAR_REVEAL_TOP_INSET -): SidebarCenteringScrollPadding | null { - if (!container.contains(element)) { - return null - } - return getCenteringScrollPadding(container, getElementScrollBounds(container, element), topInset) -} - -export function revealElementInScrollContainer(container: HTMLElement, element: Element): boolean { + behavior: ScrollBehavior +): boolean { if (!container.contains(element)) { return false } @@ -69,8 +48,17 @@ export function revealElementInScrollContainer(container: HTMLElement, element: getElementScrollBounds(container, element), WORKTREE_SIDEBAR_REVEAL_TOP_INSET ) - // Why: sidebar reveal is a focus handoff, so reposition immediately instead - // of making the user track an animated list. - container.scrollTop = Math.max(0, nextScrollTop) + if (nextScrollTop === null) { + return true + } + // Why: honor the user's reduced-motion preference by jumping instantly instead of + // animating a smooth scroll (also makes the reveal deterministic in headless + // environments that never tick the smooth-scroll animation). + const prefersReducedMotion = + typeof window !== 'undefined' && + window.matchMedia?.('(prefers-reduced-motion: reduce)').matches === true + const resolvedBehavior: ScrollBehavior = + behavior === 'smooth' && prefersReducedMotion ? 'auto' : behavior + container.scrollTo({ top: Math.max(0, nextScrollTop), behavior: resolvedBehavior }) return true } diff --git a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts index 35686a4abee..8cd870aec13 100644 --- a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts @@ -153,7 +153,9 @@ describe('forkAgentSessionFromPane', () => { launchSource: 'terminal_context_menu' }) ) - expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork') + expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork', { + sidebarRevealBehavior: 'auto' + }) expect(mockToast.success).toHaveBeenCalledWith( 'Top-level session fork opened in a new workspace' ) @@ -344,7 +346,9 @@ describe('forkAgentSessionFromPane', () => { undefined ) expect(mockLaunchAgentInNewTab).not.toHaveBeenCalled() - expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork') + expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork', { + sidebarRevealBehavior: 'auto' + }) expect(mockWriteClipboardText).toHaveBeenCalledWith( expect.stringContaining('Assistant: here is the current plan') ) diff --git a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts index 7f2d62b4782..6b36a638749 100644 --- a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts +++ b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts @@ -266,7 +266,7 @@ export async function startAgentSessionFork(fork: PreparedAgentSessionFork): Pro const forkWorktreeId = created.worktree.id if (!fork.agent) { - activateAndRevealWorktree(forkWorktreeId) + activateAndRevealWorktree(forkWorktreeId, { sidebarRevealBehavior: 'auto' }) return copyAgentSessionForkContext(fork) } await preflightForkAgentTrust({ @@ -287,7 +287,7 @@ export async function startAgentSessionFork(fork: PreparedAgentSessionFork): Pro launchSource: 'terminal_context_menu', ...(launchPlatform ? { launchPlatform } : {}) }) - activateAndRevealWorktree(forkWorktreeId) + activateAndRevealWorktree(forkWorktreeId, { sidebarRevealBehavior: 'auto' }) if (!result) { return copyAgentSessionForkContext(fork) diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 4f4b6ea8b10..d1316b98678 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -3687,6 +3687,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS startupPlan.launchToken = createBrowserUuid() } const activation = activateAndRevealWorktree(worktree.id, { + sidebarRevealBehavior: 'auto', setup: result.setup, defaultTabs: result.defaultTabs, issueCommand, diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index f8d46e38b8c..bef55602893 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -4025,6 +4025,7 @@ describe('useIpcEvents CLI-created worktree activation', () => { expect(activateAndRevealWorktree).toHaveBeenCalledTimes(1) expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-new', { setup, + sidebarRevealBehavior: 'auto', notifyHostRuntime: false }) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 80e5c2e30c0..fab0f38e013 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -931,10 +931,12 @@ export function useIpcEvents(): void { // stream and is allowed through this helper separately. return } + const existedBeforeFetch = Boolean(useAppStore.getState().getKnownWorktreeById(worktreeId)) // Why: fetch worktrees first so the activation helper can resolve // the CLI-created worktree via findWorktreeById — it arrived from // the main process and is not yet in the renderer state. await useAppStore.getState().fetchWorktrees(repoId) + const existsAfterFetch = Boolean(useAppStore.getState().getKnownWorktreeById(worktreeId)) // Why: route through activateAndRevealWorktree so CLI-created // worktrees share the canonical activation path with UI-created // ones. This records the visit in the back/forward history stack @@ -944,6 +946,7 @@ export function useIpcEvents(): void { ...(setup ? { setup } : {}), ...(startup ? { startup } : {}), ...(defaultTabs ? { defaultTabs } : {}), + ...(!existedBeforeFetch && existsAfterFetch ? { sidebarRevealBehavior: 'auto' } : {}), // Why: this activation already came from the host runtime event stream. // Echoing it back as worktree.activate can create a selection loop. notifyHostRuntime: false diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index 7d88276cc51..61f49274edb 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -216,6 +216,7 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom !isTuiAgentEnabled(agentOverride, latestStore.settings?.disabledTuiAgents) ) { activateAndRevealWorktree(worktreeId, { + sidebarRevealBehavior: 'auto', setup: result.setup }) toast.error(unavailableAgentErrorMessage()) @@ -281,6 +282,7 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom })) const activation = activateAndRevealWorktree(worktreeId, { + sidebarRevealBehavior: 'auto', setup: result.setup, defaultTabs: result.defaultTabs, ...buildDirectWorkItemStartupOpts(effectiveAgent, startupPlan, launchSource) diff --git a/src/renderer/src/lib/worktree-activation-created-agent.test.ts b/src/renderer/src/lib/worktree-activation-created-agent.test.ts index 184eb7dc7c8..174c1fab32d 100644 --- a/src/renderer/src/lib/worktree-activation-created-agent.test.ts +++ b/src/renderer/src/lib/worktree-activation-created-agent.test.ts @@ -299,10 +299,10 @@ describe('activateAndRevealWorktree created agent reopen', () => { revealWorktreeInSidebar }) - const result = activateAndRevealWorktree(worktree.id) + const result = activateAndRevealWorktree(worktree.id, { sidebarRevealBehavior: 'auto' }) expect(result).toEqual({ primaryTabId: expect.any(String) }) - expect(revealWorktreeInSidebar).toHaveBeenCalledWith(worktree.id) + expect(revealWorktreeInSidebar).toHaveBeenCalledWith(worktree.id, { behavior: 'auto' }) }) it('asks the host runtime to activate the worktree in the paired web client', async () => { diff --git a/src/renderer/src/lib/worktree-activation.ts b/src/renderer/src/lib/worktree-activation.ts index 4f735de5c17..1d94786a345 100644 --- a/src/renderer/src/lib/worktree-activation.ts +++ b/src/renderer/src/lib/worktree-activation.ts @@ -25,6 +25,7 @@ import { CLIENT_PLATFORM } from './new-workspace' import { tuiAgentToAgentKind } from './telemetry' import { agentKindToTuiAgent } from '../../../shared/agent-kind' import { useAppStore } from '@/store' +import type { PendingSidebarWorktreeReveal } from '@/store/slices/ui' import { tabHasLivePty } from '@/lib/tab-has-live-pty' import { activateWebRuntimeSessionWorktree, @@ -182,6 +183,7 @@ function ensureFolderWorkspaceInitialTerminal( export function activateAndRevealFolderWorkspace( folderWorkspaceId: string, opts?: { + sidebarRevealBehavior?: PendingSidebarWorktreeReveal['behavior'] startup?: WorktreeStartupPayload runtimeEnvironmentId?: string | null } @@ -225,7 +227,11 @@ export function activateAndRevealFolderWorkspace( resumeSleepingAgentSessionsForWorktree(workspaceKey) const primaryTabId = ensureFolderWorkspaceInitialTerminal(folderWorkspace, opts?.startup) - state.revealWorktreeInSidebar(workspaceKey) + if (opts?.sidebarRevealBehavior) { + state.revealWorktreeInSidebar(workspaceKey, { behavior: opts.sidebarRevealBehavior }) + } else { + state.revealWorktreeInSidebar(workspaceKey) + } return { primaryTabId } } @@ -283,6 +289,7 @@ export function activateAndRevealWorktree( setup?: WorktreeSetupLaunch defaultTabs?: WorktreeDefaultTabsLaunch issueCommand?: IssueCommandLaunch + sidebarRevealBehavior?: PendingSidebarWorktreeReveal['behavior'] notifyHostRuntime?: boolean revealInSidebar?: boolean } @@ -382,7 +389,11 @@ export function activateAndRevealWorktree( // 6. Reveal in sidebar if (opts?.revealInSidebar !== false) { - state.revealWorktreeInSidebar(worktreeId) + if (opts?.sidebarRevealBehavior) { + state.revealWorktreeInSidebar(worktreeId, { behavior: opts.sidebarRevealBehavior }) + } else { + state.revealWorktreeInSidebar(worktreeId) + } } if (opts?.notifyHostRuntime !== false) { diff --git a/src/renderer/src/lib/worktree-creation-flow.ts b/src/renderer/src/lib/worktree-creation-flow.ts index 3e37be6d186..9af1dc61ebe 100644 --- a/src/renderer/src/lib/worktree-creation-flow.ts +++ b/src/renderer/src/lib/worktree-creation-flow.ts @@ -222,6 +222,7 @@ async function executeWorktreeCreation( let primaryTabId: string | null if (stillActive) { activation = activateAndRevealWorktree(worktree.id, { + sidebarRevealBehavior: 'auto', ...(result.setup ? { setup: result.setup } : {}), ...(result.defaultTabs ? { defaultTabs: result.defaultTabs } : {}), ...(startupOpt ? { startup: startupOpt } : {}), diff --git a/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts b/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts index 71e071f91f4..a18a2733dcc 100644 --- a/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts +++ b/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts @@ -56,6 +56,7 @@ describe('repo slice skipped-onboarding folder startup', () => { 1, 'folder-1::/folder', { + sidebarRevealBehavior: 'auto', startup: { command: "codex '--dangerously-bypass-approvals-and-sandbox'", env: {}, @@ -76,7 +77,7 @@ describe('repo slice skipped-onboarding folder startup', () => { expect(worktreeActivation.activateAndRevealWorktree).toHaveBeenNthCalledWith( 2, 'folder-2::/folder', - undefined + { sidebarRevealBehavior: 'auto' } ) }) }) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 4f535acbce6..6fc31768e28 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -2586,7 +2586,10 @@ export const createRepoSlice: StateCreator = (set, onboarding, hadProjectBeforeAdd ) - activateAndRevealWorktree(folderWorktree.id, startup ? { startup } : undefined) + activateAndRevealWorktree(folderWorktree.id, { + sidebarRevealBehavior: 'auto', + ...(startup ? { startup } : {}) + }) } return repo } catch (err) { diff --git a/src/renderer/src/store/slices/ui.test.ts b/src/renderer/src/store/slices/ui.test.ts index 6c47388db03..eb1732f1a28 100644 --- a/src/renderer/src/store/slices/ui.test.ts +++ b/src/renderer/src/store/slices/ui.test.ts @@ -286,6 +286,7 @@ describe('createUISlice agent send target mode', () => { }) expect(store.getState().pendingRevealWorktree).toMatchObject({ worktreeId, + behavior: 'auto', highlight: true }) }) @@ -1097,12 +1098,14 @@ describe('createUISlice hydratePersistedUI', () => { const store = createUIStore() store.getState().revealWorktreeInSidebar('repo1::/feature', { + behavior: 'smooth', highlight: true, beginRename: true }) expect(store.getState().pendingRevealWorktree).toEqual({ worktreeId: 'repo1::/feature', + behavior: 'smooth', highlight: true, beginRename: true }) diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 00f40ee2f3d..321f9ce09a3 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -113,12 +113,14 @@ import { translate } from '@/i18n/i18n' export type PendingSidebarWorktreeReveal = { worktreeId: string + behavior: 'auto' | 'smooth' highlight?: boolean beginRename?: boolean } export type PendingSidebarRowReveal = { rowKey: string + behavior: 'auto' | 'smooth' highlight?: boolean } @@ -896,6 +898,7 @@ export type UISlice = { revealWorktreeInSidebar: ( worktreeId: string, options?: { + behavior?: PendingSidebarWorktreeReveal['behavior'] highlight?: boolean beginRename?: boolean } @@ -903,6 +906,7 @@ export type UISlice = { revealSidebarRow: ( rowKey: string, options?: { + behavior?: PendingSidebarRowReveal['behavior'] highlight?: boolean } ) => void @@ -986,7 +990,7 @@ export const createUISlice: StateCreator = (set, get) targets.some((target) => target.status === 'eligible') && (previousMode?.id !== args.id || previousMode.worktreeId !== args.worktreeId) ) { - get().revealWorktreeInSidebar(args.worktreeId, { highlight: true }) + get().revealWorktreeInSidebar(args.worktreeId, { behavior: 'auto', highlight: true }) } }, closeAgentSendPopoverTargetMode: (id, instanceId) => @@ -2202,6 +2206,7 @@ export const createUISlice: StateCreator = (set, get) set({ pendingRevealWorktree: { worktreeId, + behavior: options?.behavior ?? 'smooth', ...(options?.highlight ? { highlight: true } : {}), ...(options?.beginRename ? { beginRename: true } : {}) } @@ -2210,6 +2215,7 @@ export const createUISlice: StateCreator = (set, get) set({ pendingRevealSidebarRow: { rowKey, + behavior: options?.behavior ?? 'smooth', ...(options?.highlight === false ? {} : { highlight: true }) } }), diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 8e0cb6e3a30..8929f57594c 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -6412,7 +6412,7 @@ describe('setWorktreesPinnedAndReveal', () => { store.getState().setWorktreesPinnedAndReveal([wt.id], true) expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) - expect(reveal).toHaveBeenCalledWith(wt.id, { highlight: 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', () => { @@ -6427,7 +6427,7 @@ describe('setWorktreesPinnedAndReveal', () => { store.getState().setWorktreesPinnedAndReveal([wt.id], false) expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(false) - expect(reveal).toHaveBeenCalledWith(wt.id, { highlight: true }) + expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) }) it('skips a no-op toggle without requesting a reveal', () => { @@ -6492,7 +6492,7 @@ describe('setWorktreesPinnedAndReveal', () => { store.getState().setWorktreesPinnedAndReveal([alreadyPinned.id, first.id, second.id], true) expect(reveal).toHaveBeenCalledTimes(1) - expect(reveal).toHaveBeenCalledWith(first.id, { highlight: true }) + 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) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 9c350fd7652..d6fb11a315b 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -3959,7 +3959,7 @@ export const createWorktreeSlice: StateCreator // persistence is async), so the reveal below resolves against a render // where the shortcut row already exists. void get().updateWorktreesMeta(updates) - get().revealWorktreeInSidebar(revealWorktreeId, { highlight: true }) + get().revealWorktreeInSidebar(revealWorktreeId, { behavior: 'smooth', highlight: true }) }, markWorktreeUnread: (worktreeId) => { diff --git a/tests/e2e/worktree-scroll-to-current.spec.ts b/tests/e2e/worktree-scroll-to-current.spec.ts index a3bef0d095b..19c51005cfe 100644 --- a/tests/e2e/worktree-scroll-to-current.spec.ts +++ b/tests/e2e/worktree-scroll-to-current.spec.ts @@ -23,9 +23,11 @@ async function prepareSidebarForScrollTest(page: Page): Promise { test.describe('Reveal active workspace button', () => { test.beforeEach(async ({ orcaPage }) => { - // Sidebar reveal now always jumps instantly (worktree-sidebar-reveal.ts - // assigns scrollTop directly), so no reduced-motion emulation is needed - // for the geometry assertions to be deterministic. + // Why: headless Electron under xvfb never ticks a smooth-scroll animation, + // so the reveal's `scrollTo({ behavior: 'smooth' })` would never reach its + // target. Reduced-motion makes the reveal jump instantly (see + // worktree-sidebar-reveal.ts) so the geometry assertions are deterministic. + await orcaPage.emulateMedia({ reducedMotion: 'reduce' }) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) }) diff --git a/tests/e2e/worktree-switch-responsiveness.spec.ts b/tests/e2e/worktree-switch-responsiveness.spec.ts index 11b4dea8c97..45a6e6e6b2f 100644 --- a/tests/e2e/worktree-switch-responsiveness.spec.ts +++ b/tests/e2e/worktree-switch-responsiveness.spec.ts @@ -33,8 +33,8 @@ async function prepareSidebarForSwitchTest(page: Page): Promise<[string, string] if ((state.tabsByWorktree[second.id] ?? []).length === 0) { state.createTab(second.id, undefined, undefined, { pendingActivationSpawn: true }) } - state.revealWorktreeInSidebar(first.id) - state.revealWorktreeInSidebar(second.id) + state.revealWorktreeInSidebar(first.id, { behavior: 'auto' }) + state.revealWorktreeInSidebar(second.id, { behavior: 'auto' }) state.setActiveWorktree(first.id) return [first.id, second.id] })