From 5388ea2025810d4be5cf75bae090b42466cb5b70 Mon Sep 17 00:00:00 2001 From: Erik Arvidsson Date: Sat, 11 Jul 2026 11:42:51 +0200 Subject: [PATCH] fix: add permalinkRow to RowsSnapshot and related functions for permalink support --- src/core/rows.ts | 10 +++ src/core/virtualizer.dom.test.ts | 47 ++++++++++++++ src/core/virtualizer.test.ts | 1 + src/core/virtualizer.ts | 74 ++++++++++++++++------- src/react/use-zero-virtualizer.test.ts | 1 + src/solid/create-zero-virtualizer.test.ts | 1 + 6 files changed, 111 insertions(+), 23 deletions(-) diff --git a/src/core/rows.ts b/src/core/rows.ts index 867b53f..424baf7 100644 --- a/src/core/rows.ts +++ b/src/core/rows.ts @@ -31,6 +31,13 @@ export type RowsSnapshot = { atEnd: boolean; firstRowIndex: number; permalinkNotFound: boolean; + /** + * The resolved permalink target row (the single-row lookup result), when the + * current anchor is a permalink and the row has loaded; `undefined` + * otherwise. Its `getRowKey` locates the DOM row to scroll to — the + * permalink id need not equal the row key. + */ + permalinkRow: TRow | undefined; }; /** The raw results of the (up to) three staged queries. */ @@ -211,6 +218,7 @@ export function assembleRows( ? anchorIndex : anchorIndex - rowsBeforeSize, permalinkNotFound, + permalinkRow: singleRow, }; } @@ -226,6 +234,7 @@ export function assembleRows( atEnd: mainComplete && !hasMoreRows, firstRowIndex: anchorIndex, permalinkNotFound, + permalinkRow: undefined, }; } @@ -241,5 +250,6 @@ export function assembleRows( atEnd: false, firstRowIndex: anchorIndex - paginatedRowsLength, permalinkNotFound, + permalinkRow: undefined, }; } diff --git a/src/core/virtualizer.dom.test.ts b/src/core/virtualizer.dom.test.ts index 630b67c..e9d6a25 100644 --- a/src/core/virtualizer.dom.test.ts +++ b/src/core/virtualizer.dom.test.ts @@ -506,3 +506,50 @@ describe('scroll-state restore when the container mounts lazily', () => { } }); }); + +describe('permalink scroll', () => { + test('scrolls the permalink row into view when its row key differs from the permalink id', () => { + // Deep-link by a human-friendly id (`r120`) while keying rows by something + // else (`key-r120`) — the common short-id-URL / uuid-key split. The scroll + // must still land, located via the resolved row's key, not the raw id. + const h = harness({ + rowCount: 300, + options: { + permalinkID: 'r120', + getRowKey: row => `key-${row.id}`, + }, + }); + h.settle(); + + // The target row is scrolled into view near the top. Without the fix + // `findRow` looks up `r120` (the permalink id) against DOM rows keyed + // `key-r120`, never matches, and the row stays far below the viewport. + const top = h.rowTop('key-r120'); + expect(top).toBeGreaterThanOrEqual(0); + expect(top).toBeLessThan(40); // within ~a row of the top + }); +}); + +describe('persist timing', () => { + test('persists immediately on scrollend, before the debounce, so a fast navigation keeps the position', () => { + const persisted: Array> = []; + const h = harness({ + rowCount: 100, + options: { + onScrollStateChange: s => + persisted.push(s as ScrollHistoryState), + }, + }); + h.settle(); + persisted.length = 0; + + // Scroll, then the browser signals scrolling ended — without any debounce + // time elapsing. The position must already be persisted, so navigating + // away right now (e.g. clicking a row) doesn't lose it. + h.userScroll(300); + h.scroller.dispatchEvent(new Event('scrollend')); + + expect(persisted.length).toBeGreaterThan(0); + expect(persisted.at(-1)!.scrollTop).toBe(300); + }); +}); diff --git a/src/core/virtualizer.test.ts b/src/core/virtualizer.test.ts index f63db9d..470cf00 100644 --- a/src/core/virtualizer.test.ts +++ b/src/core/virtualizer.test.ts @@ -17,6 +17,7 @@ function makeRows( atEnd: false, firstRowIndex: 0, permalinkNotFound: false, + permalinkRow: undefined, ...overrides, }; } diff --git a/src/core/virtualizer.ts b/src/core/virtualizer.ts index 24e1b29..bcde3d1 100644 --- a/src/core/virtualizer.ts +++ b/src/core/virtualizer.ts @@ -203,6 +203,7 @@ const EMPTY_ROWS: RowsSnapshot = { atEnd: false, firstRowIndex: 0, permalinkNotFound: false, + permalinkRow: undefined, }; /** @@ -942,10 +943,18 @@ export class ZeroVirtualizer { }; #onScrollEnd = (): void => { - if (!this.#manual()) return; - if (!this.#fingerDown) { + if (this.#manual()) { + // A gesture still under an active finger hasn't really ended. + if (this.#fingerDown) return; this.#withNotify(() => this.#endScrolling()); } + // Flush the position the moment scrolling ends, cancelling the pending + // debounce. The debounce coalesces mid-scroll writes (and keeps us under + // history-API rate limits), but on its own it loses the position when the + // user navigates within the debounce window right after stopping — e.g. + // scrolling a list then clicking a row. `scrollend` is that "stopped" + // signal, so persist synchronously here. + this.#persistNow(); }; #onTouchStart = (): void => { @@ -1196,7 +1205,14 @@ export class ZeroVirtualizer { } const el = this.#el; if (!el) return; - const target = findRow(el, pending); + // The DOM row is keyed by `getRowKey`, which need not equal `permalinkID`: + // apps routinely deep-link by a human-friendly id (a short id / slug) while + // keying rows by something else (a uuid). Locate the row by the resolved + // target row's key, falling back to the permalink id itself until it loads. + const targetRow = this.#rows.permalinkRow; + const targetKey = + targetRow !== undefined ? this.#options.getRowKey(targetRow) : pending; + const target = findRow(el, targetKey); if (!target) { // Not rendered yet — keep the request open and retry once it loads, // unless the row genuinely doesn't exist. @@ -1374,30 +1390,42 @@ export class ZeroVirtualizer { return; } this.#lastPersistKey = key; - // Capture at schedule time (matches the old effect's closure semantics). - const anchor = this.#paging.queryAnchor.anchor; - const estimatedTotal = this.#effectiveEstimatedTotal(); - const {hasReachedStart, hasReachedEnd} = this.#paging; - const {listContextParams} = this.#options; clearTimeout(this.#persistTimer); this.#persistTimer = setTimeout(() => { - const el = this.#el; - // The element can detach during the debounce (unmount). Skip rather than - // persist a spurious scrollTop: 0 over the saved position. - if (!el) return; - onScrollStateChange({ - anchor, - // The logical committed offset: if a gesture is mid-flight with an - // owed jump held in the wrapper margin, fold it in so restore lands - // right. - scrollTop: this.#scrollOffset(el) + this.#anchorState.pendingJump, - estimatedTotal, - hasReachedStart, - hasReachedEnd, - listContextParams, - }); + this.#persistTimer = undefined; + this.#writeScrollState(); }, PERSIST_DEBOUNCE_MS); } + + // Persist the current scroll state immediately, cancelling any pending + // debounced persist. Called when scrolling ends (`scrollend`): a navigation + // in the debounce window right after the user stops scrolling must not lose + // the position. + #persistNow(): void { + clearTimeout(this.#persistTimer); + this.#persistTimer = undefined; + this.#lastPersistKey = this.#persistKey(); + this.#writeScrollState(); + } + + // The single persist write. Reads the live position at call time; skips when + // detached (the element can go away between schedule and fire) — a write + // then would clobber the saved position with a spurious scrollTop: 0. + #writeScrollState(): void { + const {onScrollStateChange, listContextParams} = this.#options; + const el = this.#el; + if (!el || !this.#isListContextCurrent() || !onScrollStateChange) return; + onScrollStateChange({ + anchor: this.#paging.queryAnchor.anchor, + // The logical committed offset: if a gesture is mid-flight with an owed + // jump held in the wrapper margin, fold it in so restore lands right. + scrollTop: this.#scrollOffset(el) + this.#anchorState.pendingJump, + estimatedTotal: this.#effectiveEstimatedTotal(), + hasReachedStart: this.#paging.hasReachedStart, + hasReachedEnd: this.#paging.hasReachedEnd, + listContextParams, + }); + } } /** diff --git a/src/react/use-zero-virtualizer.test.ts b/src/react/use-zero-virtualizer.test.ts index 85b0920..7a07d1d 100644 --- a/src/react/use-zero-virtualizer.test.ts +++ b/src/react/use-zero-virtualizer.test.ts @@ -30,6 +30,7 @@ function makeUseRowsResult( atEnd: false, firstRowIndex: 0, permalinkNotFound: false, + permalinkRow: undefined, ...overrides, }; } diff --git a/src/solid/create-zero-virtualizer.test.ts b/src/solid/create-zero-virtualizer.test.ts index 12f164f..085cd8c 100644 --- a/src/solid/create-zero-virtualizer.test.ts +++ b/src/solid/create-zero-virtualizer.test.ts @@ -34,6 +34,7 @@ function makeRows( atEnd: false, firstRowIndex: 0, permalinkNotFound: false, + permalinkRow: undefined, ...overrides, }; }