Skip to content

A11y: clickable divs/spans need keyboard + screen-reader support #268

Description

@CarlosNZ

Surfaced while writing tests for the edit flow (#264). The OK / Cancel icons in InputButtons are <div onClick> wrappers around SVG icons — no role, no aria-label, not in the natural tab order. That has two real costs:

  1. Accessibility. Screen readers don't announce these as actionable. Keyboard-only users can't focus or activate them — they can only confirm/cancel an edit via the Enter / Escape key bindings, which means there's no keyboard equivalent for the visible UI affordance. Same problem extends to the icon-style buttons elsewhere (edit, delete, copy, custom buttons).
  2. Testability. Without an accessible role/name, getByRole('button', { name: 'OK' }) doesn't work; tests fall back to class-based selectors like container.querySelectorAll('.jer-confirm-buttons > div')[0] (see JsonEditor.test.tsx — the two OK/Cancel-icon tests at the bottom of the edit-flow describe). That's fragile to markup refactors and worse for documentation than a query that reads like a sentence.

Audit scope

A quick grep shows interactive handlers across:

Around 20 click/dblClick handlers in total. Each should be assessed individually — some are deliberately divs because they wrap larger zones or non-action affordances (e.g., the collapse click-zone, the value display itself which is also draggable). Others are clearly missing button semantics.

Suggested approach

  1. Distinguish two categories:
    • Real buttons (OK, Cancel, edit, delete, copy, custom action buttons) → convert to <button type=\"button\" aria-label=\"...\"> with a CSS reset for native styling.
    • Click affordances on display elements (dblClick on a value/key, collapse click-zone, drag handles) → these aren't "buttons" semantically. Use role=\"button\" + tabIndex=0 + onKeyDown (Enter/Space) where keyboard activation matters; leave alone where it doesn't (e.g. dblClick to edit also has a keyboard path via Tab + Enter, so keyboard parity is preserved at the suite level even if the per-element handler is mouse-only).
  2. Wire labels through the existing localisation system (src/localisation.ts) so the announced text is translatable.

⚠️ Required follow-up: update the tests

This work has a tightly-coupled test-side companion. Once roles and accessible names exist, the existing class-based selectors must be replaced with role-based queries, both as a correctness check (the new roles actually work) and because it's the test layer this issue is partly motivated by.

Concretely:

  • test/JsonEditor.test.tsx — the two tests in the edit flow describe block that click the OK and Cancel icons currently use container.querySelectorAll('.jer-confirm-buttons > div')[N]. Replace with screen.getByRole('button', { name: /ok/i }) (or whatever localised label is wired up) and the matching cancel query.
  • Sweep the rest of test/JsonEditor.test.tsx and any test files added by the time this lands — anywhere .jer-* class selectors are used to interact with an element (not just to assert structure), convert to role-based queries if the element now has a role.
  • Treat "PR passes tests using only role queries for the converted buttons" as part of the acceptance criteria.

Out of scope

Full a11y audit (focus management, focus trapping during edit, ARIA-live for error messages, contrast). Worth a separate pass once button semantics are sorted, because some of those depend on having real focusable elements first.

Acceptance

  • All actionable icon buttons reachable by keyboard, focusable, announced by screen reader
  • Class-based selectors in tests that target the converted buttons replaced with getByRole(...) queries (see ⚠️ above)
  • No regressions in pnpm test or the demo

Metadata

Metadata

Assignees

No one assigned

    Labels

    V2To include in Version 2

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions