fix: close date picker on forward/backward navigation in iOS mWeb - #93617
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
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.
|
nahrinoda
left a comment
There was a problem hiding this comment.
The root cause here is in PopoverWithMeasuredContent/index.tsx (lines 31-40) — on small screens it renders <Modal> directly, bypassing Popover entirely. That means Popover's popstate listener (which handles shouldCloseWhenBrowserNavigationChanged at Popover/index.tsx:55-74) is never mounted on small screens.
Adding the listener here in DatePickerModal fixes this specific component, but the gap is one layer down. If the shouldCloseWhenBrowserNavigationChanged popstate handling were added to PopoverWithMeasuredContent's small-screen <Modal> path, every consumer (EmojiPicker, PopoverMenu, TagPickerModal, etc.) would get the fix automatically — and DatePickerModal wouldn't need its own listener.
|
@nahrinoda Thanks for checking this. I agree the generic gap is in Changing
|
|
@x-dev90, any estimation to be ready for review? |
nahrinoda
left a comment
There was a problem hiding this comment.
Fair point — scoping to DatePickerModal avoids unintended side effects for other consumers. The generic fix could be a follow-up if the same issue surfaces elsewhere. LGTM on the approach.
|
@x-dev90, gentle bump. |
|
@marufsharifi 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] |
|
@marufsharifi Updated to use |
|
@x-dev90, please check the failed unit test. |
|
@marufsharifi All done can you please do another review, thanks. |
|
I will review this by EOD. thanks. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Code changes look good to me; I am testing |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-06-19.at.4.34.31.PM.movAndroid: mWeb ChromeScreen.Recording.2026-06-19.at.4.56.46.PM.moviOS: HybridAppScreen.Recording.1405-03-29.at.9.37.42.PM.moviOS: mWeb SafariScreen.Recording.2026-06-19.at.5.26.51.PM.movMacOS: Chrome / SafariScreen.Recording.2026-06-19.at.4.17.51.PM.mov |
|
LGTM! |
|
🚧 @inimaga 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/inimaga in version: 9.4.17-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review — no changes required I reviewed the change in this PR against the help site articles under Conclusion: No help site updates are required. Why: This is a behavioral bug fix internal to the The help site documents how to use features — the steps to add a bank account remain exactly the same. This PR doesn't add, remove, or change any user-facing feature, setting, label, button, or workflow that the docs describe; it only fixes an overlay that failed to dismiss on navigation. There's nothing in Because no documentation is affected, I did not create a draft help site PR. If you believe a specific article should be updated, let me know which one and I'll take another look. @x-dev90, please confirm you agree no help site changes are needed for this PR. |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.4.17-3 🚀
|
1 similar comment
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.4.17-3 🚀
|
Explanation of Change
This PR fixes the iOS Safari mWeb date picker behavior in the Add bank account flow.
On small-screen web, DatePickerModal renders through the bottom-docked Modal path instead of the normal Popover path. That small-screen path was not closing the date picker on browser Back/Forward navigation, so the calendar could remain visible after navigating from the date of birth or company incorporation date step to a later step.
This change makes DatePickerModal close on real browser popstate navigation when it is visible, on web small screens, and when shouldCloseWhenBrowserNavigationChanged is enabled. Native platforms and wide-screen web behavior are unchanged
Fixed Issues
$ #91311
PROPOSAL: #91311 (comment)
Tests
Offline tests
Same as Tests.
QA Steps
Same as Tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
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-16.at.11.03.24.PM.mov
Android: mWeb Chrome
Screen.Recording.2026-06-16.at.10.18.56.PM.mov
iOS: Native
Screen.Recording.2026-06-16.at.10.44.12.PM.mp4
iOS: mWeb Safari
Screen.Recording.2026-06-16.at.10.55.13.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-16.at.11.13.01.PM.mov