diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 84f5ab77838..a1542323e62 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -100,6 +100,7 @@ import { type RenderRow } from './worktree-list-virtual-rows' import { + getElementCenteringScrollPadding, revealElementInScrollContainer, WORKTREE_SIDEBAR_REVEAL_TOP_INSET } from './worktree-sidebar-reveal' @@ -287,6 +288,7 @@ import { import { getFolderWorkspaceCardPrDisplay } from './folder-workspace-card-pr-display' export { + getCenteringScrollPadding, getScrollTopToRevealBounds, WORKTREE_SIDEBAR_REVEAL_TOP_INSET } from './worktree-sidebar-reveal' @@ -1320,6 +1322,11 @@ 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) @@ -1969,6 +1976,8 @@ 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, @@ -2120,10 +2129,30 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp } } 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 @@ -2199,6 +2228,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp pendingRevealRetryTick, flashRevealedRow, setRenamingWorktreeId, + centeringScrollPadding, schedulePendingRevealFrame, cancelPendingRevealFrames ]) @@ -2282,6 +2312,14 @@ 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) : null @@ -2307,6 +2345,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp } }, [ pendingRevealSidebarRow, + pendingRevealWorktree, repoMap, projectGroups, projectGrouping, @@ -2318,6 +2357,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp pendingRevealRetryTick, flashRevealedRow, clearPendingRevealSidebarRow, + centeringScrollPadding, schedulePendingRevealFrame, cancelPendingRevealFrames ]) 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 aa71d479445..f9a5a5f62ce 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,12 +1,12 @@ import { describe, expect, it, vi } from 'vitest' import { countRecordKeysByReference, + getCenteringScrollPadding, getScrollTopToRevealBounds, resolvePendingSidebarReveal, WORKTREE_SIDEBAR_REVEAL_TOP_INSET, shouldAdjustWorktreeSidebarMeasuredRowScroll } from './WorktreeList' -import { revealElementInScrollContainer } from './worktree-sidebar-reveal' import { extractWorktreeVirtualRowIndexes, estimateRenderRowSize, @@ -125,6 +125,21 @@ 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', () => { const container = makeScrollContainer(100, 400) @@ -155,22 +170,19 @@ describe('getScrollTopToRevealBounds', () => { ).toBe(136) }) - it('clamps a boundary target to the list edge instead of padding the list', () => { - // Regression (#8019 follow-up): centering first/last rows via temporary - // paddingStart/paddingEnd left a permanent phantom gap in the sidebar once - // the reveal finished, since nothing cleared the padding afterwards. - const container = { - scrollTop: 100, - clientHeight: 200, - contains: () => true, - getBoundingClientRect: () => ({ top: 0, bottom: 200 }) - } as unknown as HTMLElement - const firstRow = { - getBoundingClientRect: () => ({ top: -40, bottom: 20 }) - } as unknown as Element + it('requests trailing space when the last target would be clamped below center', () => { + const container = makeScrollContainer(100, 400, 500) - expect(revealElementInScrollContainer(container, firstRow)).toBe(true) - expect(container.scrollTop).toBe(0) + expect( + getCenteringScrollPadding( + container, + { + start: 430, + end: 470 + }, + WORKTREE_SIDEBAR_REVEAL_TOP_INSET + ) + ).toEqual({ start: 0, end: 133 }) }) }) 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 e2ec2cfaffd..850fc45704d 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 @@ -9,8 +9,8 @@ describe('getScrollTopToRevealBounds', () => { }) as HTMLElement it('centers a mounted current workspace card that starts above the viewport', () => { - // Raw result may be negative; revealElementInScrollContainer clamps it to - // the list edge so boundary reveals never pad the list. + // 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) }) diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts b/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts index 56586682a50..765efc2eabc 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts @@ -9,6 +9,11 @@ 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() @@ -31,6 +36,30 @@ export function getScrollTopToRevealBounds( 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)) + } +} + +export function getElementCenteringScrollPadding( + 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 { if (!container.contains(element)) { return false @@ -41,8 +70,7 @@ export function revealElementInScrollContainer(container: HTMLElement, 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. Boundary rows clamp to the list - // edge — padding the list so they can center leaves a phantom gap behind. + // of making the user track an animated list. container.scrollTop = Math.max(0, nextScrollTop) return true }