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 <body>. The mount now reaches through the revealed row.
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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. */}
|
||||
<PageToolbar>
|
||||
<div className={channelScoped ? "px-4 sm:px-6" : "px-4 pt-2 pb-1"}>
|
||||
{toolbar}
|
||||
@@ -892,7 +892,7 @@ export default function Feed({
|
||||
onResetState={onResetState}
|
||||
onOpen={openVideo}
|
||||
hasNextPage={!!hasNextPage}
|
||||
isFetchingNextPage={isFetchingNextPage}
|
||||
isFetching={query.isFetching}
|
||||
fetchNextPage={fetchNextPage}
|
||||
/>
|
||||
)}
|
||||
|
||||
@@ -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<HTMLDivElement | null>(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
|
||||
|
||||
@@ -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) => (
|
||||
|
||||
@@ -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 <body>.
|
||||
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(
|
||||
<DataTable
|
||||
rows={rows}
|
||||
columns={plain}
|
||||
rowKey={(x) => x.id}
|
||||
pageSize={50}
|
||||
reveal={reveal}
|
||||
/>,
|
||||
),
|
||||
);
|
||||
draw(null);
|
||||
draw({ key: "r90", seq: 1 });
|
||||
expect(host.querySelector("tbody")?.textContent).toContain("r 90");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -274,10 +274,12 @@ export default function DataTable<T>({
|
||||
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) {
|
||||
|
||||
@@ -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<T> {
|
||||
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<T>({
|
||||
rowEst,
|
||||
renderItem,
|
||||
hasNextPage = false,
|
||||
isFetchingNextPage = false,
|
||||
isFetching = false,
|
||||
fetchNextPage,
|
||||
keyboardNav = false,
|
||||
}: VirtualGridProps<T>) {
|
||||
@@ -161,17 +163,17 @@ export default function VirtualGrid<T>({
|
||||
// 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 (
|
||||
<div ref={listRef} style={{ height: virtualizer.getTotalSize(), position: "relative" }}>
|
||||
|
||||
@@ -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<T>(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 <body> (E31 S5).
|
||||
export function useProgressiveRows<T>(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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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\}/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user