diff --git a/scripts/ci/tests/board-accessible-name.test.mjs b/scripts/ci/tests/board-accessible-name.test.mjs new file mode 100644 index 0000000..3e8046a --- /dev/null +++ b/scripts/ci/tests/board-accessible-name.test.mjs @@ -0,0 +1,243 @@ +/** + * Behavioral regression test for issue #205: a board with content must not + * announce a generic name. + * + * `StaticBoardDisplay` draws its message as a wall of glyph tiles inside an + * `aria-hidden` container, so the `role="img"` label is the *only* thing a + * screen reader can read. Its default was the constant `"Board preview"`, so + * four different boards in a thumbnail grid all announced identically while a + * sighted user read four different messages. axe cannot catch this: a non-empty + * `aria-label` passes `image-alt` no matter what it says. + * + * A code-shape check would be the wrong instrument — what matters is the name + * the DOM actually exposes, for a real message, through whichever prop the + * consumer set. So this mounts the real components (React + react-dom in jsdom) + * and reads `aria-label` off the rendered board. jsdom rather than a browser + * because `release:test` runs in CI's `automation` job, which does `npm ci` but + * never `npx playwright install`; an accessible *name* is DOM state, not paint, + * so nothing here needs a compositor. Same harness shape as + * board-flap-cascade.test.mjs. + * + * What it pins down: + * 1. all three renderers put the message's text in their default name; + * 2. color markup never leaks into the name; + * 3. an explicit `previewLabel` still wins (BoardShowcase depends on it); + * 4. `messageLabel` can rebuild the wording and receives the plain text; + * 5. a board that draws no text falls back to a generic name rather than + * announcing a dangling "Board preview:"; + * 6. an empty board still announces `emptyLabel`. + */ + +import assert from "node:assert/strict"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { after, before, test } from "node:test"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { build } from "esbuild"; +import { JSDOM } from "jsdom"; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const boardDir = path.resolve(here, "../../../src/components/board"); + +const HARNESS = ` + import { createElement } from "react"; + import { createRoot } from "react-dom/client"; + import { BoardDisplay } from "./board-display"; + import { StaticBoardDisplay } from "./static-board-display"; + import { BoardTeaser } from "./board-teaser"; + + const COMPONENTS = { BoardDisplay, StaticBoardDisplay, BoardTeaser }; + + export function mount(container, name, props) { + const root = createRoot(container); + root.render(createElement(COMPONENTS[name], props)); + return () => root.unmount(); + } +`; + +const MESSAGE = "HELLO WORLD\n{red}WELCOME TO{/red}\nFIESTABOARD"; +/** What the tiles actually draw, in reading order. */ +const MESSAGE_TEXT = "HELLO WORLD WELCOME TO FIESTABOARD"; + +let tmp; +let bundleUrl; +let runCounter = 0; + +before(async () => { + tmp = await mkdtemp(path.join(tmpdir(), "board-accessible-name-")); + const outfile = path.join(tmp, "harness.mjs"); + await build({ + stdin: { contents: HARNESS, resolveDir: boardDir, sourcefile: "harness.js", loader: "js" }, + outfile, + bundle: true, + format: "esm", + platform: "browser", + jsx: "automatic", + define: { "process.env.NODE_ENV": '"development"' }, + logLevel: "silent", + }); + bundleUrl = pathToFileURL(outfile).href; +}); + +after(async () => { + if (tmp) await rm(tmp, { recursive: true, force: true }); +}); + +/** Install a jsdom window as the global environment (see board-flap-cascade). */ +function installDom() { + const dom = new JSDOM("
", { + pretendToBeVisual: true, + }); + const { window } = dom; + window.matchMedia = (query) => ({ + media: query, + matches: false, + onchange: null, + addEventListener() {}, + removeEventListener() {}, + addListener() {}, + removeListener() {}, + dispatchEvent: () => false, + }); + + const globals = [ + "window", + "document", + "navigator", + "HTMLElement", + "Element", + "Node", + "Event", + "MessageChannel", + "MessagePort", + "requestAnimationFrame", + "cancelAnimationFrame", + "getComputedStyle", + ]; + const saved = new Map(); + const put = (key, value) => { + Object.defineProperty(globalThis, key, { value, configurable: true, writable: true }); + }; + for (const key of globals) { + saved.set(key, Reflect.get(globalThis, key)); + put(key, key === "window" ? window : window[key]); + } + globalThis.IS_REACT_ACT_ENVIRONMENT = false; + + return { + window, + restore() { + for (const [key, value] of saved) put(key, value); + dom.window.close(); + }, + }; +} + +/** Mount one board and return the accessible name its `role="img"` exposes. */ +async function accessibleName(component, props) { + const dom = installDom(); + try { + // A fresh module instance per run: reduced-motion.ts resolves its + // MediaQueryList at module scope and would otherwise carry the previous + // run's window into this one. + const harness = await import(`${bundleUrl}?run=${++runCounter}`); + const container = dom.window.document.getElementById("root"); + const unmount = harness.mount(container, component, props); + + let img = null; + for (let i = 0; i < 100 && !img; i++) { + img = container.querySelector('[role="img"]'); + if (!img) await new Promise((resolve) => setTimeout(resolve, 5)); + } + if (!img) throw new Error(`${component} never rendered a role="img"`); + + const label = img.getAttribute("aria-label"); + unmount(); + // React schedules through MessageChannel, so unmount's work is still + // queued here. Let it drain before `restore()` closes the window, or a + // stray callback lands on a dead document after the test has ended. + await new Promise((resolve) => setTimeout(resolve, 10)); + return label; + } finally { + dom.restore(); + } +} + +// The heart of #205: the renderers disagreed about whether a board's *content* +// belongs in its name. StaticBoardDisplay said no, and it is the one used for +// lists and thumbnails, where telling boards apart is the whole job. +test("every renderer puts the board's own text in its default accessible name", async () => { + for (const [component, props] of [ + ["BoardDisplay", { message: MESSAGE }], + ["StaticBoardDisplay", { message: MESSAGE }], + ["BoardTeaser", { teaser: "HELLO WORLD", tiles: 15 }], + ]) { + const name = await accessibleName(component, props); + const expected = component === "BoardTeaser" ? "HELLO WORLD" : MESSAGE_TEXT; + assert.ok( + name.includes(expected), + `${component} announced ${JSON.stringify(name)}, which does not contain the text it draws ` + + `(${JSON.stringify(expected)}). A screen-reader user cannot tell two boards apart by a name that ` + + "omits their content (issue #205).", + ); + } +}); + +test("color markup never reaches the accessible name", async () => { + for (const [component, props] of [ + ["BoardDisplay", { message: MESSAGE }], + ["StaticBoardDisplay", { message: MESSAGE }], + ]) { + const name = await accessibleName(component, props); + assert.doesNotMatch(name, /[{}]|\bred\b/i, `${component} leaked color markup into its name: ${name}`); + } +}); + +test("an explicit previewLabel still wins over the derived name", async () => { + // BoardShowcase passes a hand-written label for a curated preview; #205 must + // not take that away from it. + const name = await accessibleName("StaticBoardDisplay", { + message: MESSAGE, + previewLabel: "Air Quality & Fog displayed on a split-flap board", + }); + assert.equal(name, "Air Quality & Fog displayed on a split-flap board"); +}); + +test("messageLabel rebuilds the wording and receives the board's plain text", async () => { + const name = await accessibleName("StaticBoardDisplay", { + message: MESSAGE, + messageLabel: (msg) => `Thumbnail — ${msg}`, + }); + assert.equal(name, `Thumbnail — ${MESSAGE_TEXT}`); +}); + +test("the name says what the tiles draw on a Note, where ° is a heart", async () => { + // messageToGrid substitutes code 62 for a heart on Note hardware. A name + // derived without that substitution announces "degree" for a tile showing ♥, + // which is a text alternative describing something else (WCAG 1.1.1). + for (const component of ["BoardDisplay", "StaticBoardDisplay"]) { + const name = await accessibleName(component, { message: "LOVE °", deviceType: "note" }); + assert.match(name, /LOVE ♥/, `${component} announced ${JSON.stringify(name)} for a board drawing "LOVE ♥"`); + } + // …and only on Note: everywhere else the tile really is a degree sign. + const flagship = await accessibleName("StaticBoardDisplay", { message: "52 °F" }); + assert.match(flagship, /52 °F/, `a flagship board draws °, so its name must keep it; got ${flagship}`); +}); + +test("a board that draws no text falls back to a generic name", async () => { + // Colour-only boards render tiles but no glyphs; "Board preview: " with + // nothing after it would be worse than the generic name it replaced. + const name = await accessibleName("StaticBoardDisplay", { message: "{63}{64}{65}" }); + assert.equal(name, "Board preview"); +}); + +test("an empty board still announces its emptyLabel", async () => { + assert.equal(await accessibleName("StaticBoardDisplay", { message: null }), "Empty board display"); + assert.equal(await accessibleName("StaticBoardDisplay", { message: "" }), "Empty board display"); + assert.equal( + await accessibleName("StaticBoardDisplay", { message: null, emptyLabel: "No message set" }), + "No message set", + ); +}); diff --git a/src/components/board/board-display.tsx b/src/components/board/board-display.tsx index 8c70ed2..7e48f5d 100644 --- a/src/components/board/board-display.tsx +++ b/src/components/board/board-display.tsx @@ -32,6 +32,7 @@ import { getCharIndex, isColorTile, messageToGrid, + messageToText, tokensEqual, } from "../../lib/board-characters"; import { resolveColorCode } from "../../lib/board-colors"; @@ -1236,6 +1237,9 @@ export interface BoardDisplayProps { // Module-scope default so the aria-label memo below keeps a stable dependency. const defaultMessageLabel = (msg: string) => `Board display: ${msg}`; +/** Name for a board that renders no text at all — an all-color board. */ +const NO_TEXT_LABEL = "Board display"; + export const BoardDisplay = memo( function BoardDisplay({ message, @@ -1320,13 +1324,15 @@ export const BoardDisplay = memo( const boardText = useMemo(() => { if (isLoading) return loadingLabel; if (!message) return emptyLabel; - return messageLabel( - message - .replace(/\{[^}]*\}/g, "") - .replace(/\n/g, " ") - .trim(), - ); - }, [message, isLoading, loadingLabel, emptyLabel, messageLabel]); + // `messageToText` rather than a local regex (issue #205): it reads the + // message with the same parser the tiles do, so the name says what is + // actually on the board, and all three renderers now derive it one way. + const text = messageToText(message, deviceType); + // A board of nothing but color tiles draws no text; it is not empty, so + // it gets the generic name rather than `emptyLabel` or a dangling + // "Board display: " with nothing after it. + return text ? messageLabel(text) : NO_TEXT_LABEL; + }, [message, deviceType, isLoading, loadingLabel, emptyLabel, messageLabel]); return (
diff --git a/src/components/board/board-teaser.tsx b/src/components/board/board-teaser.tsx index d680925..22b788f 100644 --- a/src/components/board/board-teaser.tsx +++ b/src/components/board/board-teaser.tsx @@ -14,7 +14,7 @@ import { memo, useMemo } from "react"; -import { type BoardToken, parseLine } from "../../lib/board-characters"; +import { type BoardToken, messageToText, parseLine } from "../../lib/board-characters"; import { resolveColorCode } from "../../lib/board-colors"; import { gapClasses, radiusClasses, sizeClasses, textSizeClasses } from "../../lib/board-metrics"; import { charLeafBoxShadow, SEAM_CLASS, seamStyle } from "./board-surfaces"; @@ -50,14 +50,9 @@ export const BoardTeaser = memo(function BoardTeaser({ // Plain-text teaser for the accessible label: color markers stripped, // whitespace collapsed. Falls back to a generic label for color-only strips. - const label = useMemo(() => { - const text = parseLine(teaser) - .map((token) => (token.type === "char" ? token.value : " ")) - .join("") - .replace(/\s+/g, " ") - .trim(); - return text || "Board teaser"; - }, [teaser]); + // `messageToText` is the derivation all three renderers share (issue #205) — + // this used to be its own copy of the same few lines. + const label = useMemo(() => messageToText(teaser) || "Board teaser", [teaser]); // Tile metrics (sizeClasses/textSizeClasses/gapClasses) come from // ../../lib/board-metrics so a teaser strip matches a full board row rendered diff --git a/src/components/board/static-board-display.stories.tsx b/src/components/board/static-board-display.stories.tsx index 73a36bc..01792b4 100644 --- a/src/components/board/static-board-display.stories.tsx +++ b/src/components/board/static-board-display.stories.tsx @@ -46,7 +46,13 @@ const meta = { }, previewLabel: { control: "text", - description: "Accessible label when a message is shown", + description: + "Fixed accessible label for a shown message, overriding the derived one. Leave unset unless a hand-written description beats the board's own text — every board given the same string announces identically (issue #205).", + }, + messageLabel: { + control: false, + description: + "Builds the accessible label for a shown message, color markup already stripped. Defaults to `Board preview: ${message}`, the same contract as BoardDisplay's prop of this name.", }, emptyLabel: { control: "text", @@ -105,6 +111,12 @@ export const Empty: Story = { /** * Thumbnail grid — the use case this variant exists for: many boards at once. * + * It is also the case that made issue #205 concrete: with a constant default + * label these four boards all announced "Board preview, image" and a + * screen-reader user had no way to tell them apart. Each now announces its own + * message — "Board preview: PAGE ONE ALERTS", and so on. Inspect the four + * `role="img"` names in the a11y panel to see it. + * * Two 22-column thumbnails need ~774px, so on a phone the grid scrolls rather * than squeezing the boards: a `sm` board's 379px floor is not negotiable * (issue #192), and until `shrink-0` was added to the tiles a squeezed cell @@ -126,3 +138,26 @@ export const ThumbnailGrid = () => (
); + +// Browser-side half of the #205 guard: four boards, four different names. The +// behavioural detail (what each name says, which prop wins) is asserted in +// jsdom by scripts/ci/tests/board-accessible-name.test.mjs; this holds the +// property a user actually depends on in a real render — that the boards in a +// grid are distinguishable by name at all. +ThumbnailGrid.play = async ({ canvasElement }: { canvasElement: HTMLElement }) => { + const names = Array.from(canvasElement.querySelectorAll('[data-slot="static-board-display"]')).map( + (board) => board.getAttribute("aria-label") ?? "", + ); + + if (names.length !== 4) throw new Error(`expected four thumbnails, found ${names.length}`); + if (new Set(names).size !== names.length) { + throw new Error( + `the thumbnails announce duplicate names (${names.join(" / ")}) — a screen-reader user cannot tell them ` + + "apart (issue #205)", + ); + } + const missing = names.filter((name) => !/PAGE (ONE|TWO|THREE|FOUR)/.test(name)); + if (missing.length > 0) { + throw new Error(`these names do not carry their board's message: ${missing.join(" / ")} (issue #205)`); + } +}; diff --git a/src/components/board/static-board-display.tsx b/src/components/board/static-board-display.tsx index 8c40484..c1edac8 100644 --- a/src/components/board/static-board-display.tsx +++ b/src/components/board/static-board-display.tsx @@ -12,7 +12,7 @@ import { memo, useMemo } from "react"; -import { messageToGrid } from "../../lib/board-characters"; +import { messageToGrid, messageToText } from "../../lib/board-characters"; import { resolveColorCode } from "../../lib/board-colors"; import { type DeviceType, isNoteArray, NOTE_COLS, NOTE_ROWS, resolveDimensions } from "../../lib/board-dimensions"; import { gapClasses, paddingClasses, radiusClasses, sizeClasses, textSizeClasses } from "../../lib/board-metrics"; @@ -28,12 +28,27 @@ export interface StaticBoardDisplayProps { notesWide?: number; /** Notes tall (for note_array device; ignored otherwise). */ notesTall?: number; - /** Accessible label when a message is shown. */ + /** Fixed accessible label for a shown message. Overrides `messageLabel`, so + * pass it only when a hand-written description beats the board's own text + * (BoardShowcase's curated previews do). Note that it makes every board it + * is passed to announce identically — the default derives the name from the + * message instead. */ previewLabel?: string; + /** Builds the accessible label for a shown message (color markup already + * stripped). Same contract as BoardDisplay's prop of this name; defaults to + * `Board preview: ${message}`. */ + messageLabel?: (message: string) => string; /** Accessible label when the board has no message. */ emptyLabel?: string; } +// Module-scope so the memoized component sees a stable prop identity, and the +// name memo below a stable dependency. +const defaultMessageLabel = (msg: string) => `Board preview: ${msg}`; + +/** Name for a board that renders no text at all — an all-color board. */ +const NO_TEXT_LABEL = "Board preview"; + export const StaticBoardDisplay = memo(function StaticBoardDisplay({ message, size = "sm", @@ -42,7 +57,8 @@ export const StaticBoardDisplay = memo(function StaticBoardDisplay({ className = "", notesWide = 1, notesTall = 1, - previewLabel = "Board preview", + previewLabel, + messageLabel = defaultMessageLabel, emptyLabel = "Empty board display", }: StaticBoardDisplayProps) { const dims = resolveDimensions(deviceType, notesWide, notesTall); @@ -56,6 +72,20 @@ export const StaticBoardDisplay = memo(function StaticBoardDisplay({ [message, dims.rows, dims.cols, deviceType], ); + // The board's whole accessible name: the tiles below are aria-hidden, so + // whatever this says is all a screen reader ever gets (issue #205). It + // carries the message by default — a thumbnail grid of four boards that all + // announce "Board preview" is four boards a screen-reader user cannot tell + // apart, while a sighted user reads four different messages. + const label = useMemo(() => { + if (!message) return emptyLabel; + if (previewLabel !== undefined) return previewLabel; + // `deviceType` matters: on Note a `°` draws as a heart, and the name has to + // say what the tiles draw. + const text = messageToText(message, deviceType); + return text ? messageLabel(text) : NO_TEXT_LABEL; + }, [message, deviceType, previewLabel, messageLabel, emptyLabel]); + // Seam gap: additional left/top margin applied at Note physical boundaries const seamGap = size === "sm" ? "6px" : size === "md" ? "8px" : "10px"; @@ -87,7 +117,7 @@ export const StaticBoardDisplay = memo(function StaticBoardDisplay({
+ parseLine(line) + .map((token) => { + const drawn = applyDeviceSubstitution(token, isNote); + return drawn.type === "char" ? drawn.value : " "; + }) + .join(""), + ) + .join(" ") + .replace(/\s+/g, " ") + .trim(); +}