Skip to content

a11y(presets): GUI3 preset accessibility (#2338 #2339 #2345 #2350 #2352) - #2357

Merged
fl0rianr merged 2 commits into
kpoin/ui-testingfrom
feat/gui3-presets-a11y
Jun 22, 2026
Merged

fl0rianr merged 2 commits into
kpoin/ui-testingfrom
feat/gui3-presets-a11y

Conversation

@kpoineal

Copy link
Copy Markdown
Collaborator

Summary

Addresses all 5 NVDA screen-reader issues against the Presets UI 3 beta. No behaviour or visual regressions. One reviewer (Lovell) required.

Changes per issue

Closes #2338 — Preset parameter controls programmatic labels

All interactive controls now have programmatic accessible names via htmlFor/id (temperature, top_p, context size, top_k, repeat penalty, steps, CFG scale, engine hint, AutoOpt result, llamacpp_args, sdcpp_args). Image width/height use aria-label since they share one visual label.

Closes #2339 — Advanced backend/device fields discoverable

llamacpp_backend and llamacpp_device converted from plain text inputs to input[list] + datalist, exposing known selectable values (9 backends, 7 devices) while keeping free-text for custom values.

Closes #2345 — Preset card richer accessible description

Card overlay button gains aria-describedby pointing to a sr-only span with: starter/manual-args status, applies_to capabilities, parameter summary, prompt name, tools state.

Closes #2350 — Capability chip radio semantics

Capability chip container gains role=radiogroup. Each chip button gains role=radio + aria-checked. Radio chosen over aria-pressed because toggleCap is single-select.

Closes #2352 — AutoOpt run selection state

Each AutoOpt run button gains aria-pressed={selectedAutoRunId === run.id}. NVDA announces pressed/not-pressed state.

Test results

61 passed / 7 skipped / 0 failed (was 50/7/0). 11 new tests A35-A45.

Review notes

Needs human reviewer (not author). Arrow key navigation for radiogroup is a follow-up enhancement.

…chip/AutoOpt/card state

#2338 — Add htmlFor/id labels to all Preset parameter controls (temperature,
top_p, context size, top_k, repeat penalty, steps, CFG scale, engine hint,
AutoOpt result, llamacpp_args, sdcpp_args); aria-label on image width/height.

#2339 — llamacpp_backend and llamacpp_device converted from plain text inputs to
input[list] + datalist, exposing selectable known values (backends: auto, cpu,
cuda, vulkan, kompute, metal, rpc, opencl, mmap; devices: Auto, CPU, CUDA0,
CUDA1, Vulkan0, Vulkan1, Metal) while staying free-text for custom values.

#2345 — PresetCard overlay button gains aria-describedby pointing to a sr-only
span that includes: starter/manual-args state, applies_to capabilities,
parameter summary, prompt name, and tools state.

#2350 — Capability chip container gains role=radiogroup + aria-label; each chip
button gains role=radio + aria-checked to expose single-select state to AT.

#2352 — AutoOpt run buttons gain aria-pressed={selectedAutoRunId === run.id}
so AT announces selected/not-selected on each run card.

Tests: 50 → 61 passed, 7 skipped, 0 failed (11 new tests A35–A45).

Refs #2338 #2339 #2345 #2350 #2352

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@kpoineal

Copy link
Copy Markdown
Collaborator Author

Internal review: CHANGES SUGGESTED (advisory — human approval required)

Authored by Mattingly; reviewed by Lovell. Reviewer lockout in effect — Mattingly may NOT self-revise; a different agent must address the blocking issue.

[BLOCKER] #2350 — role="radiogroup" declared without arrow-key navigation (PresetManager.tsx ~lines 937–943)
The APG Radio Group pattern requires ArrowDown/Up to cycle focus through role="radio" children. Without an onKeyDown handler on the radiogroup, NVDA enters forms mode, expects arrow keys to work, and gets silence. This leaves capability chips less operable than the prior aria-pressed approach (which Tab/Space handled correctly). Deferring arrow-key nav as "a follow-up enhancement" while declaring role="radiogroup" is a WCAG 2.1.1 violation, not a gap. Fix: (a) add onKeyDown to the radiogroup handling ArrowUp/Down/Home/End; or (b) revert chips to aria-pressed toggle buttons — fully operable today with no keyboard model required.

Test gap — no ArrowKey coverage (tests/a11y.spec.ts A41–A43)
A41–A43 verify the role/aria-checked exist but no test asserts keyboard.press('ArrowDown') moves focus to the next chip. The suite passes today even with broken navigation.

All other items — no blocking issues

Action required: Assign a different agent (not Mattingly) to add arrow-key navigation to the capability chip radiogroup and a corresponding Playwright ArrowDown focus test.

…fix review)

Addresses review on #2350

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@kpoineal

Copy link
Copy Markdown
Collaborator Author

Review finding #2350 item 13 is addressed in commit 5beb8a3.

Switched capability chips from role="radiogroup" / role="radio" / aria-checked to role="group" / plain buttons / aria-pressed. Tests A41-A43 updated to assert toggle-button semantics. Full suite: 61 passed, 7 skipped, 0 failed.

@kpoineal

Copy link
Copy Markdown
Collaborator Author

Internal review (re-review): LGTM (advisory — human approval required)

Reviewer: Lovell | Re-review date: 2026-06-22

The fix correctly resolves the WCAG 2.1.1 keyboard gap I flagged in the first pass. Findings:

  • role="radio" / aria-checked fully removed. grep on the branch shows zero occurrences of role="radio", aria-checked, or role="radiogroup" in PresetManager.tsx. Clean sweep.
  • Toggle-button semantics are correct. Container carries role="group" aria-label="Applies to capabilities" (PresetManager.tsx ~line 937); each chip button carries aria-pressed={appliesTo.includes(cap)} with no role override — plain <button> semantics, keyboard-accessible via Tab + Enter/Space, no arrow-key contract. WCAG 2.1.1 gap is closed.
  • Tests A41–A43 are non-trivial and precise. A41 asserts role === 'group' and aria-label is truthy. A42 loops every chip, explicitly asserts role !== 'radio', and validates aria-pressed is strictly "true" or "false". A43 uses evaluateAll to count pressed states and asserts trueCount === 1 and falseCount === total - 1. No existing tests weakened.
  • Click/select behavior unchanged. onClick={() => toggleCap(cap)} is untouched; only the ARIA attributes were added. No visual or functional regression risk.

One minor advisory note (non-blocking): A41 checks aria-label is truthy rather than matching the specific string — acceptable coverage, but worth tightening if the label text ever needs enforcement.

No blocking issues. Cleared for human approval.

@fl0rianr fl0rianr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fl0rianr
fl0rianr merged commit a8718a7 into kpoin/ui-testing Jun 22, 2026
56 of 57 checks passed
@fl0rianr
fl0rianr deleted the feat/gui3-presets-a11y branch June 22, 2026 23:11
fl0rianr pushed a commit that referenced this pull request Jul 17, 2026
…) (#2357)

* a11y(presets): label params, add combobox for backend/device, expose chip/AutoOpt/card state

#2338 — Add htmlFor/id labels to all Preset parameter controls (temperature,
top_p, context size, top_k, repeat penalty, steps, CFG scale, engine hint,
AutoOpt result, llamacpp_args, sdcpp_args); aria-label on image width/height.

#2339 — llamacpp_backend and llamacpp_device converted from plain text inputs to
input[list] + datalist, exposing selectable known values (backends: auto, cpu,
cuda, vulkan, kompute, metal, rpc, opencl, mmap; devices: Auto, CPU, CUDA0,
CUDA1, Vulkan0, Vulkan1, Metal) while staying free-text for custom values.

#2345 — PresetCard overlay button gains aria-describedby pointing to a sr-only
span that includes: starter/manual-args state, applies_to capabilities,
parameter summary, prompt name, and tools state.

#2350 — Capability chip container gains role=radiogroup + aria-label; each chip
button gains role=radio + aria-checked to expose single-select state to AT.

#2352 — AutoOpt run buttons gain aria-pressed={selectedAutoRunId === run.id}
so AT announces selected/not-selected on each run card.

Tests: 50 → 61 passed, 7 skipped, 0 failed (11 new tests A35–A45).

Refs #2338 #2339 #2345 #2350 #2352

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* a11y(presets): use aria-pressed toggle buttons for capability chips (fix review)

Addresses review on #2350

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request web ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants