Implement escape key suppression for focus trap in icationPromptPage#93570
Conversation
…icationPromptPage
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-06-16.at.09.50.35.movAndroid: mWeb ChromeScreen.Recording.2026-06-16.at.09.46.19.moviOS: HybridAppScreen.Recording.2026-06-16.at.09.34.17.moviOS: mWeb SafariScreen.Recording.2026-06-16.at.09.36.17.movMacOS: Chrome / SafariScreen.Recording.2026-06-16.at.09.52.06.mov |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@huult Have you started yet? |
|
is it ready? |
|
yes |
|
Reviewing... |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product 👍
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #93304 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
Sorry @mountiny, but I believe @rafecolton will take over this PR |
rafecolton
left a comment
There was a problem hiding this comment.
Tested on dev, works well!
a4868c3
|
@mountiny added unit tests |
mountiny
left a comment
There was a problem hiding this comment.
Thanks! All yours @rafecolton
|
@huult CI is failing, complaining that the number of items in the reviewer checklist doesn't match latest. Can you please take a look? |
|
@rafecolton all yours |
|
thanks! |
|
🚧 @rafecolton has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/rafecolton in version: 9.4.15-0 🚀
|
|
🤖 No help site changes required. This PR is a purely internal bug fix for keyboard-event handling — it does not change any documented, user-facing behavior. What it does: When a user presses Esc on the MFA prompt/validate-code pages, a single key press fires both a Why no docs are needed:
I reviewed @DylanDylann, let me know if you'd like a docs PR anyway, or if there's a user-facing behavior change here I may have missed. |
|
Hi @DylanDylann. Is it an internal PR? |
|
No, QA can test this using the biometrics test row in the troubleshoot menu |
|
@IuliiaHerets I updated the PR description to include the QA steps, similar to the test section. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.15-3 🚀
|
Explanation of Change
Root cause
Two libraries handle ESC at different event phases:
focus-traplibrarykeydownReanimatedModalkeyupA single ESC press generates both events. When the user presses ESC on
PromptPage:keydown→focus-trapcallsinterceptFocusTrapEscape→requestCancel()dispatchesSET_CANCEL_CONFIRM_VISIBLE: true.ConfirmModalmounts → itsuseEffectattaches thekeyuplistener ondocument.body.keyup(same key press, ~milliseconds later) → the brand-new listener fires → callscloseTop()→ invokes the modal'sonCancel(hideCancelConfirm) → modal closes instantly.The modal essentially intercepts the tail end of the same keystroke that opened it.
Solution
Before triggering
requestCancel(), we register a one-shot capture-phasekeyuplistener ondocument. When the matching ESCkeyuparrives,stopImmediatePropagation()preventsReanimatedModal's listener from ever seeing it, so the modal stays open. The listener self-removes on the next keyup of any kind, so it never affects subsequent keystrokes.Fixed Issues
$ #93304
PROPOSAL:
Tests
Offline tests
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
Screen.Recording.2026-06-15.at.16.54.59.mov