[WC-3418] Combobox accessibility improvements

[HedwigAR]

Pull request type

Bug fix (non-breaking change which fixes an issue)

---

Description

Fixes two WCAG accessibility violations in the combobox widget.

1. Decorative icons hidden from screen readers: Added aria-hidden="true" to DownArrow and ClearButton icon wrappers to prevent "image" announcements
2. Empty menu structures removed from accessibility tree: Menu <ul> now only renders when open, preventing screen readers from navigating to empty groups

What should be covered while testing?

Screen Reader Testing:

• Verify decorative icons (arrow, clear button) are NOT announced as "image"
• Verify no empty list structures when menu is closed
• Verify clear button announces with proper label text

Functional Testing:

• Test keyboard navigation (Tab, Enter, Arrow keys, Escape)
• Test single and multi-selection modes
• Verify validation attributes (aria-invalid, aria-describedby) still work

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
packages/pluggableWidgets/combobox-web/src/assets/icons.tsx	Added aria-hidden="true" to DownArrow and ClearButton icon wrapper spans
packages/pluggableWidgets/combobox-web/src/components/ComboboxMenuWrapper.tsx	Conditionally renders <ul> and surrounding content only when isOpen is true; extracted getMenuProps call
packages/pluggableWidgets/combobox-web/src/__tests__/SingleSelection.spec.tsx	New accessibility describe block with 7 unit tests
packages/pluggableWidgets/combobox-web/src/__tests__/__snapshots__/MultiSelection.spec.tsx.snap	Snapshot updated: aria-hidden on icon spans, menu <ul> removed when closed
packages/pluggableWidgets/combobox-web/src/__tests__/__snapshots__/SingleSelection.spec.tsx.snap	Snapshot updated: same as above
packages/pluggableWidgets/combobox-web/src/__tests__/__snapshots__/StaticSelection.spec.tsx.snap	Snapshot updated: same as above
packages/pluggableWidgets/combobox-web/e2e/Combobox.spec.js	New E2E test: menu list absent when closed, present when opened
openspec/changes/combobox-validation-a11y/**	New OpenSpec change artifacts (proposal, design, specs, tasks)
openspec/changes/enforce-validation-id-requirement/**	New OpenSpec change artifacts (proposal, design, specs, tasks — not yet implemented)

Skipped (out of scope): dist/, pnpm-lock.yaml, snapshot auto-generated content

CI checks: could not retrieve (permission denied) — manual verification recommended.

---

Findings

🔶 Medium — aria-controls broken when menu is not in DOM

File: packages/pluggableWidgets/combobox-web/src/components/ComboboxMenuWrapper.tsx line 52–59; snapshots confirm the issue

Problem: Downshift sets aria-controls="downshift-0-menu" on the combobox input element (confirmed in all three snapshots: aria-controls="downshift-0-menu"). That value references the id="downshift-0-menu" attribute that Downshift injects into the <ul> via getMenuProps. When isOpen is false, the <ul> is now removed from the DOM entirely, so aria-controls points to a non-existent element. Per ARIA spec, aria-controls must reference an element currently present in the DOM, or it is an authoring error. Screen readers (NVDA, JAWS) may either ignore or warn on a broken aria-controls reference, negating the accessibility benefit this PR aims to achieve.

The getMenuProps call was intentionally hoisted out of the conditional render (with a // Always call getMenuProps to satisfy Downshift requirements comment), but the ref that Downshift injects via getMenuProps is never attached to a DOM node when !isOpen. That means the ref is stale (pointing to null) whenever the menu closes and re-opens, which is precisely the scenario suppressRefError: true was added to silence.

Fix options (pick one):

Option A — Keep the <ul> in the DOM but hide it from the accessibility tree when closed:

<ul
    className={classNames("widget-combobox-menu-list", { ... })}
    aria-hidden={!isOpen || undefined}
    {...menuProps}
>
    {isOpen ? (isEmpty && !isLoading ? <NoOptionsPlaceholder>...</NoOptionsPlaceholder> : children) : null}
    {isOpen ? loader : null}
</ul>

This preserves the aria-controls target while hiding the empty list from AT when closed.

Option B — Remove aria-controls from the input when closed. This requires passing isOpen into the getInputProps call in SingleSelection.tsx and toggling the attribute. More invasive but semantically correct.

Option A is recommended — it restores the original DOM-always-present pattern for the <ul>, adds aria-hidden when closed (the actual fix for the empty-group problem), and keeps aria-controls intact.

---

🔶 Medium — E2E assertion uses not.toBeVisible instead of not.toBeInTheDocument

File: packages/pluggableWidgets/combobox-web/e2e/Combobox.spec.js line 181

Problem: The test comment says "Verify menu list is not in DOM when closed", but the assertion is:

await expect(menuListClosed).not.toBeVisible();

not.toBeVisible() passes if the element exists in the DOM but is hidden via CSS (e.g. visibility: hidden, display: none, opacity: 0). This test would not catch a regression where the <ul> is rendered but merely hidden — it would still pass, giving a false green. The intent is to verify the element is absent from the DOM.

Fix:

await expect(menuListClosed).toHaveCount(0);
// or equivalently:
await expect(comboBox.locator(".widget-combobox-menu-list")).not.toBeAttached();

---

⚠️ Low — Spinner test silently passes when spinner is absent

File: packages/pluggableWidgets/combobox-web/src/__tests__/SingleSelection.spec.tsx line 786–791

Note: The assertion if (spinner) { expect(spinner).not.toHaveAttribute("aria-hidden", "true"); } is wrapped in an if guard that allows the test to pass even when no spinner element is found. If the selector .widget-combobox-spinner changes or the spinner never renders in test conditions, this test does nothing. Consider asserting the spinner exists unconditionally once the loading state is triggered, or at minimum replacing the if with a direct assertion:

expect(spinner).toBeInTheDocument();
expect(spinner).not.toHaveAttribute("aria-hidden", "true");

---

⚠️ Low — CHANGELOG entry missing for this accessibility fix

File: packages/pluggableWidgets/combobox-web/CHANGELOG.md

Note: The [Unreleased] section only contains an unrelated flex-width fix. This PR changes observable widget behaviour (screen reader output, DOM structure of the menu). Per repo conventions, a changelog entry is expected. Suggest running pnpm -w changelog or manually adding under [Unreleased]:

### Fixed

- We fixed an accessibility issue where decorative icons were announced as "image" by screen readers.
- We fixed an accessibility issue where an empty menu structure was exposed to assistive technologies when the combobox was closed.

---

Positives

• The aria-hidden="true" placement on the <span> wrapper (rather than directly on <svg>) is the right approach — it hides both the container and the SVG in a single attribute.
• Extracting getMenuProps with a comment explaining the Downshift rules requirement shows good understanding of the library contract.
• The new unit tests cover the full matrix of closed/open/empty/loading states and use waitFor correctly for async open transitions.
• The OpenSpec design doc's risk/trade-off section proactively flags the aria-controls concern (though the mitigation chosen introduces the issue described above).
• The enforce-validation-id-requirement spec is thorough and well-structured for future implementation.