From 0c69a713a2bbe28134e848db654034cf24d0f9bc Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 28 May 2026 00:25:19 -0700 Subject: [PATCH] Fix sidebar scroll restore below sticky headers (#2976) * Fix sidebar scroll restore below sticky headers * Split virtualized scroll anchor state --- src/renderer/src/App.tsx | 2 +- .../sidebar/WorktreeContextMenu.tsx | 2 +- .../WorktreeList.lineage-child-card.test.ts | 11 ++++ .../src/components/sidebar/WorktreeList.tsx | 45 ++++++++++++--- src/renderer/src/components/sidebar/index.tsx | 2 +- .../components/sidebar/sleep-worktree-flow.ts | 2 +- .../sidebar/worktree-list-virtual-rows.ts | 2 +- .../worktree-scroll-to-current-button.test.ts | 18 ++++++ .../requestVirtualizedScrollAnchorRecord.ts | 2 +- .../src/hooks/useVirtualizedScrollAnchor.ts | 55 +++++++++---------- .../virtualizedScrollAnchorResolution.ts | 18 ++++++ .../src/hooks/virtualizedScrollAnchorState.ts | 14 +++++ 12 files changed, 130 insertions(+), 43 deletions(-) create mode 100644 src/renderer/src/hooks/virtualizedScrollAnchorResolution.ts create mode 100644 src/renderer/src/hooks/virtualizedScrollAnchorState.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index ff4a3268a..9451d4171 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -110,7 +110,7 @@ import { canGoBackWorktreeHistory, canGoForwardWorktreeHistory } from '@/store/slices/worktree-nav-history' -import type { VirtualizedScrollAnchor } from './hooks/useVirtualizedScrollAnchor' +import type { VirtualizedScrollAnchor } from './hooks/virtualizedScrollAnchorState' import type { RemoteWorkspacePatchResult } from '../../shared/remote-workspace-types' import type { OnboardingState } from '../../shared/types' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../shared/constants' diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index 3fc76a009..2f78b6e01 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -38,7 +38,7 @@ import { runWorktreeBatchDelete, runWorktreeDelete } from './delete-worktree-flo import { runSleepWorktrees } from './sleep-worktree-flow' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { tabHasLivePty } from '@/lib/tab-has-live-pty' -import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor' +import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/virtualizedScrollAnchorState' import { getLineageRenderInfo } from './worktree-list-groups' import { getWorkspaceStatus, getWorkspaceStatusVisualMeta } from './workspace-status' import { WorktreeOpenInSubMenu } from './WorktreeOpenInMenu' diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts index fe239ee8b..8fae6671f 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts @@ -242,6 +242,17 @@ async function renderWorktreeListMarkup(): Promise { } describe('WorktreeList lineage child card renderer', () => { + it('caps sidebar restore offsets so agent rows do not become the anchor', async () => { + const { useVirtualizedScrollAnchor } = await import('@/hooks/useVirtualizedScrollAnchor') + vi.mocked(useVirtualizedScrollAnchor).mockClear() + setLineageFixtureState() + await renderWorktreeListMarkup() + + expect(useVirtualizedScrollAnchor).toHaveBeenCalledWith( + expect.objectContaining({ maxAnchorOffset: 4 }) + ) + }) + it('renders nested inline agent rows before the nested child-count toggle', async () => { setLineageFixtureState() const markup = await renderWorktreeListMarkup() diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 1ef452596..24f9a1161 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -73,6 +73,7 @@ import { getLineageGroupKey } from './worktree-list-groups' import { + GROUP_HEADER_ROW_HEIGHT, estimateRenderRowSize, getActiveStickyHeaderIndex, getActiveStickyHeaderIndexForScroll, @@ -99,11 +100,11 @@ import { getVisibleWorktreeBrowserActivityTabs, getVisibleWorktreeTerminalActivityTabs } from './visible-worktree-activity-inputs' +import { useVirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT, - useVirtualizedScrollAnchor, type VirtualizedScrollAnchor -} from '@/hooks/useVirtualizedScrollAnchor' +} from '@/hooks/virtualizedScrollAnchorState' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { getShortcutPlatform } from '@/lib/shortcut-platform' import { SCROLL_TO_CURRENT_WORKSPACE_REVEAL_REQUEST_EVENT } from '@/lib/scroll-to-current-workspace-status' @@ -253,12 +254,24 @@ function getMountedWorktreeBounds( export function getScrollTopToRevealBounds( container: HTMLElement, - bounds: Pick + bounds: Pick, + topInset = 0, + align: 'nearest' | 'start' = 'nearest' ): number | null { const viewportTop = container.scrollTop + const visibleTop = viewportTop + topInset const viewportBottom = viewportTop + container.clientHeight - if (bounds.start < viewportTop) { - return bounds.start + if (align === 'start') { + const nextScrollTop = bounds.start - topInset + return Math.abs(nextScrollTop - viewportTop) > 1 ? nextScrollTop : null + } + const boundsHeight = bounds.end - bounds.start + const visibleHeight = container.clientHeight - topInset + if (boundsHeight > visibleHeight && (bounds.start < visibleTop || bounds.end > viewportBottom)) { + return bounds.start - topInset + } + if (bounds.start < visibleTop) { + return bounds.start - topInset } if (bounds.end > viewportBottom) { return bounds.end - container.clientHeight @@ -269,13 +282,14 @@ export function getScrollTopToRevealBounds( function revealMountedWorktreeElement( container: HTMLElement, worktreeId: string, - behavior: ScrollBehavior + behavior: ScrollBehavior, + topInset = 0 ): boolean { const bounds = getMountedWorktreeBounds(container, worktreeId) if (!bounds) { return false } - const nextScrollTop = getScrollTopToRevealBounds(container, bounds) + const nextScrollTop = getScrollTopToRevealBounds(container, bounds, topInset) if (nextScrollTop !== null) { container.scrollTo({ top: Math.max(0, nextScrollTop), behavior }) } @@ -296,6 +310,8 @@ const LINEAGE_INDENT = 18 const WORKTREE_GROUP_INDENT = 18 const PROJECT_GROUP_HEADER_INDENT = 10 const SIDEBAR_POINTER_DRAG_THRESHOLD_PX = 4 +const STICKY_HEADER_REVEAL_CLEARANCE_PX = 6 +const WORKTREE_SIDEBAR_MAX_ANCHOR_OFFSET_PX = 4 type VirtualizedWorktreeViewportProps = { rows: Row[] @@ -798,6 +814,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp const firstHeaderIndexRef = useRef(firstHeaderIndex) firstHeaderIndexRef.current = firstHeaderIndex const stickyHeaderIndexes = useMemo(() => getStickyHeaderIndexes(renderRows), [renderRows]) + // Why: sticky group headers visually cover the top of the scrollport, so + // reveal and anchor math must leave the workspace card visibly below it. + const stickyHeaderTopInset = + stickyHeaderIndexes.length > 0 ? GROUP_HEADER_ROW_HEIGHT + STICKY_HEADER_REVEAL_CLEARANCE_PX : 0 const stickyHeaderIndexesRef = useRef(stickyHeaderIndexes) stickyHeaderIndexesRef.current = stickyHeaderIndexes const activeStickyHeaderIndexRef = useRef(null) @@ -1068,7 +1088,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp revealMountedWorktreeElement( container, pendingRevealWorktree.worktreeId, - pendingRevealWorktree.behavior + pendingRevealWorktree.behavior, + stickyHeaderTopInset ) ) { if (pendingRevealWorktree.highlight) { @@ -1123,7 +1144,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp settings, projectGroups, pendingRevealRetryTick, - flashRevealedWorktree + flashRevealedWorktree, + stickyHeaderTopInset ]) const prCacheLen = useAppStore((s) => countRecordKeysByReference(s.prCache)) @@ -1186,6 +1208,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp scrollOffsetRef, hasDirectScrollInput, shouldSkipRestore: shouldSkipScrollAnchorRestore, + // Why: inline agent rows can grow after a run. Let tiny offsets survive so + // normal measurement correction does not snap, but do not let the agent row + // become the preserved anchor under the sticky project header. + maxAnchorOffset: WORKTREE_SIDEBAR_MAX_ANCHOR_OFFSET_PX, + topInset: stickyHeaderTopInset, totalSize, virtualizer }) diff --git a/src/renderer/src/components/sidebar/index.tsx b/src/renderer/src/components/sidebar/index.tsx index b7e08dc57..453ba9086 100644 --- a/src/renderer/src/components/sidebar/index.tsx +++ b/src/renderer/src/components/sidebar/index.tsx @@ -14,7 +14,7 @@ import AddRepoDialog from './AddRepoDialog' import ProjectAddedDialog from './ProjectAddedDialog' import WorktreeVisibilityDialog from './WorktreeVisibilityDialog' import OrcaYamlTrustDialog from './OrcaYamlTrustDialog' -import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' +import type { VirtualizedScrollAnchor } from '@/hooks/virtualizedScrollAnchorState' const MIN_WIDTH = 220 const MAX_WIDTH = 500 diff --git a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts index 1a35d88d1..8d529ff4e 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts @@ -1,7 +1,7 @@ import { toast } from 'sonner' import { useAppStore } from '@/store' import { clearWorktreeSleepIntent, markWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' -import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor' +import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/virtualizedScrollAnchorState' /** * Shared "sleep worktree" flow (close all panels to free memory / CPU) diff --git a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts index f334de178..dda04823b 100644 --- a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts +++ b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts @@ -2,7 +2,7 @@ import type { VirtualItem } from '@tanstack/react-virtual' import type { Row } from './worktree-list-groups' import { PINNED_GROUP_KEY } from './worktree-list-groups' -const GROUP_HEADER_ROW_HEIGHT = 28 +export const GROUP_HEADER_ROW_HEIGHT = 28 const SECONDARY_GROUP_HEADER_TOP_MARGIN = 8 type WorktreeItemRow = Extract 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 933261f20..2edf039ce 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 @@ -12,6 +12,24 @@ describe('getScrollTopToRevealBounds', () => { expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 60, end: 120 })).toBe(60) }) + it('scrolls upward when a mounted current workspace card is hidden under a sticky header', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 110, end: 180 }, 34)).toBe( + 76 + ) + }) + + it('top-aligns a tall workspace card instead of revealing only its lower agent rows', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 100), { start: 130, end: 250 }, 34)).toBe( + 96 + ) + }) + + it('does not scroll when a mounted workspace card is already visible below a sticky header', () => { + expect( + getScrollTopToRevealBounds(makeContainer(100, 200), { start: 150, end: 220 }, 34) + ).toBeNull() + }) + it('scrolls downward to reveal a mounted current workspace card below the viewport', () => { expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 250, end: 340 })).toBe(140) }) diff --git a/src/renderer/src/hooks/requestVirtualizedScrollAnchorRecord.ts b/src/renderer/src/hooks/requestVirtualizedScrollAnchorRecord.ts index 0009c60ed..44efe83f6 100644 --- a/src/renderer/src/hooks/requestVirtualizedScrollAnchorRecord.ts +++ b/src/renderer/src/hooks/requestVirtualizedScrollAnchorRecord.ts @@ -1,4 +1,4 @@ -import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from './useVirtualizedScrollAnchor' +import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from './virtualizedScrollAnchorState' /** * Asks a mounted virtualized scroller (matched by selector) to snapshot its diff --git a/src/renderer/src/hooks/useVirtualizedScrollAnchor.ts b/src/renderer/src/hooks/useVirtualizedScrollAnchor.ts index d94ef0219..b23f0911b 100644 --- a/src/renderer/src/hooks/useVirtualizedScrollAnchor.ts +++ b/src/renderer/src/hooks/useVirtualizedScrollAnchor.ts @@ -8,13 +8,13 @@ import { } from 'react' import type { Virtualizer } from '@tanstack/react-virtual' import { shouldCancelVirtualizedScrollOffsetRestore } from './virtualizedScrollOffsetRestore' +import { + VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT, + clampVirtualizedScrollAnchorOffset, + type VirtualizedScrollAnchor +} from './virtualizedScrollAnchorState' +import { resolveVirtualizedScrollAnchorKey } from './virtualizedScrollAnchorResolution' -export type VirtualizedScrollAnchor = { - fallbackKeys?: readonly string[] - key: string - offset: number -} | null -export const VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT = 'orca-record-virtualized-scroll-anchor' const RECORD_ANCHOR_SCROLL_IDLE_DELAY_MS = 150 type UseVirtualizedScrollAnchorOptions< @@ -31,6 +31,8 @@ type UseVirtualizedScrollAnchorOptions< scrollElementRef: RefObject scrollOffsetRef: MutableRefObject shouldSkipRestore?: () => boolean + maxAnchorOffset?: number + topInset?: number totalSize: number virtualizer: Virtualizer } @@ -57,23 +59,23 @@ export function useVirtualizedScrollAnchor< scrollElementRef, scrollOffsetRef, shouldSkipRestore, + maxAnchorOffset = Infinity, + topInset = 0, totalSize, virtualizer }: UseVirtualizedScrollAnchorOptions): void { const rowIndexByKey = useMemo(() => { const indexByKey = new Map() - rows.forEach((row, index) => { - indexByKey.set(getRowKey(row), index) - }) + rows.forEach((row, index) => indexByKey.set(getRowKey(row), index)) return indexByKey }, [getRowKey, rows]) - const findDomAnchor = useCallback( (scrollElement: TScrollElement) => { if (!itemElementSelector || !getItemElementKey) { return null } const scrollRect = scrollElement.getBoundingClientRect() + const visibleTop = scrollRect.top + topInset type DomAnchorItem = { element: TItemElement; key: string; rect: DOMRect } const visibleItems = Array.from( scrollElement.querySelectorAll(itemElementSelector) @@ -84,7 +86,7 @@ export function useVirtualizedScrollAnchor< return null } const rect = element.getBoundingClientRect() - if (rect.height <= 0 || rect.bottom <= scrollRect.top || rect.top >= scrollRect.bottom) { + if (rect.height <= 0 || rect.bottom <= visibleTop || rect.top >= scrollRect.bottom) { return null } return { element, key, rect } @@ -99,19 +101,20 @@ export function useVirtualizedScrollAnchor< return { fallbackKeys: visibleItems.slice(1).map((item) => item.key), key: firstVisible.key, - offset: Math.min( - firstVisible.rect.height, - Math.max(0, scrollRect.top - firstVisible.rect.top) + offset: clampVirtualizedScrollAnchorOffset( + Math.min(firstVisible.rect.height, visibleTop - firstVisible.rect.top), + maxAnchorOffset ) } }, - [getItemElementKey, itemElementSelector, rowIndexByKey] + [getItemElementKey, itemElementSelector, maxAnchorOffset, rowIndexByKey, topInset] ) const recordVirtualScrollAnchor = useCallback( (scrollTop: number) => { const virtualItems = virtualizer.getVirtualItems() - const firstVisible = virtualItems.find((item) => item.end > scrollTop) + const visibleTop = scrollTop + topInset + const firstVisible = virtualItems.find((item) => item.end > visibleTop) const row = firstVisible ? rows[firstVisible.index] : undefined if (!firstVisible || !row) { anchorRef.current = null @@ -124,10 +127,10 @@ export function useVirtualizedScrollAnchor< .filter((row): row is TRow => row != null) .map(getRowKey), key: getRowKey(row), - offset: Math.max(0, scrollTop - firstVisible.start) + offset: clampVirtualizedScrollAnchorOffset(visibleTop - firstVisible.start, maxAnchorOffset) } }, - [anchorRef, getRowKey, rows, virtualizer] + [anchorRef, getRowKey, maxAnchorOffset, rows, topInset, virtualizer] ) const recordScrollAnchor = useCallback( @@ -259,16 +262,11 @@ export function useVirtualizedScrollAnchor< return } - const resolvedKey = rowIndexByKey.has(anchor.key) - ? anchor.key - : anchor.fallbackKeys?.find((key) => rowIndexByKey.has(key)) - if (!resolvedKey) { - return - } - const index = rowIndexByKey.get(resolvedKey) - if (index === undefined) { + const resolvedAnchor = resolveVirtualizedScrollAnchorKey(anchor, rowIndexByKey) + if (!resolvedAnchor) { return } + const { index, key: resolvedKey } = resolvedAnchor const offset = resolvedKey === anchor.key ? anchor.offset : 0 const restoreFromDomElement = (): boolean => { @@ -284,7 +282,7 @@ export function useVirtualizedScrollAnchor< } const scrollRect = el.getBoundingClientRect() const rect = element.getBoundingClientRect() - const desiredTop = scrollRect.top - offset + const desiredTop = scrollRect.top + topInset - offset const delta = rect.top - desiredTop if (Math.abs(delta) > 1) { el.scrollTop += delta @@ -300,7 +298,7 @@ export function useVirtualizedScrollAnchor< return false } const maxScrollTop = Math.max(0, el.scrollHeight - el.clientHeight) - const nextScrollTop = Math.min(maxScrollTop, Math.max(0, item.start + offset)) + const nextScrollTop = Math.min(maxScrollTop, Math.max(0, item.start + offset - topInset)) if (Math.abs(el.scrollTop - nextScrollTop) > 1) { el.scrollTop = nextScrollTop } @@ -341,6 +339,7 @@ export function useVirtualizedScrollAnchor< scrollElementRef, scrollOffsetRef, shouldSkipRestore, + topInset, totalSize, virtualizer, virtualizer.isScrolling diff --git a/src/renderer/src/hooks/virtualizedScrollAnchorResolution.ts b/src/renderer/src/hooks/virtualizedScrollAnchorResolution.ts new file mode 100644 index 000000000..f7bba7e13 --- /dev/null +++ b/src/renderer/src/hooks/virtualizedScrollAnchorResolution.ts @@ -0,0 +1,18 @@ +import type { VirtualizedScrollAnchor } from './virtualizedScrollAnchorState' + +export function resolveVirtualizedScrollAnchorKey( + anchor: VirtualizedScrollAnchor, + rowIndexByKey: ReadonlyMap +): { index: number; key: string } | null { + if (!anchor) { + return null + } + const key = rowIndexByKey.has(anchor.key) + ? anchor.key + : anchor.fallbackKeys?.find((fallbackKey) => rowIndexByKey.has(fallbackKey)) + if (!key) { + return null + } + const index = rowIndexByKey.get(key) + return index === undefined ? null : { index, key } +} diff --git a/src/renderer/src/hooks/virtualizedScrollAnchorState.ts b/src/renderer/src/hooks/virtualizedScrollAnchorState.ts new file mode 100644 index 000000000..816b8fd98 --- /dev/null +++ b/src/renderer/src/hooks/virtualizedScrollAnchorState.ts @@ -0,0 +1,14 @@ +export type VirtualizedScrollAnchor = { + fallbackKeys?: readonly string[] + key: string + offset: number +} | null + +export const VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT = 'orca-record-virtualized-scroll-anchor' + +export function clampVirtualizedScrollAnchorOffset( + offset: number, + maxAnchorOffset: number +): number { + return Math.min(maxAnchorOffset, Math.max(0, offset)) +}