fix(ui): review round 2 — reveal floor, failed next page, count states, row Play

- DataTable: the reveal floor holds only in the view it was made for (Channels
  never clears its follow, so every later sort mounted rows up to it)
- Feed + Plex browse: a failed next page stops the read-ahead and shows a row
  that retries, instead of re-requesting it once a round-trip
- the YouTube Load more click uses nextPageOnClick; a contract pins NEXT_PAGE
  to the read-aheads and the click sites to nextPageOnClick
- ItemCount: a failed count says so; a stale one says updating to a screen reader
- the narrow-picture Play lift applies only where a hover row is there to clear
- PlayerModal: one description ladder for the popover and the panel, both tested
- plexImage contract reads every img src, any attribute order
- Tabs comment names who persists the sub-tab
This commit is contained in:
2026-10-03 02:25:15 +02:00
parent c4c442bc5b
commit b40440b825
17 changed files with 360 additions and 63 deletions
@@ -124,4 +124,32 @@ describe("DataTable — a long page mounts in steps", () => {
draw({ key: "r90", seq: 1 });
expect(host.querySelector("tbody")?.textContent).toContain("r 90");
});
// E31 review 2: the reveal floor held for the page's lifetime. Channels never clears its `follow`, so
// after one priority pick every later sort or search mounted every row up to that channel at once.
it("lets a revealed row go once the reader moves to another view", () => {
host = document.createElement("div");
document.body.appendChild(host);
root = createRoot(host);
const rows = many(200);
const draw = (reveal: { key: string; seq: number } | null, narrowedBy: string) =>
flushSync(() =>
root!.render(
<DataTable
rows={rows}
columns={plain}
rowKey={(x) => x.id}
pageSize={0}
reveal={reveal}
narrowedBy={narrowedBy}
/>,
),
);
draw(null, "");
const follow = { key: "r150", seq: 1 };
draw(follow, "");
expect(host.querySelector("tbody")?.textContent).toContain("r 150");
draw(follow, "s"); // a search: a new view, the same (never cleared) reveal
expect(bodyRows()).toBe(FIRST_ROWS);
});
});
+12 -3
View File
@@ -267,19 +267,28 @@ export default function DataTable<T>({
// Adjusted DURING render against the last seq seen (the house's key-change reset), so the frame
// that re-sorted the row already shows its new page. Seeded with the mount's seq: arriving with a
// stale reveal must not move a page nobody asked to move.
const viewOf = (p: number) =>
`${p}:${size}:${JSON.stringify(sort)}:${JSON.stringify(filters)}:${narrowedBy}`;
const [revealSeen, setRevealSeen] = useState(reveal?.seq);
// The view the reveal was made for: the floor below holds only there. A caller keeps its reveal
// after the focus (Channels never clears `follow`), and a floor that outlived its view mounted every
// row up to that channel on each later sort or search (E31 review 2).
const [revealView, setRevealView] = useState<string | null>(null);
if (reveal && reveal.seq !== revealSeen) {
setRevealSeen(reveal.seq);
const at = sorted.findIndex((row) => rowKey(row) === reveal.key);
if (at >= 0 && !allRows) setPage(Math.floor(at / size));
const target = at >= 0 && !allRows ? Math.floor(at / size) : safePage;
if (target !== safePage) setPage(target);
setRevealView(viewOf(target));
}
const view = viewOf(safePage);
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 — 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,
view,
reveal && revealView === view ? paged.findIndex((row) => rowKey(row) === reveal.key) : -1,
);
function toggleSort(key: string) {
@@ -70,4 +70,17 @@ describe("ItemCount", () => {
expect(screen.getByTestId("item-count").getAttribute("aria-busy")).toBe("true");
expect(screen.getByText("5 items").className).toContain("opacity-60");
});
// E31 review 2: the dim and aria-busy were the only stale cue, and a screen reader voices neither on
// plain text; a failed count looked like a first load.
it("tells a screen reader a stale count is updating", () => {
render(<ItemCount count={5} stale />);
expect(text()).toBe("5 itemsupdating");
});
it("says a count that failed, rather than looking like a first load", () => {
render(<ItemCount failed />);
expect(text()).toBe("—Couldn't count the items");
expect(screen.getByTestId("item-count").getAttribute("title")).toBe("Couldn't count the items");
});
});
+23 -8
View File
@@ -14,15 +14,26 @@ const OVERFLOW_AT = 1_000_000;
// ("999,999 items") and the content is centred within it. A count of a million or more collapses to
// "1M+", with the exact figure in the title tooltip. Deliberately generic ("item(s)", dynamic plural)
// so any surface can reuse it for whatever it counts. `stale`: the count is the previous filter's,
// kept while the new one loads (keepPreviousData) — dimmed and aria-busy, as AdminLogs dims its lines,
// so a broad search's slow count ("a": ~3 s) never reads as the new result.
export default function ItemCount({ count, stale = false }: { count?: number; stale?: boolean }) {
// kept while the new one loads (keepPreviousData) — dimmed, and "updating" for a screen reader (which
// does not voice aria-busy on plain text), so a broad search's slow count ("a": ~3 s) never reads as
// the new result. `failed`: the count could not be read — a dash that says so, not the icon-only box
// of a first load (E31 review 2).
export default function ItemCount({
count,
stale = false,
failed = false,
}: {
count?: number;
stale?: boolean;
failed?: boolean;
}) {
const { t, i18n } = useTranslation();
const lang = i18n.language;
// `count` is undefined only on a first, un-cached load — keep the fixed box (nothing shifts), just
// show the icon until the number arrives.
const label =
count == null
// `count` undefined and not `failed`: a first, un-cached load — keep the fixed box (nothing shifts),
// just show the icon until the number arrives.
const label = failed
? "—"
: count == null
? ""
: t("common.itemCount", {
count,
@@ -31,7 +42,9 @@ export default function ItemCount({ count, stale = false }: { count?: number; st
return (
<span
data-testid="item-count"
title={count == null ? undefined : formatNumber(count, lang)}
title={
failed ? t("common.countFailed") : count == null ? undefined : formatNumber(count, lang)
}
aria-busy={stale || undefined}
className="inline-flex h-7 min-w-[9rem] shrink-0 items-center justify-center gap-1.5 rounded-full bg-fg/8 px-3 text-sm whitespace-nowrap text-muted"
>
@@ -39,6 +52,8 @@ export default function ItemCount({ count, stale = false }: { count?: number; st
<span className={clsx("tabular-nums transition-opacity", stale && "opacity-60")}>
{label}
</span>
{failed && <span className="sr-only">{t("common.countFailed")}</span>}
{stale && !failed && <span className="sr-only">{t("common.countUpdating")}</span>}
</span>
);
}
+5 -3
View File
@@ -8,9 +8,11 @@ import CountBadge from "./CountBadge";
// the Settings-style side rail (full-width, left-aligned, icon + label) — from the `sm` breakpoint up.
// Below it a side rail leaves a phone too little width for the content beside it (measured at 390 px:
// the rail clipped its own labels and Settings → Account pushed its buttons over their text), so there
// it is a horizontal row that scrolls. Callers persist the active tab with lib/storage's
// useTabPersistedState (per browser tab, E31 S4b): a reload keeps the user where they were, and a
// fresh tab opens on the first sub-tab rather than wherever the last visit ended.
// it is a horizontal row that scrolls. A page whose sub-tab is worth keeping persists it with
// lib/storage's useTabPersistedState (per browser tab, E31 S4b: Admin Users, Config, Settings, Stats):
// a reload keeps the user where they were, and a fresh tab opens on the first sub-tab. Not every
// caller: the channel manager's view is account-wide (ChannelsProvider), and Downloads, a channel page
// and the player settings start on their first tab each time.
//
// An ARIA tab list (E28 review F3), not a row of toggle buttons: `tablist` / `tab` / `aria-selected`,
// ONE Tab stop (the selected tab; roving tabindex), the arrows move and select (either axis, since the
+12 -7
View File
@@ -19,23 +19,28 @@ export const MEDIA_BADGE =
"pointer-events-none absolute rounded-sm bg-black/80 px-1.5 py-0.5 text-2xs font-medium text-white tabular-nums";
/** The picture box as a size container (`pic`), so what sits on the picture can answer its width.
* Every picture box carries it: `mediaPictureBox` and the video card's own box. */
export const MEDIA_PICTURE_CONTAINER = "@container/pic";
* Every picture box carries it: `mediaPictureBox` and the video card's own box. A box that holds a
* hover row (MEDIA_BAR) also sets `--bar-room`, the band the row needs: the narrow-picture rule
* below lifts the Play by it, and a box with no row (the feed's row and tile views, a Plex card
* without actions) lifts it by nothing (E31 review 2: it sat off-centre there). */
export const MEDIA_PICTURE_CONTAINER = "@container/pic has-[.media-bar]:[--bar-room:2.5rem]";
/** The hover layer over the picture: a dim that carries the Play (mouse only, like the hover row).
* On a narrow picture the Play centres in the band ABOVE the hover row instead of on the whole
* picture: the picture's height follows the card's width, but the Play and the row grow with the
* text size, so at 1.3 a 242px thumbnail put the 63px Play 13px into the 37px row (E31 S5,
* measured). The cut is in rem, so it moves with the text size too: a picture under 14rem wide. */
* measured). The cut is in rem, so it moves with the text size too: a picture under 14rem wide.
* Only where a row is there to clear (`--bar-room`, MEDIA_PICTURE_CONTAINER). */
export const MEDIA_HOVER_LAYER =
"reveal-hover absolute inset-0 flex items-center justify-center gap-2 bg-black/45 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100 @max-[14rem]/pic:pb-10";
"reveal-hover absolute inset-0 flex items-center justify-center gap-2 bg-black/45 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100 @max-[14rem]/pic:pb-(--bar-room)";
/** The layer that holds a started title's Continue + Restart pair over the picture: a sibling of the
* picture's button, never inside it (C5). Narrow, it leaves the hover row its band as the Play does. */
export const MEDIA_RESUME_LAYER = `${HOVER_REVEAL} pointer-events-none absolute inset-0 z-base flex items-center justify-center gap-2 *:pointer-events-auto @max-[14rem]/pic:pb-10`;
export const MEDIA_RESUME_LAYER = `${HOVER_REVEAL} pointer-events-none absolute inset-0 z-base flex items-center justify-center gap-2 *:pointer-events-auto @max-[14rem]/pic:pb-(--bar-room)`;
/** Where the card's hover row (watched, save, add, "…") docks: the picture's bottom-right. */
export const MEDIA_BAR = `${HOVER_REVEAL} absolute bottom-2.5 right-1.5 z-base`;
/** Where the card's hover row (watched, save, add, "…") docks: the picture's bottom-right.
* `media-bar` marks it for the picture box's `--bar-room` (MEDIA_PICTURE_CONTAINER). */
export const MEDIA_BAR = `${HOVER_REVEAL} media-bar absolute bottom-2.5 right-1.5 z-base`;
/** The title's type, one size per density. On a SHELF (any horizontal strip: Home, the Plex rails,
* a collection strip) it is one line, cut with an ellipsis, over one meta line — every card on the