Fix: Android keyboard not opening on Magic Code screen#83229
Fix: Android keyboard not opening on Magic Code screen#83229twinkle-react wants to merge 3 commits intoExpensify:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
@CLAassistant sign |
|
test |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82d0c568b3
ℹ️ 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".
| if (DeviceCapabilities.canUseTouchScreen()) { | ||
| focusTimeoutRef.current = setTimeout(runFocusWithRetry, 0); | ||
|
|
There was a problem hiding this comment.
Restore immediate focus on Mobile Safari
This change removes the old synchronous Mobile Safari focus path and now always defers focus on touch devices via setTimeout(..., 0), but this form previously documented that delayed focus prevents the keyboard from opening on iOS Safari. In mobile web Safari, that means the magic code screen can regress to not opening the keyboard automatically again; keep the Android retry behavior, but preserve immediate focus when isMobileSafari() is true.
Useful? React with 👍 / 👎.
| return ( | ||
| <> | ||
| <MagicCodeInput | ||
| key={focusCycle} |
There was a problem hiding this comment.
Avoid remounting MagicCodeInput on each focus cycle
Setting key={focusCycle} recreates MagicCodeInput every time this screen gains focus, which resets its internal wasSubmitted guard used to prevent repeated onFulfill calls after a full code is entered. If a user returns to the focused screen with an already-complete code value, the remounted input can auto-submit that same code again and trigger duplicate verify requests.
Useful? React with 👍 / 👎.
Inline Review Comments for
|
I have read the CLA Document and I hereby sign the CLA |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Fine for product 👍
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.
|
| const focusInput = () => { | ||
| inputValidateCodeRef.current?.focusLastSelected(); | ||
| }; | ||
|
|
||
| // Force a fresh input instance on each screen focus to avoid stale native focus state on reopen. | ||
| setFocusCycle((prev) => prev + 1); | ||
|
|
||
| const runFocusWithRetry = () => { | ||
| focusInput(); | ||
| focusTimeoutRef.current = setTimeout(() => { | ||
| if (inputRef.current?.isFocused?.()) { | ||
| return; | ||
| } | ||
| focusInput(); | ||
| }, CONST.ANIMATED_TRANSITION); | ||
| }; | ||
|
|
There was a problem hiding this comment.
@twinkle-react Which use cases do we need to handle with this change?
There was a problem hiding this comment.
@suneox I fixed the Magic Code keyboard issue by improving how input focus is handled when the screen gains focus. The input is re-initialized and programmatically focused, with a retry after screen transitions to handle Android focus timing issues. This ensures the keyboard opens consistently on both initial and subsequent visits, while behavior on other platforms remains unchanged.
|
@twinkle-react Please resolve conflict, fix signed commit, eslint & typecheck |
|
@twinkle-react can you continue the work on this PR please? |
|
Conflict resolved please review |
|
@twinkle-react oh your commit is not verified, can you fix that too, please? |
Sure Let me fix it |
|
I’ll take a look at this one in about an hour |
| const inputValidateCodeRef = useRef<MagicCodeInputHandle>(null); | ||
| const [account = getEmptyObject<Account>()] = useOnyx(ONYXKEYS.ACCOUNT); | ||
| const {inputCallbackRef, inputRef} = useAutoFocusInput(false); | ||
| const [account = getEmptyObject<Account>()] = useOnyx(ONYXKEYS.ACCOUNT, { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
useAutoFocusInput already provides comprehensive focus management internally -- it tracks screen transitions via transitionEnd listener, uses InteractionManager.runAfterInteractions, handles splash screen state, popover visibility, and side panel transitions, then calls inputRef.current?.focus() at the right moment. However, the useFocusEffect callback below (lines ~170-214) implements a second, parallel focus management system with its own InteractionManager.runAfterInteractions call (line 204), setTimeout retry logic, DeviceCapabilities branching, and immediate focusLastSelected() calls -- all targeting the same input.
Two independent systems racing to focus the same MagicCodeInput makes behavior hard to predict and debug.
Consolidate into a single approach:
- If
useAutoFocusInputis the right tool, remove the manual focus logic fromuseFocusEffectand let the hook handle everything. - If the hook does not meet the requirements (e.g., it calls
focus()but you needfocusLastSelected()), either extend the hook or remove it and keep only the manual approach.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11b08db784
ℹ️ 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".
| // Delay prevents the input from gaining focus before the RHP slide-out animation finishes, | ||
| // which would cause issues with the RHP sliding out smoothly and flickering of the wide RHP in the background. | ||
| if ((wideRHPRouteKeys.length > 0 && !isMobileSafari()) || isInPageModal) { | ||
| const shouldDelayClear = !DeviceCapabilities.canUseTouchScreen() && ((wideRHPRouteKeys.length > 0 && !isMobileSafari()) || isInPageModal); |
There was a problem hiding this comment.
Restore the clear delay for touch page-modals
When isInPageModal is true (it defaults to true in ValidateCodeActionContent), this condition is now false on every native/touch device because canUseTouchScreen() returns true there, so we call MagicCodeInput.clear() immediately instead of after the transition. clear() focuses the hidden text input again, which is the exact focus-during-transition path the old code and ContactMethodDetailsPage were guarding against; on mobile validate-code page modals this can reintroduce the slide/flicker/keyboard pop while the modal is opening or after resend.
Useful? React with 👍 / 👎.
|
you have a commit from |
| } else { | ||
| inputValidateCodeRef.current?.focusLastSelected(); | ||
| if (DeviceCapabilities.canUseTouchScreen()) { | ||
| focusTimeoutRef.current = setTimeout(runFocusWithRetry, 0); |
There was a problem hiding this comment.
This issue only occurs on Android, and runFocusWithRetry also runs below we have 2 places try to run focus
| setFocusCycle((prev) => prev + 1); | ||
|
|
||
| const runFocusWithRetry = () => { | ||
| focusInput(); | ||
| focusTimeoutRef.current = setTimeout(() => { | ||
| if (inputRef.current?.isFocused?.()) { | ||
| return; | ||
| } | ||
| focusInput(); | ||
| }, CONST.ANIMATED_TRANSITION); | ||
| }; |
There was a problem hiding this comment.
Since runFocusWithRetry is called in two places, focusInput ends up being triggered four times. This seems like a hardcoded workaround. @twinkle-react what happens on the latest main with the initial solution using the useAutoFocusInput hook?
There was a problem hiding this comment.
Thanks for the feedback!
You're right — the current implementation introduces redundant focus logic.
I’m refactoring this to rely on a single focus mechanism using useAutoFocusInput and limiting the fix to Android only.
Also removing the remount-based approach and simplifying retries.
|
I have read the CLA Document and I hereby sign the CLA |
11b08db to
ef800f0
Compare
Explanation of Change
On Android, the keyboard was not automatically opening when navigating to the Magic Code screen after adding a new contact method.
The issue occurred because the
TextInputwas not explicitly focused when the screen gained focus on Android.This PR ensures the Magic Code input is programmatically focused when the screen mounts / gains focus so that the keyboard opens automatically.
Fixed Issues
# 80025
$ PROPOSAL: #80025 (comment)
Tests
Offline tests
QA Steps
// 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
Android.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari