docs(Dropdown): add accessibility testing stories#3351
Draft
rivka-ungar wants to merge 2 commits into
Draft
Conversation
Add dedicated Storybook pages under "Dropdown Testing" section with comprehensive prop coverage for Single Select, Single Searchable, Multi Select, and Multi Searchable dropdown variants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Code Review by Qodo
1. Unused Text import
|
… value - Add aria-describedby on combobox input pointing to a visually-hidden span containing the selected value text, so screen readers announce the current selection when the input receives focus - Fix aria-selected on dropdown options to reflect actual selection state instead of Downshift's highlighted index - Add inputAriaLabel and label to all dropdown accessibility test stories - Remove redundant WithLabel stories from all 4 dropdown test pages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Code Review by Qodo
1. New ARIA attributes untested
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+47
to
+82
| const selectedValueId = useRef(`dropdown-selected-${Math.random().toString(36).slice(2, 9)}`).current; | ||
| const selectedValueText = useMemo( | ||
| () => getSelectedValueText(multi, selectedItem, selectedItems), | ||
| [multi, selectedItem, selectedItems] | ||
| ); | ||
|
|
||
| return ( | ||
| <> | ||
| {searchable ? ( | ||
| <BaseInput | ||
| {...getInputProps({ | ||
| "aria-labelledby": label ? getLabelProps().id : undefined, | ||
| "aria-label": inputAriaLabel || (label ? undefined : getLabelProps()?.id), | ||
| placeholder: hasSelection ? "" : placeholder, | ||
| ref: inputRef, | ||
| ...multipleSelectionDropdownProps | ||
| })} | ||
| inputRole="combobox" | ||
| value={inputValue || ""} | ||
| autoFocus={autoFocus} | ||
| size={inputSize || size} | ||
| className={cx(styles.inputWrapper, { | ||
| [styles.hasSelected]: !multi && selectedItem && !inputValue, | ||
| [styles.small]: inputSize === "small", | ||
| [styles.multi]: multi && hasSelection, | ||
| [styles.multiSelected]: multi && hasSelection && inputSize === "small", | ||
| [styles.fullWidth]: fullWidth | ||
| })} | ||
| disabled={disabled} | ||
| readOnly={readOnly} | ||
| /> | ||
| <> | ||
| <BaseInput | ||
| {...getInputProps({ | ||
| "aria-labelledby": label ? getLabelProps().id : undefined, | ||
| "aria-label": inputAriaLabel || (label ? undefined : getLabelProps()?.id), | ||
| "aria-describedby": selectedValueText ? selectedValueId : undefined, | ||
| placeholder: hasSelection ? "" : placeholder, | ||
| ref: inputRef, | ||
| ...multipleSelectionDropdownProps | ||
| })} | ||
| inputRole="combobox" | ||
| value={inputValue || ""} | ||
| autoFocus={autoFocus} | ||
| size={inputSize || size} | ||
| className={cx(styles.inputWrapper, { | ||
| [styles.hasSelected]: !multi && selectedItem && !inputValue, | ||
| [styles.small]: inputSize === "small", | ||
| [styles.multi]: multi && hasSelection, | ||
| [styles.multiSelected]: multi && hasSelection && inputSize === "small", | ||
| [styles.fullWidth]: fullWidth | ||
| })} | ||
| disabled={disabled} | ||
| readOnly={readOnly} | ||
| /> | ||
| <span id={selectedValueId} className={styles.visuallyHidden}> | ||
| {selectedValueText} | ||
| </span> |
Contributor
There was a problem hiding this comment.
1. New aria attributes untested 📘 Rule violation ☼ Reliability
This PR adds aria-selected on dropdown options and aria-describedby on the input, but the Dropdown unit tests do not assert these accessibility attributes. Without coverage, future changes can silently regress accessibility behavior.
Agent Prompt
## Issue description
The Dropdown component now sets `aria-describedby` on the input (pointing to a visually-hidden element containing the selected value(s)) and sets `aria-selected` on each rendered option, but there are no unit tests asserting these accessibility attributes.
## Issue Context
This PR specifically introduces/changes ARIA attributes for accessibility behavior; these should be covered by tests to prevent regressions.
## Fix Focus Areas
- packages/core/src/components/Dropdown/components/Trigger/DropdownInput.tsx[47-82]
- packages/core/src/components/Dropdown/components/DropdownBaseList/DropdownBaseList.tsx[63-66]
- packages/core/src/components/Dropdown/__tests__/Dropdown.test.tsx[36-112]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+47
to
+82
| const selectedValueId = useRef(`dropdown-selected-${Math.random().toString(36).slice(2, 9)}`).current; | ||
| const selectedValueText = useMemo( | ||
| () => getSelectedValueText(multi, selectedItem, selectedItems), | ||
| [multi, selectedItem, selectedItems] | ||
| ); | ||
|
|
||
| return ( | ||
| <> | ||
| {searchable ? ( | ||
| <BaseInput | ||
| {...getInputProps({ | ||
| "aria-labelledby": label ? getLabelProps().id : undefined, | ||
| "aria-label": inputAriaLabel || (label ? undefined : getLabelProps()?.id), | ||
| placeholder: hasSelection ? "" : placeholder, | ||
| ref: inputRef, | ||
| ...multipleSelectionDropdownProps | ||
| })} | ||
| inputRole="combobox" | ||
| value={inputValue || ""} | ||
| autoFocus={autoFocus} | ||
| size={inputSize || size} | ||
| className={cx(styles.inputWrapper, { | ||
| [styles.hasSelected]: !multi && selectedItem && !inputValue, | ||
| [styles.small]: inputSize === "small", | ||
| [styles.multi]: multi && hasSelection, | ||
| [styles.multiSelected]: multi && hasSelection && inputSize === "small", | ||
| [styles.fullWidth]: fullWidth | ||
| })} | ||
| disabled={disabled} | ||
| readOnly={readOnly} | ||
| /> | ||
| <> | ||
| <BaseInput | ||
| {...getInputProps({ | ||
| "aria-labelledby": label ? getLabelProps().id : undefined, | ||
| "aria-label": inputAriaLabel || (label ? undefined : getLabelProps()?.id), | ||
| "aria-describedby": selectedValueText ? selectedValueId : undefined, | ||
| placeholder: hasSelection ? "" : placeholder, | ||
| ref: inputRef, | ||
| ...multipleSelectionDropdownProps | ||
| })} | ||
| inputRole="combobox" | ||
| value={inputValue || ""} | ||
| autoFocus={autoFocus} | ||
| size={inputSize || size} | ||
| className={cx(styles.inputWrapper, { | ||
| [styles.hasSelected]: !multi && selectedItem && !inputValue, | ||
| [styles.small]: inputSize === "small", | ||
| [styles.multi]: multi && hasSelection, | ||
| [styles.multiSelected]: multi && hasSelection && inputSize === "small", | ||
| [styles.fullWidth]: fullWidth | ||
| })} | ||
| disabled={disabled} | ||
| readOnly={readOnly} | ||
| /> | ||
| <span id={selectedValueId} className={styles.visuallyHidden}> | ||
| {selectedValueText} | ||
| </span> |
Contributor
There was a problem hiding this comment.
2. Random describedby id 🐞 Bug ☼ Reliability
DropdownInput generates selectedValueId using Math.random(), so the rendered id/aria-describedby values are non-deterministic across renders. This can break SSR hydration (server/client markup mismatch) and makes DOM output unstable for tests and automation relying on consistent ids.
Agent Prompt
### Issue description
`DropdownInput` uses `Math.random()` to generate `selectedValueId`, which makes the DOM output non-deterministic. This can cause SSR hydration mismatch warnings/errors and creates unstable ids.
### Issue Context
The id is used by both `aria-describedby` on the `<input>` and the hidden `<span>` that provides the described-by text.
### Fix Focus Areas
- packages/core/src/components/Dropdown/components/Trigger/DropdownInput.tsx[1-83]
### Suggested change
- Replace the `useRef(Math.random()...)` id generation with `useId()` (React 18).
- If you want a more readable id, prefix it: `const selectedValueId = `dropdown-selected-${useId()}`;`.
- Update the React import accordingly (add `useId`, remove `useRef` usage for the id if no longer needed).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
|
📦 Bundle Size Analysis ✅ No bundle size changes detected. Unchanged Components
📊 Summary:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
https://monday.monday.com/boards/3532714909/views/113184182/pulses/11731479098
User description
Summary
Test plan
yarn storybook) and verify the "Dropdown Testing" section appears in the sidebar🤖 Generated with Claude Code
PR Type
Documentation
Description
Add "Dropdown Testing" section to Storybook sidebar navigation
Create 4 comprehensive testing pages for all dropdown variants
Each page includes 40+ stories covering states, sizes, ARIA labels, options, groups, and behaviors
Provide MDX documentation pages rendering all stories in single scrollable view
Diagram Walkthrough
File Walkthrough
1 files
Add Dropdown Testing to sidebar navigation4 files
Create single select dropdown testing storiesCreate single searchable dropdown testing storiesCreate multi select dropdown testing storiesCreate multi searchable dropdown testing stories4 files
Create single select documentation pageCreate single searchable documentation pageCreate multi select documentation pageCreate multi searchable documentation pagePR Type
Documentation, Tests, Enhancement
Description
Add accessibility enhancements to Dropdown component with screen reader support
getSelectedValueText()helper to generate accessible descriptions for selected valuesaria-describedbyfor screen reader announcementsaria-selectedattribute to dropdown options for proper state communicationCreate comprehensive Dropdown Testing section in Storybook with 4 dedicated pages
Provide MDX documentation pages rendering all stories in single scrollable view
Update Storybook sidebar navigation to include new "Dropdown Testing" section
Diagram Walkthrough
File Walkthrough
3 files
Trigger.module.scss
Add visually hidden CSS utility classpackages/core/src/components/Dropdown/components/Trigger/Trigger.module.scss
.visuallyHiddenCSS class for screen reader-only textand clip properties
DropdownInput.tsx
Enhance dropdown input with screen reader announcementspackages/core/src/components/Dropdown/components/Trigger/DropdownInput.tsx
getSelectedValueText()helper function to generate accessible textfor selected values
aria-describedbyto announceselections to screen readers
selectedValueIdref andselectedValueTextmemoization forperformance
DropdownBaseList.tsx
Add aria-selected attribute to dropdown optionspackages/core/src/components/Dropdown/components/DropdownBaseList/DropdownBaseList.tsx
aria-selectedattribute to BaseItem component4 files
DropdownSingleSelect.stories.tsx
Add single select dropdown testing storiespackages/docs/src/pages/components/Dropdown/DropdownSingleSelect.stories.tsx
variant
groups, and behaviors
configurations
directions
DropdownSingleSearchable.stories.tsx
Add single searchable dropdown testing storiespackages/docs/src/pages/components/Dropdown/DropdownSingleSearchable.stories.tsx
variant
renderers
DropdownMultiSelect.stories.tsx
Add multi select dropdown testing storiespackages/docs/src/pages/components/Dropdown/DropdownMultiSelect.stories.tsx
min visible count
DropdownMultiSearchable.stories.tsx
Add multi searchable dropdown testing storiespackages/docs/src/pages/components/Dropdown/DropdownMultiSearchable.stories.tsx
variant
configurations
4 files
DropdownSingleSelect.mdx
Add single select dropdown MDX documentation pagepackages/docs/src/pages/components/Dropdown/DropdownSingleSelect.mdx
labels, variants, options, groups, and behavior
page
DropdownSingleSearchable.mdx
Add single searchable dropdown MDX documentation pagepackages/docs/src/pages/components/Dropdown/DropdownSingleSearchable.mdx
variants, options, groups, and behavior
DropdownMultiSelect.mdx
Add multi select dropdown MDX documentation pagepackages/docs/src/pages/components/Dropdown/DropdownMultiSelect.mdx
sizes, ARIA labels, variants, options, and groups
DropdownMultiSearchable.mdx
Add multi searchable dropdown MDX documentation pagepackages/docs/src/pages/components/Dropdown/DropdownMultiSearchable.mdx
variations
1 files
preview.tsx
Add Dropdown Testing section to Storybook sidebarpackages/docs/.storybook/preview.tsx
"Dropdown Testing"to the Storybook sidebar navigationconfiguration
"Accessibility"and"Hooks"in thenavigation hierarchy
dedicated section