fix(web): mechanical a11y fixes — breadcrumbs, headings, names, announcements - #157
Merged
themightychris merged 14 commits intoSep 9, 2026
Merged
Conversation
heyoub
added a commit
to heyoub/codeforphilly-ng
that referenced
this pull request
Aug 24, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
heyoub
marked this pull request as ready for review
September 8, 2026 22:37
The accessibility audit's third bucket — findings with one obviously-correct fix each and no design decision attached. Recording it as a plan before touching code so the DAG carries the scope, the PR stack it sits on (CodeForPhilly#154 + CodeForPhilly#155), and its relationship to issue CodeForPhilly#156, which holds the design-decision findings that are deliberately NOT implemented here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
specs/behaviors/app-shell.md carries a table of exact breadcrumb trails, and Breadcrumbs.tsx already implements them correctly — nav[aria-label], an ordered list, aria-current on the last crumb. Nothing ever imported it, so every trail in that table was spec-only. This is code brought into conformance with a spec that has not moved. Placed as a sibling above each screen's content container rather than inside it: the component supplies its own `container mx-auto px-4`, which only lands correctly as a direct child of <main> — nesting it would double the gutter. No specs/screens/*.md mentions breadcrumbs, so app-shell.md is the sole authority and there is nothing to reconcile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A screen reader's heading outline is a navigation aid, and three index screens broke it by jumping h1 -> h3: the card components render h3, which is correct where those cards sit under a section h2 (TagDetail, Home, Volunteer) but leaves a gap on the index screens. Rather than change a shared card and break it in its other homes, each index gains an sr-only h2 over its results region — the grid genuinely is a section. ProjectsIndex was checked and does not have the defect; ProjectCard already renders h2. The detail-screen aside headings go h3 -> h2 directly. They sit under the screen h1 with nothing between, are used nowhere else, and keep their classes so nothing moves: heading level and visual size are independent. PersonCard wrapped the entire card in one <a>, so its accessible name was the avatar title, the name, the project count and every tag chip concatenated into one string. Restructured to the ProjectCard idiom with a stretched pseudo-element so the whole card stays clickable. The header's navs rendered bare links; a nav without a list does not tell you how many destinations it has. Both are now ul/li matching AppFooter, with the mobile sheet's three groups as three lists so the separators and the About heading are not list children. That "About" label was a styled <p>; it is now an h3, one level under the SheetTitle that Radix renders as an h2. HelpWantedIndex wrapped FacetSidebar — which renders its own labelled aside — in a second bare <aside>, nesting two complementary landmarks with the outer one unnamed. The outer element is now a div. Result-count badges move out of the h1 on all three index screens: a heading whose accessible name changes on every keystroke is not a stable landmark. The two GitHub links in this file also pick up their new-tab cues here rather than splitting one file's edits across two commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The six formatting controls were an unlabelled div of buttons named "B", "I", "Link" — meaningless out of visual context — and six consecutive tab stops between the label and the textarea. They are now a labelled toolbar with one tab stop and arrow-key movement (ARIA APG). Each accessible name is a superset of its visible label so speech input still works. Four places changed state with nothing announced: - ProjectDetail's "Copy link" and "Share to Slack" gave no feedback at all, to anyone — the clipboard write was the entire interaction. They now raise a sonner toast, which the modals this screen already renders use for the same purpose, and surface a failed clipboard write instead of swallowing it. - Sponsor's "Copy email" signals success by renaming itself, and a control's own name changing is not announced. An sr-only live region mirrors it. - ProfileEdit's "Uploading…" appeared and vanished silently; it is now a status region that persists across both states so it can announce. - ConnectGitHubBanner was role="region", which is a landmark: it is only reachable by going looking for it. The banner renders after auth resolves, i.e. after first paint, so it needs role="status" to be heard at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three unrelated-looking defects with the same root cause: information that is obvious on screen but absent from the accessibility tree. Repeated button names. Tabbing a member list, a session table, the claim queue or a project's open roles produced "Remove, Remove, Remove" with no way to tell which row you were on — the row context lived only in visual adjacency. Each button now carries an aria-label naming its subject, with the visible text kept as a substring so speech input still reaches it (SC 2.5.3). The claim queue's labels stay fixed while a request is in flight and its buttons read "Working…"; the busy state is transient and the name should not move under a user mid-interaction. Dates. A relative string like "3 months ago" inside a title-only span is imprecise for everyone and the title is unreachable by touch and by most screen readers. These become <time dateTime> carrying the ISO instant, the idiom BlogIndex already uses, with title kept as a sighted-mouse bonus. ProjectCard's wrapper title duplicated what PersonAvatar already emits, so it is deleted rather than converted. New tabs. Every target="_blank" link now says so — an sr-only span where there is visible text, appended to the aria-label where there is not. Losing your place because a link silently opened elsewhere is a bigger problem for a screen-reader or magnifier user than for anyone else. Also: the "More ▾" menu trigger reads as an actual action list, and the stage-explainer button declares aria-haspopup="dialog" so it is not mistaken for navigation. HomeStub.tsx is skipped — nothing imports it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regression cover for the parts of this branch that are easy to undo by accident. The breadcrumb tests assert the exact trails app-shell.md prescribes, including that the final crumb is text with aria-current rather than a link — the detail that makes a trail a trail. The Revoke test asserts that two rows produce two distinct accessible names, which is the property that actually broke, rather than asserting one label's spelling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both gate runs green; breadcrumbs, copy toast, badge placement, and the PersonCard click affordance verified in headed Chrome against a seeded dev data repo. Toolbar keyboard nav rides on the jsdom coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wrapping the sheet's NavLinks in <li> left the anchors inline, so each row's tap target shrank to the width of its text. `block` on navLinkClass and on the two plain <a>s makes the rows full-width again. Swapping HelpWantedIndex's outer <aside> for a <div> removed the nested landmark but orphaned the Commitment heading and fieldset outside any landmark, since FacetSidebar renders its own aside[aria-label="Filters"] as a sibling. FacetSidebar now accepts children inside that aside, and Commitment rides there — still exactly one complementary landmark, and it holds every filter control. PeopleIndex and ProjectsIndex mount FacetSidebar directly and are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Turning the banner into role="status" traded a landmark for a live region that mounts late — and a container that appears already populated is not announced reliably, while wrapping the two buttons in a status role is invalid content for it. Revert to region + aria-label and add a sibling sr-only role="status" span carrying the headline, the same idiom Sponsor and ProfileEdit use in this PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Breadcrumbs keyed each <li> by label, which collides when a
user-authored title matches an ancestor crumb; key by index instead.
TagDetail's namespace crumb showed the raw slug ("tech") while the page
it links to is headed "Tech"; it now uses TagsNamespace's NS_LABELS so
the crumb and the destination h1 agree.
ProjectEdit rendered a blank crumb linking to /projects/ when the edit
query settled without a record; hold the loading state in that case so
the trail is only ever built from a loaded project.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
navigator.clipboard is undefined outside secure contexts and reading .writeText off it throws synchronously, before a .catch() could run, so the failure toast never fired there. One copyWithToast helper guards it and replaces the two duplicated promise chains on ProjectDetail. ProfileEdit's always-mounted status span reserved mt-1 even while empty; the margin now applies only when it has text. Three relative timestamps were still title-only: HelpWantedCard's "posted", PersonDetail's "joined" and its recent-update dates. They get the same <time dateTime title> treatment as the rest of the sweep. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
handleToolbarKeyDown read `activeButton` from its closure, which lags a focus change by a render cycle. Take the index from the button the key landed on instead; `activeButton` stays as render state for tabIndex. The name-superset test compared two literals from its own table; it now checks the rendered button's aria-label against its visible text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
This was referenced Sep 9, 2026
Open
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 CodeForPhilly#166–CodeForPhilly#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
themightychris
force-pushed
the
fix/a11y-mechanical
branch
from
September 9, 2026 02:10
ce4ea22 to
e8e9bc6
Compare
Member
|
Thanks @heyoub — this was a pleasure to review. Breadcrumbs, per-row names, the I've taken you up on "happy to rebase after they merge": #154 and #155 both picked up review patches before landing, so I rebased your eight commits onto
Plan hygiene: Notes and Follow-ups are filled in, the two placement sentences corrected (crumbs are a fragment sibling above the container; the PersonCard hover lift stays on the article), and the bigger refactors the review surfaced are filed rather than done here — #166 (shell-rendered breadcrumbs via route handles), #167 ( Full gate run from the repo root after the rebase — 🤖 Generated with Claude Code |
This was referenced Sep 9, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The "mechanical" remainder of the accessibility audit behind #154 / #155 — every fix here is contained and needs no design decision. The findings that DO need maintainer decisions are catalogued separately in #156; nothing from that issue is implemented here.
Plan record:
plans/a11y-mechanical.md. No spec changes — the one structural addition (breadcrumbs) implements whatspecs/behaviors/app-shell.mdalready prescribes.What's here
Breadcrumbs.tsxwas a correct, complete component that nothing imported. It now renders on the six screens the app-shell spec's table names, with the exact trails prescribed (Projects › title, Members › name, Tags › namespace › title, Settings, edit/create variants). Verified in a browser against a live dev API.Remove ${fullName},Revoke session on ${device}, …), always containing the visible text (SC 2.5.3).ProjectDetail/PersonDetailaside h3s → h2 (classes untouched, zero visual change); sr-only<h2>Results</h2>onPeopleIndex/HelpWantedIndexrather than bumping the shared cards, which are correctly h3 under section h2s elsewhere. (ProjectsIndexturned out to have no skip —ProjectCardalready renders h2.)role="toolbar", real names (Bold/Italic/…), and a roving tabindex; ProjectDetail's silent clipboard buttons now confirm via sonner (the screen's existing idiom) with failure toasts; Sponsor's "Copied ✓" and ProfileEdit's "Uploading…" are announced viarole="status".PersonCardno longer one giant link (ProjectCard's heading-wraps-link idiom + stretched pseudo-element keeps whole-card click); both header navs areul/lilike the footer; the sheet's "About" label is a real heading; the nestedcomplementarylandmark on HelpWantedIndex is gone; index count badges moved out of the<h1>so its name stops mutating with filters.title-only date spans →<time dateTime>(BlogIndex's existing idiom); all 16target="_blank"sites carry an "(opens in new tab)" cue;More ▾→aria-label="More actions"; stage-explainer trigger getsaria-haspopup="dialog"; ConnectGitHubBannerrole="region"→role="status"so its appearance is announced.Reviewer notes
ConnectGitHubBannerregion→status is a behavior-adjacent change (landmark → live region), not a rename — its test was updated in six places."GitHub(opens in new tab)"vs Chrome's"GitHub (opens in new tab)"); the tests use regex matchers with a comment rather than encoding the jsdom quirk.Breadcrumbs.tsxkeys crumbs bylabel, which would collide on a tag titled identically to its namespace.Validation
type-check+lintclean; web 116/116 (27 files, incl. new MarkdownEditor toolbar coverage and breadcrumb/name/time assertions), shared 75/75 — run independently twice. Headed-Chrome pass against a seeded dev data repo: breadcrumb trails on project + member detail, the "Link copied" toast, badge-outside-h1 visuals, and PersonCard's whole-card click all verified (details in the plan's validation section).apps/apiuntouched; its known Windows-only fixture failure is unrelated (noted in #154).Closes nothing on its own — the decision-level findings live in #156.
🤖 Generated with Claude Code