From 146da13b89bdc36a05f692311b5e6cd93da46500 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Tue, 10 Nov 2026 18:56:44 -0500 Subject: [PATCH] fix(Virtual, VirtualGrid): animate grid rows past row 3; scroll auto rows of displaySize items VirtualGrid: the row change read the rows' old position in applyShift, after the post-mutation flex pass had already laid out the new window. Once the window slid (row 3 on with the default buffer of 2) the rows jumped a row and the y transition did not run. The position is now read in onSelectedChanged, before the window changes. Virtual: with scroll="auto" (no wrap) and exactly displaySize items, the row never scrolled and the last item ended one slot past where it does for any other list length, out of view. That case now goes through the windowed path. Wrap and the other scroll modes keep the all-mounted path: switching them too broke "always" (last item unreachable) and wrap (items mounted twice, row off by a slot). Co-Authored-By: Claude Opus 5.5 --- src/primitives/Virtual.tsx | 8 ++- src/primitives/VirtualGrid.tsx | 8 ++- tests/contract-virtual-grid.test.tsx | 82 ++++++++++++++++++++++++++++ tests/contract-virtual.test.tsx | 47 ++++++++++++++++ 4 files changed, 142 insertions(+), 3 deletions(-) diff --git a/src/primitives/Virtual.tsx b/src/primitives/Virtual.tsx index 1a2ed32..67e5fa8 100644 --- a/src/primitives/Virtual.tsx +++ b/src/primitives/Virtual.tsx @@ -147,7 +147,13 @@ function createVirtual( const total = itemCount(); if (total === 0) return applySlice(0, noItems, 0, delta, 0, true, 0); - if (total <= props.displaySize) { + // Everything is mounted and the row does not scroll. Not for auto without + // wrap at exactly displaySize items: there, as for a longer list, the + // last item ends one slot short of displaySize, so the row scrolls. + if ( + total < props.displaySize || + (total === props.displaySize && (props.wrap || scrollType() !== 'auto')) + ) { const clamped = utils.clamp(c, 0, total - 1); return applySlice( 0, diff --git a/src/primitives/VirtualGrid.tsx b/src/primitives/VirtualGrid.tsx index 67534a7..afe94a5 100644 --- a/src/primitives/VirtualGrid.tsx +++ b/src/primitives/VirtualGrid.tsx @@ -121,6 +121,10 @@ export function VirtualGrid(props: VirtualGridProps): s.JSX.Element { setCursor(prevStart + idx); if (newRowIndex === prevRowIndex) return; + // Read before the window changes: the post-mutation flex pass can run + // before applyShift, and active.y read there is already the new layout, + // so the rows jumped a row instead of animating once the window slid. + shiftPrevRowY = this.y + active.y; setSlice(items().slice(start(), end())); // this.selected is relative to the slice @@ -149,14 +153,14 @@ export function VirtualGrid(props: VirtualGridProps): s.JSX.Element { let shiftActive!: lng.ElementNode; let shiftIdx = 0; let shiftLastIdx: number | undefined; + let shiftPrevRowY = 0; function applyShift() { const view = shiftView; const active = shiftActive; - const prevRowY = view.y + active.y; // One flex pass per window shift: lay out here only if the post-mutation // layout has not already run since the window changed. if (layoutPending) view.updateLayout(); - view.lng.y = prevRowY - active.y; + view.lng.y = shiftPrevRowY - active.y; columnScroll(shiftIdx, view, active, shiftLastIdx); } diff --git a/tests/contract-virtual-grid.test.tsx b/tests/contract-virtual-grid.test.tsx index 226cde1..7ce8767 100644 --- a/tests/contract-virtual-grid.test.tsx +++ b/tests/contract-virtual-grid.test.tsx @@ -710,4 +710,86 @@ v.describe('VirtualGrid', () => { v.expect(focusedId()).toBe('g14'); }, ); + + // Animations are on, as in an app. The DOM renderer's animations run on + // requestAnimationFrame, and jsdom's frame time is on another clock than + // the performance.now() the animations start from, so frames are driven + // here with a time far ahead: each y transition ends on its first frame. + v.it( + 'with animations on, every row change animates y from where the rows were: they do not jump when the window slides (row 3 on, buffer 2)', + async () => { + const settle = async () => { + for (let i = 0; i < 4; i++) await flush(); + }; + v.vi.stubGlobal( + 'requestAnimationFrame', + (cb: (time: number) => void) => + setTimeout(() => cb(performance.now() + 1e6), 0), + ); + lng.Config.animationsEnabled = true; + try { + let grid!: lng.ElementNode; + // Visual y of each mounted cell, by data item. + const cellYs = () => + new Map( + grid.children.map((c) => [ + (c as lng.ElementNode).item as number, + grid.lng.y + c.lng.y, + ]), + ); + let atStart: Map | undefined; + const onAnimation = { + animating(this: lng.ElementNode, name: string) { + if (this === grid && name === 'y') atStart = cellYs(); + }, + }; + dispose = await mount(() => ( + + + {(item) => } + + + )); + // Down 3 onward slides the window down (a row above is unmounted), + // and Up from there slides it back. + const steps = [ + ...[1, 2, 3, 4, 5, 6].map((row) => ['ArrowDown', row] as const), + ...[5, 4, 3, 2, 1, 0].map((row) => ['ArrowUp', row] as const), + ]; + for (const [key, row] of steps) { + const before = cellYs(); + atStart = undefined; + await press(key); + await settle(); + v.expect(focusedId()).toBe(`g${row * 3}`); + v.expect( + atStart, + `${key} to row ${row}: no y transition`, + ).toBeDefined(); + const jumped = [...atStart!].filter( + ([item, y]) => before.has(item) && before.get(item) !== y, + ); + v.expect( + jumped, + `${key} to row ${row}: cells moved before the transition`, + ).toEqual([]); + // The transition ran to the row's place. + v.expect(grid.lng.y, `${key} to row ${row}: y`).toBe( + Math.max(-160, 40 - row * 100), + ); + } + } finally { + lng.Config.animationsEnabled = false; + v.vi.unstubAllGlobals(); + } + }, + ); }); diff --git a/tests/contract-virtual.test.tsx b/tests/contract-virtual.test.tsx index 6b55b96..754f0fe 100644 --- a/tests/contract-virtual.test.tsx +++ b/tests/contract-virtual.test.tsx @@ -596,6 +596,53 @@ v.describe('VirtualRow: window and scroll modes', () => { ]); }, ); + + v.it( + 'scroll="auto", item count === displaySize: the row scrolls one slot, the last item ends where it does for a longer list', + async () => { + let row!: lng.ElementNode; + dispose = await mount(() => ( + + + {(item) => } + + + )); + const focusedX = () => row.x + (lng.activeElement() as lng.ElementNode).x; + const seen = [[focusedId(), focusedX()]]; + for (const key of [ + 'ArrowRight', + 'ArrowRight', + 'ArrowRight', + 'ArrowRight', + 'ArrowLeft', + 'ArrowLeft', + 'ArrowLeft', + ]) { + await press(key); + seen.push([focusedId(), focusedX()]); + } + // As with 5 or 12 items, the last item ends at 510, one slot short of + // 740: with no scroll it ended at 740, out of view. Left retraces the + // longer lists' path back to 50. + v.expect(seen).toEqual([ + ['v0', 50], + ['v1', 50], + ['v2', 280], + ['v3', 510], + ['v3', 510], + ['v2', 510], + ['v1', 280], + ['v0', 50], + ]); + }, + ); }); v.describe('VirtualRow: bufferSize and onEndReached', () => {