refactor: migrate accessibility props - #4750
Closed
lukewalczak wants to merge 3 commits into
Closed
lukewalczak wants to merge 3 commits into
lukewalczak wants to merge 3 commits into
Conversation
|
The mobile version of example app from this branch is ready! You can see it here. |
|
Hey @lukewalczak, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved accessibility compatibility regressions and disabled interaction-test coverage remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
refactor: migrate accessibility props updates component accessibility semantics to ARIA props and refreshes testing dependencies, tests, and snapshots.
Changes:
- Migrates legacy accessibility roles, labels, states, and live regions.
- Updates testing-library dependencies and lockfile.
- Adapts queries and refreshes component snapshots.
File summaries
| File | Reviewed change |
|---|---|
yarn.lock |
Updates locked testing dependencies. |
src/components/Tooltip/Tooltip.tsx |
Migrates live-region semantics. |
src/components/ToggleButton/ToggleButton.tsx |
Migrates toggle accessibility props. |
src/components/TextInput/Adornment/TextInputAffix.tsx |
Migrates affix role and label. |
src/components/Snackbar.tsx |
Migrates icon label. Nit (1 vote): migrate remaining live-region and action role props. |
src/components/SegmentedButtons/SegmentedButtonItem.tsx |
Migrates button accessibility props. |
src/components/Searchbar.tsx |
Migrates control labels. Nit (1 vote): migrate remaining role props. |
src/components/RadioButton/RadioButtonItem.tsx |
Migrates radio semantics. |
src/components/RadioButton/RadioButtonIOS.tsx |
Migrates radio semantics and live-region behavior. |
src/components/RadioButton/RadioButtonGroup.tsx |
Migrates radio-group semantics. |
src/components/RadioButton/RadioButtonAndroid.tsx |
Migrates radio semantics and live-region behavior. |
src/components/ProgressBar.tsx |
Migrates progress semantics and values. |
src/components/Modal.tsx |
Migrates dialog semantics. |
src/components/Menu/MenuItem.tsx |
Migrates menu-item semantics. Moderate (3 votes): retain or translate accessibilityState. |
src/components/Menu/Menu.tsx |
Migrates menu and overlay semantics. Nit (1 vote): migrate the remaining overlay role. |
src/components/List/ListAccordion.tsx |
Migrates expanded-state semantics. |
src/components/IconButton/IconButton.tsx |
Migrates button accessibility props. Moderate (2 votes): retain a legacy fallback or document the minimum RN version. |
src/components/FAB/FABGroup.tsx |
Migrates FAB group semantics. |
src/components/FAB/FAB.tsx |
Migrates FAB roles, labels, and states. Moderate (1 vote): merge accessibilityState with disabled state. |
src/components/FAB/AnimatedFAB.tsx |
Migrates animated FAB semantics. Moderate (1 vote) each: retain accessibilityState compatibility and forward ARIA state props to the interactive element. |
src/components/Drawer/DrawerItem.tsx |
Migrates drawer-item semantics. |
src/components/Drawer/DrawerCollapsedItem.tsx |
Migrates collapsed drawer-item semantics. |
src/components/Dialog/DialogTitle.tsx |
Migrates heading semantics. |
src/components/DataTable/DataTablePagination.tsx |
Migrates pagination labels. |
src/components/Chip/Chip.tsx |
Migrates chip semantics. Moderate (3 votes): retain the default “Close” label. Moderate (1 vote): preserve accessibilityRole compatibility. |
src/components/Checkbox/CheckboxItem.tsx |
Migrates checkbox semantics. |
src/components/Checkbox/CheckboxIOS.tsx |
Migrates checkbox semantics. Moderate (3 votes): preserve polite live-region behavior with aria-live. |
src/components/Checkbox/CheckboxAndroid.tsx |
Migrates checkbox semantics. Moderate (3 votes): preserve polite live-region behavior with aria-live. |
src/components/Button/Button.tsx |
Migrates button role and state. Moderate (1 vote): preserve accessibilityRole compatibility or provide migration guidance. |
src/components/BottomNavigation/BottomNavigationBar.tsx |
Migrates tab semantics. |
src/components/Banner.tsx |
Migrates alert live-region semantics. |
src/components/Appbar/AppbarContent.tsx |
Migrates app-bar semantics. |
src/components/Appbar/AppbarBackAction.tsx |
Migrates app-bar action semantics. |
src/components/Appbar/AppbarAction.tsx |
Migrates app-bar action semantics. |
src/components/ActivityIndicator.tsx |
Migrates progress semantics. |
src/components/__tests__/SegmentedButton.test.tsx |
Updates segmented-button tests. |
src/components/__tests__/Searchbar.test.tsx |
Updates hidden-element queries. Moderate (3 votes): restore clear-button interaction coverage. |
src/components/__tests__/RadioButton/__snapshots__/RadioButtonItem.test.tsx.snap |
Refreshes radio-button snapshots. |
src/components/__tests__/RadioButton/__snapshots__/RadioButtonGroup.test.tsx.snap |
Refreshes radio-group snapshots. |
src/components/__tests__/RadioButton/__snapshots__/RadioButton.test.tsx.snap |
Refreshes radio snapshots. |
src/components/__tests__/ProgressBar.test.tsx |
Updates progress tests. Moderate (3 votes): restore animated-value regression coverage. |
src/components/__tests__/MenuItem.test.tsx |
Updates menu-item tests. |
src/components/__tests__/ListImage.test.tsx |
Updates list-image tests. |
src/components/__tests__/Icon.test.tsx |
Updates icon tests. |
src/components/__tests__/FABGroup.test.tsx |
Updates FAB-group tests. |
src/components/__tests__/Dialog.test.tsx |
Updates dialog tests. Moderate (2 votes): retain custom spacing-style coverage. |
src/components/__tests__/Checkbox/CheckboxItem.test.tsx |
Updates checkbox-item tests. |
src/components/__tests__/Checkbox/__snapshots__/CheckboxItem.test.tsx.snap |
Refreshes checkbox-item snapshots. |
src/components/__tests__/Checkbox/__snapshots__/Checkbox.test.tsx.snap |
Refreshes checkbox snapshots. |
src/components/__tests__/Card/Card.test.tsx |
Updates card tests. |
src/components/__tests__/BottomNavigation.test.tsx |
Updates tab queries. Moderate (3 votes): restore onIndexChange interaction coverage. |
src/components/__tests__/Appbar/Appbar.test.tsx |
Updates app-bar tests. |
src/components/__tests__/Appbar/__snapshots__/Appbar.test.tsx.snap |
Refreshes app-bar snapshots. |
src/components/__tests__/__snapshots__/ToggleButton.test.tsx.snap |
Refreshes toggle-button snapshots. |
src/components/__tests__/__snapshots__/TextInput.test.tsx.snap |
Refreshes text-input snapshots. |
src/components/__tests__/__snapshots__/Snackbar.test.tsx.snap |
Refreshes snackbar snapshots. |
src/components/__tests__/__snapshots__/SegmentedButton.test.tsx.snap |
Refreshes segmented-button snapshots. |
src/components/__tests__/__snapshots__/Searchbar.test.tsx.snap |
Refreshes searchbar snapshots. |
src/components/__tests__/__snapshots__/ProgressBar.test.tsx.snap |
Refreshes progress-bar snapshots. |
src/components/__tests__/__snapshots__/MenuItem.test.tsx.snap |
Refreshes menu-item snapshots. |
src/components/__tests__/__snapshots__/Menu.test.tsx.snap |
Refreshes menu snapshots. |
src/components/__tests__/__snapshots__/ListItem.test.tsx.snap |
Refreshes list-item snapshots. |
src/components/__tests__/__snapshots__/ListAccordion.test.tsx.snap |
Refreshes accordion snapshots. |
src/components/__tests__/__snapshots__/IconButton.test.tsx.snap |
Refreshes icon-button snapshots. |
src/components/__tests__/__snapshots__/FAB.test.tsx.snap |
Refreshes FAB snapshots. |
src/components/__tests__/__snapshots__/DrawerItem.test.tsx.snap |
Refreshes drawer-item snapshots. |
src/components/__tests__/__snapshots__/DataTable.test.tsx.snap |
Refreshes data-table snapshots. |
src/components/__tests__/__snapshots__/Chip.test.tsx.snap |
Refreshes chip snapshots. |
src/components/__tests__/__snapshots__/Button.test.tsx.snap |
Refreshes button snapshots. |
src/components/__tests__/__snapshots__/BottomNavigation.test.tsx.snap |
Refreshes bottom-navigation snapshots. |
src/components/__tests__/__snapshots__/Banner.test.tsx.snap |
Refreshes banner snapshots. |
src/components/__tests__/__snapshots__/AnimatedFAB.test.tsx.snap |
Refreshes animated-FAB snapshots. |
src/components/__tests__/__snapshots__/ActivityIndicator.test.tsx.snap |
Refreshes activity-indicator snapshots. |
package.json |
Updates testing dependencies. |
Review details
Suppressed comments (9)
src/components/Button/Button.tsx:109
- The existing public prop is still named
accessibilityRole, but its type is narrowed fromAccessibilityRoletoRole. Existing callers using legacy React Native values such asheaderoradjustablewill no longer type-check; preserve the old prop type or introduce a separately namedroleAPI with an explicit migration.
accessibilityRole?: Role;
src/components/Chip/Chip.tsx:92
- The existing public prop is still named
accessibilityRole, but its type is narrowed fromAccessibilityRoletoRole. Existing callers using legacy React Native values such asheaderoradjustablewill no longer type-check; preserve the old prop type or introduce a separately namedroleAPI with an explicit migration.
accessibilityRole?: Role;
src/components/FAB/AnimatedFAB.tsx:67
- These new ARIA fields replace the documented
accessibilityStateprop, so existingAnimatedFABcallers lose a type-safe way to pass their current state without any deprecation path. Keep the old prop as a compatibility alias or make the breaking change explicit and provide migration guidance.
'aria-disabled'?: boolean;
'aria-selected'?: boolean;
'aria-checked'?: boolean;
'aria-expanded'?: boolean;
'aria-busy'?: boolean;
src/components/FAB/AnimatedFAB.tsx:465
- The new
aria-selected/aria-checked/aria-expanded/aria-busyprops are declared onProps, but they remain inrestand are spread only onto the outerSurface(line 381). This interactiveTouchableRipplereceives only the hard-codedaria-disabled, so callers' state props are not exposed on the button itself; destructure and forward them here, preserving the intended disabled precedence.
aria-label={accessibilityLabel}
aria-disabled={disabled}
role="button"
src/components/FAB/FAB.tsx:300
FABstill exposesaccessibilityState, but it is no longer destructured and merged withdisabled; it now falls through both{...rest}spreads unchanged. A caller-provided state such as{ disabled: false }can therefore conflict with a disabled FAB, whereas the previousnewAccessibilityStatealways forceddisabled: true. Keep the state destructured and merge/translate it before rendering.
aria-label={accessibilityLabel}
role="button"
aria-disabled={disabled}
src/components/Menu/Menu.tsx:645
- Only the overlay label is migrated here; the same
Pressablestill usesaccessibilityRole="button", leaving this node on a mixed legacy/ARIA path. Replace the role with the ARIAroleprop as well.
aria-label={overlayAccessibilityLabel}
accessibilityRole="button"
src/components/Searchbar.tsx:295
- The label props in this hunk are migrated, but
Searchbarstill passesaccessibilityRoleto the search, clear, and trailing controls (and to the input). This leaves the component partially migrated and emits legacy semantics alongside the new ARIA props; update the remaining role props in the same change.
aria-label={searchAccessibilityLabel}
src/components/Snackbar.tsx:366
- The icon label now uses ARIA, but this
Snackbarstill usesaccessibilityLiveRegionon its container andaccessibilityRoleon its action. That leaves related accessibility behavior on the legacy path; migrate those remaining props as part of this change.
src/components/tests/BottomNavigation.test.tsx:147 - This change skips the
onTabPressinteraction test, removing coverage for the tab-press callback. Please adapt the query/event setup to the upgraded testing-library version instead of disabling the test.
- Files reviewed: 73/74 changed files
- Comments generated: 9
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+129
to
+131
| role="checkbox" | ||
| aria-checked={checked} | ||
| aria-disabled={disabled} |
Comment on lines
+72
to
+74
| role="checkbox" | ||
| aria-checked={checked} | ||
| aria-disabled={disabled} |
| accessibilityLabel, | ||
| accessibilityRole = 'button', | ||
| closeIconAccessibilityLabel = 'Close', | ||
| closeIconAccessibilityLabel, |
Comment on lines
+191
to
+193
| aria-label={accessibilityLabel} | ||
| role="button" | ||
| aria-disabled={disabled} |
Comment on lines
+104
to
+108
| 'aria-disabled'?: boolean; | ||
| 'aria-selected'?: boolean; | ||
| 'aria-checked'?: boolean; | ||
| 'aria-expanded'?: boolean; | ||
| 'aria-busy'?: boolean; |
| }); | ||
|
|
||
| it('calls onIndexChange', () => { | ||
| it.skip('calls onIndexChange', () => { |
Comment on lines
+162
to
+167
| expect(getByTestId('dialog-button-1-container')).toHaveStyle({ | ||
| marginRight: 8, | ||
| }); | ||
| expect(getByTestId('dialog-button-2-container')).toHaveStyle({ | ||
| marginRight: 0, | ||
| }); |
| }); | ||
|
|
||
| it('renders progress bar with animated value', async () => { | ||
| it.skip('renders progress bar with animated value', async () => { |
| }); | ||
|
|
||
| it('defines onClearIconPress action and checks if it is called when close button is pressed', () => { | ||
| it.skip('defines onClearIconPress action and checks if it is called when close button is pressed', () => { |
Member
|
Closing as outdated on main |
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
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.
Motivation
Related issue
Test plan