Remove onClick from non-interactive Pressables to fix TalkBack a11y#86731
Remove onClick from non-interactive Pressables to fix TalkBack a11y#86731
Conversation
Co-authored-by: truph01 <truph01@users.noreply.github.com>
The patch file was missing source line 69 (onPressEnd: onPressOut) from the hunk body, causing a line count mismatch that made patch-package unable to parse the file. Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
Fixed the failing build checks: the |
|
The original check failures (ESLint, Prettier, Changed files ESLint, Check for unused styles, Translation Dry Run) were all caused by the same root cause: the patch file Current status after the fix:
|
|
The failing checks are unrelated to this PR's changes. 1.
2.
Evidence: The PR changes 3 files — No action needed on this PR — these are pre-existing/flaky failures. |
|
The failing checks are unrelated to this PR's changes (Pressable/react-native-web patch). 1. 2. This test ( Evidence: The PR only touches:
None of these files affect API middleware or Android native builds. Recommendation: Re-run the failing checks — both failures should resolve on retry. |
|
@MelvinBot Your solution doesn't work when the interactive={false} is passed in: |
…eraction
MenuItem was not forwarding the interactive prop to the pressable chain,
so GenericPressable always saw interactive=true (the default). This meant
the onPress/onLongPress suppression in BaseGenericPressable had no effect
for interactive={false} MenuItems like those in CardDetails.
Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
Good catch — Fixed in 499b86c by adding Copy-on-long-press for |
…ivePressableA11y Co-authored-by: truph01 <truph01@users.noreply.github.com>
Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
Fixed the failing Prettier check. The issue was a duplicate |
…ivePressableA11y Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
Merged with the latest
|
|
@MelvinBot, mark this PR as ready for review |
|
Android-chrome result: Screen.Recording.2026-03-31.at.22.54.37.mov |
|
Done — PR is now marked as ready for review. |
melvin-bot
bot
requested review from
trjExpensify and
truph01
and removed request for
a team and
truph01
melvin-bot
bot
requested review from
a team and
truph01
and removed request for
a team
|
@chiragsalian 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] |
|
@truph01 can you check codex review comment if it's relevant? |
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Accessibility project PR 👍
Remove the `interactive` gate from onLongPress so that long-press-to-copy still works on touch devices for copyable non-interactive MenuItems. The TalkBack a11y fix only needs onPress suppressed, not onLongPress. Co-authored-by: truph01 <truph01@users.noreply.github.com>
@Valforte That comment makes sense, and I updated the PR to align with it. cc @dominictb |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2026-04-02.at.10.42.42.moviOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-04-02.at.10.38.45.mov |
| activate" on non-interactive Pressables. Removing onClick when there are no press | ||
| handlers prevents this incorrect announcement. | ||
| ``` | ||
| - Upstream PR/issue: 🛑 |
There was a problem hiding this comment.
@truph01 This patch is a quick fix but we'll still need an upstream fix. Please create upstream PR. Don't need to do this now.
patches/react-native-web/details.md
Outdated
| ``` | ||
| - Upstream PR/issue: 🛑 | ||
| - E/App issue: https://github.com/Expensify/App/issues/79243 | ||
| - PR introducing patch: TBD |
There was a problem hiding this comment.
| - PR introducing patch: TBD | |
| - PR introducing patch: https://github.com/Expensify/App/pull/86731 |
There was a problem hiding this comment.
Updated! Replaced TBD with the actual PR link.
| + // When no press or longPress callbacks are configured, remove the onClick | ||
| + // handler so that assistive technology (e.g. TalkBack on Android mWeb) does | ||
| + // not detect a click listener and announce the element as actionable. |
There was a problem hiding this comment.
details.md already includes this
| + // When no press or longPress callbacks are configured, remove the onClick | |
| + // handler so that assistive technology (e.g. TalkBack on Android mWeb) does | |
| + // not detect a click listener and announce the element as actionable. |
There was a problem hiding this comment.
Good catch — removed the inline comment from the patch. The explanation is already in details.md.
|
@MelvinBot Please resolve these two comments #86731 (comment) |
- Replace TBD with actual PR link in details.md - Remove inline comment from patch file (already documented in details.md) Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
@dominictb Melvin resolved your two comments |
|
@Valforte All yours |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @Valforte 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Valforte in version: 9.3.52-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a purely technical accessibility bug fix — it removes the |
|
Deploy Blocker ##87002 was identified to be related to this PR |
|
Deploy Blocker #87025 was identified to be related to this PR. |
|
QA team is going to verify the following QA Steps
|
9 participants
Explanation of Change
When
interactive={false}on aMenuItem, TalkBack on Android mWeb still announces "double tap to activate" because React Native Web'sPressablealways registers anonClickevent listener — even when no press behavior is configured.This PR fixes the issue at its root with two coordinated changes:
BaseGenericPressable: Wheninteractive=false, stops passingonPressandonLongPressto the underlying RNW Pressable (previously these were passed but returned early internally).New RNW patch (013): In the Pressable component, when both
onPressandonLongPressarenull, theonClickhandler is stripped frompressEventHandlersbefore they are spread onto the DOM element. This prevents TalkBack from detecting a click listener and announcing the element as actionable.This approach (Approach A from the issue discussion) is safer than replacing
PressableWithSecondaryInteractionwith aViewwrapper because it only removes the click event listener — no layout, styling, or ARIA attribute changes.Fixed Issues
$ #79243
Tests
MenuItemcomponents (e.g., Settings > Wallet > Virtual Card details showing "Limit type", "Remaining limit")Offline tests
N/A — This is a client-side accessibility fix that doesn't involve network requests.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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