fix: Screen Reader: Many Pages: The embedded links cannot be focused and activated#79826
Conversation
|
@parasharrajat 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] |
src/components/HTMLEngineProvider/HTMLRenderers/MutedTextRenderer.tsx
Outdated
Show resolved
Hide resolved
src/components/HTMLEngineProvider/HTMLRenderers/MutedTextRenderer.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 089546593c
ℹ️ 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".
src/components/HTMLEngineProvider/HTMLRenderers/MutedTextRenderer.tsx
Outdated
Show resolved
Hide resolved
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b6f49eac0
ℹ️ 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".
src/components/HTMLEngineProvider/HTMLRenderers/MutedTextRenderer.tsx
Outdated
Show resolved
Hide resolved
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa710d5cfb
ℹ️ 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".
src/components/HTMLEngineProvider/HTMLRenderers/MutedTextRenderer.tsx
Outdated
Show resolved
Hide resolved
2f1e953 to
156005d
Compare
src/components/HTMLEngineProvider/HTMLRenderers/AccountManagerLinkRenderer.tsx
Outdated
Show resolved
Hide resolved
src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.tsx
Outdated
Show resolved
Hide resolved
src/components/HTMLEngineProvider/HTMLRenderers/ConciergeLinkRenderer.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 156005d4d2
ℹ️ 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".
src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.tsx
Outdated
Show resolved
Hide resolved
trjExpensify
left a comment
There was a problem hiding this comment.
Small accessibility update 👍
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
src/components/HTMLEngineProvider/HTMLRenderers/AccountManagerLinkRenderer.tsx
Show resolved
Hide resolved
src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.tsx
Outdated
Show resolved
Hide resolved
|
@huult will review this... |
|
@TaduJR Can you reproduce this issue on the latest main branch? I tested on web and iOS native and it’s working fine. Could you check whether it reproduces on Android? ScreenRecording_01-23-2026.15-11-06_1.MP4 |
ScreenRecording_01-23-2026.15-26-18_1.MP4I can reproduce this issue on the production app. @rushatgabhane It looks like this issue was fixed by another PR. Do you know which ticket it was related to? |
@TaduJR We need to press Control + Option + Space to open it. Screen.Recording.2026-01-25.at.07.41.14.mov |
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
The VoiceOver navigation issue was fixed by #79147 which added role={CONST.ROLE.LINK}. This allows VoiceOver to announce the element as a "link" and enables Control+Option+Space activation (VoiceOver's universal activate command).
The Control+Option+Space is VoiceOver-specific - it only works when VoiceOver is enabled. Per https://www.w3.org/WAI/ARIA/apg/patterns/link/, the Enter key is the standard keyboard activation for links. |
|
yeah feel free to fix it
|
|
@TaduJR please check #79826 (comment) and resolve the conflict. |
|
Bump @TaduJR |
Thanks @rushatgabhane! pressOnEnter is for Button components and creates a global keyboard shortcut (fires when Enter is pressed anywhere on the screen). For links, we need element-specific handling - Enter should only activate the link that currently has focus. Per https://www.w3.org/WAI/ARIA/apg/patterns/link/, "Enter executes the link" means the focused link. That's why we use onKeyDown on the Text element - it only fires when that specific element is focused and Enter is pressed. WDYT? |
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
Per W3C APG Link Pattern, elements with role=link should be activatable via Enter key. This change adds automatic Enter key handling in GenericPressable when: 1. role=link is set 2. No custom onKeyDown handler is provided 3. onPress exists This fixes keyboard accessibility for MenuItem and other Pressable-based components that use role=link (e.g., AppDownloadLinks page). Also updates useEnterKeyHandler to use CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey instead of hardcoded 'Enter' string.
|
FYI we also discussed at included this fix |
|
@TaduJR please resolve the conflict! |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-02-03.at.12.24.02.mov |
|
#79826 (comment) The original issue was already fixed by another PR. #79826 (comment) This PR fixed the issue where pressing Enter on a link did not open the URL. |
|
@TaduJR Please resolve the conflict, and once that’s done, ping me so I can approve this PR. |
…Reader-Many-Pages-The-embedded-links-cannot-be-focused-and-activated
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.13-1 🚀
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.15-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.15-10 🚀
|
Explanation of Change
This PR fixes an accessibility issue where embedded links (e.g., "Terms of Service", "Privacy") within muted text blocks couldn't be properly focused and activated by screen readers.
Fixed Issues
$ #77553
PROPOSAL: #77553 (comment)
Tests
Prerequisites: the user is not signed in
Other Occurrences:
On About: the links Terms of Service and Privacy
On Troubleshoot: 'submit a bug'
Verify that no errors appear in the JS console
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
Android-App.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Macbook-Chrome.mp4