InteractionManager migration - MoneyRequestConfirmationList, useDialogContainerFocus, useAutoFocusInput#92057
Conversation
|
@ikevin127 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: a5336b6b23
ℹ️ 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".
| }, [isNewManualExpenseFlowEnabled]), | ||
| ); | ||
|
|
||
| const isCompactMode = !showMoreFields && isScanRequest && !isInLandscapeMode; |
There was a problem hiding this comment.
Restore the delayed blur after keyboard navigation
When the old money-request flow is advanced by keyboard (for example, tab to the previous step's Next button and press Enter/Space), React Navigation's stack can leave that previous control focused while the confirmation screen is pushed. The confirmation CTA relies on pressOnEnter/useKeyboardShortcuts, and Button disables its Enter shortcut whenever useActiveElementRole() reports a focused button/textbox, so without the deleted delayed blurActiveElement() the user can land on confirmation with focus still on a hidden prior control and Enter won't submit until they click elsewhere. The removed focus effect was the only cleanup that handled this non-mouse path after the transition in the old flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
True, now it keeps back button focused after entering the confirmation screen via keyboard. Will restore the blur then
Screen.Recording.2026-06-01.at.16.04.56.mov
|
@codex review please |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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!
|
|
No product review needed |
|
@ikevin127 in case you missed this 😅 I'll fix conflict shortly |
…yRequestConfirmationList
|
errors unrelated to this pr |
Reviewer Checklist
Screenshots/VideosiOS: NativeSimulator.Screen.Recording.-.iPhone.17.Pro.Max.-.2026-06-09.at.20.09.47.movMacOS: Chrome / SafariScreen.Recording.2026-06-09.at.19.43.28.mov |
| const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
| useFocusEffect( | ||
| useCallback(() => { | ||
| // Blurring the active element after transition fights AmountField focus in the new manual flow (RHP reopen). | ||
| if (isNewManualExpenseFlowEnabled) { | ||
| return undefined; | ||
| } | ||
| focusTimeoutRef.current = setTimeout(() => { | ||
| InteractionManager.runAfterInteractions(() => { | ||
| blurActiveElement(); | ||
| }); | ||
| }, CONST.ANIMATED_TRANSITION); | ||
| return () => focusTimeoutRef.current && clearTimeout(focusTimeoutRef.current); | ||
| }, [isNewManualExpenseFlowEnabled]), | ||
| ); |
There was a problem hiding this comment.
⚠️ Issue: Potential Regression for Other Flows Using MoneyRequestConfirmationList
Important: The removed useFocusEffect + blurActiveElement logic was running for all consumers of MoneyRequestConfirmationList (when !isNewManualExpenseFlowEnabled). By removing this blurring behavior entirely without replacing it on a per-flow basis, any other flow using this component that does not also pass shouldSkipFocusAfterTransition to its header will now retain focus on the first interactive element in the header (e.g., the back button), which could break keyboard/screen reader navigation in flows that previously relied on the blur.
Why this matters: The old code was a global workaround for a local problem. Removing it is correct, but the fix (shouldSkipFocusAfterTransition) is currently only applied in IOURequestStepConfirmation.tsx. If MoneyRequestConfirmationList is used within other modals or RHP panels, they might now exhibit incorrect focus behavior.
Suggested fix / verification: Add a quick audit or grep to identify all direct consumers of MoneyRequestConfirmationList. For example:
grep -rn "MoneyRequestConfirmationList" src/pages/ --include="*.tsx"For any other non-deprecated consumers, verify if the focus landing on the back button is problematic. If it is, those flows should also receive shouldSkipFocusAfterTransition on their respective HeaderWithBackButton (or their wrapper should handle focus differently).
Severity: Medium — could cause keyboard navigation regressions in untested flows.
There was a problem hiding this comment.
Other components using MoneyRequestConfirmationList:
- SubmitDetailsPage - native mobile app only, so no need to handle keyboard navigation
- SplitBillDetailsPage - no submit button that could be triggered by tapping Enter, currently on prod after opening RHP with keyboard navigation there is a blur and tapping Enter does nothing, after these changes there is no blur so focus stays on Back button and Enter closes RHP which IMO is better than what we currently have considering the content of the RHP:
Screen.Recording.2026-06-10.at.08.35.15.mov
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM
Just one mention here to be mindful-of before merging 🙌
…yRequestConfirmationList
…yRequestConfirmationList
|
@roryabraham in case you missed it 🫡 |
|
🚧 @roryabraham 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/roryabraham in version: 9.4.9-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 I reviewed the changes in this PR and no help site documentation changes are required. Why: This is an internal focus-management refactor with no user-facing behavior change to document:
These are implementation-level changes to focus/keyboard-navigation timing and screen-reader announcement behavior. The help site articles under Since no docs changes are needed, I did not create a draft help site PR. @GCyganek, if you believe a specific user-facing behavior here should be documented (e.g., a keyboard-navigation/accessibility help article), let me know which article and I'll draft the update. |
Explanation of Change
Migrates
InteractionManager.runAfterInteractionstoTransitionTracker.runAfterTransitionsinuseAutoFocusInput,useDialogContainerFocus, and removes it fromMoneyRequestConfirmationList. Adds askipDialogContainerFocusprop toHeaderWithBackButton/Header, used inIOURequestStepConfirmationto prevent the header back button from stealing focus when Keyboard navigation is used to get to the confirmation page, so the Enter key submits the expense instead of navigating back, and to allow removal of theInteractionManagerusage inMoneyRequestConfirmationListwhich was basically calling blur to remove focus triggered inuseDialogContainerFocus.Fixed Issues
$ #83069
$ #71913
PROPOSAL: N/A
Tests
SelfDM (web):
SelfDM with Keyboard navigation (web):
Keyboard navigation (web):
Mobile native apps:
Offline tests
N/A
QA Steps
Same as tests
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
Screen.Recording.2026-06-03.at.08.41.25.mov
iOS: Native
Screen.Recording.2026-06-03.at.08.55.04.mov
Android: mWeb Chrome
Screen.Recording.2026-05-29.at.12.01.05.mov
iOS: mWeb Safari
Screen.Recording.2026-05-29.at.11.59.57.mov
MacOS: Chrome / Safari
MoneyRequestConfirmationList:
Screen.Recording.2026-05-29.at.11.54.51.mov
MoneyRequestConfirmationList with Keyboard navigation (
skipDialogContainerFocusused to skip focus inuseDialogContainerFocus):Screen.Recording.2026-06-02.at.16.09.10.mov
Screen.Recording.2026-06-02.at.16.12.49.mov