|
| 1 | +## Context |
| 2 | + |
| 3 | +The combobox widget currently has two accessibility issues affecting screen reader users: |
| 4 | + |
| 5 | +1. **Decorative icons announced as "image"**: The DownArrow and ClearButton SVG icons are exposed to assistive technologies, causing screen readers to announce them as "image" elements. These icons are purely decorative - they duplicate information already conveyed through the combobox role, button roles, and ARIA labels. |
| 6 | + |
| 7 | +2. **Empty menu structure exposed**: When the combobox has no options, the `<ul>` menu element is still rendered in the DOM and exposed to the accessibility tree, creating an empty group that screen reader users can navigate to but provides no value. |
| 8 | + |
| 9 | +**Current Implementation:** |
| 10 | + |
| 11 | +- `icons.tsx`: DownArrow and ClearButton components render SVGs wrapped in `<span className="widget-combobox-icon-container">` |
| 12 | +- `ComboboxWrapper.tsx`: Renders the DownArrow inside a container div |
| 13 | +- `ComboboxMenuWrapper.tsx`: Always renders the `<ul>` element, showing NoOptionsPlaceholder when empty |
| 14 | + |
| 15 | +**Constraints:** |
| 16 | + |
| 17 | +- Must not affect visual presentation |
| 18 | +- Must preserve existing keyboard and mouse interaction behavior |
| 19 | +- Must maintain compatibility with existing ARIA attributes (aria-invalid, aria-describedby) |
| 20 | + |
| 21 | +## Goals / Non-Goals |
| 22 | + |
| 23 | +**Goals:** |
| 24 | + |
| 25 | +- Hide decorative icon SVGs from assistive technologies using `aria-hidden="true"` |
| 26 | +- Prevent empty menu structures from being exposed to screen readers when no options exist |
| 27 | +- Maintain WCAG 2.2 AA compliance |
| 28 | +- Preserve all existing functionality and visual design |
| 29 | + |
| 30 | +**Non-Goals:** |
| 31 | + |
| 32 | +- Not changing the validation ARIA attributes (already correctly implemented) |
| 33 | +- Not modifying icon visual appearance or interaction behavior |
| 34 | +- Not addressing other accessibility improvements beyond these two specific issues |
| 35 | + |
| 36 | +## Decisions |
| 37 | + |
| 38 | +### Decision 1: Add aria-hidden to icon wrapper spans |
| 39 | + |
| 40 | +**Approach:** Add `aria-hidden="true"` to the `<span className="widget-combobox-icon-container">` wrapper in both DownArrow and ClearButton components. |
| 41 | + |
| 42 | +**Rationale:** |
| 43 | + |
| 44 | +- These icons are purely decorative - they reinforce information already available through: |
| 45 | + - DownArrow: The combobox role implies expandability; the actual button has proper ARIA labels |
| 46 | + - ClearButton: The button element has `aria-label` with clear text (from `clearButtonAriaLabel` prop) |
| 47 | +- Adding aria-hidden on the wrapper span hides both the span and its SVG child from the accessibility tree |
| 48 | +- This is simpler and more maintainable than adding aria-hidden to each SVG element individually |
| 49 | + |
| 50 | +**Alternatives considered:** |
| 51 | + |
| 52 | +- Add aria-hidden directly on SVG elements: More granular but requires changes in multiple places; wrapper approach is cleaner |
| 53 | +- Use `role="presentation"`: Less explicit than aria-hidden for hiding decorative content |
| 54 | +- Remove icons from DOM when read-only: Overcomplicates the code for minimal benefit |
| 55 | + |
| 56 | +### Decision 2: Conditionally render menu list when empty |
| 57 | + |
| 58 | +**Approach:** In `ComboboxMenuWrapper.tsx`, only render the `<ul>` element when the menu is open AND there are items or loading state. When empty and closed, skip rendering the list structure entirely. |
| 59 | + |
| 60 | +**Rationale:** |
| 61 | + |
| 62 | +- An empty `<ul>` with no `<li>` children creates an empty group/list in the accessibility tree |
| 63 | +- Screen readers can navigate to this empty group, providing no value and creating confusion |
| 64 | +- Conditional rendering ensures the menu structure only exists when it has meaningful content |
| 65 | +- The NoOptionsPlaceholder should still be visible when open but empty (useful for sighted users) |
| 66 | + |
| 67 | +**Implementation approach:** |
| 68 | + |
| 69 | +```tsx |
| 70 | +// Only render <ul> when menu is open |
| 71 | +{ |
| 72 | + isOpen && ( |
| 73 | + <ul className="..."> |
| 74 | + {isEmpty && !isLoading ? <NoOptionsPlaceholder>{noOptionsText}</NoOptionsPlaceholder> : children} |
| 75 | + {loader} |
| 76 | + </ul> |
| 77 | + ); |
| 78 | +} |
| 79 | +``` |
| 80 | + |
| 81 | +**Alternatives considered:** |
| 82 | + |
| 83 | +- Add aria-hidden to empty `<ul>`: Would still render unnecessary DOM elements |
| 84 | +- Use `role="presentation"` on empty list: Still exposed to some assistive technologies |
| 85 | +- Always render but hide with CSS: Doesn't solve accessibility tree exposure |
| 86 | + |
| 87 | +### Decision 3: Preserve spinner loader accessibility |
| 88 | + |
| 89 | +**Approach:** Keep the SpinnerLoader in `ComboboxWrapper.tsx` accessible (no aria-hidden) as it provides meaningful loading state information. |
| 90 | + |
| 91 | +**Rationale:** |
| 92 | + |
| 93 | +- Unlike the decorative arrow, the spinner indicates an active loading state |
| 94 | +- Screen readers should announce loading status to users |
| 95 | +- The spinner is already rendered conditionally (`isLoading ? <SpinnerLoader /> : <DownArrow />`) |
| 96 | + |
| 97 | +## Risks / Trade-offs |
| 98 | + |
| 99 | +**[Risk]** Adding aria-hidden might hide icons from assistive technologies that users rely on for spatial navigation |
| 100 | +→ **Mitigation:** These icons are purely decorative and duplicate information already available through proper ARIA labels and roles. The underlying button elements remain fully accessible. |
| 101 | + |
| 102 | +**[Risk]** Conditional rendering of `<ul>` might cause layout shifts or affect CSS selectors |
| 103 | +→ **Mitigation:** The menu is already hidden when closed via CSS classes. Removing it from DOM when closed won't affect visible behavior. Test thoroughly to ensure no CSS regressions. |
| 104 | + |
| 105 | +**[Risk]** Changes might affect existing E2E tests that query for menu elements when closed |
| 106 | +→ **Mitigation:** Review and update E2E tests if they attempt to query closed menu elements. This is actually a test improvement - tests should verify elements exist only when they should be visible. |
| 107 | + |
| 108 | +**[Trade-off]** Slightly more complex conditional rendering logic in ComboboxMenuWrapper |
| 109 | +→ **Accepted:** The improvement in accessibility tree cleanliness outweighs the minor complexity increase. The logic is straightforward and well-documented. |
0 commit comments