docs(plans): a11y defect bundle — 2-task plan (#62)
This commit is contained in:
@@ -0,0 +1,256 @@
|
|||||||
|
# Accessibility Defect Bundle — Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Fix five remaining a11y defects — label-id collision, unnamed drawer/breadcrumb, untranslated combobox strings (Task 1); invalid table-row semantics, missing pill focus ring, unannounced table load/error states (Task 2).
|
||||||
|
|
||||||
|
**Architecture:** Task 1 is a labelling/i18n cluster across four small components plus 5 new i18n keys. Task 2 reworks the objects-table data rows to use a real `<Link>` with `aria-current`, restores `focusRing` on the filter pills, and adds `aria-busy` + a live `<caption>` + `role="alert"` for load/error announcement.
|
||||||
|
|
||||||
|
**Tech Stack:** React 19 + TS + pnpm, React Router 7, Base UI, react-i18next, Vitest 4 (jsdom) + RTL + MSW.
|
||||||
|
|
||||||
|
**Conventions:** pnpm; **no `any`/`eslint-disable`/`@ts-ignore`**; no codename; en/sv parity; app source double-quote+semicolon; `components/ui/*` untouched; token classes only (`focusRing` is token-based). Run a single test pass.
|
||||||
|
|
||||||
|
**Spec:** `docs/superpowers/specs/2026-06-08-a11y-defect-bundle-design.md`
|
||||||
|
|
||||||
|
**Key facts:**
|
||||||
|
- Existing i18n keys: `common.noMatches` ("No matches"), `common.loading` ("Loading"), `nav.objects`, `objects.loadError` ("Could not load objects"), `actions.closeDetail`. NEW keys to add (en/sv): `common.clear`, `common.open`, `nav.breadcrumb`, `objects.detailTitle`, `objects.tableLabel`.
|
||||||
|
- `lib/focus-ring.ts` exports `focusRing` (a class string). Imported elsewhere as `import { focusRing } from "../lib/focus-ring";`.
|
||||||
|
- `components/label-editor.test.tsx`, `objects/options-combobox.test.tsx`, `shell/breadcrumb.test.tsx` exist. `objects/object-detail-drawer.test.tsx` does NOT.
|
||||||
|
- `objects/objects-table.test.tsx`: imports `renderApp`, `objectsPage` (from `../test/fixtures`), `ObjectsTable`, `ObjectDetail`, `i18n`, `Routes`/`Route`. Its `tree()` mounts `ObjectsTable` at `/objects` and `ObjectDetail` at `/objects/:id` as siblings. Fixtures: `objectsPage.items[0]` = `{ object_number: "LM-0042", object_name: "Amphora", … }`, `[1]` = `"LM-0043"`/`"Bronze fibula"`. The "clicking a row deep-links…" test clicks the **name** ("Amphora"), which stays plain text — it survives unchanged.
|
||||||
|
- `combobox.tsx` wrapper: `ComboboxClear`/`ComboboxTrigger` pass `aria-label` through to Base UI; `ComboboxEmpty` renders children. Do NOT modify `components/ui/combobox.tsx`.
|
||||||
|
- `object-detail-drawer.tsx`: `DrawerContent` spreads `...props` onto the Base UI `Drawer.Popup`, so `aria-label` passes through.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# Task 1: Labelling + i18n cluster (label-editor, combobox, breadcrumb, drawer)
|
||||||
|
|
||||||
|
**Files:** Modify `web/src/i18n/en.json`, `web/src/i18n/sv.json`, `web/src/components/label-editor.tsx`, `web/src/objects/options-combobox.tsx`, `web/src/shell/breadcrumb.tsx`, `web/src/objects/object-detail-drawer.tsx`; tests `web/src/components/label-editor.test.tsx`, `web/src/objects/options-combobox.test.tsx`, `web/src/shell/breadcrumb.test.tsx`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add the 5 i18n keys (both locales, parity).** In `web/src/i18n/en.json`, add to the relevant blocks: under `common` → `"clear": "Clear", "open": "Open"`; under `nav` → `"breadcrumb": "Breadcrumb"`; under `objects` → `"detailTitle": "Object detail", "tableLabel": "Objects"`. In `web/src/i18n/sv.json`, the same keys: `common.clear` = `"Rensa"`, `common.open` = `"Öppna"`, `nav.breadcrumb` = `"Brödsmulor"`, `objects.detailTitle` = `"Objektdetalj"`, `objects.tableLabel` = `"Objekt"`. (Valid JSON; mind commas. Place each new key beside its existing siblings in the same nested object.)
|
||||||
|
|
||||||
|
- [ ] **Step 2: `label-editor.tsx` — `useId()`.** Add `useId` to the React import (`import { useId } from "react";`). Inside the component, add `const inputId = useId();` and change the two lines to:
|
||||||
|
```tsx
|
||||||
|
<Label htmlFor={inputId}>{t("labels.label")}</Label>
|
||||||
|
<Input id={inputId} value={current} onChange={(e) => set(e.target.value)} />
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: `options-combobox.tsx` — translate.** Add `import { useTranslation } from "react-i18next";` and `const { t } = useTranslation();` at the top of the component body. Change:
|
||||||
|
```tsx
|
||||||
|
<ComboboxClear aria-label={t("common.clear")} />
|
||||||
|
<ComboboxTrigger aria-label={t("common.open")} />
|
||||||
|
```
|
||||||
|
and
|
||||||
|
```tsx
|
||||||
|
<ComboboxEmpty>{t("common.noMatches")}</ComboboxEmpty>
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: `breadcrumb.tsx` — translate the nav label.** Add `import { useTranslation } from "react-i18next";` and `const { t } = useTranslation();` inside `Breadcrumb` (before the `if (trail.length === 0)` guard). Change `<nav aria-label="Breadcrumb" …>` to `<nav aria-label={t("nav.breadcrumb")} …>`.
|
||||||
|
|
||||||
|
- [ ] **Step 5: `object-detail-drawer.tsx` — name the dialog.** Change `<DrawerContent>` to `<DrawerContent aria-label={t("objects.detailTitle")}>` (the `t` from `useTranslation` is already in scope in this file).
|
||||||
|
|
||||||
|
- [ ] **Step 6: Tests.**
|
||||||
|
- **`label-editor.test.tsx`** — append (reuse the file's existing render harness / providers, e.g. `renderApp` or whatever wraps `useConfig`; read the top of the file first):
|
||||||
|
```tsx
|
||||||
|
test("each LabelEditor instance gets a unique input id", () => {
|
||||||
|
renderApp(
|
||||||
|
<>
|
||||||
|
<LabelEditor value={[]} onChange={() => {}} />
|
||||||
|
<LabelEditor value={[]} onChange={() => {}} />
|
||||||
|
</>,
|
||||||
|
);
|
||||||
|
const inputs = screen.getAllByLabelText(/label/i);
|
||||||
|
expect(inputs).toHaveLength(2);
|
||||||
|
expect(inputs[0].id).not.toBe("");
|
||||||
|
expect(inputs[0].id).not.toBe(inputs[1].id);
|
||||||
|
});
|
||||||
|
```
|
||||||
|
(If `LabelEditor` needs config context that `renderApp` doesn't provide, mirror the wrapper the existing tests in this file use. Keep existing tests green.)
|
||||||
|
- **`options-combobox.test.tsx`** — append (mirror the file's existing render of `OptionsCombobox`):
|
||||||
|
```tsx
|
||||||
|
test("the clear and open controls and empty text are translated", async () => {
|
||||||
|
// render OptionsCombobox with empty options so the empty state is reachable,
|
||||||
|
// using the same harness the other tests in this file use.
|
||||||
|
// …render…
|
||||||
|
expect(screen.getByRole("button", { name: /open/i })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
```
|
||||||
|
(Adapt to the existing test's setup. The key assertion: the open/clear controls have accessible names from `t()`. If the existing test already opens the popup, also assert `screen.getByText("No matches")`.)
|
||||||
|
- **`breadcrumb.test.tsx`** — append:
|
||||||
|
```tsx
|
||||||
|
test("the breadcrumb nav has a translated accessible name", () => {
|
||||||
|
// render Breadcrumb with a non-empty trail using the file's existing harness
|
||||||
|
// (it needs a BreadcrumbContext provider with a trail).
|
||||||
|
expect(screen.getByRole("navigation", { name: /breadcrumb/i })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
```
|
||||||
|
(Mirror the existing breadcrumb test's provider setup; if the file already renders a trail, just add the `getByRole("navigation", { name })` assertion.)
|
||||||
|
|
||||||
|
- [ ] **Step 7: Verify (vitest ONCE), typecheck, lint:**
|
||||||
|
```bash
|
||||||
|
cd web && pnpm vitest run src/components/label-editor.test.tsx src/objects/options-combobox.test.tsx src/shell/breadcrumb.test.tsx src/i18n && pnpm typecheck && pnpm lint
|
||||||
|
```
|
||||||
|
Expected: green (incl. i18n parity covering the 5 new keys). Keep all existing tests in those files green.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
```bash
|
||||||
|
cd /Users/olsson/Laboratory/biggus-dickus
|
||||||
|
git add web/src/i18n/en.json web/src/i18n/sv.json web/src/components/label-editor.tsx web/src/objects/options-combobox.tsx web/src/shell/breadcrumb.tsx web/src/objects/object-detail-drawer.tsx web/src/components/label-editor.test.tsx web/src/objects/options-combobox.test.tsx web/src/shell/breadcrumb.test.tsx
|
||||||
|
git commit -m "fix(web): a11y labelling — useId, named drawer/breadcrumb, translated combobox (#62)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# Task 2: objects-table — real-link rows, pill focus ring, announced load/error
|
||||||
|
|
||||||
|
**Files:** Modify `web/src/objects/objects-table.tsx`, `web/src/objects/objects-table.test.tsx`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Import `focusRing`.** Add `import { focusRing } from "../lib/focus-ring";` to `objects-table.tsx`. (`Link` is already imported from `react-router-dom`.)
|
||||||
|
|
||||||
|
- [ ] **Step 2: Filter pills — add the focus ring.** In the `toolbar`, change the pill `className` to:
|
||||||
|
```tsx
|
||||||
|
className={`${focusRing} rounded-md px-2 py-1 ${active ? "bg-primary text-primary-foreground" : "border"}`}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Rows — real link + `aria-current`, plain `<tr>`.** Replace the data-row `<tr>` (the `role="link"` one) with:
|
||||||
|
```tsx
|
||||||
|
<tr
|
||||||
|
key={object.id}
|
||||||
|
onClick={() => navigate(`/objects/${object.id}?${params}`)}
|
||||||
|
className={`cursor-pointer border-b text-sm ${
|
||||||
|
selected ? "bg-primary/10" : "hover:bg-muted"
|
||||||
|
}`}
|
||||||
|
>
|
||||||
|
<td className="px-3 py-2 text-muted-foreground">
|
||||||
|
<Link
|
||||||
|
to={`/objects/${object.id}?${params}`}
|
||||||
|
aria-current={selected ? "page" : undefined}
|
||||||
|
onClick={(event) => event.stopPropagation()}
|
||||||
|
className={`${focusRing} rounded-sm hover:underline`}
|
||||||
|
>
|
||||||
|
{object.object_number}
|
||||||
|
</Link>
|
||||||
|
</td>
|
||||||
|
<td className="px-3 py-2 font-medium">{object.object_name}</td>
|
||||||
|
<td className="px-3 py-2">
|
||||||
|
<VisibilityBadge visibility={object.visibility} />
|
||||||
|
</td>
|
||||||
|
<td className="px-3 py-2 text-muted-foreground">{object.current_location ?? "—"}</td>
|
||||||
|
<td className="px-3 py-2 text-right tabular-nums">{object.number_of_objects}</td>
|
||||||
|
<td className="px-3 py-2 text-muted-foreground">{formatUpdated(object.updated_at)}</td>
|
||||||
|
</tr>
|
||||||
|
```
|
||||||
|
(Drops `role="link"`, `tabIndex={0}`, `aria-selected`, and `onKeyDown` from the `<tr>`; the object-number cell now holds the `<Link>`. Every other cell is unchanged.)
|
||||||
|
|
||||||
|
- [ ] **Step 4: Error cell — `role="alert"`.** In the `isError` branch, change the error `<td>` to:
|
||||||
|
```tsx
|
||||||
|
<td colSpan={6} role="alert" className="px-3 py-6 text-center text-sm text-destructive">
|
||||||
|
{t("objects.loadError")}
|
||||||
|
</td>
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Table — `aria-busy` + live caption.** Change the `<table>` element and add the caption as its first child:
|
||||||
|
```tsx
|
||||||
|
<table className="w-full border-collapse" aria-busy={isLoading || undefined}>
|
||||||
|
<caption className="sr-only" aria-live="polite">
|
||||||
|
{isLoading ? t("common.loading") : t("objects.tableLabel")}
|
||||||
|
</caption>
|
||||||
|
{columns}
|
||||||
|
{body}
|
||||||
|
</table>
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Tests — extend `objects-table.test.tsx`.** Add a nested-route helper (so `ObjectsTable` is mounted WITH a `:id` param, mirroring the real `ObjectsPage` nesting) and the new assertions. Add near the existing `tree()`:
|
||||||
|
```tsx
|
||||||
|
function nestedTree() {
|
||||||
|
return (
|
||||||
|
<Routes>
|
||||||
|
<Route
|
||||||
|
path="/objects"
|
||||||
|
element={
|
||||||
|
<>
|
||||||
|
<ObjectsTable />
|
||||||
|
<Outlet />
|
||||||
|
</>
|
||||||
|
}
|
||||||
|
>
|
||||||
|
<Route path=":id" element={<div>detail pane</div>} />
|
||||||
|
</Route>
|
||||||
|
</Routes>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
(Add `Outlet` to the `react-router-dom` import.) Then add these tests:
|
||||||
|
```tsx
|
||||||
|
test("the object number cell is a real link", async () => {
|
||||||
|
renderApp(tree(), { route: "/objects" });
|
||||||
|
expect(await screen.findByRole("link", { name: "LM-0042" })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("the selected row's link is marked aria-current=page", async () => {
|
||||||
|
// objectsPage.items[0] has object_number "LM-0042"; read its id from the fixture.
|
||||||
|
const first = objectsPage.items[0];
|
||||||
|
renderApp(nestedTree(), { route: `/objects/${first.id}` });
|
||||||
|
const link = await screen.findByRole("link", { name: first.object_number });
|
||||||
|
expect(link).toHaveAttribute("aria-current", "page");
|
||||||
|
// a different row's link is not current
|
||||||
|
const other = await screen.findByRole("link", { name: objectsPage.items[1].object_number });
|
||||||
|
expect(other).not.toHaveAttribute("aria-current");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("the table is marked aria-busy while loading", async () => {
|
||||||
|
server.use(
|
||||||
|
http.get("/api/admin/objects", async () => {
|
||||||
|
await delay(50);
|
||||||
|
return HttpResponse.json(objectsPage);
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
renderApp(tree(), { route: "/objects" });
|
||||||
|
expect(screen.getByRole("table")).toHaveAttribute("aria-busy", "true");
|
||||||
|
await screen.findByRole("link", { name: "LM-0042" });
|
||||||
|
expect(screen.getByRole("table")).not.toHaveAttribute("aria-busy");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a failed objects fetch is announced via role=alert", async () => {
|
||||||
|
server.use(http.get("/api/admin/objects", () => new HttpResponse(null, { status: 500 })));
|
||||||
|
renderApp(tree(), { route: "/objects" });
|
||||||
|
expect(await screen.findByRole("alert")).toHaveTextContent(/could not load/i);
|
||||||
|
});
|
||||||
|
```
|
||||||
|
(Add `delay` to the `msw` import: `import { delay, http, HttpResponse } from "msw";`. The existing "clicking a row deep-links…" test clicks "Amphora" — the name cell, still plain text + whole-row `onClick` — so it stays green. If `objectsPage.items[0]` doesn't carry an `id`, read `src/test/fixtures.ts` to use the correct id field.)
|
||||||
|
|
||||||
|
- [ ] **Step 7: FULL FRONTEND GATE (run tests EXACTLY ONCE):**
|
||||||
|
```bash
|
||||||
|
cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size && pnpm check:colors
|
||||||
|
```
|
||||||
|
All green. Report test totals, largest chunk (gz), and the `check:colors` line.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Codename + status:**
|
||||||
|
```bash
|
||||||
|
cd /Users/olsson/Laboratory/biggus-dickus
|
||||||
|
git grep -in 'biggus\|dickus' -- web/src; echo "codename-exit=$?"
|
||||||
|
git status --short
|
||||||
|
```
|
||||||
|
Expected: no matches (`codename-exit=1`).
|
||||||
|
|
||||||
|
- [ ] **Step 9: Manual smoke (recommended).** `pnpm dev`: tab into the objects table — the visibility pills show a focus ring; Tab reaches each row's object-number link (Enter opens; Cmd/middle-click opens a new tab); the open object's row link is `aria-current`; a slow/failed load is announced.
|
||||||
|
|
||||||
|
- [ ] **Step 10: Commit**
|
||||||
|
```bash
|
||||||
|
cd /Users/olsson/Laboratory/biggus-dickus
|
||||||
|
git add web/src/objects/objects-table.tsx web/src/objects/objects-table.test.tsx
|
||||||
|
git commit -m "fix(web): objects-table a11y — real-link rows, pill focus ring, announced load/error (#62)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-Review (completed)
|
||||||
|
|
||||||
|
**Spec coverage:** AC1 LabelEditor useId (T1 S2); AC2 row real-link + aria-current + plain tr + pill focusRing (T2 S2-S3); AC3 aria-busy + live caption + role=alert (T2 S4-S5); AC4 drawer + breadcrumb names + combobox translation (T1 S3-S5); AC5 gate/parity/codename (T2 S7-S8, T1 S1/S7). ✓
|
||||||
|
|
||||||
|
**Placeholder scan:** every code step shows full code; tests give concrete role/name assertions; the two "mirror the existing harness" notes (label-editor/options-combobox/breadcrumb tests) point at named existing files to copy from, not vague TODOs; the fixture-id note names the exact field to read. No TBD. ✓
|
||||||
|
|
||||||
|
**Type/consistency:** `focusRing` (string) imported once in T2 and used on pills + row link; `aria-current={selected ? "page" : undefined}` consistent; the 5 i18n keys added in T1 S1 are consumed in T1 S3-S5 (`common.clear/open`, `nav.breadcrumb`, `objects.detailTitle`) and T2 S5 (`objects.tableLabel`). ✓
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
- No new dependency. `components/ui/*` untouched (combobox/drawer wrappers unchanged; only props passed from callers). `check:colors` stays green — `focusRing` uses `ring-ring` tokens, no raw palette.
|
||||||
|
- The combobox wrapper's own raw-palette internals and the segmented-control extraction are #66, not here.
|
||||||
Reference in New Issue
Block a user