fix: defer BOTTOM_DOCKED modal open until keyboard dismissed on mobile web#87143
Conversation
Checklist Notes
|
|
I think the video looks good to me? Might be nice to see a very clear side-by-side before and after video? |
|
@neerajbachani It seems I've encountered a regression; the 3-dot menu no longer appears in the web (Chrome): Screen.Recording.2026-04-06.at.11.51.02.movCan you reproduce it as well? Thank you. |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product 👍
|
Hi @brunovjk, thanks for catching that! The issue was that KeyboardUtils.dismiss().then(openMenu) was unconditional even on desktop where the Promise resolves immediately, .then() still defers openMenu to the microtask queue, which broke the click/focus event sequencing. Fixed by guarding with isMobile(): desktop now calls openMenu() synchronously (same as the original code), while mobile web still waits for the keyboard to fully close before opening the menu. Could you re-test on desktop Chrome when you get a chance? Thanks! |
|
Thanks @shawnborton @joekaufmanexpensify @brunovjk for the review! Here's a side-by-side video comparing staging (left) vs the fix (right) - no blank space on mobile web, and the desktop regression is resolved too. lv_0_20260408142720.mp4 |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
LGTM, as long as design is cool with it 👍
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb Chrome87143_android_web.moviOS: HybridAppiOS: mWeb Safari87143_ios_web.movMacOS: Chrome / Safari87143_web_chrome.mov |
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.
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @rlinoz 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/rlinoz in version: 9.3.55-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a UI bug fix for mobile web (blank-space flash when opening the three-dot menu while the keyboard is open). The changes are purely internal rendering fixes — keyboard dismissal timing in |
|
Hi @neerajbachani. QA team failed this PR on iOS with an original issue 1775698973200.ScreenRecording_04-08-2026_20-39-53_1.mp4 |
That's odd, I tested this. @neerajbachani can you check? I'm AFK now. |
Explanation of Change
When a user taps the three-dot menu while the software keyboard is open on mobile web, the bottom-docked modal ("Remove" option) was rendering while the keyboard was still animating closed. This caused a large blank space above the menu item for ~300-400ms (the keyboard animation duration).
Root cause: In
ThreeDotsMenu/index.tsx,onIconPress?.()(which callsblurActiveElement()+KeyboardUtils.dismiss()) was called AFTERshowPopoverMenu(), so the modal opened before the keyboard started dismissing.Two targeted fixes:
1.
src/components/ThreeDotsMenu/index.tsxonIconPress?.()to fire BEFOREshowPopoverMenu()so keyboard dismissal starts immediatelyKeyboardUtils.dismiss().then(openMenu)using the existing project utility resolves immediately if keyboard is not open, waits for full dismissal if it is. Works correctly on both iOS Safari and Android Chrome.2.
src/styles/utils/generators/ModalStyleUtils.tsisMobileSafari()toisMobile()-interactive-widget=resizes-contentis Chrome-only; Android Firefox, Samsung Internet, and older Android Chrome also ignore it and have the samewindow.innerHeightstability issueheight: 'fit-content'alongsidemaxHeightso the modal container collapses to actual content size instead of expanding to fillmaxHeight. Preserves original PR Fix test drive modal not moving when keyboard openes #62799 behavior for large modals (content larger than viewport still scrolls correctly).Fixed Issues
$ #86639
PROPOSAL: #86639 (comment)
Tests
with no delay ✓
Offline tests
QA Steps
Same as Tests section above.
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
andoid-preview.mp4
iOS: Native
iOS: mWeb Safari
ios-safari-preview.mp4
MacOS: Chrome / Safari