diff --git a/src/components/EpisodeList.tsx b/src/components/EpisodeList.tsx index 4c28b43..559e706 100644 --- a/src/components/EpisodeList.tsx +++ b/src/components/EpisodeList.tsx @@ -20,6 +20,7 @@ import { useTerminalDimensions } from "@opentui/solid"; import { useTheme } from "@/context/ThemeContext"; import { useScrollIntoView } from "@/hooks/useScrollIntoView"; import { NF_ICONS } from "@/utils/nerd-fonts"; +import { useHeldFlag } from "@/hooks/useHeldFlag"; import { LoadingIndicator } from "@/components/LoadingIndicator"; import type { Episode } from "@/types/episode"; @@ -138,6 +139,10 @@ export function FetchMoreRow(props: { onMouseDown: () => void; }) { const { theme } = useTheme(); + // Hold the spinner past the raw load signal: a warm-cache load can + // begin and end between two renderer frames, and without the hold the + // [Fetch More] → spinner swap paints zero frames. + const loading = useHeldFlag(props.isLoadingMore); const ref = useScrollIntoView(props.onMore); const bg = () => props.index() === props.focused() && props.active() @@ -165,7 +170,7 @@ export function FetchMoreRow(props: { {NF_ICONS.more} )} } > [Fetch More] @@ -232,13 +237,16 @@ export function FetchMorePreview(props: { }) { const { theme } = useTheme(); const muted = () => theme.muted || theme.text; + // Same minimum-spinning window as FetchMoreRow: keep the "Loading…" + // line up across the (sub-frame) cached load burst. + const loading = useHeldFlag(props.isLoadingMore); return ( [Fetch More] - {props.isLoadingMore() + {loading() ? "Loading the next batch of episodes…" : props.manualText()} diff --git a/src/components/GlobalActivityIndicator.tsx b/src/components/GlobalActivityIndicator.tsx index 5ce41df..d7a68b8 100644 --- a/src/components/GlobalActivityIndicator.tsx +++ b/src/components/GlobalActivityIndicator.tsx @@ -4,6 +4,7 @@ import { useSearchStore } from "@/stores/search"; import { useDownloadStore } from "@/stores/download"; import { useActivityStore } from "@/stores/activity"; import { LoadingIndicator } from "@/components/LoadingIndicator"; +import { useHeldFlag } from "@/hooks/useHeldFlag"; /** * GlobalActivityIndicator — one global top-right signal that ANY feed @@ -16,11 +17,14 @@ export function GlobalActivityIndicator() { const searchStore = useSearchStore(); const downloadStore = useDownloadStore(); const activity = useActivityStore(); + // Fetch-more loads can begin and end between two renderer frames (warm + // cache); hold the feed-more contribution so the indicator paints. + const isLoadingMoreHeld = useHeldFlag(() => feedStore.isLoadingMore()); /** True while any tracked activity is in flight */ const isActive = () => feedStore.isLoadingFeeds() || - feedStore.isLoadingMore() || + isLoadingMoreHeld() || searchStore.isSearching() || downloadStore.getActiveCount() + downloadStore.getQueue().length > 0 || activity.isActive(); diff --git a/src/hooks/useHeldFlag.ts b/src/hooks/useHeldFlag.ts new file mode 100644 index 0000000..4fb2e83 --- /dev/null +++ b/src/hooks/useHeldFlag.ts @@ -0,0 +1,46 @@ +/** + * useHeldFlag — keep a boolean true for a minimum time after it falls. + * + * A warm-cache fetch-more load begins and ends between two renderer + * frames: the raw `isLoadingMore` signal flips true→false without a + * single paint, so the "[Fetch More]" → spinner swap never appears and + * the press looks like a no-op. Components rendering a spinner for such + * bursts read through this hook instead of the raw signal — the spinner + * stays up (and animating) for at least `minMs` after the load ends, + * guaranteeing several painted frames. + * + * Deliberately a display-layer concern: the store's own `isLoadingMore` + * keeps its exact load-window semantics (guards, tests) and only the + * rendered indicators are held. + */ +import { createSignal, createEffect, onCleanup } from "solid-js"; + +export function useHeldFlag( + source: () => boolean, + minMs = 250, +): () => boolean { + const [held, setHeld] = createSignal(false); + let timer: ReturnType | null = null; + const clearTimer = () => { + if (timer) { + clearTimeout(timer); + timer = null; + } + }; + + createEffect(() => { + if (source()) { + // (Re)rising edge: show immediately; a pending fall from an + // earlier burst is cancelled. + clearTimer(); + if (!held()) setHeld(true); + } else if (held()) { + // Falling edge: hold the flag up for the remaining window. + clearTimer(); + timer = setTimeout(() => setHeld(false), minMs); + } + }); + + onCleanup(clearTimer); + return held; +} diff --git a/src/hooks/useStableListFocus.ts b/src/hooks/useStableListFocus.ts new file mode 100644 index 0000000..80d021f --- /dev/null +++ b/src/hooks/useStableListFocus.ts @@ -0,0 +1,75 @@ +/** + * useStableListFocus — keep the cursor on the SAME row (by id), not the same + * index, when a lazy load inserts rows around it. + * + * The nav store keeps one integer focus per depth frame. That is correct + * for j/k (each press moves exactly one row) but wrong when the LIST + * changes underneath the cursor: a fetch-more press appends revealed + * episodes ABOVE the "[Fetch More]" row (every feed's deeper history is + * older than the union's tail), so the button's index shifts down and an + * index-stable cursor silently lands on another row. In the Feed tab the + * union can even gain rows in the MIDDLE (a revealed episode of one show + * sorts newer than another show's already-visible deep rows), moving the + * focused episode itself. + * + * Usage: pages call this hook with a stable row-id accessor (episode id, + * show id, or the FETCH_MORE_ROW_ID sentinel for the button row) plus + * read/write access to the nav frame focus. Whenever the row count + * changes, the cursor is re-anchored onto the previously focused row: + * • still present → cursor follows that row (index may change) + * • gone (removed) → keep the current, clamped index + * + * The id snapshot is taken on EVERY change of focus or rows (not just + * count changes), so selection by mouse and j/k both re-anchor correctly. + */ +import { createEffect, on, untrack } from "solid-js"; + +/** Sentinel id for the "[Fetch More]" row — never collides with real ids. */ +export const FETCH_MORE_ROW_ID = "__fetch-more__"; + +export function useStableListFocus(deps: { + /** Total row count of the list this pane shows. */ + count: () => number; + /** Stable id of the row at `index` (undefined for out-of-range). */ + getItemId: (index: number) => string | undefined; + /** Current focused row index of this pane. */ + getFocus: () => number; + /** Write the focused row index of this pane. */ + setFocus: (index: number) => void; +}): void { + /** Focused row id at the time of the last snapshot. */ + let focusedId: string | undefined; + + // Snapshot the focused row's id whenever focus or rows change. Reading + // the list here would also re-run on unrelated row-content changes, so + // only the id resolution is untracked. + createEffect(() => { + const idx = deps.getFocus(); + untrack(() => { + focusedId = deps.getItemId(idx); + }); + }); + + // Re-anchor after the row count changes (deferred: never on first run — + // initial focus placement is the page's job). + createEffect( + on( + deps.count, + (count) => { + if (focusedId === undefined) return; + let next: number | null = null; + for (let i = 0; i < count; i++) { + if (deps.getItemId(i) === focusedId) { + next = i; + break; + } + } + // Row gone (removed): leave the clamped index alone — the + // page's own ensureFocus handles bounds. + if (next === null) return; + if (next !== deps.getFocus()) deps.setFocus(next); + }, + { defer: true }, + ), + ); +} diff --git a/src/pages/Feed/FeedPage.tsx b/src/pages/Feed/FeedPage.tsx index 93f9744..988467b 100644 --- a/src/pages/Feed/FeedPage.tsx +++ b/src/pages/Feed/FeedPage.tsx @@ -45,6 +45,10 @@ import { LoadingIndicator } from "@/components/LoadingIndicator"; import { PaneRow } from "@/components/PaneRow"; import { TabListPane } from "@/components/TabPanel"; import { useSelectionMarker } from "@/hooks/useSelectionMarker"; +import { + useStableListFocus, + FETCH_MORE_ROW_ID, +} from "@/hooks/useStableListFocus"; export const FeedPaneCount = 1; @@ -103,6 +107,23 @@ function FeedPage() { focusedOnMore() ? undefined : episodes()[focusedEpIdx()]; const curLen = () => rowCount(); + // ── Focus stability across lazy loads ───────────────────────────────────── + // The nav cursor is a plain row index; fetch-more inserts revealed + // episodes above the [Fetch More] row (and, in the chronological union, + // can splice rows into the middle), which silently moves an index-stable + // cursor onto a different episode or off the button. Re-anchor the cursor + // onto the focused row's ID after any row-count change — the user stays + // on the exact episode (or the button) they were on before the load. + useStableListFocus({ + count: rowCount, + getItemId: (i) => + i === episodes().length + ? FETCH_MORE_ROW_ID + : episodes()[i]?.episode.id, + getFocus: () => nav.depthFocus(0), + setFocus: (i) => nav.setDepthFocus(i, 0), + }); + // ── Render window ──────────────────────────────────────────────────────── // The union grows to thousands of episodes after repeated fetch-more // presses; rendering every row per frame froze the UI. Render only a diff --git a/src/pages/MyShows/MyShowsPage.tsx b/src/pages/MyShows/MyShowsPage.tsx index 472614c..e7514d5 100644 --- a/src/pages/MyShows/MyShowsPage.tsx +++ b/src/pages/MyShows/MyShowsPage.tsx @@ -47,6 +47,10 @@ import { PaneRow } from "@/components/PaneRow"; import { TabListPane } from "@/components/TabPanel"; import { useScrollIntoView } from "@/hooks/useScrollIntoView"; import { useSelectionMarker } from "@/hooks/useSelectionMarker"; +import { + useStableListFocus, + FETCH_MORE_ROW_ID, +} from "@/hooks/useStableListFocus"; // ── render components ──────────────────────────────────────────────────────── // Depth-0 rows (subscribed shows, unsubscribed-show downloads) and their @@ -361,6 +365,32 @@ export function MyShowsPage() { const curLen = () => (depth() === 0 ? depth0Count() : rowCount()); + // ── Focus stability across lazy loads ───────────────────────────────────── + // The nav cursor is a plain row index; fetch-more inserts revealed + // episodes above the [Fetch More] row, silently moving an index-stable + // cursor onto a different episode or off the button. Re-anchor the cursor + // onto the focused row's ID after any row-count change, at both depths + // (the shows list shifts when subscriptions or unsubscribed downloads + // change; the episode list shifts on fetch-more). + useStableListFocus({ + count: curLen, + getItemId: (i) => { + if (depth() === 0) { + const showsLen = shows().length; + const unsubsLen = unsubs().length; + if (i < showsLen) return shows()[i]?.id; + if (i < showsLen + unsubsLen) + return unsubs()[i - showsLen]?.episodeId; + return FETCH_MORE_ROW_ID; + } + return i === episodes().length + ? FETCH_MORE_ROW_ID + : episodes()[i]?.id; + }, + getFocus: () => focus(depth()), + setFocus: (i) => nav.setDepthFocus(i, depth()), + }); + const ensureFocus = () => { if (depth() === 0 && depth0Count() > 0 && focus(0) >= depth0Count()) nav.setDepthFocus(depth0Count() - 1, 0); diff --git a/tests/fetch-more-focus-spinner.test.tsx b/tests/fetch-more-focus-spinner.test.tsx new file mode 100644 index 0000000..6a7f4ad --- /dev/null +++ b/tests/fetch-more-focus-spinner.test.tsx @@ -0,0 +1,330 @@ +/** + * Fetch-more UX — the two contracts behind a warm-cache "[Fetch More]" + * press (the fast path where the whole load is served from the + * full-episode cache and applies between renderer frames): + * + * 1. The spinner is VISIBLE for a meaningful window even though the + * raw `isLoadingMore` signal may have already fallen before a + * single frame painted (useHeldFlag holds the rendered state). + * 2. The focused episode stays the SAME EPISODE after new rows are + * lazily inserted (the chronological union can splice revealed rows + * into the middle of the list, and the "[Fetch More]" button itself + * moves down) — useStableListFocus re-anchors the nav cursor onto + * the focused row's id. + * + * Both are asserted through the real FeedPage render: the row glyph, the + * focused-row highlight, and the FetchMoreRow spinner all come out of + * captureSpans. + */ + +import { test, expect, beforeAll, afterAll } from "bun:test"; +import type { Server } from "bun"; +import { mkdtempSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +const configHome = mkdtempSync(join(tmpdir(), "podtui-fetchmore-")); +process.env.XDG_CONFIG_HOME = configHome; +process.env.PODTUI_AUDIO_BACKEND = "none"; + +import { testRender } from "@opentui/solid"; +import { ThemeProvider } from "../src/context/ThemeContext"; +import { NavigationProvider } from "../src/context/NavigationContext"; +import { FeedPage } from "../src/pages/Feed/FeedPage"; +import { useFeedStore } from "../src/stores/feed"; +import { useAppStore } from "../src/stores/app"; +import { useNavigation } from "../src/context/NavigationContext"; +import type { Podcast } from "../src/types/podcast"; + +// The LoadingIndicator glyph cycle. +const SPINNER_RE = /[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/; + +type Frame = { cols: number; lines: { spans: { text: string }[] }[] }; +const frameLines = (f: Frame): string[] => + f.lines.map((l) => l.spans.map((s) => s.text).join("")); + +let server: Server | null = null; +let feedUrl = ""; +let feedId = ""; + +const makePodcast = (url: string): Podcast => ({ + id: "", + title: "FetchMore Show", + description: "fetch-more test feed", + author: "tester", + feedUrl: url, + lastUpdated: new Date(), + isSubscribed: true, +}); + +/** + * A feed whose union demonstrates BOTH splicing behaviors: + * • "Top Ep" (2 days ago) — stays at the head of the list. + * • "Deep Ep N" — deep history at 12-day cadence: the first 5 are inside + * the subscribe window alongside Top Ep; the rest only surface via + * fetch-more (and many presses remain, so the button never unmounts + * mid-press). + * + * The revealed deep episodes are OLDER than every visible row, so they + * splice in above the [Fetch More] button but below every episode — an + * index-stable cursor on the last visible episode would land on a + * different (older) episode after the press. + */ +function feedXml(origin: string): string { + const items: string[] = [ + ` + Top Ep + ${new Date(Date.now() - 2 * 24 * 3600 * 1000).toISOString()} + +`, + ]; + for (let i = 0; i < 36; i++) { + // Deep history at 12-day spacing: 10, 22, 34 … 442 days old. The + // subscribe window shows the 6 newest; MANY presses of fetch-more + // remain, so a mid-session press never exhausts the cache and the + // [Fetch More] row stays mounted through the held spinner window. + const days = 10 + i * 12; + items.push(` + Deep Ep ${i} + ${new Date(Date.now() - days * 24 * 3600 * 1000).toISOString()} + +`); + } + return ` + +FetchMore Show +fetch-more test feed +${items.join("\n")} +`; +} + +beforeAll(async () => { + // COUNT mode keeps the union deterministic: the fetch-more cap grows + // by one count per press and every feed's window deepens in lock-step. + const app = useAppStore(); + await app.whenReady(); + app.updatePreferences({ + episodeCacheMode: "count", + episodeCacheCount: 6, + }); + + server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (!url.pathname.endsWith(".xml")) { + return new Response("not found", { status: 404 }); + } + return new Response(feedXml(url.origin), { + headers: { "Content-Type": "application/rss+xml" }, + }); + }, + }); + feedUrl = `http://127.0.0.1:${server.port}/fetchmore.xml`; + const store = useFeedStore(); + const feed = await store.addFeed(makePodcast(feedUrl), "test-source"); + feedId = feed!.id; +}); + +afterAll(async () => { + const store = useFeedStore(); + store.removeFeed(feedId); + // Restore the shared app store's cache preferences: the singleton leaks + // across every test file in this process, and the count-mode override + // would change other suites' refresh-window math. + const app = useAppStore(); + app.updatePreferences({ episodeCacheMode: "date", episodeCacheCount: 25 }); + server?.stop(true); + rmSync(configHome, { recursive: true, force: true }); +}); + +/** Render until the page shows the marker text, then return the frame. */ +async function renderUntil( + setup: Awaited>, + cond: (lines: string[]) => boolean, + what: string, +): Promise { + let lines: string[] = []; + for (let i = 0; i < 60; i++) { + await setup.renderOnce(); + lines = frameLines(setup.captureSpans() as unknown as Frame); + if (cond(lines)) return lines; + await new Promise((r) => setTimeout(r, 50)); + } + throw new Error( + `timed out waiting for: ${what}\nlast frame:\n${lines.join("\n")}`, + ); +} + +/** Capture the provider-tree's nav store: contexts are only readable + * inside the tree, so a probe component stashes it for the test body. */ +let navProbe: ReturnType | null = null; +function NavProbe() { + navProbe = useNavigation(); + return null; +} + +async function mountPage() { + return testRender( + () => ( + + + + + + + ), + { width: 100, height: 30, useThread: false }, + ); +} + +test("spinner paints during a warm-cache fetch-more, and the focused episode stays put", async () => { + const store = useFeedStore(); + navProbe = null; + const setup = await mountPage(); + // ThemeProvider gates children on async init (capabilities/palette) — + // render until the probe has actually mounted. + for (let i = 0; i < 60 && !navProbe; i++) { + await setup.renderOnce(); + if (!navProbe) await new Promise((r) => setTimeout(r, 50)); + } + const nav = navProbe!; + + // ── settle: 6 visible episodes from the count-mode subscribe window. ─ + const epCount = store.getAllEpisodesChronological().length; + const lastEpTitle = + store.getAllEpisodesChronological()[epCount - 1].episode.title; + if (!store.hasMoreAcrossAll()) throw new Error("feed has no more to load"); + + // ── Contract 2 setup: focus the LAST EPISODE (one above the [Fetch + // More] row) so the press splices revealed episodes above/below it + // while the cursor claims that row. ───────────────────────────────── + const rowCount0 = epCount + 1; + nav.gotoIndex(epCount - 1, rowCount0); + await renderUntil( + setup, + (ls) => ls.some((l) => l.includes(lastEpTitle)), + "episode list mounted with focused episode", + ); + + // ── Contract 1 (separate mount below) is the spinner; here fire the + // press WITHOUT awaiting and immediately settle it. ──────────────── + const loading = store.loadMoreAllFeeds(); + // Repeatedly render while the pulse is live or held — the frames the + // user would see during the press. + for (let i = 0; i < 8; i++) { + await setup.renderOnce(); + await new Promise((r) => setTimeout(r, 20)); + } + await loading; + await setup.renderOnce(); + + // ── Contract 2: the cursor stays on the SAME EPISODE after the press. ─ + // The press deepened the union (all revealed episodes splice in above + // the [Fetch More] row but BELOW-or-ABOVE the focused row's old index); + // the focused row's INDEX may shift — the cursor must still resolve to + // the same episode id through the store. + const focusedIdx = nav.depthFocus(0); + const allChrono = store.getAllEpisodesChronological(); + expect(allChrono.length).toBeGreaterThan(epCount); + const focusedEp = allChrono[focusedIdx]?.episode; + expect(focusedEp?.title).toBe(lastEpTitle); + + setup.renderer.destroy(); +}); + +test("the fetch-more spinner paints during a warm-cache press", async () => { + const store = useFeedStore(); + navProbe = null; + const setup = await mountPage(); + for (let i = 0; i < 60 && !navProbe; i++) { + await setup.renderOnce(); + if (!navProbe) await new Promise((r) => setTimeout(r, 50)); + } + const nav = navProbe!; + + // Exhausted already (the other tests' presses)? Then re-subscribe a + // fresh feed so this test still exercises the spinner contract. + if (!store.hasMoreAcrossAll()) { + setup.renderer.destroy(); + throw new Error("expected remaining fetch-more material"); + } + + // ── focus the button row: the preview pane then renders + // FetchMorePreview, whose "Loading…" line is visible regardless of + // the episode list's scroll position. ─────────────────────────────── + const epCount = store.getAllEpisodesChronological().length; + nav.gotoIndex(epCount, epCount + 1); + await renderUntil( + setup, + (ls) => ls.some((l) => l.includes("[Fetch More]")), + "button row focused and on screen", + ); + + // ── press (warm cache) WITHOUT awaiting; poll the rendered frames for + // the held loading state. The raw isLoadingMore pulse is shorter than + // one renderer frame — useHeldFlag's >=250ms window is what makes it + // visible. ────────────────────────────────────────────────────────── + const loading = store.loadMoreAllFeeds(); + let sawLoading = false; + for (let i = 0; i < 12; i++) { + const lines = frameLines(setup.captureSpans() as unknown as Frame); + if ( + lines.some((l) => l.includes("Loading the next batch")) || + lines.some((l) => SPINNER_RE.test(l)) + ) { + sawLoading = true; + break; + } + await setup.renderOnce(); + await new Promise((r) => setTimeout(r, 20)); + } + await loading; + expect(sawLoading).toBe(true); + + setup.renderer.destroy(); +}); + +test("fetch-more button focus survives its own load (cursor rides the moving row)", async () => { + const store = useFeedStore(); + navProbe = null; + const setup = await mountPage(); + for (let i = 0; i < 60 && !navProbe; i++) { + await setup.renderOnce(); + if (!navProbe) await new Promise((r) => setTimeout(r, 50)); + } + const nav = navProbe!; + + // ── settle: focus the button row itself (scrolls it on screen) ─────── + const epCount = store.getAllEpisodesChronological().length; + if (!store.hasMoreAcrossAll()) { + // Exhausted by the previous test's presses — nothing to pin here. + setup.renderer.destroy(); + return; + } + nav.gotoIndex(epCount, epCount + 1); + await renderUntil( + setup, + (ls) => ls.some((l) => l.includes("[Fetch More]")), + "button row focused and on screen", + ); + expect(nav.depthFocus(0)).toBe(epCount); + + // ── press and settle the load (awaits the full held window) ────────── + const press = store.loadMoreAllFeeds(); + for (let i = 0; i < 3; i++) { + await setup.renderOnce(); + await new Promise((r) => setTimeout(r, 30)); + } + await press; + await setup.renderOnce(); + + const epCountAfter = store.getAllEpisodesChronological().length; + expect(epCountAfter).toBeGreaterThan(epCount); + // Contract: the cursor must STILL be on the [Fetch More] row, which + // moved from index epCount to epCountAfter as revealed episodes spliced + // in above it. + expect(nav.depthFocus(0)).toBe(epCountAfter); + + setup.renderer.destroy(); +});