From 4ef9ab7e59f60d6fb94a4a4d36cd7286ce9568f6 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Thu, 13 Aug 2026 17:46:30 -0400 Subject: [PATCH] perf(ui): render only a bounded window around the focused row --- src/pages/Feed/FeedPage.tsx | 42 ++++++- src/pages/MyShows/MyShowsPage.tsx | 69 +++++++++-- tests/show-row-wrap.test.tsx | 189 ++++++++++++++++++++++++++++++ 3 files changed, 286 insertions(+), 14 deletions(-) create mode 100644 tests/show-row-wrap.test.tsx diff --git a/src/pages/Feed/FeedPage.tsx b/src/pages/Feed/FeedPage.tsx index c0b1c43..3687281 100644 --- a/src/pages/Feed/FeedPage.tsx +++ b/src/pages/Feed/FeedPage.tsx @@ -107,6 +107,30 @@ function FeedPage() { focusedOnMore() ? undefined : episodes()[focusedEpIdx()]; const curLen = () => rowCount(); + // ── 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 + // bounded slice around the focus (real indexes preserved) — the scrollbox + // still keeps the focused row in view. Spacers above/below the window + // restore the full content height so the scrollbar tracks the real list. + // Each EpisodeRow is 3 lines tall (title, subtitle, date). + const LIST_WINDOW = 30; + const ROW_HEIGHT = 3; + const listWindow = createMemo<[number, number]>(() => { + const len = episodes().length; + // Focusing the Fetch More button keeps the window anchored at the + // last episode — no jump when the focus crosses onto the button. + const f = focusedOnMore() ? len - 1 : focusedEpIdx(); + return [ + Math.max(0, f - LIST_WINDOW), + Math.min(len, f + LIST_WINDOW + 1), + ]; + }); + const visibleEpisodes = createMemo(() => { + const [start, end] = listWindow(); + return episodes().slice(start, end); + }); + const ensureFocus = () => { if (rowCount() > 0 && focus() >= rowCount()) nav.setDepthFocus(rowCount() - 1, 0); @@ -245,17 +269,22 @@ function FeedPage() { } > - + } > - + {/* Spacers keep the scrollbox content at the FULL list height so + the scrollbar reflects the real list, not the render window. */} + 0}> + + + {(item, index) => ( item.feed.customName || item.feed.podcast.title} - index={index} + index={() => listWindow()[0] + index()} focused={focusedEpIdx} active={isActive} selected={() => nav.isSelected(item.episode.id)} @@ -264,11 +293,14 @@ function FeedPage() { marker={marker} onMouseDown={() => { nav.setActivePane(DEPTH_CENTER_PANE); - nav.setDepthFocus(index(), 0); + nav.setDepthFocus(listWindow()[0] + index(), 0); }} /> )} + 0}> + + episodes().length} @@ -286,7 +318,7 @@ function FeedPage() { - + diff --git a/src/pages/MyShows/MyShowsPage.tsx b/src/pages/MyShows/MyShowsPage.tsx index c75fe53..2e2a24c 100644 --- a/src/pages/MyShows/MyShowsPage.tsx +++ b/src/pages/MyShows/MyShowsPage.tsx @@ -89,13 +89,23 @@ function ShowRow(props: { backgroundColor={bg()} onMouseDown={props.onMouseDown} > - {isFocused() ? props.marker() : " "} - {props.title} - + + {isFocused() ? props.marker() : " "} + + {/* Long titles truncate with middle-ellipsis instead of wrapping — + a wrapped title grows the row to 2+ lines and shifts every row + below (see EpisodeList for the same guard). The episode-count + and watchlist cells are flexShrink=0 so they never shrink or + wrap; the flexible title takes the remaining width. */} + + {props.title} + + ({props.feed.episodes.length}) focusedOnMore() ? undefined : episodes()[focusedEpIdx()]; + // ── Render window ──────────────────────────────────────────────────────── + // The drilled show's list grows deep after repeated fetch-more presses; + // rendering every row per frame froze the UI. Render only a bounded slice + // around the focus (real indexes preserved) — the scrollbox still keeps + // the focused row in view. Spacers above/below the window restore the + // full content height so the scrollbar tracks the real list. + // Each episode row is 2 lines tall (title, date) — no subtitle here. + const LIST_WINDOW = 30; + const ROW_HEIGHT = 2; + const listWindow = createMemo<[number, number]>(() => { + const len = episodes().length; + // Focusing the Fetch More button keeps the window anchored at the + // last episode — no jump when the focus crosses onto the button. + const f = focusedOnMore() ? len - 1 : focusedEpIdx(); + return [ + Math.max(0, f - LIST_WINDOW), + Math.min(len, f + LIST_WINDOW + 1), + ]; + }); + const visibleEpisodes = createMemo(() => { + const [start, end] = listWindow(); + return episodes().slice(start, end); + }); + const curLen = () => (depth() === 0 ? depth0Count() : rowCount()); const ensureFocus = () => { @@ -537,9 +571,17 @@ export function MyShowsPage() { paddingRight={1} backgroundColor={focused() ? theme.border : undefined} > - {focused() ? marker() : " "} - {showTitle(feed)} - ({feed.episodes.length}) + + {focused() ? marker() : " "} + + {/* 20%-wide parent pane truncates hard — same + middle-ellipsis guard as the depth-0 rows. */} + + {showTitle(feed)} + + + ({feed.episodes.length}) + ); }} @@ -622,11 +664,17 @@ export function MyShowsPage() { } > - + {/* Spacers keep the scrollbox content at the FULL list + height so the scrollbar reflects the real list, not the + render window. */} + 0}> + + + {(ep, index) => ( listWindow()[0] + index()} focused={focusedEpIdx} active={isActive} selected={() => nav.isSelected(ep.id)} @@ -635,11 +683,14 @@ export function MyShowsPage() { marker={marker} onMouseDown={() => { nav.setActivePane(DEPTH_CENTER_PANE); - nav.setDepthFocus(index(), 1); + nav.setDepthFocus(listWindow()[0] + index(), 1); }} /> )} + 0}> + + episodes().length} diff --git a/tests/show-row-wrap.test.tsx b/tests/show-row-wrap.test.tsx new file mode 100644 index 0000000..a1b1b1e --- /dev/null +++ b/tests/show-row-wrap.test.tsx @@ -0,0 +1,189 @@ +/** + * Show-row height regression — My Shows depth-0 rows must stay exactly one + * line tall: marker + show title + episode count (+ watchlist dot). The + * flexible title carries `wrapMode="none"` + `truncate` (middle-ellipsis: + * head and tail of the title stay visible) and the fixed-width cells carry + * `flexShrink={0}`, so Yoga can never shrink them and wrap the row — a + * wrapped row grows to 2+ lines and the episode count + watchlist dot shift + * below the title while scrolling (the original bug). Same guard for the + * 20%-wide parent-pane shows list at depth ≥1. + * + * Rendered at 70 columns so the 35-col current pane / 14-col parent pane are + * narrow enough to force truncation on the long title; at the default + * 100-col/50-col pane the same rows show everything in full. + */ + +import { describe, test, expect, afterAll } from "bun:test"; +import type { JSX } from "solid-js"; +import { testRender } from "@opentui/solid"; +import { ThemeProvider } from "../src/context/ThemeContext"; +import { PaneRow } from "../src/components/PaneRow"; + +type Frame = { cols: number; lines: { spans: { text: string }[] }[] }; + +function frameText(spans: Frame): string[] { + return spans.lines.map((l) => l.spans.map((s) => s.text).join("")); +} + +const LONG_TITLE = + "Out of Whiskey and Reaching for the Rotgut (Members Only #338)"; + +// The exact depth-0 row shape MyShowsPage renders: marker + title + count + +// watchlist dot. Static text, no store hooks — pure layout probe. +const ShowRowFixed = () => ( + + ❯ + + {LONG_TITLE} + + (123) + ● + +); + +// Pre-fix shape: no wrapMode/truncate/flexShrink props — the long title +// wraps at the shrunken width and pushes the count + dot onto wrapped lines. +const ShowRowNaive = () => ( + + ❯ + {LONG_TITLE} + (123) + ● + +); + +// The depth-1 parent-pane shows-list row (marker + title + count). +const ParentRowFixed = () => ( + + ❯ + + {LONG_TITLE} + + (123) + +); + +const ParentRowNaive = () => ( + + ❯ + {LONG_TITLE} + (123) + +); + +async function renderRow( + row: () => JSX.Element, + pane: "current" | "parent", + width = 70, +): Promise<{ lines: string[]; destroy: () => Promise }> { + const setup = await testRender( + () => ( + + + + ), + { width, height: 10, useThread: false }, + ); + // ThemeProvider mounts children only once the theme resolves; poll for + // the header row so the captured frame is a mounted PaneRow. + let lines: string[] | null = null; + for (let i = 0; i < 40 && !lines; i++) { + await setup.renderOnce(); + const frame = setup.captureSpans() as unknown as Frame; + const ls = frameText(frame); + if (ls.some((l) => l.includes("List"))) lines = ls; + else await new Promise((r) => setTimeout(r, 100)); + } + if (!lines) throw new Error("PaneRow did not render before timeout"); + return { + lines, + destroy: async () => { + setup.renderer.destroy(); + }, + }; +} + +const cleanups: (() => void | Promise)[] = []; +afterAll(async () => { + for (const c of cleanups) { + try { + await c(); + } catch { + // renderer already torn down — ignore + } + } +}); + +describe("show row height in the current pane (70-wide → 35-col pane)", () => { + test("long title stays on one line — middle-ellipsis keeps head AND tail, count + dot stay aligned", async () => { + const { lines, destroy } = await renderRow(ShowRowFixed, "current"); + cleanups.push(destroy); + + // Middle-ellipsis: the title head and its tail both survive, on a + // single line (end-truncation would drop the tail). + expect(lines.filter((l) => l.includes("Out of Wh"))).toHaveLength(1); + expect(lines.filter((l) => l.includes("#338)"))).toHaveLength(1); + // The count and watchlist dot sit on that same line — nothing wrapped. + const aligned = lines.filter( + (l) => + l.includes("Out of Wh") && + l.includes("#338)") && + l.includes("(123)") && + l.includes("●"), + ); + expect(aligned).toHaveLength(1); + // Row occupies exactly 1 content line below the header. + const content = lines.filter( + (l) => l.includes("Out of Wh") || l.includes("(123)"), + ); + expect(content).toHaveLength(1); + }); + + test("naive row (pre-fix props) wraps the title — the regression the test guards", async () => { + const { lines, destroy } = await renderRow(ShowRowNaive, "current"); + cleanups.push(destroy); + + // The title's wrapped fragments span 3 frame lines instead of 1 — + // every row below shifts while scrolling. + const titleFragments = lines.filter( + (l) => + l.includes("Out of Whiskey") || + l.includes("Rotgut") || + l.includes("#338)"), + ); + expect(titleFragments).toHaveLength(3); + }); +}); + +describe("show row in the parent pane (70-wide → 14-col pane)", () => { + test("long title stays on one line with count aligned", async () => { + const { lines, destroy } = await renderRow(ParentRowFixed, "parent"); + cleanups.push(destroy); + + // The 14-col slot truncates the title to a head stub (too narrow for + // head + tail), but the row stays one line and the count pins to it. + expect(lines.filter((l) => l.includes("O..."))).toHaveLength(1); + expect( + lines.filter((l) => l.includes("O...") && l.includes("(123)")), + ).toHaveLength(1); + }); + + test("naive parent row wraps the title — the regression the test guards", async () => { + const { lines, destroy } = await renderRow(ParentRowNaive, "parent"); + cleanups.push(destroy); + + const titleFragments = lines.filter( + (l) => + l.includes("Out of") || + l.includes("Whiskey") || + l.includes("Rotgut") || + l.includes("#3"), + ); + expect(titleFragments.length).toBeGreaterThan(1); + }); +});