name the unlabeled inline form controls on the a11y sweep's high-value pages - #4307
Merged
Conversation
…pages (#4156) Adds accessible names to the previously-unlabeled inline inputs on CharacterSheet, GitHub, Sharing, Browser, MoodBoardDetail and VideoTimelineEditor: aria-label where the control lives in a compact toolbar/inline row, and a FormField (visible label + htmlFor/id) for the GitHub add-secret form, which is a stacked column with room for one. The repo-wide guard in a11yConventions.test.js could not see through a page-local field wrapper that renders its children inside a <label> (PipelineSeries.jsx's <Field label="…">), so three already-accessible controls sat in PREEXISTING_INPUT_NAME_ALLOWLIST purely as a static-analysis gap. Teach the guard to recognise same-file label-wrapping components and drop those entries too — 20 allowlist entries removed in total, so these controls are now enforced.
…pper detection (#4156) - Skip the wrapper's parameter list with the string-aware balancedCallAt so a default value containing ')' cannot point the body scan at the destructuring. - Require {children} to sit inside a <label> with no intervening </label>, so a component rendering sibling labels around its children can't register as a wrapper and exempt controls it never labels. - Track every still-open wrapper instance rather than only the outermost, so an inner <Field label="…"> nested in an unlabeled outer one still counts.
atomantic
added a commit
that referenced
this pull request
Aug 15, 2026
…ace and Agent tabs (#4156) Continues the accessible-name sweep past the page-level slice shipped in #4307, working four whole component subtrees rather than scattered hits: client/src/components/goals, brain/tabs, meatspace/tabs and agents/tabs. Each candidate was read in context first — controls already named by a wrapper were left alone. Where the surface had room the control gets a visible label wired with htmlFor/id (the shared FormField, or an existing orphan <label> given the pairing it was missing); compact toolbars and inline table-edit rows get aria-label instead. MemoryTab's five add/edit forms moved to FormField so every field carries a visible label rather than a placeholder alone. LifestyleTab's page-local FieldGroup rendered a <label> that pointed at nothing. It now takes an htmlFor prop for its three single-control groups and falls back to role="group" + aria-labelledby for the button sets, which have no labelable control. Wrapping was not an option there — an implicit <label> would pull the live value readout and the hint paragraph beside each slider into the accessible name. The a11y guard learns that shape: hasMatchingExplicitLabel now also credits a same-file wrapper that forwards htmlFor onto a <label> it fills with its own `label` prop, under the same limits as the existing label-wrapper recognizer. A probe test pins the bypasses shut — a forwarder whose <label> holds no text, and a call site with no `label=`, both still count as unnamed. All 80 corresponding PREEXISTING_INPUT_NAME_ALLOWLIST entries are removed, so these controls are now enforced by the guard instead of exempted.
atomantic
added a commit
that referenced
this pull request
Aug 15, 2026
…ace and Agent tabs (#4156) Continues the accessible-name sweep past the page-level slice shipped in #4307, working four whole component subtrees rather than scattered hits: client/src/components/goals, brain/tabs, meatspace/tabs and agents/tabs. Each candidate was read in context first — controls already named by a wrapper were left alone. Where the surface had room the control gets a visible label wired with htmlFor/id (the shared FormField, or an existing orphan <label> given the pairing it was missing); compact toolbars and inline table-edit rows get aria-label instead. MemoryTab's five add/edit forms moved to FormField so every field carries a visible label rather than a placeholder alone. LifestyleTab's page-local FieldGroup rendered a <label> that pointed at nothing. It now takes an htmlFor prop for its three single-control groups and falls back to role="group" + aria-labelledby for the button sets, which have no labelable control. Wrapping was not an option there — an implicit <label> would pull the live value readout and the hint paragraph beside each slider into the accessible name. The a11y guard learns that shape: hasMatchingExplicitLabel now also credits a same-file wrapper that forwards htmlFor onto a <label> it fills with its own `label` prop, under the same limits as the existing label-wrapper recognizer. A probe test pins the bypasses shut — a forwarder whose <label> holds no text, and a call site with no `label=`, both still count as unnamed. All 80 corresponding PREEXISTING_INPUT_NAME_ALLOWLIST entries are removed, so these controls are now enforced by the guard instead of exempted.
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.
Summary
Continues the unlabeled-form-control sweep from the 2026-07-23 accessibility audit.
Instances.jsxlanded in #4274; this PR covers the rest of the "highest value first" page list in the issue.Each candidate was verified in context rather than mass-applied — the raw scan's true-positive rate is roughly half, and most hits on these pages turned out to be already-labeled controls (wrapped in
FormField, a page-local<label>wrapper, or paired viahtmlFor/id),<select>/<input>mentions inside comments, orid={expr}/htmlFor={expr}pairs the naive scan can't resolve.Controls given a name (14):
CharacterSheet.jsxaria-label— compact inline action rows, no room for a visible labelGitHub.jsxaria-label— toolbarGitHub.jsxFormField— stacked column with room for a visible labelSharing.jsxaria-label— inline strip sharing a row with its Save buttonBrowser.jsxaria-label— single-line inline formMoodBoardDetail.jsxaria-label— per-card inline fieldVideoTimelineEditor.jsxaria-label— header/transport chromeGuard change. The repo-wide
gives every input an accessible nametest inclient/src/a11yConventions.test.jscould not see through a page-local field wrapper that renders its children inside a real<label>(PipelineSeries.jsx's<Field label="…">). Three already-accessible controls were sitting inPREEXISTING_INPUT_NAME_ALLOWLISTpurely because of that static-analysis gap — adding a redundantaria-labelto each would have shadowed the visible text.isNestedInLabelWrappingComponent()now detects same-file components whose body renders{children}between<label>and</label>, mirroring the existingisNestedInLabeledFormField()check.Net: 20 entries removed from
PREEXISTING_INPUT_NAME_ALLOWLIST(the migration tracked in #4297), so every control above is now enforced by the guard instead of exempted.Pages from the issue list needing no change, verified per-tag:
PipelineSeries.jsx(all controls inside the localFieldlabel wrapper),CatalogIngredient.jsx(the one hit is a<input type=color>mention inside a code comment),OpenClaw.jsxandLoraDatasetDetail.jsx(already labeled).Test plan
cd client && npx vitest run src/a11yConventions.test.js— 10 passed.cd client && npx vitest run src/pages/{GitHub,Sharing,Browser,CharacterSheet,MoodBoardDetail,VideoTimelineEditor,PipelineSeries}— 7 files, 44 tests passed.cd client && npx biome lint --error-on-warningson every changed file — clean.isNestedInLabelWrappingComponent()tofalsefails the suite with exactly the threePipelineSeries.jsxoffenders and nothing else — confirming the helper is load-bearing and does not over-exempt any other file.Refs #4156
Remaining
The issue's "highest value first" page list is now complete, but the long tail behind it is not. Re-running the issue's scan across every tracked
client/src/**/*.jsxstill reports ~370 candidates (again roughly half true positives), concentrated in components rather than pages:components/cos/tabs/JobsTab.jsx(~22),components/brain/tabs/MemoryTab.jsx(~21),components/agents/tabs/WorldTab.jsx(~19),components/meatspace/tabs/AlcoholTab.jsx(~12),components/meatspace/tabs/NicotineTab.jsxandcomponents/apps/tabs/CustomTasksSection.jsx(~8 each), then a long tail of 1–7 per file.<input>only. Extendinggives every input an accessible nameto<select>and<textarea>is worth doing, but it surfaces a fresh allowlist and belongs in its own PR.Both are the same work as the
PREEXISTING_INPUT_NAME_ALLOWLISTmigration already tracked in #4297; #4156 stays open for them.