From b1d87f310da50aa893cdb052e133745fb545d542 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 28 May 2026 01:22:00 -0700 Subject: [PATCH] Revert "Fix sidebar scroll restore below sticky headers" (#2985) --- 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, 43 insertions(+), 130 deletions(-) delete mode 100644 src/renderer/src/hooks/virtualizedScrollAnchorResolution.ts delete mode 100644 src/renderer/src/hooks/virtualizedScrollAnchorState.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 9451d4171..ff4a3268a 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/virtualizedScrollAnchorState' +import type { VirtualizedScrollAnchor } from './hooks/useVirtualizedScrollAnchor' 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 2f78b6e01..3fc76a009 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/virtualizedScrollAnchorState' +import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor' 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 8fae6671f..fe239ee8b 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,17 +242,6 @@ 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 24f9a1161..1ef452596 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -73,7 +73,6 @@ import { getLineageGroupKey } from './worktree-list-groups' import { - GROUP_HEADER_ROW_HEIGHT, estimateRenderRowSize, getActiveStickyHeaderIndex, getActiveStickyHeaderIndexForScroll, @@ -100,11 +99,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/virtualizedScrollAnchorState' +} from '@/hooks/useVirtualizedScrollAnchor' 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' @@ -254,24 +253,12 @@ function getMountedWorktreeBounds( export function getScrollTopToRevealBounds( container: HTMLElement, - bounds: Pick, - topInset = 0, - align: 'nearest' | 'start' = 'nearest' + bounds: Pick ): number | null { const viewportTop = container.scrollTop - const visibleTop = viewportTop + topInset const viewportBottom = viewportTop + container.clientHeight - 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.start < viewportTop) { + return bounds.start } if (bounds.end > viewportBottom) { return bounds.end - container.clientHeight @@ -282,14 +269,13 @@ export function getScrollTopToRevealBounds( function revealMountedWorktreeElement( container: HTMLElement, worktreeId: string, - behavior: ScrollBehavior, - topInset = 0 + behavior: ScrollBehavior ): boolean { const bounds = getMountedWorktreeBounds(container, worktreeId) if (!bounds) { return false } - const nextScrollTop = getScrollTopToRevealBounds(container, bounds, topInset) + const nextScrollTop = getScrollTopToRevealBounds(container, bounds) if (nextScrollTop !== null) { container.scrollTo({ top: Math.max(0, nextScrollTop), behavior }) } @@ -310,8 +296,6 @@ 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[] @@ -814,10 +798,6 @@ 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) @@ -1088,8 +1068,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp revealMountedWorktreeElement( container, pendingRevealWorktree.worktreeId, - pendingRevealWorktree.behavior, - stickyHeaderTopInset + pendingRevealWorktree.behavior ) ) { if (pendingRevealWorktree.highlight) { @@ -1144,8 +1123,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp settings, projectGroups, pendingRevealRetryTick, - flashRevealedWorktree, - stickyHeaderTopInset + flashRevealedWorktree ]) const prCacheLen = useAppStore((s) => countRecordKeysByReference(s.prCache)) @@ -1208,11 +1186,6 @@ 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 453ba9086..b7e08dc57 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/virtualizedScrollAnchorState' +import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' 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 8d529ff4e..1a35d88d1 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/virtualizedScrollAnchorState' +import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor' /** * 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 dda04823b..f334de178 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' -export const GROUP_HEADER_ROW_HEIGHT = 28 +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 2edf039ce..933261f20 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,24 +12,6 @@ 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 44efe83f6..0009c60ed 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 './virtualizedScrollAnchorState' +import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from './useVirtualizedScrollAnchor' /** * 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 b23f0911b..d94ef0219 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,8 +31,6 @@ type UseVirtualizedScrollAnchorOptions< scrollElementRef: RefObject scrollOffsetRef: MutableRefObject shouldSkipRestore?: () => boolean - maxAnchorOffset?: number - topInset?: number totalSize: number virtualizer: Virtualizer } @@ -59,23 +57,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) @@ -86,7 +84,7 @@ export function useVirtualizedScrollAnchor< return null } const rect = element.getBoundingClientRect() - if (rect.height <= 0 || rect.bottom <= visibleTop || rect.top >= scrollRect.bottom) { + if (rect.height <= 0 || rect.bottom <= scrollRect.top || rect.top >= scrollRect.bottom) { return null } return { element, key, rect } @@ -101,20 +99,19 @@ export function useVirtualizedScrollAnchor< return { fallbackKeys: visibleItems.slice(1).map((item) => item.key), key: firstVisible.key, - offset: clampVirtualizedScrollAnchorOffset( - Math.min(firstVisible.rect.height, visibleTop - firstVisible.rect.top), - maxAnchorOffset + offset: Math.min( + firstVisible.rect.height, + Math.max(0, scrollRect.top - firstVisible.rect.top) ) } }, - [getItemElementKey, itemElementSelector, maxAnchorOffset, rowIndexByKey, topInset] + [getItemElementKey, itemElementSelector, rowIndexByKey] ) const recordVirtualScrollAnchor = useCallback( (scrollTop: number) => { const virtualItems = virtualizer.getVirtualItems() - const visibleTop = scrollTop + topInset - const firstVisible = virtualItems.find((item) => item.end > visibleTop) + const firstVisible = virtualItems.find((item) => item.end > scrollTop) const row = firstVisible ? rows[firstVisible.index] : undefined if (!firstVisible || !row) { anchorRef.current = null @@ -127,10 +124,10 @@ export function useVirtualizedScrollAnchor< .filter((row): row is TRow => row != null) .map(getRowKey), key: getRowKey(row), - offset: clampVirtualizedScrollAnchorOffset(visibleTop - firstVisible.start, maxAnchorOffset) + offset: Math.max(0, scrollTop - firstVisible.start) } }, - [anchorRef, getRowKey, maxAnchorOffset, rows, topInset, virtualizer] + [anchorRef, getRowKey, rows, virtualizer] ) const recordScrollAnchor = useCallback( @@ -262,11 +259,16 @@ export function useVirtualizedScrollAnchor< return } - const resolvedAnchor = resolveVirtualizedScrollAnchorKey(anchor, rowIndexByKey) - if (!resolvedAnchor) { + 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) { return } - const { index, key: resolvedKey } = resolvedAnchor const offset = resolvedKey === anchor.key ? anchor.offset : 0 const restoreFromDomElement = (): boolean => { @@ -282,7 +284,7 @@ export function useVirtualizedScrollAnchor< } const scrollRect = el.getBoundingClientRect() const rect = element.getBoundingClientRect() - const desiredTop = scrollRect.top + topInset - offset + const desiredTop = scrollRect.top - offset const delta = rect.top - desiredTop if (Math.abs(delta) > 1) { el.scrollTop += delta @@ -298,7 +300,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 - topInset)) + const nextScrollTop = Math.min(maxScrollTop, Math.max(0, item.start + offset)) if (Math.abs(el.scrollTop - nextScrollTop) > 1) { el.scrollTop = nextScrollTop } @@ -339,7 +341,6 @@ 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 deleted file mode 100644 index f7bba7e13..000000000 --- a/src/renderer/src/hooks/virtualizedScrollAnchorResolution.ts +++ /dev/null @@ -1,18 +0,0 @@ -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 deleted file mode 100644 index 816b8fd98..000000000 --- a/src/renderer/src/hooks/virtualizedScrollAnchorState.ts +++ /dev/null @@ -1,14 +0,0 @@ -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)) -}