Skip to content

fix(activity): stuck confirmation on back after tx + missing lending deposit token icon#33157

Draft
vinnyhoward wants to merge 2 commits into
mainfrom
fix-tmcu-1001-activity-confirmation-back-navigation
Draft

fix(activity): stuck confirmation on back after tx + missing lending deposit token icon#33157
vinnyhoward wants to merge 2 commits into
mainfrom
fix-tmcu-1001-activity-confirmation-back-navigation

Conversation

@vinnyhoward

@vinnyhoward vinnyhoward commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

Two related Activity-redesign bug fixes on one branch.

1. Stuck confirmation on back after a transaction (TMCU-1001).
After submitting a full-screen confirmation (send, Earn pooled-staking, or stablecoin lending), the app redirects to the Activity screen. With the Money-account feature enabled, Activity is registered as a root-level screen, so the redirect pushed it on top of the still-mounted confirmation — whose approval had already been consumed (deleteAfterResult). Pressing back returned to that consumed confirmation, which rendered an infinite loader/skeleton.

The fix adds a shared navigateToActivityAfterConfirmation helper that replaces the confirmation's stack with Activity in a single StackActions.replace, removing the stale confirmation from the back stack. It's wired into the shared transaction confirm hook (useTransactionConfirm), batch approvals (useConfirmActions), the Earn lending deposit/withdrawal views, and the legacy staking footer.

Approach note: I first tried to make "back" return to the transaction build screen (amount / recipient / input). On a native-stack that means removing the nested confirmation while pushing a root-level Activity — two screen changes that react-native-screens animates at the same time, producing a double-navigation on the way in and a flashing half-finished pop on the way back. After several attempts to work around the animation, the reliable single-transition solution was to replace the confirmation's whole stack. As a result: send/staking → back returns to Wallet home; lending → back returns to the input screen (its input lives in a separate stack that survives the replace).

2. Missing token icon/amount on lending deposit rows (TMCU-1082).
Lending deposit Activity rows derived their token from the pool contract (txParams.to) instead of the underlying deposited asset, so the row had no resolvable icon, symbol, or amount. The supplied token is now resolved from simulation data / the outgoing Transfer log (falling back to the pool address), and lending token metadata is enriched from the tokens API; the row avatar uses the resolved token.

Changelog

CHANGELOG entry: Fixed the Activity screen back button returning to a stuck confirmation screen after a transaction, and fixed the missing token icon and amount on Earn lending deposit rows.

Related issues

Fixes: TMCU-1001
Fixes: TMCU-1082

Manual testing steps

Feature: Activity navigation after a transaction

  Background:
    Given the Money-account and Activity-redesign feature flags are enabled

  Scenario: Back after a send or staking transaction is not stuck
    Given I complete a native/ERC-20/NFT send, or an ETH pooled-staking deposit/unstake/claim
    When I am redirected to the Activity screen and tap the back button
    Then I return to Wallet home
    And I never land on the consumed confirmation screen or an infinite loader

  Scenario: Back after a lending deposit returns to the input screen
    Given I complete a stablecoin (USDC/USDT/DAI) lending deposit or withdrawal
    When I am redirected to the Activity screen and tap the back button
    Then I return to the lending input screen
    And I never land on the consumed confirmation screen

Feature: Lending deposit Activity row

  Scenario: Lending deposit row shows the deposited token
    Given I complete a stablecoin lending deposit
    When I open the Activity list
    Then the lending deposit row shows the underlying token's icon, symbol, and amount

Also covered by automated tests: real-navigator integration tests for all three navigation flows (navigateToActivityAfterConfirmation.test.tsx), the redirect call sites, and the lending row/adapter changes (ActivityListItemRow, local-transaction).

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

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.

Lending deposit rows derived their token from the pool contract (txParams.to) instead of the underlying deposited asset, so the Activity row had no resolvable icon, symbol, or amount. Resolve the supplied token from simulation data / the outgoing Transfer log (falling back to the pool address), and enrich lending token metadata from the tokens API so the row shows the correct avatar and amount.

TMCU-1082
…ter a transaction

After a full-screen confirmation submitted, the redirect pushed the Activity screen on top of the now-consumed confirmation (approval deleted via deleteAfterResult), so pressing back returned to it and it rendered an infinite loader. A new navigateToActivityAfterConfirmation helper replaces the confirmation's stack with Activity in a single StackActions.replace, avoiding the stale screen and the pop+push double-animation. Wired into the shared transaction confirm hook, batch approvals, the Earn lending deposit/withdrawal views, and the legacy staking footer.

TMCU-1001
@github-actions

Copy link
Copy Markdown
Contributor

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.

@mm-token-exchange-service mm-token-exchange-service Bot added the team-mobile-ux Mobile UX team label Jul 10, 2026
@mm-token-exchange-service

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Screenshots/Recordings section is empty. Add an image/video for user-facing changes, logs/console output for non-user-facing changes, or write N/A if no evidence is applicable.
  • Pre-merge author checklist has unchecked items (e.g. "I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors."). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: 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 several related changes:

  1. New navigateToActivityAfterConfirmation utility: Replaces direct navigation.navigate(Routes.TRANSACTIONS_VIEW) calls with a smarter StackActions.replace approach that prevents stale confirmation screens from appearing in the back stack. This affects post-confirmation navigation across multiple flows.

  2. Staking confirmation (FooterButtonGroup.tsx): Uses the new navigation utility after staking/unstaking confirmation. This directly impacts SmokeStake flows.

  3. Lending confirmation views (EarnLendingDepositConfirmationView, EarnLendingWithdrawalConfirmationView): Use the new navigation utility after lending deposit/withdrawal. Also impacts SmokeStake (lending is part of Earn/Stake).

  4. Transaction confirmation hooks (useConfirmActions.ts, useTransactionConfirm.ts): Updated for batch transactions (EIP-7702) and full-screen confirmations. This affects SmokeConfirmations broadly, and SmokeSwap since swap flows use full-screen confirmations.

  5. local-transaction.ts: Fixed lending deposit source token resolution - now correctly resolves the underlying asset (USDC/USDT) from simulation data or Transfer logs instead of the pool address. This fixes activity list display for lending deposits.

  6. useActivityListItemRowContent.ts: Added enrichTokenFromApi function and lending-specific token enrichment for activity list items, improving display of lending transactions in the activity feed. This affects SmokeWalletPlatform (activity/transaction history display) and SmokeStake.

Tags selected:

  • SmokeStake: Directly affected - staking/unstaking/lending confirmation navigation changed, lending activity display fixed
  • SmokeConfirmations: Transaction confirmation navigation changed for batch transactions and full-screen confirmations; per tag description, SmokeStake flows also require SmokeConfirmations
  • SmokeWalletPlatform: Activity list item display changes for lending transactions affect transaction history view
  • SmokeSwap: Full-screen confirmation navigation changes could affect swap confirmation flows; per tag description, SmokeSwap also requires SmokeConfirmations

The changes are medium risk - they modify navigation behavior after confirmation which is a critical user flow, but the logic is well-contained and the fallback behavior is preserved.

Performance Test Selection:
The changes are focused on post-confirmation navigation logic and activity list item display improvements. None of these changes affect app launch, login, onboarding, account list rendering, swap quote fetching performance, or other performance-sensitive flows measured by the available performance test tags. The navigation change (StackActions.replace vs navigate) is a correctness fix, not a performance-sensitive change.

View GitHub Actions results

@github-actions

Copy link
Copy Markdown
Contributor

🧪 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/UI/ActivityListItemRow/ActivityListItemRow.test.tsx 0/50 0/216 0/388

AI-detected flaky patterns

app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The 'display currency conversion' describe block restores specific mock return values in afterEach but never calls jest.clearAllMocks(). This means call counts for mockCurrency, mockConversionRate, and mockUsdConversionRate accumulate across every test in this block. If any future test in this file or a sibling describe block asserts toHaveBeenCalledTimes(N) on these selectors, it will see a higher count than expected and fail intermittently depending on test execution order. Additionally, if a test throws mid-body after calling mockCurrency.mockReturnValue('eur'), the afterEach still runs and resets the return value — but the call count from the failed test is never cleared, leaving stale state for subsequent tests. The outer 'row content' describe has jest.clearAllMocks() in beforeEach, but this inner describe does not inherit that guard.
    • Suggested fix in app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx:1:
      -describe('ActivityListItemRow — display currency conversion', () => {
      -  const mockCurrency = jest.mocked(selectCurrentCurrency);
      -  const mockConversionRate = jest.mocked(selectConversionRateByChainId);
      -  const mockUsdConversionRate = jest.mocked(selectUSDConversionRateByChainId);
      -  afterEach(() => {
      -    mockCurrency.mockReturnValue('usd');
      -    mockConversionRate.mockReturnValue(2500);
      -    mockUsdConversionRate.mockReturnValue(2500);
      -  });
      +describe('ActivityListItemRow — display currency conversion', () => {
      +  const mockCurrency = jest.mocked(selectCurrentCurrency);
      +  const mockConversionRate = jest.mocked(selectConversionRateByChainId);
      +  const mockUsdConversionRate = jest.mocked(selectUSDConversionRateByChainId);
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockCurrency.mockReturnValue('usd');
      +    mockConversionRate.mockReturnValue(2500);
      +    mockUsdConversionRate.mockReturnValue(2500);
      +  });
      +  afterEach(() => {
      +    jest.clearAllMocks();
      +  });
  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The 'ERC-20 fiat address casing' describe block uses afterEach to restore only mockContractExchangeRates to a specific return value, but never calls jest.clearAllMocks(). Call counts for mockContractExchangeRates accumulate across tests in this block. If a test in this block sets mockContractExchangeRates.mockReturnValue(ratesFor(USDC_CHECKSUM)) and then a subsequent test (or a test in a later describe block that runs after this one) checks toHaveBeenCalledTimes, it will see inflated counts. The missing clearAllMocks also means that any other mocks touched inside these tests (e.g. getNetworkImageSource, useTokensData) retain their call history across tests in this block, creating order-dependent failures.
    • Suggested fix in app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx:1:
      -describe('ActivityListItemRow — ERC-20 fiat address casing (TMCU-937)', () => {
      -  const mockContractExchangeRates = jest.mocked(
      -    selectContractExchangeRatesByChainId,
      -  );
      -  const USDC_LOWER = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
      -  const USDC_CHECKSUM = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
      -  const ratesFor = (address: string) =>
      -    ({ [address]: { price: 0.0004 } }) as ReturnType<
      -      typeof selectContractExchangeRatesByChainId
      -    >;
      -  afterEach(() => {
      -    mockContractExchangeRates.mockReturnValue(ratesFor(LINEA_MUSD_ADDRESS));
      -  });
      +describe('ActivityListItemRow — ERC-20 fiat address casing (TMCU-937)', () => {
      +  const mockContractExchangeRates = jest.mocked(
      +    selectContractExchangeRatesByChainId,
      +  );
      +  const USDC_LOWER = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
      +  const USDC_CHECKSUM = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
      +  const ratesFor = (address: string) =>
      +    ({ [address]: { price: 0.0004 } }) as ReturnType<
      +      typeof selectContractExchangeRatesByChainId
      +    >;
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockContractExchangeRates.mockReturnValue(ratesFor(LINEA_MUSD_ADDRESS));
      +  });
      +  afterEach(() => {
      +    jest.clearAllMocks();
      +  });

This check is informational only and does not block merging.

@sonarqubecloud

Copy link
Copy Markdown

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant