Fix: gate forceKeyboardIfAlreadyFocused per-platform to remove Android discard-changes keyboard regression - #98808
Fix: gate forceKeyboardIfAlreadyFocused per-platform to remove Android discard-changes keyboard regression#98808MelvinBot wants to merge 2 commits into
Conversation
…d discard-keyboard regression Co-authored-by: Kevin Brian Bader <ikevin127@users.noreply.github.com>
Co-authored-by: Kevin Brian Bader <ikevin127@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
No code fix is needed — every real CI check on this PR is now green. When you filed this request, several checks were still pending (all Current check status
Next step: get an independent reviewer to approve the PR and that check will clear. Nothing to push here. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@ikevin127 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] |
|
🤖 Done,
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppscreen-20260817-170441-1787011447337.mp4Android: mWeb Chromescreen-20260804-170521-1785888299711.mp4iOS: HybridAppSimulator.Screen.Recording.-.iPhone.17.Pro.Max.-.2026-08-04.at.16.53.57.moviOS: mWeb SafariScreenRecording_08-04-2026.16-49-16_1.MP4MacOS: Chrome / SafariScreen.Recording.2026-08-04.at.16.42.03.mov |
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM - Follow-up PR to fix Android specific regression by gating previous PRs logic specifically to iOS, making sure Android is not affected by the change
Explanation of Change
#97823 fixed the iOS bug where the description field lost focus/keyboard after canceling the "Discard changes?" modal, by passing
forceKeyboardIfAlreadyFocused = truetofocusComposerWithDelayin the discard-modalonCancelhandler:That third argument is a platform-agnostic flag gating a platform-divergent native action.
requestKeyboardForFocusedComposer(called under that flag) only has anindex.ts(web NOOP) and anindex.native.ts, so Android silently inherited a fix that was reasoned about and QA-verified only for iOS — and it regressed: after tapping Cancel on Android Native the keyboard no longer returns.Why the two platforms diverge under the same flag:
KeyboardController.setFocusTo('current')isreloadInputViews+ focus on the current first responder: cheap and idempotent. The input never lost first-responder status (onlyeditable={false}suppressed the IME), so re-requesting it is exactly right.ReactEditText.requestFocusProgrammatically(), whoseshowSoftKeyboard()is gated behindisInTouchMode. The discard modal is a separateDialogwindow and the back path (useDiscardChangesConfirmation/index.native.tswires it tohardwareBackPress) can leave the app window out of touch mode, sosuper.requestFocus()succeeds (caret returns) butshowSoftKeyboard()is skipped — focus without keyboard. On Android the early-return was already correct: once the Dialog dismisses, the system restores focus and the IME on its own.Fix: gate the flag at our call site only, via a platform-resolved helper — no shared file is touched.
src/libs/shouldForceKeyboardIfAlreadyFocused/index.ios.ts→true(preserves the Fix: return focus and soft keyboard to description input on iOS after canceling discard modal #97823 iOS fix).src/libs/shouldForceKeyboardIfAlreadyFocused/index.ts→false(Android/web).DynamicIOURequestStepDescription.tsxnow passesshouldForceKeyboardIfAlreadyFocused()instead of a hardcodedtrue.This follows the same platform-resolved-file pattern as
src/libs/shouldBreakAccessibilityGrouping/rather than an inlinePlatform.OScheck.Why it's safe: Metro resolves the helper to
index.tson Android (noindex.android.ts/index.native.tsexists) and toindex.ios.tson iOS. On Android the call becomes(true, undefined, false)— argument-identical to pre-#97823main, not just similar — so the regression is removed. iOS keeps(true, undefined, true). Web/mWeb are unaffected (requestKeyboardForFocusedComposer/index.tsis a NOOP), andComposerWithSuggestionsis untouched since no shared file changed.Unit test: the one-liner was previously untestable, which is part of why this shipped unverified twice.
tests/ui/IOURequestStepDescriptionTest.tsxnow mocks the helper and pins both platform behaviors: helpertrue→focusComposerWithDelaycalled with(true, undefined, true); helperfalse→(true, undefined, false). Either assertion fails loudly if the argument is ever hardcoded again during a cleanup.Fixed Issues
$ #97604
PROPOSAL:
Tests
ComposerWithSuggestionscontainment check (Android + iOS). Open a chat, tap an existing message to edit it; verify the keyboard appears. Should be unchanged (no shared file modified).Automated:
tests/ui/IOURequestStepDescriptionTest.tsxpins both platform argument values (helpertrue→(true, undefined, true); helperfalse→(true, undefined, false)).Offline tests
Identical to the Tests above. This change only affects focus and keyboard behavior and never touches the network, so every step should behave the same with the connection off.
QA Steps
Same as tests. The Android: Native, hardware back button path (test 1) is the critical one — QA on a physical device / release build is required to confirm the soft keyboard returns after Cancel, since this native touch-mode behavior can't be verified in a dev simulator.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Automated smoke check of the Description discard-changes Cancel flow on standalone Android NewDot (Melvin
app-android-tester). Text is preserved and focus returns to the field with no crash. Keyboard visibility could not be independently confirmed by the harness — it injects text via its own IME helper rather than a rendered soft keyboard — so the hardware-back keyboard-return behavior still needs QA on a device build (Tests step 1).Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari