fix(web): mechanical a11y fixes — breadcrumbs, headings, names, announcements - #157
Draft
heyoub wants to merge 26 commits into
Draft
fix(web): mechanical a11y fixes — breadcrumbs, headings, names, announcements#157heyoub wants to merge 26 commits into
heyoub wants to merge 26 commits into
Conversation
Issue CodeForPhilly#153 walks the live site on desktop and mobile and collects five findings. Four are shippable together because they all land in the app shell or in one screen's outbound links; the fifth (replace the Home hero CTA with a mailing-list invite) has no mechanism to build against, so the plan records the block up front rather than inventing one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Specs lead, so both files change before the code does. app-shell: the header's single "primary nav" table conflated content navigation with utilities, which is why the Volunteer CTA ended up buried between Members and About. Splitting it into a content cluster and a right-pinned utility cluster makes the CTA's position a stated rule rather than an accident, and gives the GitHub link and the auth control a declared home. Also states the sheet's accessible name and the icon-only-controls labelling rule, both of which the header violated. volunteer: the whole codeforphilly.gitbook.io space now returns 404 "Content owner not found", so the spec was prescribing two dead targets. The Meetup group and the CodeForPhilly/partnerships first-steps doc are the live equivalents; both verified reachable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An accessibility audit found a set of verified ARIA correctness defects in apps/web — prohibited attributes on roleless elements, listboxes owning non-option children, dangling references, and toggle/progress state withheld from assistive tech. Record the scope up front so the code commits that follow have a reviewed target, and so the deliberately-excluded audit categories are on the record rather than looking like oversights. specs/behaviors/app-shell.md already requires keyboard-navigable dropdowns, so this plan needs no spec change — it brings code into conformance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Site check (CodeForPhilly#153) found the Volunteer button buried mid-nav between Members and About, where it read as one more section link rather than the call to action. It now closes the utility cluster, after the auth control, per the reordered app-shell spec. About joins the content links; a GitHub icon link is added to both breakpoints. The mobile sheet had no horizontal padding at all: SheetContent's base classes carry none and the only override here was a pt-8 hack, so nav items and the search box sat flush against the panel edge. Replaced with the structure shadcn intends — SheetHeader + SheetTitle, which bring their own p-4 — plus explicit px-4 on the nav and search. SheetTitle also gives the underlying Radix dialog the accessible name it never had. Three ARIA defects fixed while the file was open: aria-label on a roleless skeleton div (prohibited; now aria-hidden), a hand-written aria-expanded duplicating what Dialog.Trigger already supplies, and an aria-label overriding the About trigger's own visible text. The account-menu label stays — below sm the person's name is display:none, so it is the only accessible name there. Per-child ml-1 margins are gone; the parent gap-2 is now the single source of spacing at the same effective density. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The whole codeforphilly.gitbook.io space now returns 404 "Content owner not found", so both of the Volunteer screen's outbound CTAs were dead. PR CodeForPhilly#128 fixed the Home screen's copy of the same URL; these are the last two in the SPA. "When we meet" goes to the Meetup group, which is where hack nights are actually announced and which the footer already links to; "Read the guide" goes to the partnerships repo's first-steps doc, the surviving source of the GitBook page it replaces. The footer's "view this site on GitHub" link still named the repo codeforphilly-rewrite. That only resolves through GitHub's rename redirect, which is not something to depend on indefinitely. New Volunteer test asserts both hrefs and, following the Home dead-link idiom, that no gitbook.io URL survives anywhere in the rendered screen — so a copy-paste of the old constant cannot come back unnoticed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sheet is a full-height flex column with no scroll container, so a nav list taller than the viewport was simply unreachable below the fold. That was already latent; adding the GitHub row makes it one row likelier on short phones. min-h-0 lets the flex child shrink at all, and overflow-y-auto gives it somewhere to put the excess. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The widget was invalid ARIA on every axis: aria-expanded/aria-controls sat on
an implicit searchbox, the popup claimed role="listbox" while owning a bare
<p> and unroled <div>s, and every option hardcoded aria-selected={false}.
Worse, the results were unreachable — Tab blurred the input and a 150ms
setTimeout unmounted the dropdown before anything could be clicked, so only
Enter and Escape ever did anything.
specs/behaviors/app-shell.md already requires "All dropdowns are
keyboard-navigable", so this is conformance work, not new behavior.
Rebuilt to the combobox-with-listbox pattern: focus stays on the input and the
active option is pointed at with aria-activedescendant, which removes the blur
race structurally for keyboard users. The popup swallows mousedown so a
pointer click cannot blur the input either — hence the timeout is deleted
rather than retuned. Group headers become role="group" + aria-labelledby and
the status text moves outside the listbox, so the listbox owns only groups and
options; "See all results" becomes its final option.
Options stay <a href> (option is an allowed role for a[href]) with
tabIndex={-1}, keeping middle-click and "open in new tab" alive, while plain
activation routes through useNavigate() instead of reloading the whole SPA.
The hardcoded id="search-results-dropdown" becomes useId()-derived — it was
duplicated whenever the desktop and mobile-sheet instances both rendered. The
print stylesheet's hook moves to [data-search-dropdown] so app-shell.md's
Print section stays true.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
role="listbox" sat on a <ul> whose <li> children carried no role and wrapped <button>s, the driving Input had no combobox ARIA at all, there was no Escape or arrow handling, and the `label` prop rendered a <Label> associated with nothing — so ProjectEdit, ProfileEdit and PostHelpWantedModal each shipped several unlabelled inputs. Same pattern as SearchBox: useId() ties the Label to the input, the <li> become the interactive role="option" targets driven by aria-activedescendant, and arrows/Enter/Escape work. Enter still falls back to the historical exact-match then first-match then create-tag chain when no option is active, and Backspace-removes-last is untouched, so existing muscle memory survives. The popup no longer renders when the only reason to show it was a query that cannot produce an option — that previously left an empty listbox on screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each of these either lies to assistive tech or withholds state that sighted users get for free. Grouped into one commit because they are all one-line attribute repairs with no shared behavior change. Prohibited attributes on roleless elements (aria-label needs a role to attach to, so today it is simply dropped): PersonAvatar's initials span gains role="img"; StageProgressBar's wrapper loses its aria-label in favour of real role="progressbar" semantics on the bar that actually encodes the percentage. Announcements that never fire: the LoginPlaceholder and AccountClaim spinners put aria-live + aria-label on an empty roleless div, which announces nothing — now role="status" with sr-only text and the spinner hidden. TopProgressBar was permanently exposed (it only fades via opacity), so every page read out a finished "Page loading" bar; it is hidden from AT while idle. Names that fight their labels: NetworkErrorBanner's button reads "Retry" but was named "Dismiss error" (SC 2.5.3), and ConnectGitHubBanner duplicated its own visible "Dismiss" text. Both aria-labels go. Pagination's page buttons were named only "3"; the filter chips on ProjectsIndex and HelpWantedIndex never said they remove the filter. State conveyed by styling alone: TagChip and the Home activity filters get aria-pressed, matching StageFilterRow. Also: StageBadge's tooltip triggers were non-focusable, so the stage description was hover-only; the sessions table's <th>s had no scope; the skip link — the one control that exists purely for keyboard users — ended its class list with focus:outline-none; and ManageMembersModal's inline role field was labelled only by its placeholder. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
aria-invalid was set on several controls but the error <p> beneath was never
referenced, so a screen-reader user heard "invalid" with no way to reach the
reason — the one piece of information the error exists to convey. Every
audited error message now carries an ${id}-error id and its control an
aria-describedby, applied the same way everywhere: conditional, present only
while the error is.
Fields covered: AddMemberModal, ProjectEdit (5), ProjectBuzzNew (4),
PostHelpWantedModal, TagEditModal (2), ProfileEdit (2) and MarkdownEditor.
Where a field showed an error without aria-invalid, that is set too so the
pair stays consistent.
Two related labelling fixes on the same screens. ProjectEdit's debounced slug
check ("Checking…" / "✓ Available" / "✗ Taken") was never announced, so a
non-sighted author could submit a slug already known to be taken; it becomes a
role="status" the input describes. ProfileEdit's "Avatar" Label pointed at
nothing and its file input had no id — the wrapping <label> is now a div so it
cannot compete for the accessible name.
MarkdownEditor also drops aria-live from its preview pane: the preview is the
whole document re-rendered on every debounce, so the live region read the
entire text back on each pause in typing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixing the Label-in-Name violation on NetworkErrorBanner turned up a deeper disagreement: the button says "Retry", app-shell.md prescribes "[Retry]", and the handler only calls clearError(). Spec and label agree; the code does not. That is a behavior decision, not an ARIA repair, so it does not belong in this plan's scope — but it should not evaporate either, and the aria-label removed here was the only remaining trace of what the button really does. Record it as a follow-up needing its own spec decision so nobody later "fixes" it by quietly renaming whichever side is easiest to reach. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ticks the criteria verified during implementation and, deliberately, leaves two unticked rather than rewriting them to match what was achievable: the browser pass belongs to whoever does UI QA, and the all-workspaces test gate cannot close on this Windows dev box. Notes record why. apps/api fails ten tests here on a tree whose API code is byte-identical to develop, because the fixtures assume POSIX — store.test.ts injects a write failure via /dev/null/impossible-path, which Windows will happily create, so the expected rejection never comes. Reproduces with the files run alone, so it is not runner contention. Filed as a follow-up rather than fixed: cross-platform fixtures are their own scope, and silently ticking a gate that did not run is worse than leaving the box open. Also flags HomeStub.tsx, which carries the same stale repo URL the footer had but is imported by nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the numbers behind the unticked all-workspaces gate so a future reader can tell a known Windows baseline from a real regression, and notes the develop re-run that confirms it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With showLabel the nested StageBadge is already a focusable trigger for the same tooltip, so the wrapper's tabIndex added a second, redundant stop right next to it. The wrapper now joins the tab order only in the bar-only variant, where it is the sole way to reach the stage description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Desktop header verified in headed Chrome at 1400px; the sheet verified via its portal at desktop width because the automation harness could not shrink a maximized window below md — same limitation web-shell.md's plan recorded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
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
1 participant
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