From 5d33b4815bd818192c44a5789afc15925cb66655 Mon Sep 17 00:00:00 2001 From: Ivan Cheung Date: Wed, 29 Jul 2026 13:01:06 +0000 Subject: [PATCH 1/2] =?UTF-8?q?viewer:=20Pages=20stepper=20UX=20=E2=80=94?= =?UTF-8?q?=20labeled=20Prev/Next,=20dots/progress,=20keyboard=20nav,=20ne?= =?UTF-8?q?sted=20hierarchy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Pages pager is used heavily as a step-through debugger (nested Pages inside Pages). Replace the numbered Pagination row with a stepper built for that: labeled Prev/Next with focusable end-disabled states, the current title shown prominently with an 'N of M' count, clickable title-tooltipped dots (<=16 pages) or a progress bar plus a jump-by-title select (>16), arrow/Home/End keys while a bar has focus (stopPropagation keeps nested pagers independent), a subtle page-in transition honoring prefers-reduced-motion, and depth-context sizing + inset so inner pagers read as subordinate. Authoring contract unchanged: {pages:[{title,node}...]} renders one lazy page at a time. Evidence: reviews/pages-stepper/ before/after Playwright shots. 6 unit tests + 5 keyboard e2e checks added. --- packages/viewer/e2e/keyboard.e2e.mjs | 32 +++ .../viewer/src/client/renderers/pages.tsx | 200 ++++++++++++++++-- packages/viewer/test/pages.test.tsx | 74 +++++++ reviews/pages-stepper/after-dark.png | Bin 0 -> 25275 bytes reviews/pages-stepper/after-longtitle.png | Bin 0 -> 50389 bytes reviews/pages-stepper/after-narrow.png | Bin 0 -> 24755 bytes reviews/pages-stepper/after-nested.png | Bin 0 -> 61451 bytes reviews/pages-stepper/after-p1.png | Bin 0 -> 44173 bytes reviews/pages-stepper/after-scale20-mid.png | Bin 0 -> 44024 bytes reviews/pages-stepper/after-scale20.png | Bin 0 -> 43802 bytes .../pages-stepper/before-live-debugger.png | Bin 0 -> 285492 bytes reviews/pages-stepper/before-longtitle.png | Bin 0 -> 52988 bytes reviews/pages-stepper/before-narrow.png | Bin 0 -> 25225 bytes reviews/pages-stepper/before-nested.png | Bin 0 -> 64647 bytes reviews/pages-stepper/before-scale20.png | Bin 0 -> 45064 bytes 15 files changed, 285 insertions(+), 21 deletions(-) create mode 100644 packages/viewer/test/pages.test.tsx create mode 100644 reviews/pages-stepper/after-dark.png create mode 100644 reviews/pages-stepper/after-longtitle.png create mode 100644 reviews/pages-stepper/after-narrow.png create mode 100644 reviews/pages-stepper/after-nested.png create mode 100644 reviews/pages-stepper/after-p1.png create mode 100644 reviews/pages-stepper/after-scale20-mid.png create mode 100644 reviews/pages-stepper/after-scale20.png create mode 100644 reviews/pages-stepper/before-live-debugger.png create mode 100644 reviews/pages-stepper/before-longtitle.png create mode 100644 reviews/pages-stepper/before-narrow.png create mode 100644 reviews/pages-stepper/before-nested.png create mode 100644 reviews/pages-stepper/before-scale20.png diff --git a/packages/viewer/e2e/keyboard.e2e.mjs b/packages/viewer/e2e/keyboard.e2e.mjs index 2014c7c8..33904d48 100644 --- a/packages/viewer/e2e/keyboard.e2e.mjs +++ b/packages/viewer/e2e/keyboard.e2e.mjs @@ -139,6 +139,38 @@ try { skip("modal focus-trap (no clear-all in this environment)"); } + // --- Pages stepper: arrow-key nav + nested isolation ----------------------------------------- + await push("pager", "component", JSON.stringify({ + type: "Pages", + props: { pages: [ + { title: "One", node: { type: "Pages", props: { pages: [ + { title: "i1", node: { type: "Text", children: ["inner 1"] } }, + { title: "i2", node: { type: "Text", children: ["inner 2"] } }, + ] } } }, + { title: "Two", node: { type: "Text", children: ["outer two"] } }, + { title: "Three", node: { type: "Text", children: ["outer three"] } }, + ] }, + })); + await p.evaluate(() => { location.hash = "#kbd%2Fpager"; }); + await p.waitForSelector(".tc-pages .tc-pages", { timeout: 8000 }); + const bodyHas = (s) => p.evaluate((t) => (document.body.textContent || "").includes(t), s); + + await p.evaluate(() => document.querySelector('.tc-pages > .tc-pages-bar [aria-label="next page"]')?.focus()); + await p.keyboard.press("ArrowRight"); + ok("ArrowRight steps the focused pager forward", await bodyHas("outer two")); + await p.keyboard.press("ArrowLeft"); + ok("ArrowLeft steps it back", await bodyHas("inner 1")); + + await p.evaluate(() => document.querySelector('.tc-pages .tc-pages [aria-label="next page"]')?.focus()); + await p.keyboard.press("ArrowRight"); + ok("nested pager arrows step the inner pager only", (await bodyHas("inner 2")) && !(await bodyHas("outer two"))); + + await p.evaluate(() => document.querySelector('.tc-pages > .tc-pages-bar [aria-label="next page"]')?.focus()); + await p.keyboard.press("End"); + ok("End jumps the pager to the last page", await bodyHas("outer three")); + await p.keyboard.press("Home"); + ok("Home jumps back to the first page", await bodyHas("inner 1")); + // --- Regression: Escape with nothing open is harmless --------------------------------------- await p.keyboard.press("Escape"); ok("Escape with no overlay does not throw", true); diff --git a/packages/viewer/src/client/renderers/pages.tsx b/packages/viewer/src/client/renderers/pages.tsx index b86c4c88..99a9e70e 100644 --- a/packages/viewer/src/client/renderers/pages.tsx +++ b/packages/viewer/src/client/renderers/pages.tsx @@ -1,10 +1,11 @@ -import { Group, Pagination, Text } from "@mantine/core"; -import { useState } from "react"; +import { Button, Text } from "@mantine/core"; +import { createContext, useContext, useState } from "react"; +import { injectStyle } from "./inject-style.js"; import { resolve } from "./component-resolver.js"; /** * `Pages` — top-level board pagination: one page rendered at a - * time with a Pagination control above and below the content. + * time with a stepper control above and below the content. * Off-page trees are never resolved, so their images are never * fetched — a 400-node board pays only for the page in view. * @@ -16,38 +17,195 @@ import { resolve } from "./component-resolver.js"; * `pages` is a plain-data prop (the MapDetail.items convention), * so the resolver leaves the trees raw and this component * resolves only the active one via `resolve()`. + * + * The control is built for step-through use (algorithm debuggers + * arrow through stages, often with a Pages nested inside a page): + * labeled Prev/Next with end-disabled states, the current title + * shown prominently, arrow-key navigation while the bar has + * focus, and a progress affordance that scales — clickable dots + * up to 16 pages, a slim bar plus a jump-by-title select beyond. + * Nested pagers read a depth context and render smaller + inset + * so outer/inner controls are visually distinct. */ export interface PageEntry { title?: string; node?: unknown; } +/** Dot-per-page stops scaling past this; switch to progress bar + jump select. */ +const DOTS_MAX = 16; + +const PagesDepth = createContext(0); + +const CSS = ` +.tc-pages-bar { display: flex; align-items: center; gap: 8px; margin: 10px 0; } +.tc-pages-mid { flex: 1 1 0; min-width: 0; display: flex; align-items: baseline; gap: 8px; } +.tc-pages-dots { display: flex; align-items: center; gap: 5px; flex-wrap: wrap; margin: 4px 0 8px; } +.tc-pages-dot { width: 8px; height: 8px; border-radius: 4px; border: none; padding: 0; cursor: pointer; + background: var(--mantine-color-default-border, #adb5bd); + transition: width 120ms ease, background-color 120ms ease; } +.tc-pages-dot:hover { background: var(--mantine-color-dimmed, #868e96); } +.tc-pages-dot[aria-current="page"] { width: 22px; background: var(--mantine-primary-color-filled, #228be6); } +.tc-pages-dot:focus-visible { outline: 2px solid var(--mantine-primary-color-filled, #228be6); outline-offset: 2px; } +.tc-pages-track { height: 4px; border-radius: 2px; margin: 4px 0 8px; + background: var(--mantine-color-default-border, #dee2e6); overflow: hidden; } +.tc-pages-fill { height: 100%; border-radius: 2px; background: var(--mantine-primary-color-filled, #228be6); + transition: width 160ms ease; } +.tc-pages-jump { flex: 0 1 auto; min-width: 0; font: inherit; font-weight: 600; font-size: 13px; + color: inherit; background: transparent; cursor: pointer; + border: 1px solid var(--mantine-color-default-border, #ced4da); border-radius: 6px; padding: 2px 6px; } +.tc-pages-jump:focus-visible { outline: 2px solid var(--mantine-primary-color-filled, #228be6); outline-offset: 1px; } +.tc-pages-page { animation: tc-pages-in 160ms ease-out; } +@keyframes tc-pages-in { from { opacity: 0; transform: translateY(4px); } to { opacity: 1; transform: none; } } +/* nested pagers are subordinate: inset with a rule, tighter bars, smaller dots */ +.tc-pages .tc-pages { padding-left: 10px; border-left: 2px solid var(--mantine-color-default-border, #dee2e6); } +.tc-pages .tc-pages .tc-pages-bar { margin: 6px 0; } +.tc-pages .tc-pages .tc-pages-dot { width: 6px; height: 6px; } +.tc-pages .tc-pages .tc-pages-dot[aria-current="page"] { width: 18px; } +@media (prefers-reduced-motion: reduce) { + .tc-pages-page { animation: none; } + .tc-pages-dot, .tc-pages-fill { transition: none; } +} +`; + export function Pages(props: { pages?: PageEntry[] }) { const pages = Array.isArray(props.pages) ? props.pages : []; + const depth = useContext(PagesDepth); const [page, setPage] = useState(1); if (pages.length === 0) return null; - const idx = Math.min(Math.max(page, 1), pages.length) - 1; + injectStyle("tc-pages", CSS); + + const total = pages.length; + const idx = Math.min(Math.max(page, 1), total) - 1; // clamp: live patches may shrink `pages` const entry = pages[idx] ?? {}; - const title = String(entry.title ?? `Page ${idx + 1}`); - const control = (where: string) => ( - - - - {idx + 1}/{pages.length} · {title} - - + const titleOf = (i: number) => String(pages[i]?.title ?? `Page ${i + 1}`); + const title = titleOf(idx); + const go = (n: number) => setPage(Math.min(Math.max(n, 1), total)); + + const nested = depth > 0; + const btnSize = nested ? "compact-xs" : "compact-sm"; + const txtSize = nested ? "xs" : "sm"; + + const content = ( + +
+ {resolve(entry.node)} +
+ ); + + if (total === 1) { + return ( +
+ + {title} + + {content} +
+ ); + } + + // Arrow-step while focus is anywhere in a bar. stopPropagation keeps a nested + // pager's keys from also driving the outer one (and board-level handlers). + const onKeyDown = (e: React.KeyboardEvent) => { + if (e.ctrlKey || e.metaKey || e.altKey) return; + if ((e.target as HTMLElement).tagName === "SELECT") return; // jump select owns its keys + const next = + e.key === "ArrowLeft" ? idx : e.key === "ArrowRight" ? idx + 2 : e.key === "Home" ? 1 : e.key === "End" ? total : 0; + if (next === 0) return; + e.preventDefault(); + e.stopPropagation(); + go(next); + }; + + const bar = (where: "top" | "bottom") => ( +
+ {/* data-disabled (not disabled) keeps the button focusable at the ends, + so arrow-key stepping survives reaching page 1 / page N */} + + {/* polite live region (top bar only) so a step change is announced once */} +
+ {total > DOTS_MAX ? ( + + ) : ( + <> + + {title} + + + {idx + 1} of {total} + + + )} +
+ +
+ ); + + const rail = + total > DOTS_MAX ? ( +