fix: mWeb - Keyboard doesn't pop-up, when foreground the app using device menu button#81809
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
Code Review SummaryI've reviewed PR #81809 and identified the following issues: Issues Found1. [PERF-12] Memory leak risk ( 2. [CONSISTENCY-2] Magic number ( 3. [PERF-8] Module-level state concerns ( 4. [CONSISTENCY-5] Empty listener ( Positive Aspects
Recommendations
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8af5c0718
ℹ️ 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".
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.
|
…yboard-doesnt-pop-up-when-foreground-the-app-using-device-menu-button
…yboard-doesnt-pop-up-when-foreground-the-app-using-device-menu-button
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cee074552
ℹ️ 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".
|
@TaduJR Could you please go through all the AI suggestions, react to each one with 👍 or 👎, and provide a brief explanation for your decision? Thanks! |
…yboard-doesnt-pop-up-when-foreground-the-app-using-device-menu-button
|
|
Friendly Bump @truph01 when you have moment Thanks! |
|
@TaduJR I have two comments. Also, could you merge main? |
…yboard-doesnt-pop-up-when-foreground-the-app-using-device-menu-button
|
@TaduJR In your PR, you added the This PR should focus on syncing the focus state with the keyboard state. |
…yboard-doesnt-pop-up-when-foreground-the-app-using-device-menu-button # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
The validation skip prevents a regression our fix introduces. The original visibilitychange blur was safe because document.hasFocus() is false at that point, so FormProvider skips validation automatically. The window.focus blur happens when document.hasFocus() is true by definition, which triggers validation showing errors on form inputs when returning from background. The flag is the minimal fix for this. |
…yboard-doesnt-pop-up-when-foreground-the-app-using-device-menu-button
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-03-15.at.22.20.10.movAndroid: mWeb ChromeScreen.Recording.2026-03-15.at.22.19.01.moviOS: HybridAppScreen.Recording.2026-03-15.at.22.14.00.moviOS: mWeb SafariScreen.Recording.2026-03-15.at.22.12.11.movMacOS: Chrome / SafariThis platform doesn't have a keyboard. |
|
🚧 @marcaaron 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/marcaaron in version: 9.3.42-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.3.42-3 🚀
|
Explanation of Change
On Android mWeb (Chrome), the keyboard doesn't appear when returning from background via the device menu button, even though the input has focus.
Root cause: The existing keyboard restoration uses
visibilitychange, which doesn't fire when using the device menu without switching apps. When it does fire (after switching apps), the immediateblur()/focus()fails because Android hasn't fully restored the window yet.Fix: Replace the
visibilitychangehandler with awindow.focuslistener that blurs and refocuses with a 300ms delay (CONST.ANIMATED_TRANSITION), giving Android time to restore the window state. A module-level flag preventsFormProviderfrom triggering validation during the blur/focus cycle.Fixed Issues
$ #75055
PROPOSAL: #75055 (comment)
Tests
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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-Native.mp4
Android: mWeb Chrome
Android-mWeb.mp4
iOS: Native
iOS-Native.mp4
iOS: mWeb Safari
iOS-Safari.mp4
MacOS: Chrome / Safari