Skip to content

Commit e8e9bc6

Browse files
chore(plans): record review closeout for a11y-mechanical
Fill in the Notes and Follow-ups that were left as placeholders, correct two sentences that no longer matched the code (crumbs are a fragment sibling above the content container, not its first child; the PersonCard hover lift stays on the article, there is no group-hover), describe the post-rebase state of the banner, landmark and toolbar items, and record the full validation gate across every workspace. Follow-ups filed as #166–#170 for the refactors the review surfaced but deliberately kept out of this PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
1 parent dc9055b commit e8e9bc6

1 file changed

Lines changed: 94 additions & 23 deletions

File tree

‎plans/a11y-mechanical.md‎

Lines changed: 94 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ table of trails. **No spec change is needed anywhere in this plan.**
4545
`apps/web/src/components/Breadcrumbs.tsx` renders `nav[aria-label="Breadcrumb"]
4646
> ol > li` with `aria-current="page"` on the last crumb — correct as written,
4747
imported by nothing. Wired into the six screens the spec's table names, each
48-
placed as the first child of the screen's content container (the spec's "row
49-
below the header"):
48+
rendered as a fragment sibling immediately *above* the screen's content
49+
container (the spec's "row below the header"):
5050

5151
| Route | Trail |
5252
|---|---|
@@ -97,7 +97,9 @@ The six formatting buttons were a bare `<div>` of buttons named "B", "I",
9797
`aria-label`'d with a full name that contains its visible label as a
9898
substring (B ⊂ Bold, I ⊂ Italic, Link ⊂ Insert link, List ⊂ Bulleted list),
9999
and a roving tabindex: only the active button is tabbable,
100-
ArrowLeft/ArrowRight move focus (wrapping), Home/End jump to the ends.
100+
ArrowLeft/ArrowRight move focus (wrapping), Home/End jump to the ends. The
101+
keydown handler derives its starting index from the event target rather
102+
than the `activeButton` state, which lags a focus change by one render.
101103

102104
### 5. Status announcements
103105

@@ -109,33 +111,40 @@ Three places changed state visually with nothing announced:
109111
- `Sponsor`'s "Copy email" swaps its label to "Copied ✓" — visible text kept,
110112
with an `sr-only role="status"` mirror added.
111113
- `ProfileEdit`'s "Uploading…" span becomes `role="status"`.
112-
- `ConnectGitHubBanner` was `role="region"`, which is never announced; the
113-
banner appears *after* auth resolves, so it becomes `role="status"`.
114+
- `ConnectGitHubBanner` is a `role="region"` that mounts only after auth
115+
resolves, so its arrival is never announced. It *stays* a region (a
116+
late-mounted `status` container is not read reliably either, and it would
117+
wrap the two buttons) and gains a sibling sr-only `role="status"` span
118+
carrying the headline — the same mirror idiom as `Sponsor`.
114119

115120
### 6. `<time dateTime>` for machine-readable dates
116121

117122
`title` is not exposed to most screen readers and never on touch. Every date
118123
rendered as relative text inside a `title`-only `<span>` becomes
119124
`<time dateTime={iso} title={absolute}>` — the `BlogIndex.tsx` idiom.
120-
Covers `ActivityCard` (×2), `ProjectDetail` (×2), `BlogDetail`, `Account`,
121-
`StaffAccountClaimQueue`, `AccountClaim`. `ProjectCard`'s wrapper
125+
Covers `ActivityCard` (×2), `ProjectDetail` (×2), `PersonDetail` (×2),
126+
`HelpWantedCard`, `BlogDetail`, `Account`, `StaffAccountClaimQueue`,
127+
`AccountClaim`. `ProjectCard`'s wrapper
122128
`title={m.fullName}` is deleted outright — `PersonAvatar` already emits it.
123129

124130
### 7. Structure and one-liners
125131

126132
- `PersonCard` was one giant `<Link>`, so its accessible name concatenated
127133
avatar + name + project count + every tag chip. Restructured to the
128134
`ProjectCard` idiom: `<article>` with the `h3` wrapping the link. The hover
129-
lift moves to the article via `group-hover`, so the affordance is unchanged.
135+
lift (`hover:shadow-md hover:-translate-y-0.5`) stays on the article, so
136+
the affordance is unchanged.
130137
- `AppHeader`'s two navs render bare links; wrapped in `<ul>/<li>` matching
131138
`AppFooter`. Flex/gap classes move to the `ul`; `li` contributes nothing.
132139
Sheet separators sit between the two lists rather than inside one.
133140
- The mobile sheet's "About" group label was a styled `<p>` → `<h3>` (one
134141
level below the Radix `SheetTitle`, which renders `h2`), same classes.
135142
- `HelpWantedIndex`'s bare outer `<aside>` wrapped `FacetSidebar`, which
136143
renders its own labelled `<aside>` — two nested `complementary` landmarks.
137-
Outer becomes a `<div>`. (`PeopleIndex`/`ProjectsIndex` render
138-
`FacetSidebar` directly and never had this.)
144+
`FacetSidebar` now accepts `children` inside its aside, and the Commitment
145+
heading + fieldset ride there: one landmark ("Filters") holds every filter
146+
control instead of leaving Commitment orphaned beside it. (`PeopleIndex`/
147+
`ProjectsIndex` render `FacetSidebar` directly and never had this.)
139148
- Result-count badges move **out** of the `h1` into a flex sibling on all
140149
three index screens, so the heading's accessible name stops mutating as
141150
filters change.
@@ -155,13 +164,23 @@ Covers `ActivityCard` (×2), `ProjectDetail` (×2), `BlogDetail`, `Account`,
155164
`ProjectDetail` / `PersonDetail` aside headings are `h2`.
156165
- [x] The `MarkdownEditor` toolbar exposes `role="toolbar"`, named buttons,
157166
and a working roving tabindex (Arrow/Home/End).
158-
- [x] Copy actions on `ProjectDetail` and `Sponsor` announce; `ProfileEdit`
159-
upload and `ConnectGitHubBanner` are live regions.
167+
- [x] Copy actions on `ProjectDetail` and `Sponsor` announce (including a
168+
failure toast where `navigator.clipboard` is absent); `ProfileEdit`'s
169+
upload is a live region; `ConnectGitHubBanner`'s arrival is announced
170+
by its sibling status span.
160171
- [x] Dates expose `datetime`; no date is `title`-only.
161-
- [x] Exactly one `complementary` landmark per index screen.
162-
- [x] `npm run -w packages/shared build && npm run type-check && npm run lint
163-
&& npm run -w apps/web test && npm run -w packages/shared test` clean
164-
(web 116/116, shared 75/75; run twice — implementer and coordinator).
172+
- [x] Exactly one `complementary` landmark per index screen, and on
173+
`HelpWantedIndex` it contains the Commitment controls
174+
(`HelpWantedIndex.test.tsx`).
175+
- [x] Full gate from the repo root, all workspaces: `npm ci && npm run -w
176+
packages/shared build && npm run type-check && npm run lint && npm
177+
test` clean after the rebase onto `develop` and the review fix-ups
178+
(api 434/434 across 35 files, web 124/124 across 28, shared 75/75
179+
across 3). The root `npm test` invocation was OOM-killed once on the
180+
closeout machine, so the api suite was re-run alone with
181+
`--maxWorkers=2`; every workspace suite ran to completion. The
182+
contributor's earlier pass ran web + shared only (web 116/116,
183+
shared 75/75).
165184
- [x] Browser test (headed Chrome against the live dev stack — api booted on
166185
a `setup-dev-data` repo with two seeded records): breadcrumb trails
167186
verified on `/projects/qa-sandbox` ("Projects › QA Sandbox Project")
@@ -174,17 +193,69 @@ Covers `ActivityCard` (×2), `ProjectDetail` (×2), `BlogDetail`, `Account`,
174193

175194
- **Low.** Almost every change is attribute-level or a wrapper element.
176195
- The two structural edits (`PersonCard`, `AppHeader` nav lists) touch files
177-
PR #154 rewrote. Both keep every existing behavior — the sheet's `onClick`
178-
close handlers, the separators, the NavLink active styling — and are
179-
covered by the existing `AppHeader.test.tsx` suite plus updated name
196+
PR #154 rewrote. Both keep every existing behavior — the sheet closing on
197+
navigation (derived from `location.key` since #154, so the NavLinks carry
198+
no per-item `onClick`), the separators, the NavLink active styling — and
199+
are covered by the existing `AppHeader.test.tsx` suite plus updated name
180200
matchers.
181-
- The `apps/api` suite is deliberately not run: it has a known pre-existing
182-
Windows fixture failure and no `apps/api` file changes here.
201+
- The `apps/api` suite was skipped on the contributor's Windows pass (known
202+
fixture failure there, tracked in #162); it was run in full on Linux at
203+
closeout — see Validation.
183204

184205
## Notes
185206

186-
_(filled in at closeout)_
207+
- **`aria-label` supersets on visible-text buttons are deliberate.** "More ▾"
208+
→ `aria-label="More actions"`, "Mark filled" → `Mark filled: <role>`,
209+
"Remove" → `Remove <name>`, and the toolbar's "B" → "Bold". In every case
210+
the visible text is a prefix (or, for the toolbar, a substring) of the
211+
accessible name, which is what SC 2.5.3 Label in Name asks for and what
212+
the specs already assume for the per-row actions. Speech-input users
213+
saying what they see still hit the control.
214+
- **jsdom's accname drops the leading space in sr-only cues** — it
215+
computes `"GitHub(opens in new tab)"` where Chrome computes
216+
`"GitHub (opens in new tab)"`. Tests match with `\s*` regexes rather than
217+
encoding either engine's answer.
218+
- **Rebase.** The branch was cut from a local merge of #154 and #155; both
219+
were patched during review before landing on `develop`, so the eight
220+
commits here were rebased onto `develop` after #155 merged. The one
221+
conflict was `AppHeader.tsx`: #154 replaced the per-NavLink `onClick`
222+
closers with a sheet state derived from `location.key`, and this branch
223+
wrapped the same links in `<ul>/<li>`. Resolved by keeping the list
224+
structure without the closers (the two plain `<a>`s — Contact, GitHub —
225+
keep theirs, since they don't navigate client-side).
226+
- **Review fix-ups applied on top of the six commits:** `block` on sheet
227+
anchors so the `<li>` wrap doesn't shrink their tap targets; Commitment
228+
moved inside `FacetSidebar`'s aside (via a new `children` slot) instead
229+
of sitting outside every landmark; `ConnectGitHubBanner` kept as a region
230+
with a sibling status announcer; `Breadcrumbs` keyed by index (the PR's
231+
own "pre-existing, flagged" item); the namespace crumb labelled via
232+
`TagsNamespace`'s `NS_LABELS` so it matches the destination `h1`;
233+
`ProjectEdit` holds its loading state when the edit query settles empty
234+
instead of rendering a blank crumb to `/projects/`; a `copyWithToast`
235+
helper on `ProjectDetail` that guards `navigator.clipboard` (undefined in
236+
insecure contexts, and reading `.writeText` off it throws synchronously,
237+
so the old `.catch()` never fired); `ProfileEdit`'s status span only
238+
reserves `mt-1` while it has text; three `<time>`s the first sweep missed
239+
(`HelpWantedCard`, `PersonDetail` ×2); the toolbar keydown handler reads
240+
its index from the event target rather than the `activeButton` closure.
241+
- `NS_LABELS` is now exported from `screens/TagsNamespace.tsx` and imported
242+
by `screens/TagDetail.tsx` — a screen importing from a screen, chosen as
243+
the smallest change. `FacetSidebar` carries its own copy of the same map;
244+
a shared tags-label module could absorb both.
187245

188246
## Follow-ups
189247

190-
_(filled in at closeout)_
248+
- Issue [#166](https://github.com/CodeForPhilly/codeforphilly-ng/issues/166)
249+
— render breadcrumbs from the shell via route `handle`s (`useMatches`)
250+
per app-shell.md, so skip-to-main lands past them rather than on them.
251+
- Issue [#167](https://github.com/CodeForPhilly/codeforphilly-ng/issues/167)
252+
— `ExternalLink` component for the 16 hand-rolled "(opens in new tab)"
253+
copies.
254+
- Issue [#168](https://github.com/CodeForPhilly/codeforphilly-ng/issues/168)
255+
— `RelativeTime` component for the 13 `<time dateTime title>` sites.
256+
- Issue [#169](https://github.com/CodeForPhilly/codeforphilly-ng/issues/169)
257+
— `MarkdownEditor` toolbar should use Radix `Toolbar` from the
258+
already-installed `radix-ui` instead of the hand-rolled roving tabindex.
259+
- Issue [#170](https://github.com/CodeForPhilly/codeforphilly-ng/issues/170)
260+
— card heading level should be a prop; `TagDetail` renders a section `h2`
261+
followed by `ProjectCard` `h2`s.

0 commit comments

Comments
 (0)