Fix: forward aria-checked for screen readers on checkbox and radio#81755
Fix: forward aria-checked for screen readers on checkbox and radio#81755marcochavezf merged 4 commits intomainfrom
Conversation
…tyState to Checkbox - Forward accessibilityState.checked as aria-checked in BaseGenericPressable so web screen readers correctly announce checkbox/radio state - Add accessibilityState with checked property to Checkbox component so native platforms (Android TalkBack) correctly announce state Co-authored-by: Marco Chávez <marcochavezf@users.noreply.github.com>
|
The only failing check is
To fix this, edit the PR description and check off ( |
|
@bernhardoj Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
The
All checks are now passing — no code changes were needed. |
|
All checks on this PR are currently passing. Here's the status:
If there were previously failing checks, they appear to have been resolved. Let me know if you're seeing something different or if new failures appear. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-02-06.at.10.55.24.pm.movScreen.Recording.2026-02-06.at.10.58.52.pm.mov |
src/components/Checkbox.tsx
Outdated
| onKeyDown={handleSpaceOrEnterKey} | ||
| role={CONST.ROLE.CHECKBOX} | ||
| accessibilityState={{ | ||
| checked: isIndeterminate ? ('mixed' as const) : isChecked, |
There was a problem hiding this comment.
Unless this causes TS error:
| checked: isIndeterminate ? ('mixed' as const) : isChecked, | |
| checked: isIndeterminate ? 'mixed' : isChecked, |
There was a problem hiding this comment.
Applied in 7ad4c03 -- removed the unnecessary as const cast.
There was a problem hiding this comment.
Already applied — the as const cast was removed in 7ad4c03.
BaseGenericPressable already forwards accessibilityState.checked as aria-checked, making the explicit prop unnecessary. Co-authored-by: Marco Chávez <marcochavezf@users.noreply.github.com>
|
@MelvinBot don't remove |
|
The failing Analysis: The CI found a reviewer checklist comment with 0 out of 58 items checked. This check requires the PR reviewer to fill out their checklist by checking each box. It cannot be fixed by pushing code changes. What needs to happen: The assigned reviewer (@bernhardoj based on the comment thread) needs to go through the Reviewer Checklist comment and check each item. Once all 58 items are checked, this CI check will pass on the next run. All other checks (typecheck, ESLint, Prettier, builds, tests, verifySignedCommits, etc.) are passing. |
|
Hmm, looks like @MelvinBot doesn't wanna address my feedback. |
Reverts 3278d5f per reviewer feedback. The explicit aria-checked prop should be kept alongside accessibilityState.checked. Co-authored-by: Mykhailo <mkhutornyi@users.noreply.github.com>
|
The only failing check is PR Reviewer Checklist — it failed because the reviewer checklist comment has 0 out of 58 items checked. This is not a code issue; it requires the reviewer (@bernhardoj) to manually complete their checklist by checking each box. All code-related checks (ESLint, Prettier, TypeScript, signed commits, spellcheck, builds, tests) are passing or still in progress on the latest run. No code fix is needed here. |
Addresses reviewer feedback from mkhutornyi. Co-authored-by: Mykhailo <mkhutornyi@users.noreply.github.com>
|
The Analysis: The failure is because @mkhutornyi posted a Reviewer Checklist comment with 0 out of 58 items checked. The CI check enforces that all reviewer checklist items must be marked complete. Resolution: @mkhutornyi needs to go through the reviewer checklist and check off each item, or the checklist comment needs to be deleted if it was posted prematurely. Once the checklist is completed, the check will pass on re-run. |
|
The failing The CI requires that the Reviewer Checklist comment (posted by @mkhutornyi) has all boxes checked. Currently 0 out of 58 items are completed. This cannot be fixed by pushing code changes; the reviewer needs to go through and check off the items in their Reviewer Checklist comment. All other checks are passing (ESLint, typecheck, Prettier, builds, tests, verifySignedCommits, etc.). |
|
The only failing check is The All review feedback from @mkhutornyi has been addressed: |
mkhutornyi
left a comment
There was a problem hiding this comment.
Tested with VoiceOver on MacOS, iPhone.
Works well.
|
🚧 @marcochavezf has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Fix: forward aria-checked for screen readers on checkbox and radio (cherry picked from commit d3321f1) (cherry-picked to staging by Julesssss)
|
🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.3.15-7 🚀
|
Explanation of Change
Screen readers were announcing incorrect checkbox/radio states because:
BaseGenericPressableforwardsaria-selectedandaria-disabledbut did not forwardaccessibilityState.checkedasaria-checked. This meant radios had no announced state and checkboxes were always announced as unchecked on web.Checkboxcomponent setaria-checked(web-only) but did not provideaccessibilityState, so TalkBack had no state to announce.This PR implements @ashu75575's suggested fix:
accessibilityState.checkedasaria-checkedinBaseGenericPressableso web screen readers correctly announce state.accessibilityState={{ checked }}(with'mixed'for indeterminate) toCheckboxso native platforms announce state correctly.Fixed Issues
$ #81654
Tests
Offline tests
N/A - Accessibility attributes do not depend on network state.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari