fix(web): repair invalid ARIA and make search/tag pickers real comboboxes - #155
Draft
heyoub wants to merge 8 commits into
Draft
fix(web): repair invalid ARIA and make search/tag pickers real comboboxes#155heyoub wants to merge 8 commits into
heyoub wants to merge 8 commits into
Conversation
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>
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>
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>
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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
heyoub
force-pushed
the
fix/aria-correctness
branch
from
August 24, 2026 16:55
ce8eb21 to
54b24ad
Compare
This was referenced Aug 24, 2026
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.
An ARIA-correctness pass over
apps/web, scoped to verified defects: markup that is invalid per the ARIA spec (prohibited attributes, illegal listbox ownership, dangling references) or that withholds state assistive technology needs. No spec changes —specs/behaviors/app-shell.mdalready requires "All dropdowns are keyboard-navigable"; this PR brings the code into conformance. Plan record:plans/aria-correctness.md. No visual redesigns: out-of-scope-by-choice items (heading hierarchy, color contrast,document.title, motion controls, etc.) are listed in the plan's follow-ups instead of bundled here.Headline changes
SearchBoxrebuilt as an APG combobox. It previously putaria-expanded/aria-controlson a plain search input, had arole="listbox"owning bare<p>/<div>s, hardcodedaria-selected={false}, and its results were keyboard-unreachable (Tab blurred the input and a 150 ms timeout unmounted the popup). Now:role="combobox"+aria-activedescendant, valid listbox ownership (status text outside, groups labelled), Arrow/Home/End/Enter/Escape, and the blur race replaced by swallowingmousedownon the popup. Options stay<a href>(middle-click works) but plain activation routes throughuseNavigate()— clicking a result no longer full-page-reloads the SPA. The print stylesheet's hook moved from the old hardcoded id (which duplicated when both header and sheet search rendered) to[data-search-dropdown].TagPickerrebuilt the same way — it drove a popup with no combobox ARIA at all,role="listbox"on a<ul>of unroled<li><button>s, no Escape/arrows, and itslabelprop was never associated with the input (affects ProjectEdit, ProfileEdit, PostHelpWantedModal). Also fixes a latent empty-popup render whenallowCreatewas set but the query wasn't a valid slug.aria-invalidwas set in many forms but the error text was never referenced; every audited error now has an id and a conditionalaria-describedby(AddMemberModal, ProjectEdit ×5, ProjectBuzzNew ×4, PostHelpWantedModal, TagEditModal ×2, ProfileEdit ×2, MarkdownEditor). ProjectEdit's slug-availability status is now announced.Smaller repairs
Prohibited
aria-labelon roleless elements (PersonAvatar →role="img", StageBadge bar → realrole="progressbar", auth skeletons, spinner wrappers →role="status"+sr-onlytext); label-in-name mismatch on the network banner's Retry button; the always-mountedTopProgressBarhidden from AT when idle;aria-pressedon toggle filters (TagChip, Home activity row — matching the existingStageFilterRowidiom); redundant labels removed;aria-controlson the login disclosure;Page Nnames in Pagination;scope="col"on the sessions table; tooltip-only stage descriptions now keyboard-reachable; the skip link's suppressed focus ring restored; avatar file input labelled.Notes for reviewers
app-shell.mdprescribes) but its handler only dismisses. Recorded in the plan's follow-ups — it needs its own spec decision.SearchBox.test.tsxandTagPicker.test.tsx(keyboard paths, attribute contracts) — 11 tests.Validation
type-checkandlintclean; web tests 100/100 (25 files), shared 75/75. Noapps/api,specs/, orcomponents/ui/files touched. This PR is independent of #154 (verified: clean merge in either order).🤖 Generated with Claude Code