fix: show preparation state for Money Account deposits cp-8.3.0 by pedronfigueiredo · Pull Request #33325 · MetaMask/metamask-mobile

pedronfigueiredo · 2026-07-15T10:07:46Z

Description

Money Account deposits could appear unresponsive for several seconds after the user pressed Continue because the amount-entry keypad remained visible while transaction calldata and quote prerequisites were prepared.

This change adds a local preparation state for Money Account deposits. It immediately replaces the keypad with the existing review-loading presentation, keeps the From and Pay with rows visible but non-interactive, disables amount editing and duplicate submission, and displays fee/time/total skeletons with a disabled Add funds button. The amount retains its normal visual emphasis while its edit action is unavailable. The same UI remains visible while TransactionPayController fetches quotes, and the amount, account, and payment token stay locked until quote loading settles. This avoids a perceived loading restart, stale review details, or an enabled-button flicker between the two phases.

If preparation fails, the amount-entry keypad is restored and the existing error toast is retained. Relay quote latency and quote failures remain separate from this local preparation state.

Changelog

CHANGELOG entry: Fixed missing loading feedback while Money Account deposits were being prepared

Related issues

Refs: #33317

Manual testing steps

Feature: Money Account deposit preparation feedback

  Scenario: user continues from Money Account deposit amount entry
    Given a user is entering an Add funds amount for a Money Account deposit

    When the user presses Continue
    Then the keypad is immediately replaced by a loading review
    And the From and Pay with rows remain visible but cannot be changed
    And the Add funds button remains disabled until quote loading settles

  Scenario: Money Account deposit preparation fails
    Given a user pressed Continue from Money Account deposit amount entry

    When transaction preparation fails
    Then the amount-entry keypad is restored
    And the existing amount-update error toast is displayed

Automated verification:

yarn jest app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx --runInBand --silent
yarn lint:tsc
Targeted ESLint (no errors; pre-existing deprecation warnings remain)

Screenshots/Recordings

Before

N/A — no simulator recording was captured for this code-only change.

After

N/A — the preparation transition is covered by targeted component unit tests.

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
I've completed the PR template to the best of my ability
I've included tests if applicable
I've documented my code using JSDoc format if applicable
I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Performance checks (if applicable)

I've tested on Android — N/A; manual device testing was not performed in this session.
I've tested with a power user scenario — N/A; this change targets immediate visual feedback and has focused unit coverage.
I've instrumented key operations with Sentry traces for production performance metrics — N/A; instrumentation and underlying controller optimization are separate follow-up work.

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches confirmation UX and amount-commit/quote handoff timing for money deposits; regressions could affect button state, stale quotes, or non-money flows, though tests target those paths.

Overview
Money Account deposit Continue no longer leaves the amount keypad up while calldata and quotes are prepared. CustomAmountInfo now enters a local preparation / loading review as soon as Done runs: the keypad closes, fee/time/total skeletons show, From / Pay with stay visible but review rows use pointerEvents: 'none', amount edit is blocked (with normal text styling), duplicate Done is guarded via a ref, and Add funds stays disabled through controller quote loading.

Preparation ends when TransactionPay loading finishes or quotesLastUpdated changes after the commit (new useTransactionPayQuotesLastUpdated selector/hook), with logic to ignore stale quote timestamps mid-commit and a zero-timeout handoff for synchronous updates. Failures reopen the keypad and keep the existing amount-update toast. Payment detail rendering is centralized in a Quote helper; ConfirmButton also respects isAmountUpdating.

Coverage adds a Money Account quote preparation test block and related rerender helpers.

^{Reviewed by Cursor Bugbot for commit 43f430c. Bugbot is set up for automated code reviews on this repo. Configure here.}

github-actions · 2026-07-15T10:07:54Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

github-actions · 2026-07-15T13:15:09Z

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File	7d	15d	30d
`app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx`	0/122	0/164	0/372

AI-detected flaky patterns

`app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx`

J1 — Missing act() around async state updates (high)

The test wraps two fireEvent.press calls in a synchronous act(), but the button's handler calls updateTokenAmount which returns a Promise (the deferred). Synchronous act() does not flush microtasks or await the async handler, so React state updates triggered by the resolved/rejected promise can escape the act() boundary and cause 'Warning: An update to ... inside a test was not wrapped in act(...)' — an intermittent failure depending on microtask scheduling. The fix is to use await act(async () => { ... }) so that all pending microtasks are flushed before the assertion phase.

Suggested fix in app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx:

-it('blocks review rows, amount editing, and duplicate submission during preparation', () => {
-      const { updateTokenAmount } = arrangePendingPreparation();
-      const { getByTestId } = render({
-        supportAccountSelection: true,
-        transactionType: TransactionType.moneyAccountDeposit,
-      });
-
-      const doneButton = getByTestId('deposit-keyboard-done-button');
-      act(() => {
-        fireEvent.press(doneButton);
-        fireEvent.press(doneButton);
-      });
+it('blocks review rows, amount editing, and duplicate submission during preparation', async () => {
+      const { updateTokenAmount } = arrangePendingPreparation();
+      const { getByTestId } = render({
+        supportAccountSelection: true,
+        transactionType: TransactionType.moneyAccountDeposit,
+      });
+
+      const doneButton = getByTestId('deposit-keyboard-done-button');
+      await act(async () => {
+        fireEvent.press(doneButton);
+        fireEvent.press(doneButton);
+      });

J1 — Missing act() around async state updates (high)

fireEvent.press triggers the done-button handler which calls updateTokenAmount (returning a deferred Promise). This async state update is not wrapped in act(), so React may process the resulting state changes outside the test's act boundary, causing intermittent 'not wrapped in act(...)' warnings that can turn into test failures in strict mode or under CI load. The press should be wrapped in await act(async () => { ... }).

Suggested fix in app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx:

-it('keeps the loading review throughout quote loading', async () => {
-      const { deferred } = arrangePendingPreparation();
-      const view = render({
-        transactionType: TransactionType.moneyAccountDeposit,
-      });
-      fireEvent.press(view.getByTestId('deposit-keyboard-done-button'));
-      useIsTransactionPayLoadingMock.mockReturnValue(true);
+it('keeps the loading review throughout quote loading', async () => {
+      const { deferred } = arrangePendingPreparation();
+      const view = render({
+        transactionType: TransactionType.moneyAccountDeposit,
+      });
+      await act(async () => {
+        fireEvent.press(view.getByTestId('deposit-keyboard-done-button'));
+      });
+      useIsTransactionPayLoadingMock.mockReturnValue(true);

J1 — Missing act() around async state updates (high)

fireEvent.press on the done button triggers an async handler (updateTokenAmount returning a deferred Promise) without being wrapped in act(). The subsequent mock update (useTransactionPayQuotesLastUpdatedMock.mockReturnValue(2)) happens synchronously after the press, but any React state updates from the async handler can still escape the act boundary. Wrapping the press in await act(async () => { ... }) ensures all microtasks from the async handler are flushed before the mock is updated.

Suggested fix in app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx:

-it('ignores stale quote updates while the amount is still committing', async () => {
-      useTransactionPayQuotesLastUpdatedMock.mockReturnValue(1);
-      const { deferred } = arrangePendingPreparation();
-      const view = render({
-        transactionType: TransactionType.moneyAccountDeposit,
-      });
-      fireEvent.press(view.getByTestId('deposit-keyboard-done-button'));
-      useTransactionPayQuotesLastUpdatedMock.mockReturnValue(2);
+it('ignores stale quote updates while the amount is still committing', async () => {
+      useTransactionPayQuotesLastUpdatedMock.mockReturnValue(1);
+      const { deferred } = arrangePendingPreparation();
+      const view = render({
+        transactionType: TransactionType.moneyAccountDeposit,
+      });
+      await act(async () => {
+        fireEvent.press(view.getByTestId('deposit-keyboard-done-button'));
+      });
+      useTransactionPayQuotesLastUpdatedMock.mockReturnValue(2);

J1 — Missing act() around async state updates (high)

Same pattern as the other 'keeps the loading review' tests: fireEvent.press triggers an async handler (the deferred updateTokenAmount) without act() wrapping. React state updates from the async handler can escape the act boundary, causing intermittent 'not wrapped in act(...)' warnings/failures. The press should be wrapped in await act(async () => { ... }).

Suggested fix in app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx:

-it('keeps preparation active while the amount update is pending', async () => {
-      const { deferred } = arrangePendingPreparation();
-      const view = render({
-        transactionType: TransactionType.moneyAccountDeposit,
-      });
-      fireEvent.press(view.getByTestId('deposit-keyboard-done-button'));
-      useIsTransactionPayLoadingMock.mockReturnValue(true);
+it('keeps preparation active while the amount update is pending', async () => {
+      const { deferred } = arrangePendingPreparation();
+      const view = render({
+        transactionType: TransactionType.moneyAccountDeposit,
+      });
+      await act(async () => {
+        fireEvent.press(view.getByTestId('deposit-keyboard-done-button'));
+      });
+      useIsTransactionPayLoadingMock.mockReturnValue(true);

J3 — Missing jest.clearAllMocks()/resetAllMocks() between tests (medium)
- mockToastRef.current.closeToast is a jest.fn() created once at module level. jest.resetAllMocks() in beforeEach resets its call history and implementation, which is correct. However, mockShowToast is also reset separately with mockShowToast.mockReset() after jest.resetAllMocks() — this double-reset is redundant but harmless. The real risk is that mockToastRef is a shared object reference passed as a context value to every render. If any test mutates mockToastRef.current (e.g. replaces showToast or closeToast), that mutation persists into subsequent tests because the object is never re-created. Tests that rely on the original mockShowToast reference could then fail intermittently depending on test execution order. The fix is to recreate mockToastRef in beforeEach so each test gets a fresh reference.
- Suggested fix in app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx:
```
-const mockToastRef = {
-  current: { showToast: mockShowToast, closeToast: jest.fn() },
-};
-
-// ...
-
-beforeEach(() => {
-    jest.resetAllMocks();
-    mockShowToast.mockReset();
+let mockToastRef: { current: { showToast: jest.Mock; closeToast: jest.Mock } };
+
+// ...
+
+beforeEach(() => {
+    jest.resetAllMocks();
+    mockToastRef = {
+      current: { showToast: mockShowToast, closeToast: jest.fn() },
+    };
```

This check is informational only and does not block merging.

sonarqubecloud · 2026-07-15T14:18:32Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

matthewwalsh0 · 2026-07-15T15:35:02Z

      updateTokenAmount,
    ]);

+    useEffect(() => {


Why do we need to wait for quote completion explicitly, can't we just immediately clear after the update in a finally block and the quote loading will already be triggered automatically and keep the skeletons visible?

matthewwalsh0 · 2026-07-15T15:37:49Z

    const [isKeyboardVisible, setIsKeyboardVisible] = useState(
      !isAddMusdIntent && !isDepositPrefillEnabled,
    );
+    const [isPreparingQuote, setIsPreparingQuote] = useState(false);


Why do we need both state and a ref?

If we're setting state and are therefore happy with the re-render, what purpose does the ref serve?

Kept the ref, but renamed it to isAmountUpdateInProgressRef. isPreparingQuote controls rendering, while the ref synchronously blocks a second Done press before React commits the state update. Added regression coverage that fires two presses in the same batch and verifies updateTokenAmount runs once. Addressed in bf771c752b.

matthewwalsh0 · 2026-07-15T15:38:08Z

    const [isKeyboardVisible, setIsKeyboardVisible] = useState(
      !isAddMusdIntent && !isDepositPrefillEnabled,
    );
+    const [isPreparingQuote, setIsPreparingQuote] = useState(false);


Should we be explicit and call this isAmountUpdating for clarity?

matthewwalsh0 · 2026-07-15T15:38:36Z

+    const isQuoteLoading = useIsTransactionPayQuoteLoading();
    const isQuotesLoading = useIsTransactionPayLoading();
+    const showLoadingReview = isPreparingQuote || isQuotesLoading;
+    const showMoneyAccountLoadingReview =


Does this need to be type specific, won't this logic be applicable for any flow?

Ideally we minimise type specific logic here for simplicity sake.

matthewwalsh0 · 2026-07-15T15:39:03Z

+        return;
+      }
+
+      if (isMoneyAccountDeposit) {


Can we apply this logic universally so immediately close keyboard always?

matthewwalsh0 · 2026-07-15T15:40:14Z

            }
-            onPress={handleAmountPress}
+            onPress={
+              showMoneyAccountLoadingReview ? undefined : handleAmountPress


isAmountUpdating?

Won't impact other flows as they are synchronous.

Already addressed in c21b192447: the shared render state is named isAmountUpdating and is used for all flows. Synchronous flows resolve it immediately, while asynchronous flows keep the loading state active. The focused tests cover both Money and non-Money updates.

matthewwalsh0 · 2026-07-15T15:42:10Z

 function ConfirmButton({
  alertTitle,
  disableConfirm,
+  isLoadingReview,


isAmountUpdating?

matthewwalsh0 · 2026-07-15T15:43:18Z

            !hideBuyForNoFunds &&
            !isDepositPrefillEnabled && <BuySection />}
-          {!isKeyboardVisible && (
+          {(!isKeyboardVisible || showMoneyAccountLoadingReview) && (


If we immediately hide the keyboard, why do we also need to check showMoneyAccountLoadingReview ?

matthewwalsh0 · 2026-07-15T15:43:58Z

-            />
-          )}
+          {isKeyboardVisible &&
+            !showMoneyAccountLoadingReview &&


Also here, ideally we minimise the criteria for these render conditions, so can't we rely on the existing isKeyboardVisible since we hide keyboard explicitly when starting update?

matthewwalsh0 · 2026-07-15T15:46:20Z

@@ -375,7 +423,9 @@
                <PayWithRow isResultReady />
              )}
              {!hasAccountNoFunds &&


This JSX is quickly getting scary, could we make a Quote component in this file to handle the quote rows and skeletons? So we can early return and remove all these ternary statements?

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit f29e5ae. Configure here.}

github-actions · 2026-07-16T08:59:35Z

🔍 Smart E2E Test Selection

Selected E2E tags: SmokeMoney, SmokePerps, SmokeStake, SmokeConfirmations, SmokeWalletPlatform, SmokeSwap
Selected Performance tags: None (no tests recommended)
Risk Level: medium
AI Confidence: 85%

click to see 🤖 AI reasoning details

E2E Test Selection:

The PR introduces UX improvements to the custom amount entry flow used in Money (card/ramps) and Perps withdraw confirmations:

transactionPayController.ts - New selector selectTransactionPayQuotesLastUpdatedByTransactionId to track quote update timestamps. This is a new selector that doesn't modify existing behavior.
useTransactionPayData.ts - New hook useTransactionPayQuotesLastUpdated() exposing the new selector.
custom-amount-info.tsx - Significant UI logic changes:
- Added isAmountUpdating / isAmountUpdateComplete state with a ref-based synchronous guard to prevent double-taps
- Shows a loading skeleton (PaymentDetailsSkeleton) while quotes refresh after amount update
- Disables interaction (pointerEvents='none') during loading
- Refactored quote display into a new Quote sub-component
- Added isAmountUpdating prop to ConfirmButton to disable it during updates
- New useEffect to transition from loading state once quotes are refreshed
custom-amount-info.testIds.ts - Added REVIEW_ROWS test ID.

Affected flows:

SmokeMoney: CustomAmountInfo is used in money account deposit/withdraw confirmations (card/ramps flows). The loading state changes directly affect this flow.
SmokePerps: CustomAmountInfo is used in perps-withdraw-info.tsx for the Perps withdraw flow.
SmokeStake: Lending deposit/withdraw flows use this component for amount entry.
SmokeConfirmations: Required by SmokeMoney, SmokePerps, and SmokeStake per tag descriptions.
SmokeWalletPlatform: Required by SmokePerps (Perps is a section inside Trending).
SmokeSwap: Required by SmokeMoney for Add Funds flows that execute swaps.

The changes are medium risk - they modify UI state management logic in a shared component used across multiple flows, but the changes are well-scoped to the amount update loading state and don't touch core transaction submission logic.

Performance Test Selection:
The changes are UI state management improvements (loading states, double-tap prevention) in the custom amount entry component. While they affect rendering behavior, they don't impact core performance-sensitive flows like app launch, login, onboarding, or asset loading. The changes add minor state tracking (refs and useState) but no performance-critical paths are modified. No performance test tags are warranted.

View GitHub Actions results

fix: show preparation state for money account deposits

38766a2

metamask-ci Bot added the team-confirmations Push issues to confirmations team label Jul 15, 2026

github-actions Bot added the size-M label Jul 15, 2026

pedronfigueiredo marked this pull request as ready for review July 15, 2026 10:11

pedronfigueiredo requested a review from a team as a code owner July 15, 2026 10:11

github-actions Bot added the risk:medium AI analysis: medium risk label Jul 15, 2026

cursor Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx Outdated

fix: keep money deposit loading state continuous

525c3fe

github-actions Bot added risk:high AI analysis: high risk and removed risk:medium AI analysis: medium risk labels Jul 15, 2026

fix: use disabled add funds during money loading

04c4b55

github-actions Bot added risk:medium AI analysis: medium risk and removed risk:high AI analysis: high risk labels Jul 15, 2026

pedronfigueiredo added 2 commits July 15, 2026 14:35

fix: preserve money deposit amount emphasis while loading

a4d0b00

fix: complete money loading handoff for fast quotes

2dd975f

pedronfigueiredo changed the title ~~fix: show preparation state for Money Account deposits~~ fix: show preparation state for Money Account deposits cp-8.3.0 Jul 15, 2026

cursor Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx Outdated

matthewwalsh0 self-requested a review July 15, 2026 14:45

matthewwalsh0 requested changes Jul 15, 2026

View reviewed changes

pedronfigueiredo added 8 commits July 15, 2026 17:20

fix: ignore stale quotes during amount preparation

fe32006

refactor: hand off quote loading after amount update

57fdf70

refactor: clarify synchronous amount update guard

bf771c7

refactor: rename amount preparation state

46685a7

refactor: share amount update loading state

c21b192

test: cover universal keyboard dismissal

874e347

test: cover universal amount update recovery

7ab75c8

refactor: rename amount update button prop

fa3cbfd

pedronfigueiredo added 3 commits July 15, 2026 20:35

refactor: rely on keyboard visibility for keypad

bfdef16

refactor: rely on keyboard visibility for confirm

d63d0db

refactor: extract quote details component

4e5fed8

github-actions Bot added size-L and removed size-M labels Jul 16, 2026

cursor Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx Outdated

fix: preserve loading through quote handoff

f29e5ae

cursor Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx

fix: clear synchronous quote handoff

43f430c

pedronfigueiredo requested a review from matthewwalsh0 July 16, 2026 11:13

Uh oh!

Uh oh!

Conversation

pedronfigueiredo commented Jul 15, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

Pre-merge reviewer checklist

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 Flaky unit test detection

Run history flaky detection

AI-detected flaky patterns

app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 15, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026

🔍 Smart E2E Test Selection

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pedronfigueiredo commented Jul 15, 2026 •

edited by cursor Bot

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading

`app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx`