Skip to content

a11y(forms,omni,icons): combobox + durable labels + icon names (#2347 #2349 #2353) - #2364

Merged
fl0rianr merged 1 commit into
kpoin/ui-testingfrom
feat/gui3-forms-icons-a11y
Jun 23, 2026
Merged

fl0rianr merged 1 commit into
kpoin/ui-testingfrom
feat/gui3-forms-icons-a11y

Conversation

@kpoineal

Copy link
Copy Markdown
Collaborator

Summary

Group F accessibility fixes for blind NVDA user on the GUI3 branch. Three issues resolved in prototype/ui-redesign/.


#2347 — OmniComponentPicker full combobox semantics

File: src/components/ModelManager.tsx (OmniComponentPicker, ~lines 807–920)

The custom Omni model component picker lacked combobox ARIA semantics entirely. Changes:

  • Added role="combobox", aria-expanded, aria-controls (pointing to the listbox id), aria-activedescendant (tracking the focused option by id), and aria-autocomplete="list" to the input.
  • Keyboard navigation: ArrowDown/Up move the active option through the flat options list; Enter selects the active option; Escape closes without selecting.
  • Converted <button role="option"> to <div role="option"> — buttons are not allowed to own role="option" per ARIA spec.
  • Moved the HuggingFace search action outside role="listbox" to avoid aria-required-children violations.
  • Label now associated via htmlFor/id pair.
  • Added .omni-component-picker__option--focused CSS for keyboard-navigation highlight.
  • Clear button gained aria-label matching its title.

Closes #2347


#2349 — ConnectView cloud/marketplace form durable labels

File: src/components/ConnectView.tsx

The cloud provider form (provider name, base URL, API key) and the marketplace search relied solely on placeholders. Changes:

  • Cloud provider form: three bare inputs now have <label className="sr-only" htmlFor=...> with matching id attributes.
  • Edit-API-key inline input (per-provider row): aria-label naming the provider.
  • Marketplace search: aria-label="Search marketplace apps".
  • Added aria-describedby hint span for the base URL format.
  • Server URL and API key fields in the Server section were already correctly labeled — untouched.

Closes #2349


#2353 — Icon-only/title-only controls reliable accessible names

Files: src/components/LogViewer.tsx, src/components/MarkdownMessage.tsx, src/components/ModelManager.tsx

Swept components outside the Groups A–E conflict zone:

  • LogViewer: search input → aria-label="Filter logs"; Clear button → aria-label="Clear log output"; Reconnect button → aria-label="Reconnect to log stream".
  • MarkdownMessage: code-block copy button (built as raw HTML string) → aria-label="Copy code" added to the template.
  • OmniComponentPicker clear button → aria-label matching the role name (also in item 10 scope).

Closes #2353


Tests

9 new Playwright tests (A35–A43) added to tests/a11y.spec.ts:

  • A35–A38: Omni picker combobox — role=combobox present, aria-expanded toggles, Escape closes, listbox visible on open, htmlFor/id label association verified.
  • A39–A40: Connect form fields found by getByLabel; marketplace search aria-label verified.
  • A41–A43: LogViewer search aria-label; Clear button aria-label; Omni clear button aria-label.

Test result: 59 passed / 7 skipped / 0 failed (was 50/7/0 before this PR; +9 new tests all green).


A human reviewer should verify NVDA screen reader UX on the Omni collection form before merging.

@kpoineal

Copy link
Copy Markdown
Collaborator Author

Internal review: LGTM (advisory — human approval required)

Authored by Mattingly; reviewed by Lovell.

  • OmniComponentPicker combobox (a11y: Custom Omni component picker has incomplete combobox semantics #2347, ModelManager.tsx ~lines 807–920): The key fix — <button role="option"> → <div role="option"> — is correct; ARIA prohibits buttons from owning role="option". role="combobox" + aria-expanded + aria-controls (pointing to listbox) + aria-activedescendant (tracking active option by id) + aria-autocomplete="list" is the correct complete ARIA combobox pattern. HF search action moved outside role="listbox" removes the aria-required-children violation. role="group" + aria-label on each option group is valid inside a listbox. Group label div gets aria-hidden="true" to prevent duplication. ✅
  • ConnectView.tsx cloud form (a11y: Connect/cloud forms still have placeholder-only fields #2349): <label className="sr-only" htmlFor=...> with matching id on all three cloud provider inputs is durable labeling (survives placeholder=empty, placeholder attribute loss). aria-describedby hint span for base URL format is a useful addition. Inline API key edit input uses aria-label naming the provider — correct for a dynamically created input with no adjacent persistent label. ✅
  • LogViewer.tsx / MarkdownMessage.tsx (a11y: Some icon-only or title-only controls lack reliable accessible names #2353): aria-label on search, Clear, and Reconnect buttons is straightforward. Code-block copy button receives aria-label="Copy code" in the generated HTML string — the title attribute remains for pointer users. ✅
  • Test A43 (Omni picker clear button) is conditionally weak — if no value is pre-selected, the clear button is absent and the test passes trivially. Low risk for this PR (the clear button's aria-label is a static template attribute), but consider seeding a pre-selected value in the test fixture to make the assertion unconditional.
  • Tests A35–A43: 9 new tests; A35–A38 cover the combobox ARIA attributes and keyboard open/close/ArrowDown behavior; A39–A40 cover durable form labels using getByLabel (which exercises the actual accessible name computation, not just presence of an attribute). ✅
  • No behavior or visual regressions detected.

@github-actions github-actions Bot added app documentation Improvements or additions to documentation labels Jun 22, 2026
@kpoineal

Copy link
Copy Markdown
Collaborator Author

Rebased onto merged base a8718a7. Conflicts resolved in ests/a11y.spec.ts (branch's new tests renumbered to A71–A79) and ACCESSIBILITY.md (union merge). Final suite: 70 passed / 7 skipped / 0 failed. — Mattingly

@kpoineal
kpoineal force-pushed the feat/gui3-forms-icons-a11y branch 2 times, most recently from e8e4600 to a1a9d26 Compare June 23, 2026 15:06
@kpoineal

Copy link
Copy Markdown
Collaborator Author

Rebase round 2 — rebased onto \kpoin/ui-testing\ @ e0d1c92 (#2360 backend merged)

Conflicts resolved in \ ests/a11y.spec.ts\ and \ACCESSIBILITY.md\ by keeping both sides (base's A35–A45 presets + A51–A58 backend AND this branch's A71–A79 forms/omni/icons tests). \styles.css\ auto-merged cleanly. No renumbering needed — ranges already disjoint.

Final suite: 78 passed / 7 skipped / 0 failed

— Mattingly

@kpoineal
kpoineal force-pushed the feat/gui3-forms-icons-a11y branch from a1a9d26 to 389dfee Compare June 23, 2026 15:56
@kpoineal

Copy link
Copy Markdown
Collaborator Author

Rebase round 3 complete — rebased onto \kpoin/ui-testing\ @ 7eb0c6c (post #2359 models merge).

Conflicts resolved by keeping both sides:

  • \ModelManager.tsx: auto-merged cleanly — base's row-action accessible-name changes (a11y(models): qualify model row action accessible names (#2341) #2359, lines ~460–530) and this branch's OmniComponentPicker combobox changes (~lines 807–920) are in different regions of the file; no manual intervention needed.
  • \ ests/a11y.spec.ts: retained base A35–A58 (presets/models/backend) and this branch's A71–A79 (Omni picker combobox, connect form labels, icon-button names). No renumbering.
  • \ACCESSIBILITY.md: union-merged both sides' table rows and last-updated line.

Full suite result: 83 passed / 7 skipped / 0 failed. Branch force-pushed to \origin/feat/gui3-forms-icons-a11y.

— Mattingly

…-button names

Closes #2347 #2349 #2353

- role=combobox, aria-expanded, aria-controls, aria-activedescendant, aria-autocomplete=list
- Arrow key navigation (Down/Up move active option), Enter selects, Escape closes
- Converted <button role=option> to <div role=option> (buttons can't own role=option)
- Moved HF search action outside role=listbox to avoid aria-required-children violation
- Label associated via htmlFor/id pair; options get stable id for activedescendant
- Added .omni-component-picker__option--focused CSS for keyboard-navigation highlight
- Clear button gained aria-label

- Cloud provider form: 3 bare inputs now have <label class=sr-only htmlFor>+id
- Edit-API-key inline input: aria-label naming the provider
- Marketplace search: aria-label='Search marketplace apps'
- aria-describedby hint span for base URL format

- LogViewer search input: aria-label='Filter logs'
- LogViewer Clear button: aria-label='Clear log output'
- LogViewer Reconnect button: aria-label='Reconnect to log stream'
- MarkdownMessage code-block copy button: aria-label='Copy code' in generated HTML
- OmniComponentPicker clear button: aria-label=Clear {role}

Tests: 9 new tests A35-A43; full suite 59 passed / 7 skipped / 0 failed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@kpoineal
kpoineal force-pushed the feat/gui3-forms-icons-a11y branch from 389dfee to 6bc7fbe Compare June 23, 2026 16:19
@kpoineal

Copy link
Copy Markdown
Collaborator Author

Rebase round 4 complete — rebased onto \1c3bb93f\ (merged base after #2362 download a11y).

Conflict resolution: kept both sides throughout:

Test results: 87 passed / 7 skipped / 0 failed ✅

— Mattingly

@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 2d431d1 into kpoin/ui-testing Jun 23, 2026
17 checks passed
@fl0rianr
fl0rianr deleted the feat/gui3-forms-icons-a11y branch June 23, 2026 16:27
fl0rianr pushed a commit that referenced this pull request Jul 17, 2026
…-button names (#2364)

Closes #2347 #2349 #2353

- role=combobox, aria-expanded, aria-controls, aria-activedescendant, aria-autocomplete=list
- Arrow key navigation (Down/Up move active option), Enter selects, Escape closes
- Converted <button role=option> to <div role=option> (buttons can't own role=option)
- Moved HF search action outside role=listbox to avoid aria-required-children violation
- Label associated via htmlFor/id pair; options get stable id for activedescendant
- Added .omni-component-picker__option--focused CSS for keyboard-navigation highlight
- Clear button gained aria-label

- Cloud provider form: 3 bare inputs now have <label class=sr-only htmlFor>+id
- Edit-API-key inline input: aria-label naming the provider
- Marketplace search: aria-label='Search marketplace apps'
- aria-describedby hint span for base URL format

- LogViewer search input: aria-label='Filter logs'
- LogViewer Clear button: aria-label='Clear log output'
- LogViewer Reconnect button: aria-label='Reconnect to log stream'
- MarkdownMessage code-block copy button: aria-label='Copy code' in generated HTML
- OmniComponentPicker clear button: aria-label=Clear {role}

Tests: 9 new tests A35-A43; full suite 59 passed / 7 skipped / 0 failed

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

app documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants