ReviewOS

stacks/bench-review

a11y: review-form star ratings fail WCAG — unnamed icon buttons, color-only state, hidden focus, no skip link

#58
Open glennmichael123 opened this 9 days ago ·
9 days ago

Problem

Bench Review's core conversion flow — rating a judge — is practically unusable with a screen reader. Blind and low-vision litigants are squarely in this platform's audience, and as a US public-facing site this is ADA Title III / Unruh exposure on top of the usability gap. The codebase has deliberate a11y work elsewhere (tablist filters, role="dialog" mobile menu, aria-live error banner in the review form itself), so this is not a full-audit ask: a targeted one-day pass on the review flow closes the worst of it.

Findings

  • All four star-rating widgets have no accessible name or state. The fairness (resources/components/Bench/ReviewForm.stx:457-468), knowledge (:478-489), demeanor (:499-510), and overall (:539-550) rows render five <button>s carrying only type/class/x-class/@click/@mouseenter/@mouseleave; the <Icon name="star"/> child expands to a bare <svg> with no <title>/aria-hidden. A screen-reader user hears five identical unnamed buttons per row. Confirmed in built output: dist/judges/review.html contains a single aria-label on the whole page.
  • Dangling label for= attributes. ReviewForm.stx:454/475/496/536 use for="fairness"/"knowledge"/"demeanor"/"rating", but no element with those ids exists in the rendered page — the targets are the unlabeled buttons.
  • Selected state is color-only (WCAG 1.4.1). x-class toggles text-yellow-400 vs text-gray-300 (ReviewForm.stx:462/483/504/544) with no non-color cue (fill vs outline, checkmark, text).
  • focus:outline-none with no replacement (WCAG 2.4.7). Exactly the four star rows (ReviewForm.stx:461/482/503/543) plus one non-tabbable menu container (resources/components/Bench/Judge/ProfileHeader.stx:161). All other focus:outline-none uses in Bench components already pair a focus:ring-2/focus-visible:ring-2 replacement, so scope is contained.
  • No skip link. No skip-to-content anchor anywhere under resources/; resources/layouts/default.stx:3 has <main> but no skip target.
  • <html lang="en"> is verified present in built output (dist/judges/review.html) — no action needed.

Acceptance criteria

  • Each star widget exposes accessible names and state: either role="radiogroup" with per-star role="radio"/aria-checked, or per-button aria-label="Rate {n} of 5" + aria-pressed
  • The four label for= attributes resolve to real ids, or labels become group captions wired via aria-labelledby
  • Selected stars are distinguishable without color (e.g., filled vs outlined icon)
  • focus:outline-none on the four star rows is replaced with a visible focus-visible:ring-2 (or equivalent) style
  • A skip-to-content link precedes the header and targets <main id="main"> in resources/layouts/default.stx
  • Manual VoiceOver pass: a user can complete a full review submission (all four ratings) end-to-end

From the 2026-06-10 production-readiness audit.


Runtime pass against a live form (6b7cb792)

The body's checkboxes were stale — five were landed across the earlier passes and never ticked here. They are ticked now. The sixth, the manual VoiceOver pass, is still open and is still not something I can do.

What I could do that the earlier passes could not: drive the real widget against a working API. Every previous comment on this issue says some version of "markup inspection, not assistive-technology testing", and one notes the form could not be reached at all because the judge lookup needs the API. A stub API on :4321 serving the built dist/ closes that gap.

A role that promised what nothing implemented

role="radiogroup" is a contract, not a label: it tells a screen reader that Tab enters the group once and arrow keys move within it. The five stars were separately tabbable buttons with no key handling — so the role announced a keyboard behaviour that did nothing, which is exactly the defect the "More" menu had (role="menu" with no roving focus). There the answer was to drop the roles, because it was a disclosure. Here the role is correct for a single-select of five, so the contract is now implemented: roving tabindex, arrows with wrap, Home/End, selection following focus.

Two defects the live pass found

The live region announced the wrong rating. hoverRating takes precedence over rating in both the star fill and the aria-live readout, and nothing cleared it on a keyboard change — mouseleave only fires for a pointer. After hovering one star and arrowing to another, the DOM read:

aria-checked  → "Rate 4 of 5"
aria-live     → "Poor"          (which is 1)

That region is the only confirmation a non-sighted user gets that anything registered, and it was confidently wrong. Not reachable by reading the template — both values had to be read out of a live DOM.

A held arrow key never advanced. The first implementation computed the next value from the star that received the event, but focus moves on a later tick, so every repeat of a held key arrived at the star the user started on and re-set the same value. Measured: three presses from star 1 landed on star 2. It reads the rating signal now, so repeats advance.

Checkbox name, 258 → 28 characters. The anonymity <label> wrapped both the heading and the full explanation, so a screen reader announced all 258 characters as the control's name before ever saying "checkbox". aria-labelledby now names it and the explanation moved to aria-describedby, which is announced after the name and is skippable. The whole card still toggles on click.

Verified end to end, keyboard only

Title typed → rating set with arrow keys → body typed in the editor reached by Tab → Submit reached by Tab and fired with Return → "Review submitted". No pointer involved after the judge was chosen. Also confirmed live: "Skip to content" is the first focusable element, exactly one star is in the tab order at a time, aria-checked tracks the selection, End→5 and a further Right wraps to 1, and the rich-text editable is Tab-reachable — a tabIndex === -1 property reading suggested otherwise, and driving it proved the property wrong rather than the page.

Gate check 25

The repo has now hit "ARIA role promising keyboard behaviour that nothing implements" twice, so it is guarded: a file claiming radiogroup/menu/menubar/tablist/listbox/tree/grid has to handle keys somewhere. It strips comments first — its own first run flagged ProfileHeader.stx, whose comment explains the roles it removed.

What is still open

Only the VoiceOver pass. Everything above is DOM state, computed accessible names, and real keyboard driving — it is a much stronger claim than markup inspection, and it is still not the same as listening to the page. NVDA/JAWS behaviour in particular can differ from what the accessibility tree suggests, and the aria-live politeness and whether the readout interrupts are things only a listen will settle.

8 days ago

Most of this is done in 12c556e. Scope note first: the audit described four rating rows (fairness / knowledge / demeanor / overall); the form has one now, so this covers that one plus the sitewide items.

  • Accessible names and state — role="radiogroup" named through aria-labelledby, each star a role="radio" with aria-label="Rate N of 5" and aria-checked. The value readout beside it is aria-live="polite", which is the only confirmation a non-sighted user gets that a click registered.
  • Dangling label for= — removed. There was nothing for it to name: the widget is five buttons, not one control. It is a caption with an id now, referenced by the group.
  • Selected state without colour — filled glyph when chosen, outline when not. Worth noting this was only possible as of 2ae27d8: until then <Icon name="heroicons:star-solid"> rendered an empty element (#91), so the stars were invisible buttons — a worse problem than the one this issue describes, and invisible to the audit because the markup looked right.
  • focus:outline-none replaced — now pairs with focus-visible:ring-2 on the stars.
  • Skip link — first focusable element in resources/layouts/default.stx, visually hidden until focused, targeting the <main id="main"> the layout already owned. Verified: one Tab from a cold load focuses "Skip to content", which renders as a visible pill and points at #main.
  • Manual VoiceOver pass — still outstanding.

One thing I could not verify and am not claiming: the radiogroup's runtime behaviour end to end. The form sits behind a judge selection that needs the API, and with it down bindIf removes the form before it can be inspected. The markup is confirmed present in the built artifact, and the x-aria-* bindings resolve through stx's generic x-<attr> path — confirmed live on the header's x-aria-expanded, which is consumed and writes aria-expanded false→true on toggle.

Leaving this open for the VoiceOver pass and a run against a live API.

8 days ago

Triage — partially landed, still the best-value open issue. Keeping it open.

Landed since filing:

  • Skip link. resources/layouts/default.stx carries a skip-to-content link and <main id="main"> for it to target.
  • Icons are real elements now. 49 <Icon> tags across 24 files used to render nothing — no Icon component existed anywhere, and stx drops unresolved PascalCase tags silently. resources/components/Icon.stx exists and they render, which is a precondition for naming any of them.
  • Some aria-label coverage has appeared in ReviewForm.stx (3 occurrences).
  • Related pattern work: the status-filter and sort controls use role="group" + toggle buttons with aria-pressed, deliberately not role="tab" — the tab role promises a tabpanel and roving focus this app does not have, and a screen reader announced "tab, 1 of 4" with nowhere to go.

Not done, and these are the substance of the issue:

  • Star ratings still need a proper accessible control (name, state not conveyed by colour alone, visible focus)
  • A real VoiceOver pass — nothing here has been driven with a screen reader, only reasoned about
  • A runtime check against the live API rather than the static artifact

Flagging my own limitation honestly: everything above is inspection of markup, not assistive-technology testing. The remaining work needs someone to actually listen to the page.

8 days ago

Five of six criteria are met (9d269e8e, 0d6fc6cf). The sixth needs a human with a screen reader.

  • Star widget exposes names and state. role="radiogroup" named by #rating-label, five children with role="radio", aria-checked, and aria-label="Rate N of 5". A screen reader hears "Rate 4 of 5, radio button, selected" where it used to hear five identical unnamed buttons.
  • No dangling label for=. for="rating" became a <span id="rating-label"> wired by aria-labelledby — there is no single control called "rating", so a label was the wrong shape. for="review" was the harder one: MediumEditor builds the editable at runtime, so no template id can exist. The host now marks itself data-richtext-label and RichTextEditor sets aria-labelledby on the editable after construction, plus role="textbox" and aria-multiline. Measured on the built page: zero labels pointing at a missing id.
  • Selected stars survive without colour. Filled vs outlined icon, not amber vs grey. At rating 4, stars 1–4 render solid and star 5 outlined; at rating 1, only star 1 is solid.
  • Focus is visible. Keyboard focus on a star matches :focus-visible and paints a blue ring. The residual nobody had spotted: the title input paired focus:outline-none with a focus ring only in its healthy x-class branch, so a field flagged invalid had no focus indicator at the exact moment a keyboard user was sent back to it.
  • Skip link precedes the header and targets <main id="main">.
  • Manual VoiceOver pass. Not done, and not something I can do — everything above is DOM inspection and keyboard driving, not listening to the page.

Three of the four widgets in this issue no longer exist. The per-aspect fairness/knowledge/demeanor scorecard was dropped for a single overall rating, so the finding reduced to one widget.

Two defects found while here, outside the issue's text:

  • The judge profile's mobile "More" menu was entirely unreachable by keyboard — role="menu" + role="menuitem" + tabindex="-1" on all three elements. That pattern promises arrow-key roving focus that nothing implements, so the trigger was the only focusable thing and opening it led nowhere. It is a disclosure, not a menu; the roles and negative tabindexes are gone and both items are back in the tab order.
  • Its aria-expanded was a static "false", so the panel opened while a screen reader was told it stayed shut. It tracks state now.

Gate check 22 — "nothing removes the focus outline without a replacement" — now guards both shapes repo-wide, including the x-class-branch case a reviewer cannot see in built output because only one branch ever renders. It found the settings email field (readonly, so still tabbable, with no focus indicator) and the menu panel's dead focus:outline-none on its first run.

Leaving this open for the VoiceOver pass.

Sign in to comment on this issue.