Skip to content

Generalize the Instances-only input-accessible-name a11y test repo-wide #4282

Description

@atomantic

Context

PR #4274 (issue #4156) added a regression test in client/src/a11yConventions.test.js (gives the inline Instances form controls accessible names, ~line 435) that scans a single hardcoded file — src/pages/Instances.jsx — for <input> elements missing aria-label/aria-labelledby.

A /simplify review of that PR (three independent review passes: reuse, simplification, altitude) converged on the same finding: every sibling rule in this same test file generalizes across the whole tree via trackedJsxFiles() (see gives every icon-only button an accessible name, ~line 408, and role="switch" without aria-checked, ~line 396) — this new test is the only one hardcoded to one page. Left as-is, a newly introduced unlabeled <input> on any other page ships with zero regression coverage.

Why it wasn't generalized in #4274

A naive repo-wide port of the same regex (<input\b + check for aria-label/aria-labelledby) would false-positive heavily: grep -rl '<input\b' client/src --include='*.jsx' matches 308 files, and the project's own documented convention (client/src/CLAUDE.md: "Form labels need htmlFor/id pairing") means most of those inputs are legitimately accessible via a paired <label htmlFor="...">, not via aria-label. A blind port would either fail CI across dozens of already-correct files or (if loosened to silence that) fail to actually catch anything new.

Decision (ready to implement)

Generalize the check to trackedJsxFiles(), but an input counts as accessibly-named if any of:

  1. aria-label or aria-labelledby is present on the tag (current check), OR
  2. The tag has id="X" and the same file contains <label ... htmlFor="X" (or htmlFor={'X'} / template-literal X) — reuse openingTagAt/lineOf helpers already in the test file, OR
  3. The <input> is nested inside a <label>...</label> element (implicit label wrapping) — a simple "does an enclosing <label> tag exist before this index with no intervening close" scan is sufficient; false negatives here are acceptable (a stricter follow-on can tighten it) as long as we don't false-positive on this common pattern, OR
  4. type="hidden" — excluded outright, no user-facing control to name.

Land this as a single generalized test (replacing the Instances-only one, keeping its doc comment about compact rows), not two tests. Run it once locally before opening the PR and hand-fix (or exempt with a documented reason, mirroring the button test's allowlist pattern) any genuine pre-existing offenders it surfaces — do not silently narrow the regex to avoid them.

Files

  • client/src/a11yConventions.test.js (~line 408 for the pattern to mirror, ~line 435 for the test to replace)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

in-progressClaimed and being workedplanTracked by /do:replan

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions