diff --git a/design-log/147 - jay-html validation rules catalog.md b/design-log/147 - jay-html validation rules catalog.md index c3699945..ada136ff 100644 --- a/design-log/147 - jay-html validation rules catalog.md +++ b/design-log/147 - jay-html validation rules catalog.md @@ -64,15 +64,21 @@ Package: `@jay-framework/a11y-validator` (monorepo, dev dependency) | Rule | Severity | Element | WCAG | What it checks | | ------------------------------ | -------- | ----------------------------------- | ----- | ------------------------------------------------------------------------------------- | | Image missing alt | error | `` | 1.1.1 | No `alt` attribute | -| Form input without label | error | ``, ``, `` | 1.3.1 | No ``, no wrapping ``, no `aria-label`/`aria-labelledby` | +| Form input without label | error | ``, ``, `` | 1.3.1 | No ``, no wrapping ``, no usable `aria-label`/`aria-labelledby` | +| Empty `aria-label` | error | labelable controls | 4.1.2 | `aria-label` present but empty/whitespace (DL#163) | +| Broken `aria-labelledby` | error | labelable controls | 1.3.1 | Empty `aria-labelledby` or id token(s) missing in file (DL#163) | +| Duplicate `id` | error | any | 4.1.1 | Same `id` value used more than once in the file (DL#163) | | Button without accessible name | error | `` | 4.1.2 | No text, no `aria-label`, no `aria-labelledby`, no child `` | | Media autoplay without muted | error | ``, `` | 1.4.2 | `autoplay` attribute present without `muted` | | Invalid ARIA role | error | any | 4.1.2 | `role` attribute value not in WAI-ARIA role list | | Viewport disables zoom | error | `` | 1.4.4 | `user-scalable=no` or `maximum-scale` < 2 in viewport meta (via `ctx.head`) | | Positive tabindex | warning | interactive + `[role]` | 2.4.3 | `tabindex` > 0 disrupts natural tab order | | Focusable without role | warning | non-interactive | 4.1.2 | `` or similar without `role` — screen readers don't know what it is | +| Multiple controls in `` | warning | `` | 1.3.1 | More than one labelable control nested in one label (DL#163) | +| Orphan `label[for]` | warning | `` | 1.3.1 | `for` points to an `id` that does not exist in the file (DL#163) | +| Adjacent duplicate text | warning | any | — | Adjacent siblings with identical visible text (screen readers announce twice) | -The form label rule skips `type="hidden"`, `type="submit"`, `type="button"`, and `type="reset"` inputs. +The form label rule skips `type="hidden"`, `type="submit"`, `type="button"`, and `type="reset"` inputs. Labelable inputs include `checkbox` and `radio` (DL#163). Empty `aria-label` / unresolved `aria-labelledby` do not count as an accessible name. ## Rule Overlap diff --git a/design-log/163 - a11y form and label validation rules.md b/design-log/163 - a11y form and label validation rules.md new file mode 100644 index 00000000..80080faf --- /dev/null +++ b/design-log/163 - a11y form and label validation rules.md @@ -0,0 +1,166 @@ +# Design Log #163 — A11y Form and Label Validation Rules + +## Background + +`@jay-framework/a11y-validator` (DL#145, DL#147) already checks that text-like inputs, selects, and textareas have an associated label via ``, wrapping ``, `aria-label`, or `aria-labelledby`. + +Gaps remain for common form/accessibility mistakes that AI agents and designers make in jay-html: + +- `checkbox` / `radio` are skipped by `LABELABLE_INPUTS` +- empty `aria-label=""` and unresolved `aria-labelledby` pass as “labeled” +- multiple form controls inside one `` are accepted +- duplicate `id` values are not flagged +- orphan `` pointing at a missing `id` is not flagged + +## Problem + +Static validation under-reports WCAG 1.3.1 / 4.1.2 form naming issues. Agents self-correct only when findings include actionable `suggestion` text. + +## Questions and Answers + +### Q1: One PR or several? + +**A:** One PR for the whole form/label scope. Nesting of interactive elements, `lang`, and runtime axe stay out of scope. + +### Q2: Severity? + +**A:** + +| Finding | Severity | +| ------- | -------- | +| Missing label (incl. checkbox/radio) | error | +| Empty `aria-label` | error | +| `aria-labelledby` with missing/empty id refs | error | +| Duplicate `id` | error | +| Multiple labelable controls in one `` | warning | +| Orphan `label[for]` (no matching `id`) | warning | + +### Q3: Does wrapping `` still count for checkbox/radio? + +**A:** Yes — same association rules as other inputs. Explicit `for`/`id` preferred when multiple controls share a visual group; `fieldset`/`legend` is not required in this PR. + +### Q4: How does `aria-labelledby` resolve? + +**A:** Split on whitespace; every token must match an element `id` in the same jay-html file. Missing any token → error. Empty attribute → error. + +### Q5: Version bump in the PR? + +**A:** No. Maintainers bump `@jay-framework/a11y-validator` on release (currently `0.22.2`). + +## Design + +### Rules (additions / tightenings) + +1. **Extend labelable inputs** — add `checkbox`, `radio` to `LABELABLE_INPUTS`. Still skip `hidden`, `submit`, `button`, `reset`. + +2. **Empty `aria-label`** — if attribute is present and `trim()` is empty → error (do not treat as labeled). + +3. **Broken `aria-labelledby`** — if present: empty / only whitespace → error; any id token not found in the document → error. If all ids resolve, treat as labeled (skip “no label” finding). + +4. **Multiple controls in one ``** — count labelable descendants (`input` except ignored types, `select`, `textarea`). If count > 1 → warning on ``. + +5. **Duplicate `id`** — collect all `id` attributes; any value used more than once → error per duplicate occurrence after the first (or one finding listing the id). + +6. **Orphan `label[for]`** — if `for` is non-empty and no element has that `id` → warning. + +### Implementation approach + +Single pre-pass over the DOM: + +- `Set` / `Map` of all `id` → count +- `Set` of existing ids for ARIA/`for` lookup +- for each ``, count labelable descendants and check `for` + +Then existing `walkElements` + tightened `checkLabel`. + +### Examples + +✅ Good: + +```html +Email + + + Agree + + +Option A +``` + +❌ Bad: + +```html + + + +From To + + +Name +``` + +## Implementation Plan + +### Phase 1: Design log + index + +1. This document + `design-log/index.md` entry under validation/plugins. + +### Phase 2: Tests + +2. Vitest cases for each rule (pass + fail). No `toContain` on code files. + +### Phase 3: Implementation + +3. Update `a11y-validator.ts` helpers and `validate`. +4. Run package tests. + +### Phase 4: Catalog + results + +5. Append new rows to DL#147 a11y table. +6. Append Implementation Results here. + +## Verification Criteria + +1. checkbox/radio without association → error +2. `aria-label=""` → error; non-empty → passes label check +3. `aria-labelledby` missing target → error +4. two inputs in one label → warning +5. duplicate ids → error +6. `label for` without matching id → warning +7. Existing label/`alt`/button/tabindex tests still pass + +## Trade-offs + +| Decision | Benefit | Cost | +| -------- | ------- | ---- | +| Per-file id uniqueness only | Matches jay-html validation model | Won't catch cross-file collisions | +| No fieldset/legend rule yet | Keeps PR focused | Radio groups still weak without legend | +| Warning for multi-control label | Avoids hard break on legacy templates | Agents may ignore warnings | + +## Out of Scope + +- Nested interactive (`` in ``) +- `` +- Runtime axe / focus / toast timing +- Color-only / contrast (design-system-validator) + +## Implementation Results + +### Phase 2–3: Tests + code + +- Extended `LABELABLE_INPUTS` with `checkbox` / `radio` +- Tightened `checkLabel` for empty `aria-label` and resolved `aria-labelledby` tokens against file ids +- Pre-pass: duplicate `id` counts, orphan `label[for]`, multi-control `` +- Tests added in `a11y-validator.test.ts` + +### Test results + +`packages/plugins/a11y-validator`: **54/54 passing** + +### Catalog + +Updated DL#147 a11y rules table with the new rows. + +### Deviations from design + +None material. Multi-control warning message wording uses “contains N form controls” (same intent as designed). diff --git a/design-log/index.md b/design-log/index.md index 4754b7fa..0802abd3 100644 --- a/design-log/index.md +++ b/design-log/index.md @@ -111,6 +111,7 @@ Quick reference to find relevant design logs by topic. Design logs capture desig | 160 | [deprecate editor packages](160%20-%20deprecate%20editor%20packages) | Move editor-client/protocol/server to \_deprecated; remove from stack-cli | | 161 | [markdown image url resolution](161%20-%20markdown%20image%20url%20resolution) | Rewrite relative image URLs in markdown; copy media to public; CDN mapping | | 162 | [structural headfull components](162%20-%20structural%20headfull%20components) | Allow headfull components without .ts code file; passthrough from contract | +| 163 | [a11y form and label validation rules](163%20-%20a11y%20form%20and%20label%20validation%20rules) | Extend a11y-validator: checkbox/radio, ARIA name integrity, duplicate ids, label hygiene | --- @@ -164,6 +165,7 @@ Quick reference to find relevant design logs by topic. Design logs capture desig | 153 | [npm create jay](153%20-%20npm%20create%20jay) | Interactive project scaffolding: name, plugin selection, agent-kit, setup banner | | 158 | [staged npm publish](158%20-%20staged%20npm%20publish) | Two-phase publish: stage all packages without OTP, then bulk-approve with single OTP | | 147 | [jay-html validation rules catalog](147%20-%20jay-html%20validation%20rules%20catalog) | Complete catalog of all validation rules across wix-media, SEO, and a11y | +| 163 | [a11y form and label validation rules](163%20-%20a11y%20form%20and%20label%20validation%20rules) | Extend a11y-validator: checkbox/radio, ARIA name integrity, duplicate ids, label hygiene | --- diff --git a/packages/plugins/a11y-validator/lib/validators/a11y-validator.ts b/packages/plugins/a11y-validator/lib/validators/a11y-validator.ts index 120f3c78..e2e57288 100644 --- a/packages/plugins/a11y-validator/lib/validators/a11y-validator.ts +++ b/packages/plugins/a11y-validator/lib/validators/a11y-validator.ts @@ -108,13 +108,33 @@ const LABELABLE_INPUTS = new Set([ 'color', 'file', 'range', + 'checkbox', + 'radio', ]); +const IGNORED_INPUT_TYPES = new Set(['hidden', 'submit', 'button', 'reset']); + export const validate: JayHtmlValidatorFn = (ctx) => { const findings: JayHtmlValidationFinding[] = []; const labelForIds = new Set(); + const allIds = new Set(); + const idCounts = new Map(); + + collectDomIndex(ctx.body, labelForIds, allIds, idCounts); + + for (const [id, count] of idCounts) { + if (count > 1) { + findings.push({ + severity: 'error', + message: `Duplicate id="${id}" used ${count} times (WCAG 4.1.1)`, + suggestion: + 'Give each element a unique id. Duplicate ids break label associations and ARIA references.', + attribute: 'id', + }); + } + } - collectLabelForIds(ctx.body, labelForIds); + checkLabelsStructure(ctx.body, allIds, findings); walkElements(ctx.body, ctx, (el) => { const tag: string | undefined = el.rawTagName?.toLowerCase(); @@ -139,14 +159,14 @@ export const validate: JayHtmlValidatorFn = (ctx) => { // --- Rule: input/select/textarea must have label --- if (tag === 'input') { const type = (el.getAttribute?.('type') || 'text').toLowerCase(); - if (type === 'hidden' || type === 'submit' || type === 'button' || type === 'reset') { + if (IGNORED_INPUT_TYPES.has(type)) { return; } if (!LABELABLE_INPUTS.has(type)) return; - checkLabel(el, tag, findings, labelForIds); + checkLabel(el, tag, findings, labelForIds, allIds); } if (tag === 'select' || tag === 'textarea') { - checkLabel(el, tag, findings, labelForIds); + checkLabel(el, tag, findings, labelForIds, allIds); } // --- Rule: button must have accessible name --- @@ -285,12 +305,64 @@ function checkLabel( tag: string, findings: JayHtmlValidationFinding[], labelForIds: Set, + allIds: Set, ): void { const id = el.getAttribute?.('id'); const ariaLabel = el.getAttribute?.('aria-label'); const ariaLabelledBy = el.getAttribute?.('aria-labelledby'); - if (ariaLabel || ariaLabelledBy) return; + let hasAccessibleName = false; + + if (ariaLabel !== undefined && ariaLabel !== null) { + if (!String(ariaLabel).trim()) { + findings.push({ + severity: 'error', + message: `<${tag}> has empty aria-label (WCAG 4.1.2)`, + suggestion: + 'Provide a non-empty aria-label, use aria-labelledby with an existing id, ' + + 'or associate a .', + element: `<${tag}>`, + attribute: 'aria-label', + }); + } else { + hasAccessibleName = true; + } + } + + if (ariaLabelledBy !== undefined && ariaLabelledBy !== null) { + const tokens = String(ariaLabelledBy) + .trim() + .split(/\s+/) + .filter(Boolean); + if (tokens.length === 0) { + findings.push({ + severity: 'error', + message: `<${tag}> has empty aria-labelledby (WCAG 4.1.2)`, + suggestion: + 'Set aria-labelledby to one or more element ids that exist in this file, ' + + 'or use a non-empty aria-label / .', + element: `<${tag}>`, + attribute: 'aria-labelledby', + }); + } else { + const missing = tokens.filter((token) => !allIds.has(token)); + if (missing.length > 0) { + findings.push({ + severity: 'error', + message: `<${tag}> aria-labelledby references missing id(s): ${missing.join(', ')} (WCAG 1.3.1)`, + suggestion: + `Add element(s) with id="${missing[0]}" (or fix the aria-labelledby tokens), ` + + 'or use a / non-empty aria-label instead.', + element: `<${tag}>`, + attribute: 'aria-labelledby', + }); + } else { + hasAccessibleName = true; + } + } + } + + if (hasAccessibleName) return; if (id && labelForIds.has(id)) return; // Check if wrapped in a @@ -311,14 +383,84 @@ function checkLabel( }); } -function collectLabelForIds(el: any, ids: Set): void { +function collectDomIndex( + el: any, + labelForIds: Set, + allIds: Set, + idCounts: Map, +): void { + const id = el.getAttribute?.('id'); + if (id) { + allIds.add(id); + idCounts.set(id, (idCounts.get(id) ?? 0) + 1); + } + if (el.rawTagName?.toLowerCase() === 'label') { const forId = el.getAttribute?.('for'); - if (forId) ids.add(forId); + if (forId) labelForIds.add(forId); + } + + for (const child of el.childNodes ?? []) { + if (child.nodeType === 1) collectDomIndex(child, labelForIds, allIds, idCounts); } +} + +function isLabelableControl(el: any): boolean { + const tag = el.rawTagName?.toLowerCase(); + if (tag === 'select' || tag === 'textarea') return true; + if (tag !== 'input') return false; + const type = (el.getAttribute?.('type') || 'text').toLowerCase(); + if (IGNORED_INPUT_TYPES.has(type)) return false; + return LABELABLE_INPUTS.has(type); +} + +function countLabelableDescendants(el: any): number { + let count = 0; for (const child of el.childNodes ?? []) { - if (child.nodeType === 1) collectLabelForIds(child, ids); + if (child.nodeType !== 1) continue; + if (isLabelableControl(child)) count += 1; + count += countLabelableDescendants(child); } + return count; +} + +function checkLabelsStructure( + root: any, + allIds: Set, + findings: JayHtmlValidationFinding[], +): void { + function walk(el: any): void { + if (el.rawTagName?.toLowerCase() === 'label') { + const forId = el.getAttribute?.('for'); + if (forId && !allIds.has(forId)) { + findings.push({ + severity: 'warning', + message: ` has no matching id in this file (WCAG 1.3.1)`, + suggestion: + `Add id="${forId}" to the related form control, or fix the for attribute.`, + element: '', + attribute: 'for', + }); + } + + const controlCount = countLabelableDescendants(el); + if (controlCount > 1) { + findings.push({ + severity: 'warning', + message: ` contains ${controlCount} form controls — screen readers only associate the first (WCAG 1.3.1)`, + suggestion: + 'Use a separate for each input (or one wrapping label per control). ' + + 'multiple form controls inside one label is not reliable.', + element: '', + }); + } + } + + for (const child of el.childNodes ?? []) { + if (child.nodeType === 1) walk(child); + } + } + walk(root); } function getVisibleText(el: any): string { diff --git a/packages/plugins/a11y-validator/test/validators/a11y-validator.test.ts b/packages/plugins/a11y-validator/test/validators/a11y-validator.test.ts index b9384fcf..52c52f6c 100644 --- a/packages/plugins/a11y-validator/test/validators/a11y-validator.test.ts +++ b/packages/plugins/a11y-validator/test/validators/a11y-validator.test.ts @@ -114,6 +114,166 @@ describe('a11y-validator', () => { }), ]); }); + + it('flags checkbox without label', async () => { + const ctx = makeContext(''); + const findings = await validate(ctx); + expect(findings).toEqual([ + expect.objectContaining({ + severity: 'error', + element: '', + message: expect.stringContaining('WCAG 1.3.1'), + }), + ]); + }); + + it('passes checkbox wrapped in label', async () => { + const ctx = makeContext(' Agree'); + const findings = await validate(ctx); + expect(findings).toEqual([]); + }); + + it('flags radio without label', async () => { + const ctx = makeContext(''); + const findings = await validate(ctx); + expect(findings).toEqual([ + expect.objectContaining({ + severity: 'error', + element: '', + message: expect.stringContaining('WCAG 1.3.1'), + }), + ]); + }); + + it('passes radio with label[for]', async () => { + const ctx = makeContext( + 'Option A', + ); + const findings = await validate(ctx); + expect(findings).toEqual([]); + }); + + it('flags empty aria-label on input', async () => { + const ctx = makeContext(''); + const findings = await validate(ctx); + expect(findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + severity: 'error', + attribute: 'aria-label', + message: expect.stringContaining('empty aria-label'), + }), + ]), + ); + }); + + it('flags aria-labelledby pointing to missing id', async () => { + const ctx = makeContext(''); + const findings = await validate(ctx); + expect(findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + severity: 'error', + attribute: 'aria-labelledby', + message: expect.stringContaining('aria-labelledby'), + }), + ]), + ); + }); + + it('passes aria-labelledby when target id exists', async () => { + const ctx = makeContext( + 'Search', + ); + const findings = await validate(ctx); + expect(findings).toEqual([]); + }); + + it('flags empty aria-labelledby', async () => { + const ctx = makeContext(''); + const findings = await validate(ctx); + expect(findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + severity: 'error', + attribute: 'aria-labelledby', + }), + ]), + ); + }); + }); + + describe('multiple controls in label', () => { + it('flags label with two inputs', async () => { + const ctx = makeContext( + 'From To ', + ); + const findings = await validate(ctx); + expect(findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + severity: 'warning', + element: '', + message: expect.stringContaining('form controls'), + }), + ]), + ); + }); + + it('passes label with a single input', async () => { + const ctx = makeContext('Name '); + const findings = await validate(ctx); + expect(findings).toEqual([]); + }); + }); + + describe('duplicate ids', () => { + it('flags duplicate id attributes', async () => { + const ctx = makeContext(''); + const findings = await validate(ctx); + expect(findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + severity: 'error', + attribute: 'id', + message: expect.stringContaining('Duplicate id="x"'), + }), + ]), + ); + }); + + it('passes unique ids', async () => { + const ctx = makeContext( + 'A', + ); + const findings = await validate(ctx); + expect(findings).toEqual([]); + }); + }); + + describe('orphan label for', () => { + it('flags label for without matching id', async () => { + const ctx = makeContext('Name'); + const findings = await validate(ctx); + expect(findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + severity: 'warning', + element: '', + attribute: 'for', + message: expect.stringContaining('for="missing"'), + }), + ]), + ); + }); + + it('passes label for with matching id', async () => { + const ctx = makeContext( + 'Name', + ); + const findings = await validate(ctx); + expect(findings).toEqual([]); + }); }); describe('button accessible name', () => {