fix(A11Y): Low hanging fruit, aria labels - #1189
Conversation
🦋 Changeset detectedLatest commit: a19ddfc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
gjones
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 977eaf2. Configure here.
There was a problem hiding this comment.
🟡 Changes recommended
Several updated components introduce accessibility regressions (e.g., interactive elements hidden from assistive tech or missing proper interactive semantics) that should be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses WCAG audit findings by improving accessible naming/labeling across multiple UI components (icon-only controls, form errors, progress indicators, tabs, and sortable/table controls) and updates tests to assert the new accessibility behavior.
Changes:
- Add/adjust ARIA attributes (e.g.,
aria-label,aria-describedby,aria-invalid,aria-current,aria-sort,aria-busy,role="alert"/"status"), and mark decorative icons asaria-hidden. - Improve keyboard accessibility patterns in
FileTabs(rovingtabIndex, arrow-key navigation) and add targeted unit/VR test assertions. - Introduce consistent error element IDs/roles to enable
aria-describedbylinking for form controls.
File summaries
| File | Description |
|---|---|
| tests/display/multi-accordion.spec.ts | Updates VR assertions to validate trigger naming via title text. |
| tests/display/badge.spec.ts | Aligns VR tests with updated dismiss button accessible name. |
| tests/display/alert.spec.ts | Aligns VR tests with updated dismiss button accessible name. |
| tests/display/accordion.spec.ts | Updates VR assertions to validate trigger naming via title text. |
| src/components/VerticalStepper/VerticalStepper.tsx | Adds aria-current and hides decorative check icon. |
| src/components/Toast/Toast.tsx | Hides decorative toast icon; labels dismiss control. |
| src/components/TextField/TextField.tsx | Adds aria-busy, improves clear button label, hides icons. |
| src/components/Table/Table.tsx | Adds header scopes, aria-sort, and labels for selection/actions; hides decorative icons. |
| src/components/Table/Table.test.tsx | Adds test coverage for checkbox accessible names. |
| src/components/Switch/Switch.tsx | Removes redundant aria-label in favor of associated label. |
| src/components/SplitButton/SplitButton.tsx | Attempts to label secondary dropdown trigger; hides chevron icon. |
| src/components/Select/common/InternalSelect.tsx | Adds trigger labeling support and error associations; labels search/clear controls. |
| src/components/SearchField/SearchField.tsx | Hides decorative search/filter icon. |
| src/components/RadioGroup/RadioGroup.tsx | Adds group labeling and error associations; removes redundant item aria-label. |
| src/components/ProgressBar/ProgressBar.tsx | Adds progressbar semantics; labels/hides cancel control based on dismissable state. |
| src/components/ProgressBar/ProgressBar.test.tsx | Adds regression test to ensure cancel button isn’t inside the progressbar role subtree. |
| src/components/PasswordField/PasswordField.tsx | Adds toggle button labeling and pressed state; hides icon. |
| src/components/Pagination/Pagination.tsx | Labels pagination controls and page/rows-per-page inputs. |
| src/components/NumberField/NumberField.tsx | Adds aria-busy; hides loading icon. |
| src/components/NumberField/NumberField.test.tsx | Updates tests for new loading semantics (aria-busy, spinner presence). |
| src/components/MultiAccordion/MultiAccordion.tsx | Marks accordion icons aria-hidden. |
| src/components/InputWrapper/InputWrapper.tsx | Adds error context to apply aria-invalid/aria-describedby to wrapped inputs. |
| src/components/Icon/Icon.tsx | Stops exposing accessible names for aria-hidden icons by removing role/aria-label. |
| src/components/FormContainer/FormContainer.tsx | Adds htmlFor override and error element IDs/roles. |
| src/components/Flyout/Flyout.tsx | Hides decorative close icons. |
| src/components/FileUpload/FileUploadItem.tsx | Hides decorative icons; labels retry/remove actions. |
| src/components/FileUpload/FileUploadArea.tsx | Hides decorative upload icon. |
| src/components/FileUpload/FileUpload.tsx | Adds label to hidden file input. |
| src/components/FileUpload/FileMultiUpload.tsx | Adds label to hidden multi-file input. |
| src/components/FileTabs/FileTabs.tsx | Adds ARIA tab semantics and keyboard navigation; hides decorative tab icons; labels close action. |
| src/components/FileTabs/FileTabs.test.tsx | Adds tests for selection semantics and arrow-key navigation. |
| src/components/FileTabs/FileTabs.module.css | Shows close/indicator controls on focus-within (keyboard parity with hover). |
| src/components/Dialog/Dialog.tsx | Hides decorative close icon. |
| src/components/DatePicker/Common.tsx | Adds nav button labeling and supports hiding in views where nav is inert. |
| src/components/CrossButton/CrossButton.tsx | Adds an accessible name to the close button. |
| src/components/Collapsible/Collapsible.tsx | Adds aria-expanded, updates trigger labeling, hides decorative indicators. |
| src/components/CodeBlock/CodeBlock.tsx | Adds accessible names and pressed state for icon-only controls. |
| src/components/Checkbox/Checkbox.tsx | Removes redundant aria-label; hides decorative indicator icon. |
| src/components/CardPromotion/CardPromotion.tsx | Labels dismiss button; hides decorative icon. |
| src/components/Badge/Badge.tsx | Converts dismiss icon into a real button with an accessible name; hides icon. |
| src/components/Badge/Badge.module.css | Adds button-reset styling for the new badge close button. |
| src/components/Alert/Alert.tsx | Adds role based on alert severity; improves dismiss control labeling and icon handling. |
| src/components/Accordion/Accordion.tsx | Marks accordion icons aria-hidden. |
| .changeset/fix-aria-wcag-aa.md | Adds patch changeset describing the accessibility improvements. |
Review details
Suppressed comments (2)
src/components/InputWrapper/InputWrapper.tsx:189
- Same issue as InputElement: NumberInputElement spreads props after aria-describedby, allowing callers to accidentally override and lose the error linkage. Merge described-by values and apply the final aria-* attributes after spreading props.
<input
ref={ref}
aria-invalid={invalid || undefined}
aria-describedby={describedBy}
{...props}
src/components/InputWrapper/InputWrapper.tsx:213
- Same issue as InputElement: TextAreaElement spreads props after aria-describedby, so consumer props can override and remove the error association. Merge described-by values and ensure the final aria-* attributes are applied after the prop spread.
<textarea
ref={ref}
aria-invalid={invalid || undefined}
aria-describedby={describedBy}
{...props}
- Files reviewed: 44/44 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ble name. Co-authored-by: Cursor <cursoragent@cursor.com>
…name. Restore the banner layout spacer as a non-tabbable button, drop :has() for stylelint, and update ProgressBar snapshots now that cancel only renders when dismissable. Co-authored-by: Cursor <cursoragent@cursor.com>
Storybook Preview Deployed✅ Preview URL: https://click-dfsfhwnx0-clickhouse.vercel.app Built from commit: |
XOP
left a comment
XOP
left a comment
There was a problem hiding this comment.
The intention is great, however by doing so many updates the risk of unpredictable waterfall changes is growing exponentially.
My recommendation is to follow this plan:
- implement CR fixes, and iterate with the follow-up review
- create an artifact that we can test against project
- observe if there are any issues and if adjustments are mechanical or need more detailed work
- if more work is needed then I would suggest convert this to Draft and splitting this PR into 4 stages, where each stage is dedicated to the component tier (1 - form elements, 2 - navigation, 3 - tables, 4 - presentation and feedback)
- new PRs should be carried out sequentially, not simultaneously
Similar approach applies to #1190.
| } | ||
| role="tab" | ||
| // Explicit name so the close button's aria-label is not folded into the tab. | ||
| aria-label={child.props.text} |
There was a problem hiding this comment.
Use child?.props?.text, and guard the Close ${text} label at line 315.
| className={styles['custom-select']} | ||
| onSelect={onPageSizeChange} | ||
| value={pageSize.toString()} | ||
| aria-label="Rows per page" |
There was a problem hiding this comment.
Use aria-labelledby (hidden label id + trigger id) so "Rows per page NN rows" is announced.
|
|
||
| .tab:hover [data-type='close'] { | ||
| .tab:hover [data-type='close'], | ||
| .tabs-sortable > div:focus-within [data-type='close'] { |
There was a problem hiding this comment.
the display toggle under :focus-within can never receive focus in Chrome; the "reachable from the keyboard" claim is false
Drop the toggle, make the close icon mouse-only (tabIndex={-1}, aria-hidden), add Delete-key close in the tab's onKeyDown, and remove the claim from the changeset.
| data-testid="select-search-input" | ||
| onKeyDown={onKeyDown} | ||
| $showSearch={showSearch} | ||
| aria-label="Search options" |
There was a problem hiding this comment.
aria-label={showSearch ? 'Search options' : undefined}; add aria-hidden + tabIndex={-1} when !showSearch.
|
|
||
| const progressAria = { | ||
| role: 'progressbar' as const, | ||
| 'aria-label': ariaLabel, |
There was a problem hiding this comment.
Default 'aria-label': ariaLabel ?? (typeof label === 'string' ? label : 'Progress').
| <IconButton | ||
| disabled={disabled} | ||
| onClick={togglePasswordViewer} | ||
| aria-label={viewPassword ? 'Hide password' : 'Show password'} |
There was a problem hiding this comment.
Static aria-label="Show password" + aria-pressed, or changing label without aria-pressed.
| data-testid="progressbar-close" | ||
| className={closeButtonVariants({ dismissable })} | ||
| /> | ||
| {dismissable && ( |
There was a problem hiding this comment.
Calls for an additional story and respective tests update, both unit and visual
| id={id} | ||
| id={groupId} | ||
| dir={itemDir} | ||
| aria-labelledby={label ? labelId : undefined} |
There was a problem hiding this comment.
Merge ids like InputWrapper does.
| ref={fileInputRef} | ||
| accept={acceptedFileTypes} | ||
| onChange={handleFileSelect} | ||
| aria-label="Browse file" |
There was a problem hiding this comment.
aria-label on a display: none input is never read.
| name="check" | ||
| size="xs" | ||
| className={styles.step__check} | ||
| aria-hidden |
There was a problem hiding this comment.
aria-hidden on the completed-step check removed the only non-visual completion signal
Add visually hidden text ("Completed", "Upload complete", "Error:" / "Warning:").
Why?
A recent WCAG Audit showed that a few of our components were missing some aria-labels. This PR adds those relevant labels.