fix: SR: Settings Security: The status messages is not announced.#85031
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e466618cf
ℹ️ 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".
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@Krishna2323 can you mark it ready for review |
| <Text | ||
| key={statusAnnouncement.id} | ||
| role={CONST.ROLE.ALERT} | ||
| accessibilityLiveRegion="assertive" |
There was a problem hiding this comment.
Since accessibilityLiveRegion is Android-only, wouldn't this likely still miss VoiceOver on iOS devices?
There was a problem hiding this comment.
@ShridharGoel added AccessibilityInfo.announceForAccessibility(message); for handling iOS and removed accessibilityLiveRegion since the android app is already working fine without the prop.
I can't test on iOS so I couldn't verify, please test it if you can verify that. Thanks!
There was a problem hiding this comment.
We might need to ask someone with a physical iOS device to help with testing.
|
@ShridharGoel 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@ShridharGoel bump |
|
|
||
| const announceStatus = (message: string) => { | ||
| setStatusAnnouncement((prev) => ({id: prev.id + 1, text: message})); | ||
| AccessibilityInfo.announceForAccessibility(message); |
There was a problem hiding this comment.
Do we not need a delay here? See useFormHelpMessageAccessibilityAnnouncement.ios.ts.
There was a problem hiding this comment.
I think that's only needed for iOS?
|
alright, asking to trigger a build so we can get ios tested |
|
are all other platforms working fine? |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Yes, checked on Android Native, Windows and Mac with NVDA, JAWS, VoiceOver. |
|
@ShridharGoel Could you please complete testing on your end so we don’t have to wait after confirming on the iOS native app? |
|
Yes, will do it |
| function useAccessibilityAnnouncement(message: string | ReactNode, shouldAnnounceMessage: boolean, options?: UseAccessibilityAnnouncementOptions) { | ||
| const previousAnnouncedMessageRef = useRef(''); | ||
| const delay = options?.delay ?? DELAY_FOR_ACCESSIBILITY_TREE_SYNC; | ||
| const previousKeyRef = useRef(_options?.announcementKey); |
There was a problem hiding this comment.
iOS app crashes on this line:
Property '_options' doesn't exist
|
iOS still says "Copy copied" |
|
This works perfect on iOS without code changes in this PR. No need to call App/src/pages/settings/Security/TwoFactorAuth/CopyCodesPage.tsx Lines 116 to 130 in e53a325 pass |
…nnounceForAccessibility Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@aimane-chnaif Thanks a lot for the solution ❤️. Could you please check again? |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
| Clipboard.setString(account?.recoveryCodes ?? ''); | ||
| setError(''); | ||
| setCodesAreCopied(); | ||
| announceStatus(translate('common.copied')); |
There was a problem hiding this comment.
No "Copied" announcement on iOS with this line. As this is for web only, can you add guard?
There was a problem hiding this comment.
Same happens on android. Maybe re-rendering component (because of state update) interrupts default announcement.
There was a problem hiding this comment.
@aimane-chnaif added the guard but now the initial label is re-announced after copied. Seems expected?
cc: @rushatgabhane
Monosnap.screencast.2026-03-25.01-42-27.mp4
There was a problem hiding this comment.
Yes that is okay, I'm assuming it's saying Copied the second time?
There was a problem hiding this comment.
I'm assuming it's saying Copied?
Yes
There was a problem hiding this comment.
Copy, Recovery codes when Copy button is highlighted
Copied when Copy button is pressed
Copy, Recovery codes when animation is finished
There was a problem hiding this comment.
cool that's correct behavior then
There was a problem hiding this comment.
ok, btw re-announcement is android only. Not on iOS
There was a problem hiding this comment.
understood! not worth blocking
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- Using || intentionally so empty string tooltip/text values fall through to the next fallback | ||
| (!isActive ? accessibilityLabelChecked || accessibilityLabelProp : accessibilityLabelProp) || (!isActive ? tooltipTextChecked || textChecked : tooltipText || text) || text || ''; |
There was a problem hiding this comment.
Let's get rid of this complex ternary
| const isWeb = getPlatform() === CONST.PLATFORM.WEB; | ||
|
|
||
| const announceStatus = (message: string) => { | ||
| if (!isWeb) { |
There was a problem hiding this comment.
Out of curiosity, why announcement are only for web?
There was a problem hiding this comment.
On native, PressableWithDelayToggle already handles the announcement by changing its accessibilityLabel to accessibilityLabelChecked (e.g., "Copied") when pressed — VoiceOver/TalkBack naturally reads the updated label. On web, screen readers don't detect dynamic label changes the same way, so a hidden role="alert" element is needed. Additionally, the state update from setStatusAnnouncement causes a re-render that interrupts the native screen reader's announcement of the changed label.
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Failing check is not related |
It's fixed now on main |
|
thanks! |
|
@MonilBhavsar forgot to merge? 😅 |
|
🚧 @MonilBhavsar 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. |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.3.49-0 🚀
Bundle Size Analysis (Sentry): |
|
Hi @Krishna2323. This is an accessibility PR, which is out of QA scope for retesting. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.3.49-2 🚀
|
7 participants
Explanation of Change
Fixed Issues
$ #76928
PROPOSAL:
Tests
Prerequisites:
Offline tests
Prerequisites:
QA Steps
Prerequisites:
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
Monosnap.screencast.2026-03-16.20-31-40.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Monosnap.screencast.2026-03-12.13-04-39.mp4