From 15ec67b0290bbe57e69e665565a1dfbb4f7153a4 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 2 Oct 2026 23:42:31 +0200 Subject: [PATCH] fix(ui): a read-ahead waits out a background refetch, and a revealed row is mounted before it is focused NEXT_PAGE joins ANY fetch in flight: a read-ahead during an invalidation or a poll joined that refetch and appended no page, and nothing asked again until the reader moved the range (the channel page during a backfill). The read-ahead now waits while the list fetches anything (mayReadAhead, re-run on isFetching); a Load older click cancels a poll instead (nextPageOnClick). A page turn to a revealed row restarted the progressive mount at 30 rows; Channels focuses that row in the next commit, and at index 40 of a 50-row page it was not there, so focus fell to . The mount now reaches through the revealed row. --- frontend/src/components/AdminLogs.tsx | 7 ++- frontend/src/components/Feed.tsx | 8 +-- frontend/src/components/PlexBrowse.tsx | 8 +-- frontend/src/components/VirtualFeed.tsx | 6 +- .../ui/DataTable.progressive.dom.test.tsx | 24 ++++++++ frontend/src/components/ui/DataTable.tsx | 4 +- frontend/src/components/ui/VirtualGrid.tsx | 16 +++--- frontend/src/components/ui/progressiveRows.ts | 15 +++-- frontend/src/lib/nextPage.dom.test.ts | 57 ++++++++++++++++++- frontend/src/lib/nextPage.ts | 16 ++++++ .../src/lib/nextPage.wiring.contract.test.ts | 32 +++++++++++ 11 files changed, 165 insertions(+), 28 deletions(-) create mode 100644 frontend/src/lib/nextPage.wiring.contract.test.ts diff --git a/frontend/src/components/AdminLogs.tsx b/frontend/src/components/AdminLogs.tsx index 656ce33f..7c4a13d7 100644 --- a/frontend/src/components/AdminLogs.tsx +++ b/frontend/src/components/AdminLogs.tsx @@ -13,7 +13,7 @@ import { api, type LogFacet, type LogQuery, type ServerLogRow } from "../lib/api import { copyText } from "../lib/clipboard"; import { formatDateTimeSec } from "../lib/format"; import { nextNavId, notifyNav, rememberNavPosition } from "../lib/history"; -import { NEXT_PAGE } from "../lib/nextPage"; +import { nextPageOnClick } from "../lib/nextPage"; import { LEVEL_GROUPS, type LevelGroup } from "../lib/log"; import { DEFAULT_LOGS_STATE, @@ -157,7 +157,8 @@ export default function AdminLogs() { ? "countsFailed" : null; const retryFailed = () => { - if (logs.isFetchNextPageError) void logs.fetchNextPage(NEXT_PAGE); + if (logs.isFetchNextPageError) + void logs.fetchNextPage(nextPageOnClick(logs.isFetchingNextPage)); else if (logs.isError) void logs.refetch(); if (facets.isError) void facets.refetch(); }; @@ -191,7 +192,7 @@ export default function AdminLogs() { const loadOlder = () => { setLive(false); - void logs.fetchNextPage(NEXT_PAGE); + void logs.fetchNextPage(nextPageOnClick(logs.isFetchingNextPage)); }; const copy = async () => { diff --git a/frontend/src/components/Feed.tsx b/frontend/src/components/Feed.tsx index 3eb7d287..4a32e5c0 100644 --- a/frontend/src/components/Feed.tsx +++ b/frontend/src/components/Feed.tsx @@ -550,7 +550,7 @@ export default function Feed({ onResetState={onResetState} onOpen={openVideo} hasNextPage={false} - isFetchingNextPage={false} + isFetching={false} fetchNextPage={() => {}} /> {ytQuery.hasNextPage && ( @@ -826,8 +826,8 @@ export default function Feed({ it stays put while the cards scroll (bug 4). On the feed page that slot is the shell's top band; on a channel page ChannelPage overrides the slot with its own sticky sub-header, so the toolbar rides with the tabs. Channel pages get no chips: their Feed starts from its OWN - baseline (channel-scoped, whole-catalog, show-all), so each would read as a "filter you - applied" when it's just the page being itself. */} + baseline (channel-scoped, whole-catalog, the watch state the channel page remembers), so + each would read as a "filter you applied" when it's just the page being itself. */}
{toolbar} @@ -892,7 +892,7 @@ export default function Feed({ onResetState={onResetState} onOpen={openVideo} hasNextPage={!!hasNextPage} - isFetchingNextPage={isFetchingNextPage} + isFetching={query.isFetching} fetchNextPage={fetchNextPage} /> )} diff --git a/frontend/src/components/PlexBrowse.tsx b/frontend/src/components/PlexBrowse.tsx index a9fdfac2..6b80d6df 100644 --- a/frontend/src/components/PlexBrowse.tsx +++ b/frontend/src/components/PlexBrowse.tsx @@ -9,7 +9,7 @@ import { } from "@tanstack/react-query"; import { User, X } from "lucide-react"; import { qk } from "../lib/queryKeys"; -import { NEXT_PAGE } from "../lib/nextPage"; +import { mayReadAhead, NEXT_PAGE } from "../lib/nextPage"; import { healPlexField } from "../lib/plexSearch"; import { healPlexSort, plexSortOptions, PLEX_SORT_FALLBACK } from "../lib/plexSort"; import type { SortState } from "../lib/columnSort"; @@ -317,19 +317,19 @@ export default function PlexBrowse() { // mount/unmount, so the observer always watches the live node; a plain ref would keep observing the // detached old node after Back and never fire fetchNextPage again (feed stuck at the loaded pages). const [sentinelEl, setSentinelEl] = useState(null); - const { hasNextPage, isFetchingNextPage, fetchNextPage } = browseQ; + const { hasNextPage, isFetching, isFetchingNextPage, fetchNextPage } = browseQ; useEffect(() => { if (!sentinelEl) return; const io = new IntersectionObserver( (entries) => { - if (entries[0]?.isIntersecting && hasNextPage && !isFetchingNextPage) + if (entries[0]?.isIntersecting && mayReadAhead(hasNextPage, isFetching)) void fetchNextPage(NEXT_PAGE); }, { rootMargin: "600px" }, ); io.observe(sentinelEl); return () => io.disconnect(); - }, [sentinelEl, hasNextPage, isFetchingNextPage, fetchNextPage]); + }, [sentinelEl, hasNextPage, isFetching, fetchNextPage]); function onCard(card: PlexCard) { scrollRef.current = scroller()?.scrollTop ?? 0; // remember grid position for the trip back diff --git a/frontend/src/components/VirtualFeed.tsx b/frontend/src/components/VirtualFeed.tsx index 521d5d39..3fc2d66a 100644 --- a/frontend/src/components/VirtualFeed.tsx +++ b/frontend/src/components/VirtualFeed.tsx @@ -29,7 +29,7 @@ export interface VirtualFeedProps { onResetState: (id: string) => void; onOpen: (v: Video, startAt?: number | null) => void; hasNextPage: boolean; - isFetchingNextPage: boolean; + isFetching: boolean; fetchNextPage: (options: typeof NEXT_PAGE) => unknown; } @@ -44,7 +44,7 @@ export default function VirtualFeed({ onResetState, onOpen, hasNextPage, - isFetchingNextPage, + isFetching, fetchNextPage, }: VirtualFeedProps) { const { minCol, colMaxWidth, maxCols, thumb } = FEED_VIEW_SPEC[view]; @@ -64,7 +64,7 @@ export default function VirtualFeed({ maxCols={maxCols} rowEst={ROW_EST[view]} hasNextPage={hasNextPage} - isFetchingNextPage={isFetchingNextPage} + isFetching={isFetching} fetchNextPage={fetchNextPage} keyboardNav renderItem={(v, itemWidth, index) => ( diff --git a/frontend/src/components/ui/DataTable.progressive.dom.test.tsx b/frontend/src/components/ui/DataTable.progressive.dom.test.tsx index db15fa1c..42942f37 100644 --- a/frontend/src/components/ui/DataTable.progressive.dom.test.tsx +++ b/frontend/src/components/ui/DataTable.progressive.dom.test.tsx @@ -100,4 +100,28 @@ describe("DataTable — a long page mounts in steps", () => { expect(bodyRows()).toBe(FIRST_ROWS); await waitFor(() => expect(bodyRows()).toBe(150)); }); + + // E31 S5: Channels turns the page to the row a priority pick moved and focuses it in the commit + // that follows. That row was at index 40 of a 50-row page: not mounted yet, focus fell to . + it("mounts a revealed row in the render that turns to its page", () => { + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + const rows = many(100); + const draw = (reveal: { key: string; seq: number } | null) => + flushSync(() => + root!.render( + x.id} + pageSize={50} + reveal={reveal} + />, + ), + ); + draw(null); + draw({ key: "r90", seq: 1 }); + expect(host.querySelector("tbody")?.textContent).toContain("r 90"); + }); }); diff --git a/frontend/src/components/ui/DataTable.tsx b/frontend/src/components/ui/DataTable.tsx index db8c02a9..e60a75b2 100644 --- a/frontend/src/components/ui/DataTable.tsx +++ b/frontend/src/components/ui/DataTable.tsx @@ -274,10 +274,12 @@ export default function DataTable({ if (at >= 0 && !allRows) setPage(Math.floor(at / size)); } const paged = allRows ? sorted : sorted.slice(safePage * size, safePage * size + size); - // A long page ("All", 100) mounts its first rows at once and the rest in the background. + // A long page ("All", 100) mounts its first rows at once and the rest in the background — but + // always through the revealed row, which the caller focuses as soon as this frame commits. const shown = useProgressiveRows( paged, `${safePage}:${size}:${JSON.stringify(sort)}:${JSON.stringify(filters)}:${narrowedBy}`, + reveal ? paged.findIndex((row) => rowKey(row) === reveal.key) : -1, ); function toggleSort(key: string) { diff --git a/frontend/src/components/ui/VirtualGrid.tsx b/frontend/src/components/ui/VirtualGrid.tsx index fcc95e92..325f36bc 100644 --- a/frontend/src/components/ui/VirtualGrid.tsx +++ b/frontend/src/components/ui/VirtualGrid.tsx @@ -10,7 +10,7 @@ import { import { useVirtualizer } from "@tanstack/react-virtual"; import { useListNav } from "../../lib/listNav"; import { scrollParent } from "../../lib/listPaging"; -import { NEXT_PAGE } from "../../lib/nextPage"; +import { mayReadAhead, NEXT_PAGE } from "../../lib/nextPage"; import { GRID_GAP, columnsFor, itemWidthFor } from "./gridColumns"; import { shouldPrefetchNext } from "./readAhead"; @@ -66,7 +66,9 @@ export interface VirtualGridProps { keyboardNav?: boolean; /** Infinite scroll — omit all three for a finite client-side list. */ hasNextPage?: boolean; - isFetchingNextPage?: boolean; + /** Any fetch of the list in flight (a next page OR a background refetch): the read-ahead waits for + * it and runs again when it settles (`lib/nextPage` mayReadAhead). */ + isFetching?: boolean; /** Called with `NEXT_PAGE`, so a read-ahead that fires again joins the page already loading. */ fetchNextPage?: (options: typeof NEXT_PAGE) => unknown; } @@ -80,7 +82,7 @@ export default function VirtualGrid({ rowEst, renderItem, hasNextPage = false, - isFetchingNextPage = false, + isFetching = false, fetchNextPage, keyboardNav = false, }: VirtualGridProps) { @@ -161,17 +163,17 @@ export default function VirtualGrid({ // Infinite-scroll read-ahead: pull the next page once the last rendered row is within the // read-ahead window of the loaded end (see readAhead.ts — the window scales with the rendered row // count, clamped so a tall window doesn't prefetch unboundedly). A no-op for finite lists (no - // fetchNextPage / hasNextPage false). Because this effect re-runs when a page settles - // (isFetchingNextPage flips false) it CHAINS — refilling the buffer as the user consumes it, so a + // fetchNextPage / hasNextPage false). Because this effect re-runs when a fetch settles + // (isFetching flips false) it CHAINS — refilling the buffer as the user consumes it, so a // fast scroll reaches the loaded end far less often. useEffect(() => { - if (!fetchNextPage || !hasNextPage || isFetchingNextPage) return; + if (!fetchNextPage || !mayReadAhead(hasNextPage, isFetching)) return; const last = virtualRows[virtualRows.length - 1]; if (!last) return; if (shouldPrefetchNext(last.index, rows.length, virtualRows.length)) { fetchNextPage(NEXT_PAGE); } - }, [virtualRows, rows.length, hasNextPage, isFetchingNextPage, fetchNextPage]); + }, [virtualRows, rows.length, hasNextPage, isFetching, fetchNextPage]); return (
diff --git a/frontend/src/components/ui/progressiveRows.ts b/frontend/src/components/ui/progressiveRows.ts index 6580176a..d420c3bf 100644 --- a/frontend/src/components/ui/progressiveRows.ts +++ b/frontend/src/components/ui/progressiveRows.ts @@ -18,16 +18,21 @@ export const STEP_ROWS = 40; // `view` names what the rows show (page, size, sort, filters). A new view starts over from the first // rows, so the urgent render after a click or a key stays small. New data for the same view (a refetch) // does not: it would drop the rows the reader is looking at. -export function useProgressiveRows(rows: T[], view: string): T[] { +// +// `through` is a row that must be in the DOM in THIS render: the row a page turn was made to show +// (DataTable's `reveal`). Its page is a new view, and the caller focuses that row right after the +// commit — at index 40 of a 50-row page it was not mounted yet, and focus fell to (E31 S5). +export function useProgressiveRows(rows: T[], view: string, through = -1): T[] { const [seen, setSeen] = useState(view); const [limit, setLimit] = useState(FIRST_ROWS); if (seen !== view) { setSeen(view); setLimit(FIRST_ROWS); } - const more = rows.length > limit; + const count = Math.max(limit, through + 1); + const more = rows.length > count; useEffect(() => { - if (more) startTransition(() => setLimit(limit + STEP_ROWS)); - }, [more, limit]); - return more ? rows.slice(0, limit) : rows; + if (more) startTransition(() => setLimit(count + STEP_ROWS)); + }, [more, count]); + return more ? rows.slice(0, count) : rows; } diff --git a/frontend/src/lib/nextPage.dom.test.ts b/frontend/src/lib/nextPage.dom.test.ts index 46eff88b..3a27d86f 100644 --- a/frontend/src/lib/nextPage.dom.test.ts +++ b/frontend/src/lib/nextPage.dom.test.ts @@ -5,9 +5,22 @@ import { InfiniteQueryObserver } from "@tanstack/react-query"; import { describe, expect, it } from "vitest"; -import { NEXT_PAGE } from "./nextPage"; +import { mayReadAhead, NEXT_PAGE, nextPageOnClick } from "./nextPage"; import { createQueryClient } from "./queryClient"; +function probe(sent: (string | null)[], delay = 5) { + return new InfiniteQueryObserver(createQueryClient(), { + queryKey: ["probe"], + initialPageParam: null as string | null, + queryFn: async ({ pageParam }) => { + sent.push(pageParam); + await new Promise((r) => setTimeout(r, delay)); + return { next: `c${sent.length}` }; + }, + getNextPageParam: (last: { next: string }) => last.next, + }); +} + describe("NEXT_PAGE", () => { it("sends each page once when the read-ahead calls twice in a row", async () => { const sent: (string | null)[] = []; @@ -32,4 +45,46 @@ describe("NEXT_PAGE", () => { expect(observer.getCurrentResult().data?.pages).toHaveLength(2); unsubscribe(); }); + + // E31 S5: the join takes ANY fetch in flight. During a background refetch (an invalidation, a + // poll) it reloads the pages already there and appends none — why a read-ahead waits it out. + it("joins a background refetch and appends no page", async () => { + const sent: (string | null)[] = []; + const observer = probe(sent); + const unsubscribe = observer.subscribe(() => {}); + await observer.refetch(); + + void observer.refetch(); + await observer.fetchNextPage(NEXT_PAGE); + + expect(observer.getCurrentResult().data?.pages).toHaveLength(1); + unsubscribe(); + }); +}); + +describe("a read-ahead", () => { + it("waits while the list fetches anything", () => { + expect(mayReadAhead(true, false)).toBe(true); + expect(mayReadAhead(true, true)).toBe(false); + expect(mayReadAhead(false, false)).toBe(false); + }); +}); + +describe("a next page asked for by a click", () => { + it("loads the page even while a background refetch runs", async () => { + const sent: (string | null)[] = []; + const observer = probe(sent); + const unsubscribe = observer.subscribe(() => {}); + await observer.refetch(); + + void observer.refetch(); + await observer.fetchNextPage(nextPageOnClick(false)); + + expect(observer.getCurrentResult().data?.pages).toHaveLength(2); + unsubscribe(); + }); + + it("joins a next page already loading", () => { + expect(nextPageOnClick(true)).toBe(NEXT_PAGE); + }); }); diff --git a/frontend/src/lib/nextPage.ts b/frontend/src/lib/nextPage.ts index 60798abd..0f6f4f58 100644 --- a/frontend/src/lib/nextPage.ts +++ b/frontend/src/lib/nextPage.ts @@ -5,5 +5,21 @@ * re-runs on every scroll frame, and a frame that lands before `isFetchingNextPage` turns true calls * again. On prod (E31 S4e) every feed page went out 2–3 times; on a 0.8 s title sort the copies * queued to 2.3 s. With this a second call joins the fetch already running. + * + * The join takes ANY fetch in flight, not only a next page: a call during a background refetch (an + * invalidation, a poll) joins THAT, which reloads the pages already there and appends none (E31 S5). + * So a read-ahead also waits while the list is fetching at all (`mayReadAhead`) and runs again when + * the fetch settles; a click uses `nextPageOnClick`. */ export const NEXT_PAGE = { cancelRefetch: false } as const; + +/** Whether an infinite list's read-ahead may ask for the next page now. Not while ANY fetch of the + * list is in flight: that call would join it (see NEXT_PAGE). The caller re-runs on `isFetching`. */ +export const mayReadAhead = (hasNextPage: boolean, isFetching: boolean) => + hasNextPage && !isFetching; + +/** The options for a next page a person asked for (a "Load older" button): join the page already + * loading, else cancel a background refetch and load the page — a click must not be swallowed by a + * poll that happened to be running. */ +export const nextPageOnClick = (isFetchingNextPage: boolean) => + isFetchingNextPage ? NEXT_PAGE : ({ cancelRefetch: true } as const); diff --git a/frontend/src/lib/nextPage.wiring.contract.test.ts b/frontend/src/lib/nextPage.wiring.contract.test.ts new file mode 100644 index 00000000..a12aa837 --- /dev/null +++ b/frontend/src/lib/nextPage.wiring.contract.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; + +// Every infinite list's read-ahead waits while the list fetches ANYTHING and runs again when the fetch +// settles (E31 S5). A read-ahead that only checks `isFetchingNextPage` calls during a background +// refetch, joins it through NEXT_PAGE and loads no page: the grid stalls at its loaded end until the +// reader scrolls the range. The jsdom virtualizer measures nothing, so the wiring is pinned in source. +const files = import.meta.glob(["../components/**/*.tsx", "!../components/**/*.test.tsx"], { + query: "?raw", + import: "default", + eager: true, +}); +const source = (name: string) => { + const hit = Object.entries(files).find(([path]) => path.endsWith(`/${name}`)); + if (!hit) throw new Error(`no ${name}`); + return hit[1] as string; +}; + +describe("an infinite list's read-ahead", () => { + it.each(["ui/VirtualGrid.tsx", "PlexBrowse.tsx"])( + "%s gates on mayReadAhead and re-runs on isFetching", + (file) => { + const src = source(file); + expect(src).toMatch(/mayReadAhead\(hasNextPage, isFetching\)/); + expect(src).toMatch(/\[[^\]]*\bisFetching\b[^\]]*fetchNextPage\]\);/); + expect(src).not.toMatch(/hasNextPage && !isFetchingNextPage/); + }, + ); + + it("the feed hands the grid the query's isFetching", () => { + expect(source("Feed.tsx")).toMatch(/isFetching=\{query\.isFetching\}/); + }); +});