Open keyboard automatically on Android magic code screen#90771
Open keyboard automatically on Android magic code screen#90771wildan-m wants to merge 8 commits into
Conversation
…oid keyboard Replaces the deprecated InteractionManager.runAfterInteractions approach with React Navigation's transitionEnd event listener — the canonical replacement recommended in contributingGuides/INTERACTION_MANAGER.md and the same pattern used by useAutoFocusInput across 100+ screens. Falls back to setTimeout(CONST.SCREEN_TRANSITION_END_TIMEOUT) when transitionEnd does not fire (e.g. when the screen is already focused).
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
melvin-bot
Bot
requested review from
flaviadefaria and
gijoe0295
and removed request for
a team
|
@gijoe0295 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Android only opens the soft keyboard once the app window has focus, so we | ||
| // chain: wait for the screen transition to finish, then for the window-focus | ||
| // signal (a no-op on iOS and web), then focus the input. | ||
| let didFocus = false; |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The transitionEnd listener + setTimeout fallback + isWindowReadyToFocus() pattern introduced here duplicates the same pattern already implemented in src/hooks/useAutoFocusInput.ts (lines 100-126). Both use navigation.addListener('transitionEnd', ...), check event?.data?.closing, fall back with setTimeout(..., CONST.SCREEN_TRANSITION_END_TIMEOUT), and chain with isWindowReadyToFocus() before focusing.
Consider extracting the shared "focus after transition" logic into a reusable hook (e.g., useTransitionEndCallback) that both useAutoFocusInput and this component can consume, or adapt useAutoFocusInput to work for this use case. This would eliminate the maintenance burden of keeping two copies of this non-trivial pattern in sync.
Reviewed at: 0ac8e59 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Acknowledged but keeping the inlined approach. The proposal explicitly evaluated reusing useAutoFocusInput and rejected it because adapting it for MagicCodeInput requires adding an inputCallbackRef prop to the input's public API — which would cascade through every existing MagicCodeInput consumer, well outside the scope of an Android-keyboard-only bug fix on one screen. Extracting a brand-new shared hook is the same amount of scope creep with the added downside of two callers immediately depending on a hook with no other proven users.
The current inlining is a localized 30-line block that's tightly bound to this one useFocusEffect; the maintenance burden of keeping it loosely in sync with useAutoFocusInput is low, and the C+ reviewer already approved this approach. Happy to revisit as a follow-up if there's a third consumer in the future.
| }; | ||
|
|
||
| jest.mock('@react-navigation/native', () => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The eslint-disable-next-line @typescript-eslint/no-unsafe-assignment directive lacks a justification comment explaining why the rule is disabled.
Add a comment explaining the reason, e.g.:
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- jest.requireActual returns unknown; safe cast for test mock
const actualNav = jest.requireActual('@react-navigation/native');Reviewed at: 0ac8e59 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Applied in 0dd1382 — added a justification comment to the eslint-disable directive explaining that jest.requireActual returns unknown and this is the standard pattern for spreading the actual module in jest.mock factories.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ac8e59061
ℹ️ 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".
| isWindowReadyToFocus().then(() => { | ||
| inputValidateCodeRef.current?.focusLastSelected(); |
There was a problem hiding this comment.
On Android, isWindowReadyToFocus() can remain pending until the next AppState focus event (src/libs/isWindowReadyToFocus/index.android.ts), but this .then is not cancelled when the route blurs; cleanup only removes the listener/timer. If the magic-code screen starts this wait and then loses navigation focus while the app window is still blurred, the promise later resolves and focuses this screen's input even though another screen is active, stealing focus/keyboard. Track an isActive flag (or equivalent) and check it before calling focusLastSelected().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — applied in 0dd1382.
Added an isCancelled flag local to the useFocusEffect callback, set to true in the cleanup function and checked inside the isWindowReadyToFocus().then(...) callback before calling focusLastSelected(). That way if the screen unfocuses (or unmounts) while the window-ready promise is still pending on Android, the late resolution becomes a no-op instead of stealing keyboard focus on whatever screen is now active.
Also added a unit test covering the scenario (does not steal focus if the screen unfocuses before isWindowReadyToFocus resolves) — it mounts the form, fires transitionEnd, unmounts before resolving the promise, then resolves it and asserts focusLastSelected was never called.
1 participant
Explanation of Change
On Android, the soft keyboard did not open automatically when reaching the magic code input screen after adding a new contact method. The form was relying on a fixed 300 ms delay before focusing the input, which has no relationship to whether the app window actually holds focus at that moment. Android's documented behavior is to silently drop the keyboard request whenever the window has not regained focus by the time the focus call is made, so the keyboard never appeared. iOS and web were unaffected because they do not have the same window-focus requirement.
The fix replaces the fixed delay with two combined signals. First, the form listens for the navigation transition-end event so it focuses the input exactly when the screen transition finishes (deterministic instead of a guessed timeout). Then it awaits a small helper that resolves only once the app window has regained focus — a no-op on iOS and web, and on Android a Promise tied to the system's window focus events. A safety fallback timer fires if the transition-end event never arrives (for example when the screen is already focused while the effect runs), and a guard flag ensures the listener and the fallback never both run.
The mobile Safari path is intentionally left as a synchronous focus call, because that browser only opens the keyboard when the focus call originates inside the user gesture handler.
Fixed Issues
$ #80025
PROPOSAL: #80025 (comment)
Tests
Offline tests
The fix only changes when and how the magic code input is focused after a screen transition; it does not alter any network behavior. With the device offline, opening the magic code screen still focuses the input and opens the keyboard exactly as in the online case (verification of the magic code itself still fails offline as before, since that requires a network request).
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
Kapture.2026-05-16.at.20.15.14.mp4
Android: mWeb Chrome
Kapture.2026-05-16.at.20.24.15.mp4
iOS: Native
Kapture.2026-05-16.at.20.32.39.mp4
iOS: mWeb Safari
Kapture.2026-05-16.at.20.34.35.mp4
MacOS: Chrome / Safari
Kapture.2026-05-16.at.20.26.57.mp4