-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix accessibility roles and states for radio buttons and tabs #79704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… tabs
This commit addresses three accessibility issues:
1. Radio buttons in Reports filters (SingleSelectListItem):
- Changed from role="checkbox" to role="radio"
- Added accessibilityState with selected state
- Screen readers will now properly announce selection state
2. Tabs in Track Distance feature (TabSelectorItem):
- Changed from role="button" to role="tab"
- Added accessibilityState with selected state
- Screen readers will now properly announce tab selection state
3. Checkboxes (MultiSelectListItem):
- Already correctly implemented with role="checkbox" and checked state
- No changes needed
The Checkbox component was enhanced to support dynamic roles:
- Added optional accessibilityRole prop (defaults to 'checkbox')
- Added optional accessibilityState prop
- Automatically computes correct state based on role:
- For radio: uses {selected: boolean}
- For checkbox: uses {checked: boolean | 'mixed'}
Fixes: #79240
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
| const children = ( | ||
| <AnimatedPressableWithFeedback | ||
| accessibilityLabel={title} | ||
| accessibilityRole={CONST.ROLE.TAB} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ PERF-4 (docs)
Creating a new object {selected: isActive} on every render breaks memoization of the AnimatedPressableWithFeedback child component, causing unnecessary re-renders.
Reasoning: The parent TabSelectorItem is not optimized by React Compiler, while the child AnimatedPressableWithFeedback (via PressableWithFeedback) is memoized. Each new object reference forces the child to re-render even when isActive hasn't changed.
Suggested fix:
const accessibilityState = useMemo(() => ({selected: isActive}), [isActive]);
// Then use:
accessibilityState={accessibilityState}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a valid comment as isActive is a variable though I think it should comment on line 119
|
Note: This PR also addresses issue #77466 (checkboxes). The MultiSelectListItem component was already correctly implemented with role="checkbox" and checked state, so no code changes were needed for checkboxes - they were already WCAG compliant. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppNA Android: mWeb ChromeNA iOS: HybridAppNA iOS: mWeb SafariNA MacOS: Chrome / SafariNA |
eh2077
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll be better to fix #79704 (comment) and prettier check, otherwise looks good to me!
Explanation of Change
This PR fixes accessibility issues where radio buttons, checkboxes, and tabs were not properly announcing their roles and states to screen readers. The changes ensure WCAG 4.1.2 compliance by implementing proper accessibility properties.
Changes made:
The Checkbox component was refactored to accept optional
accessibilityRoleandaccessibilityStateprops, defaulting to correct behavior for both checkbox and radio button use cases.Fixed Issues
$ #79240
$ #77540
$ #74848
Tests
Offline tests
N/A - This is a screen reader accessibility fix that works the same offline
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)Avataris modified, I verified thatAvataris working as expected in all cases)Screenshots/Videos
N/A - This is a screen reader accessibility change with no visual differences. Testing requires screen reader software (VoiceOver on iOS/macOS or TalkBack on Android).