fix(a11y): update picker-select fork, remove Android TalkBack workaround, keep iOS Done button role#85244
Conversation
…und, keep iOS Done button role
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
@eVoloshchak 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] |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 587a08e214
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
|
No product review needed |
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Can confirm this is working as expected on Android. TalkBack also doesn't announce the picker on mWeb Android But on iOS the picker doesn't work at all (and not announced too) Screen.Recording.2026-03-17.at.18.54.21.movPlease always include all of the recordings for all of the platforms, even if the original bug and the changes are platform-specific, this is a way to catch this type of issue. |
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
Just tried it again, its working.
Also this one works. Could you double check this one?
Interesting. I thought it was working fine since the OP was reported only for Android. Raised upstream PR Expensify/react-native-picker-select#22 |
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated # Conflicts: # src/components/Picker/BasePicker.tsx
@TaduJR, please always test all of the test scenarios on all of the platforms in the PR checklist. Even if the bug is reported on a single platform and we fix it for a single platform, this is a good way to verify that nothing has been broken. Both PR author and C+ always go through all of the platforms when testing a PR, both for App and for forks (in this case, https://github.com/Expensify/react-native-picker-select, please check Expensify/react-native-picker-select#22 on all of the platforms) |
Ohh I see. I focused on Android since the first step from the OP was
Thanks so much for the clarification @eVoloshchak Really appreciate your work, and support since the issue milestone began.
You got it 👍. |
Explanation of Change
Updates the
react-native-picker-selectfork toacf8e54, which includes:lawnstarter/react-native-picker-selectinto the Expensify forkaccessibilityRole="combobox",accessibilityLabel,accessibilityState,onAccessibilityActionhandler, andimportantForAccessibility="no-hide-descendants"on the inner containerSince the fork now handles Android TalkBack accessibility internally, this PR removes the consumer-level workaround added in #81013:
importantForAccessibility: 'no-hide-descendants'fromtextInputPropstouchableWrapperProps(accessible, combobox role, label, state)The iOS
touchableDoneProps(accessibilityRole: 'button') is kept because the fork does not add a built-in accessibility role to the iOS Done button it relies on the consumer to provide it viatouchableDoneProps.Fixed Issues
$ #77542
PROPOSAL: #77542 (comment)
Tests
Here are the test steps based on the issue:
Tests
Precondition: You must be logged out.
Test 1: Android TalkBack & iOS VoiceOver - Language picker announcement and activation
Collapsed, Language, combobox(not "button disabled")Test 2: iOS VoiceOver - Done button role
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
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
Screen_Recording_20260122_065943_New.Expensify.Dev.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari