perf(ui): render only a bounded window around the focused row
This commit is contained in:
@@ -107,6 +107,30 @@ function FeedPage() {
|
|||||||
focusedOnMore() ? undefined : episodes()[focusedEpIdx()];
|
focusedOnMore() ? undefined : episodes()[focusedEpIdx()];
|
||||||
const curLen = () => rowCount();
|
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 = () => {
|
const ensureFocus = () => {
|
||||||
if (rowCount() > 0 && focus() >= rowCount())
|
if (rowCount() > 0 && focus() >= rowCount())
|
||||||
nav.setDepthFocus(rowCount() - 1, 0);
|
nav.setDepthFocus(rowCount() - 1, 0);
|
||||||
@@ -245,17 +269,22 @@ function FeedPage() {
|
|||||||
</text>
|
</text>
|
||||||
}
|
}
|
||||||
>
|
>
|
||||||
<LoadingIndicator label="Refreshing…" />
|
<LoadingIndicator />
|
||||||
</Show>
|
</Show>
|
||||||
</box>
|
</box>
|
||||||
}
|
}
|
||||||
>
|
>
|
||||||
<For each={episodes()}>
|
{/* Spacers keep the scrollbox content at the FULL list height so
|
||||||
|
the scrollbar reflects the real list, not the render window. */}
|
||||||
|
<Show when={listWindow()[0] > 0}>
|
||||||
|
<box height={listWindow()[0] * ROW_HEIGHT} />
|
||||||
|
</Show>
|
||||||
|
<For each={visibleEpisodes()}>
|
||||||
{(item, index) => (
|
{(item, index) => (
|
||||||
<EpisodeRow
|
<EpisodeRow
|
||||||
episode={item.episode}
|
episode={item.episode}
|
||||||
subtitle={() => item.feed.customName || item.feed.podcast.title}
|
subtitle={() => item.feed.customName || item.feed.podcast.title}
|
||||||
index={index}
|
index={() => listWindow()[0] + index()}
|
||||||
focused={focusedEpIdx}
|
focused={focusedEpIdx}
|
||||||
active={isActive}
|
active={isActive}
|
||||||
selected={() => nav.isSelected(item.episode.id)}
|
selected={() => nav.isSelected(item.episode.id)}
|
||||||
@@ -264,11 +293,14 @@ function FeedPage() {
|
|||||||
marker={marker}
|
marker={marker}
|
||||||
onMouseDown={() => {
|
onMouseDown={() => {
|
||||||
nav.setActivePane(DEPTH_CENTER_PANE);
|
nav.setActivePane(DEPTH_CENTER_PANE);
|
||||||
nav.setDepthFocus(index(), 0);
|
nav.setDepthFocus(listWindow()[0] + index(), 0);
|
||||||
}}
|
}}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
</For>
|
</For>
|
||||||
|
<Show when={episodes().length - listWindow()[1] > 0}>
|
||||||
|
<box height={(episodes().length - listWindow()[1]) * ROW_HEIGHT} />
|
||||||
|
</Show>
|
||||||
<Show when={showFetchMore()}>
|
<Show when={showFetchMore()}>
|
||||||
<FetchMoreRow
|
<FetchMoreRow
|
||||||
index={() => episodes().length}
|
index={() => episodes().length}
|
||||||
@@ -286,7 +318,7 @@ function FeedPage() {
|
|||||||
</Show>
|
</Show>
|
||||||
<Show when={feedStore.isLoadingFeeds()}>
|
<Show when={feedStore.isLoadingFeeds()}>
|
||||||
<box alignItems="center" paddingTop={1}>
|
<box alignItems="center" paddingTop={1}>
|
||||||
<LoadingIndicator label="Refreshing…" />
|
<LoadingIndicator />
|
||||||
</box>
|
</box>
|
||||||
</Show>
|
</Show>
|
||||||
</Show>
|
</Show>
|
||||||
|
|||||||
@@ -89,13 +89,23 @@ function ShowRow(props: {
|
|||||||
backgroundColor={bg()}
|
backgroundColor={bg()}
|
||||||
onMouseDown={props.onMouseDown}
|
onMouseDown={props.onMouseDown}
|
||||||
>
|
>
|
||||||
<text fg={fg()}>{isFocused() ? props.marker() : " "}</text>
|
<text flexShrink={0} fg={fg()}>
|
||||||
<text fg={fg()}>{props.title}</text>
|
{isFocused() ? props.marker() : " "}
|
||||||
<text fg={isFocused() ? theme.surface : muted()}>
|
</text>
|
||||||
|
{/* 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. */}
|
||||||
|
<text wrapMode="none" truncate fg={fg()}>
|
||||||
|
{props.title}
|
||||||
|
</text>
|
||||||
|
<text flexShrink={0} fg={isFocused() ? theme.surface : muted()}>
|
||||||
({props.feed.episodes.length})
|
({props.feed.episodes.length})
|
||||||
</text>
|
</text>
|
||||||
<Show when={props.wlScope()}>
|
<Show when={props.wlScope()}>
|
||||||
<text
|
<text
|
||||||
|
flexShrink={0}
|
||||||
fg={
|
fg={
|
||||||
isFocused()
|
isFocused()
|
||||||
? theme.surface
|
? theme.surface
|
||||||
@@ -312,6 +322,30 @@ export function MyShowsPage() {
|
|||||||
const focusedEpisode = () =>
|
const focusedEpisode = () =>
|
||||||
focusedOnMore() ? undefined : episodes()[focusedEpIdx()];
|
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 curLen = () => (depth() === 0 ? depth0Count() : rowCount());
|
||||||
|
|
||||||
const ensureFocus = () => {
|
const ensureFocus = () => {
|
||||||
@@ -537,9 +571,17 @@ export function MyShowsPage() {
|
|||||||
paddingRight={1}
|
paddingRight={1}
|
||||||
backgroundColor={focused() ? theme.border : undefined}
|
backgroundColor={focused() ? theme.border : undefined}
|
||||||
>
|
>
|
||||||
<text fg={fg()}>{focused() ? marker() : " "}</text>
|
<text flexShrink={0} fg={fg()}>
|
||||||
<text fg={fg()}>{showTitle(feed)}</text>
|
{focused() ? marker() : " "}
|
||||||
<text fg={muted()}>({feed.episodes.length})</text>
|
</text>
|
||||||
|
{/* 20%-wide parent pane truncates hard — same
|
||||||
|
middle-ellipsis guard as the depth-0 rows. */}
|
||||||
|
<text wrapMode="none" truncate fg={fg()}>
|
||||||
|
{showTitle(feed)}
|
||||||
|
</text>
|
||||||
|
<text flexShrink={0} fg={muted()}>
|
||||||
|
({feed.episodes.length})
|
||||||
|
</text>
|
||||||
</box>
|
</box>
|
||||||
);
|
);
|
||||||
}}
|
}}
|
||||||
@@ -622,11 +664,17 @@ export function MyShowsPage() {
|
|||||||
</box>
|
</box>
|
||||||
}
|
}
|
||||||
>
|
>
|
||||||
<For each={episodes()}>
|
{/* Spacers keep the scrollbox content at the FULL list
|
||||||
|
height so the scrollbar reflects the real list, not the
|
||||||
|
render window. */}
|
||||||
|
<Show when={listWindow()[0] > 0}>
|
||||||
|
<box height={listWindow()[0] * ROW_HEIGHT} />
|
||||||
|
</Show>
|
||||||
|
<For each={visibleEpisodes()}>
|
||||||
{(ep, index) => (
|
{(ep, index) => (
|
||||||
<EpisodeRow
|
<EpisodeRow
|
||||||
episode={ep}
|
episode={ep}
|
||||||
index={index}
|
index={() => listWindow()[0] + index()}
|
||||||
focused={focusedEpIdx}
|
focused={focusedEpIdx}
|
||||||
active={isActive}
|
active={isActive}
|
||||||
selected={() => nav.isSelected(ep.id)}
|
selected={() => nav.isSelected(ep.id)}
|
||||||
@@ -635,11 +683,14 @@ export function MyShowsPage() {
|
|||||||
marker={marker}
|
marker={marker}
|
||||||
onMouseDown={() => {
|
onMouseDown={() => {
|
||||||
nav.setActivePane(DEPTH_CENTER_PANE);
|
nav.setActivePane(DEPTH_CENTER_PANE);
|
||||||
nav.setDepthFocus(index(), 1);
|
nav.setDepthFocus(listWindow()[0] + index(), 1);
|
||||||
}}
|
}}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
</For>
|
</For>
|
||||||
|
<Show when={episodes().length - listWindow()[1] > 0}>
|
||||||
|
<box height={(episodes().length - listWindow()[1]) * ROW_HEIGHT} />
|
||||||
|
</Show>
|
||||||
<Show when={showFetchMore()}>
|
<Show when={showFetchMore()}>
|
||||||
<FetchMoreRow
|
<FetchMoreRow
|
||||||
index={() => episodes().length}
|
index={() => episodes().length}
|
||||||
|
|||||||
189
tests/show-row-wrap.test.tsx
Normal file
189
tests/show-row-wrap.test.tsx
Normal file
@@ -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 = () => (
|
||||||
|
<box flexDirection="row" gap={1} paddingRight={1}>
|
||||||
|
<text flexShrink={0}>❯</text>
|
||||||
|
<text wrapMode="none" truncate>
|
||||||
|
{LONG_TITLE}
|
||||||
|
</text>
|
||||||
|
<text flexShrink={0}>(123)</text>
|
||||||
|
<text flexShrink={0}>●</text>
|
||||||
|
</box>
|
||||||
|
);
|
||||||
|
|
||||||
|
// 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 = () => (
|
||||||
|
<box flexDirection="row" gap={1} paddingRight={1}>
|
||||||
|
<text>❯</text>
|
||||||
|
<text>{LONG_TITLE}</text>
|
||||||
|
<text>(123)</text>
|
||||||
|
<text>●</text>
|
||||||
|
</box>
|
||||||
|
);
|
||||||
|
|
||||||
|
// The depth-1 parent-pane shows-list row (marker + title + count).
|
||||||
|
const ParentRowFixed = () => (
|
||||||
|
<box flexDirection="row" gap={1} paddingRight={1}>
|
||||||
|
<text flexShrink={0}>❯</text>
|
||||||
|
<text wrapMode="none" truncate>
|
||||||
|
{LONG_TITLE}
|
||||||
|
</text>
|
||||||
|
<text flexShrink={0}>(123)</text>
|
||||||
|
</box>
|
||||||
|
);
|
||||||
|
|
||||||
|
const ParentRowNaive = () => (
|
||||||
|
<box flexDirection="row" gap={1} paddingRight={1}>
|
||||||
|
<text>❯</text>
|
||||||
|
<text>{LONG_TITLE}</text>
|
||||||
|
<text>(123)</text>
|
||||||
|
</box>
|
||||||
|
);
|
||||||
|
|
||||||
|
async function renderRow(
|
||||||
|
row: () => JSX.Element,
|
||||||
|
pane: "current" | "parent",
|
||||||
|
width = 70,
|
||||||
|
): Promise<{ lines: string[]; destroy: () => Promise<void> }> {
|
||||||
|
const setup = await testRender(
|
||||||
|
() => (
|
||||||
|
<ThemeProvider mode="dark">
|
||||||
|
<PaneRow
|
||||||
|
parent={pane === "parent" ? row : null}
|
||||||
|
current={pane === "current" ? row : null}
|
||||||
|
preview={null}
|
||||||
|
currentLabel="List"
|
||||||
|
/>
|
||||||
|
</ThemeProvider>
|
||||||
|
),
|
||||||
|
{ 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<void>)[] = [];
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user