[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 wrappers
packages/pluggableWidgets/combobox-web/src/components/ComboboxMenuWrapper.tsx	Conditionally render <ul> only when isOpen; extract menuProps outside JSX
packages/pluggableWidgets/combobox-web/src/components/SingleSelection/SingleSelection.tsx	New aria-label / aria-labelledby logic based on open/selected state; added aria-busy
packages/pluggableWidgets/combobox-web/src/components/MultiSelection/MultiSelection.tsx	Same aria-label / aria-labelledby / aria-busy changes as SingleSelection
packages/pluggableWidgets/combobox-web/src/__tests__/SingleSelection.spec.tsx	New accessibility unit tests
packages/pluggableWidgets/combobox-web/src/__tests__/__snapshots__/*.snap	Snapshot updates reflecting all changes
packages/pluggableWidgets/combobox-web/e2e/Combobox.spec.js	New E2E test for menu-list DOM presence
openspec/changes/combobox-validation-a11y/	OpenSpec design artifacts

Skipped (out of scope): dist/, pnpm-lock.yaml, openspec/ (non-code), snapshots reviewed for correctness only.
Note: gh pr checks was not available in this environment — CI status could not be verified automatically.

---

Findings

🔶 Medium — aria-labelledby dropped in the default empty-closed state (regression)

Files: SingleSelection.tsx line 120, MultiSelection.tsx line 153

Problem: The new logic sets aria-labelledby={hasLabel && isOpen ? inputProps["aria-labelledby"] : undefined}. When the combobox is closed with no selection and hasLabel=true (the most common initial state), the condition is false, so aria-labelledby becomes undefined. The aria-label branch also produces undefined in this case because none of its conditions match:

aria-label={
    !isOpen && selectedItem && hasLabel  // false — no selectedItem
        ? ...
    : !isOpen && selectedItem            // false — no selectedItem
        ? ...
    : !hasLabel                          // false — hasLabel is true
        ? options.ariaLabel
    : undefined                          // ← reached here
}

So the input has no accessible name when empty and closed with a visible label. This is a regression from the original aria-labelledby={hasLabel ? inputProps["aria-labelledby"] : undefined} which always applied the label.

Fix: Preserve aria-labelledby as the base case when there is a label:

aria-label={
    !isOpen && selectedItem && hasLabel
        ? `${labelText}, ${selector.caption.get(selectedItem)}`
        : !isOpen && selectedItem
          ? selector.caption.get(selectedItem)
          : !hasLabel
            ? options.ariaLabel
            : undefined
}
aria-labelledby={hasLabel ? inputProps["aria-labelledby"] : undefined}
// Remove the && isOpen guard — the label should always be associated when present

The selected-value announcement is already handled by aria-label taking precedence when set (because aria-label overrides aria-labelledby when both present). When aria-label is undefined, aria-labelledby takes over naturally.

---

🔶 Medium — aria-controls references an element that no longer exists when closed

File: ComboboxMenuWrapper.tsx (indirect — Downshift sets aria-controls on the input via getInputProps)

Problem: Downshift's getInputProps returns aria-controls="downshift-N-menu" pointing to the <ul id="downshift-N-menu">. After this PR, the <ul> is only in the DOM when isOpen. When closed, aria-controls references a non-existent ID, which is an ARIA spec violation. Some screen readers (JAWS in particular) rely on aria-controls to locate the listbox and may behave unexpectedly.

The snapshot confirms this — aria-controls="downshift-0-menu" is present on the input even when aria-expanded="false" and the <ul> is gone.

Fix (preferred): Keep the <ul> in the DOM at all times (original approach), but conditionally render its children — which was the original pattern. That keeps aria-controls valid and avoids the dangling reference:

<ul className={...} {...menuProps}>
    {isOpen
        ? isEmpty && !isLoading
            ? <NoOptionsPlaceholder>{noOptionsText}</NoOptionsPlaceholder>
            : children
        : null}
    {isOpen && loader}
</ul>

The empty <ul role="listbox"> is harmless when closed — screen readers skip empty listboxes. Conditionally hiding the header/footer via isOpen && is still fine.

Alternatively: If conditional rendering of <ul> is kept, use Downshift's getMenuProps with a stable container ref that always exists (the outer <div>) to keep the referenced element in DOM.

---

🔶 Medium — Missing CHANGELOG entry

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

Problem: The [Unreleased] section has no entry for these accessibility fixes. These changes alter observable screen-reader behavior (icons no longer announced as "image", empty list no longer navigable), which qualifies as a user-visible bug fix per the changelog conventions.

Fix: Add under [Unreleased] > Fixed:

- We fixed an accessibility issue where decorative icons (down arrow, clear button) were announced as "image" by screen readers.
- We fixed an accessibility issue where an empty menu list structure was exposed to the accessibility tree when the combobox was closed.

Run pnpm -w changelog or edit manually.

---

⚠️ Low — E2E test assertion doesn't verify the element is absent from the DOM

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

Note: The test asserts await expect(menuListClosed).not.toBeVisible(). In Playwright, not.toBeVisible() passes both when the element doesn't exist and when it's hidden via CSS — so this would also have passed before this PR when the <ul> was always rendered but hidden. Use not.toBeAttached() to assert the element is actually absent from the DOM, which is what this change specifically aims to achieve:

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

---

⚠️ Low — labelText DOM read can produce a leading-comma aria-label on first render

Files: SingleSelection.tsx line 63, MultiSelection.tsx line 43

Note: const labelText = inputLabel?.textContent?.trim() || "" reads from the DOM element at render time. If the label element exists in the DOM (hasLabel=true) but has no text content yet (race on initial mount), labelText is "". The resulting aria-label would be ", selectedCaption" with a leading comma and space, which sounds odd through a screen reader. Consider using a useLayoutEffect or useMemo guard to default to the empty-label fallback rather than producing a malformed string:

const labelText = inputLabel?.textContent?.trim() ?? "";
// Guard in the aria-label expression:
: !isOpen && selectedItem && hasLabel && labelText
    ? `${labelText}, ${selector.caption.get(selectedItem)}`

---

Positives

• aria-hidden="true" on the icon wrapper <span> is the correct placement — it hides both the span and its SVG child from the accessibility tree in one attribute, and the clear button's parent <button> retains its aria-label for screen readers.
• Extracting menuProps before the JSX return (const menuProps = getMenuProps?.(...)) is the right way to satisfy Downshift's requirement that getMenuProps is called unconditionally, even when the <ul> is conditionally rendered.
• The new unit tests are well-structured using waitFor appropriately for async state changes, and cover all four key scenarios (closed, open with items, open empty, loading) as specified in the task list.
• aria-busy addition is a nice low-cost improvement that helps screen reader users understand when options are loading.

[r0b1n]

I believe we better create a helper function that covers this, also as AI review mentioned, there is a logic error where in empty it will have no label at all, we should address this in a helper function that we can unit test.