Skip to content

refactor(confirmations): migrate Badge and BadgeWrapper to MMDS - #34307

Open
brianacnguyen wants to merge 26 commits into
mainfrom
refactor/confirmations-badge
Open

refactor(confirmations): migrate Badge and BadgeWrapper to MMDS#34307
brianacnguyen wants to merge 26 commits into
mainfrom
refactor/confirmations-badge

Conversation

@brianacnguyen

@brianacnguyen brianacnguyen commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

Component-library Badge / BadgeWrapper are deprecated in favor of MMDS. Confirmation UIs still used the legacy network-badge pattern (BadgeVariant.Network + BadgePosition.BottomRight), which blocked cleanup and kept us on the old API.

This PR migrates those call sites to @metamask/design-system-react-native BadgeNetwork + BadgeWrapper (imageSourcesrc, badgeElementbadge, badgePositionposition). Shared AccountBase / AccountBalance badgeProps now take { name?, src } instead of component-library BadgeProps, and callers (ApproveTransactionHeader, AddressFrom) are updated. Tests assert network badges via testID on real MMDS components (no Badge mocks).

What changed:

  • Migrated confirmation network badges in hero token/NFT, token icon, gas fee token icon, token conversion header, send token/NFT rows, transaction details account row, AddressElement, and staking contract interaction details
  • Updated AccountBase badge rendering and badgeProps shape; updated ApproveTransactionHeader and AddressFrom
  • Strengthened unit tests to query MMDS BadgeNetwork via testID

Breaking change: AccountBase / AccountBalance badgeProps no longer accept component-library BadgeProps (variant, imageSource). Callers must pass { name?: string; src?: ... } for MMDS BadgeNetwork.

Changelog

CHANGELOG entry: null

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/DSYS-985 https://consensyssoftware.atlassian.net/browse/DSYS-983

Manual testing steps

Feature: confirmation network badges render with MMDS

  Scenario: user reviews a send confirmation with a network badge on the token hero
    Given the app is unlocked with an account that has ETH on a network with a known network image
    When the user starts a send of native ETH and reaches the confirmation screen
    Then the token hero shows a bottom-right network badge and the amount looks unchanged

  Scenario: user opens NFT send confirmation
    Given the app has an NFT on a network with a network image
    When the user starts an NFT send and reaches confirmation
    Then the NFT hero shows a network badge overlay and collection/token id remain visible

  Scenario: user opens legacy approve confirmation header
    Given a dapp approval confirmation that uses the legacy approve header
    When the confirmation is shown
    Then the account avatar still shows a network badge and balance row renders as before

Unit tests:

yarn jest app/components/Views/confirmations/components/hero-nft
yarn jest app/components/Views/confirmations/components/hero-token
yarn jest app/components/Views/confirmations/components/UI/token
yarn jest app/components/Views/confirmations/components/UI/nft
yarn jest app/components/Views/confirmations/legacy/components/AddressElement
yarn jest app/components/Views/confirmations/components/activity/transaction-details-account-row
yarn jest app/component-library/components-temp/Accounts/AccountBase
yarn jest app/component-library/components-temp/Accounts/AccountBalance

Screenshots/Recordings

N/A — internal component migration; visual parity expected (network badge still bottom-right on the same avatars)

Before

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

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
Wide UI-only refactor across many confirmation entry points with a breaking AccountBase badgeProps API; behavior should stay visually the same but regressions are possible if network image gating differs from legacy Badge.

Overview
Replaces deprecated component-library Badge / BadgeWrapper network overlays in confirmation and account UI with @metamask/design-system-react-native BadgeNetwork + BadgeWrapper (badgeElementbadge, badgePositionposition, imageSourcesrc).

AccountBase / AccountBalance now take badgeProps: { name?, src } instead of BadgeProps; AccountBase only renders the network badge when src is set. Callers AddressFrom, ApproveTransactionHeader, and related flows pass the new shape.

Confirmation surfaces updated include hero token/NFT, send token/NFT rows, token icon, gas fee token icon, token conversion header, transaction details account row, legacy AddressElement, and AddressFrom. Most sites gate the badge on a present network image (previously some could render with undefined sources). Staking network row swaps Badge for AvatarNetwork with layout tweaks.

Tests assert MMDS badges via dedicated **testID**s and add coverage for missing network image / showNetwork: false where applicable.

Reviewed by Cursor Bugbot for commit 1174f77. Bugbot is set up for automated code reviews on this repo. Configure here.

Replace component-library network badges with BadgeNetwork and BadgeWrapper, including AccountBase badgeProps.

Co-authored-by: Cursor <cursoragent@cursor.com>
@brianacnguyen brianacnguyen self-assigned this Aug 4, 2026
@brianacnguyen
brianacnguyen requested review from a team as code owners August 4, 2026 22:09
@brianacnguyen brianacnguyen added the team-design-system All issues relating to design system in Mobile label Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

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.

@github-actions github-actions Bot added size-M risk:medium AI analysis: medium risk labels Aug 4, 2026
Comment thread app/components/Views/confirmations/components/hero-nft/hero-nft.test.tsx Outdated
Comment thread app/components/Views/confirmations/components/UI/nft/nft.test.tsx Outdated
@github-actions

github-actions Bot commented Aug 4, 2026

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/Views/confirmations/components/hero-token/hero-token.test.tsx 0/86 0/266 0/367
app/components/Views/confirmations/legacy/components/AddressElement/AddressElement.test.tsx 0/86 0/266 0/367

AI-detected flaky patterns

app/components/Views/confirmations/components/hero-token/hero-token.test.tsx

  • J4 — waitFor with async callback (high)
    • Passing an async callback to waitFor is explicitly flagged as a flaky pattern (J4). When the async callback throws, the rejection can be swallowed silently, causing the test to pass even when assertions fail. waitFor is designed to retry a synchronous predicate; an async callback breaks the retry semantics and can produce non-deterministic results. The same pattern appears in the other three tests in this file (lines 65, 87, and 107). All async callbacks inside waitFor should be converted to synchronous ones.
    • Suggested fix in app/components/Views/confirmations/components/hero-token/hero-token.test.tsx:42:
      -    await waitFor(async () => {
      -      expect(queryByTestId('avatar-with-badge-avatar-token-ETH')).toBeTruthy();
      -      expect(queryByTestId('avatar-token-network-badge')).toBeOnTheScreen();
      -      expect(getByText('0.0556 ETH')).toBeDefined();
      -      expect(getByText('$199.79')).toBeDefined();
      -    });
      +    await waitFor(() => {
      +      expect(queryByTestId('avatar-with-badge-avatar-token-ETH')).toBeOnTheScreen();
      +      expect(queryByTestId('avatar-token-network-badge')).toBeOnTheScreen();
      +      expect(getByText('0.0556 ETH')).toBeDefined();
      +      expect(getByText('$199.79')).toBeDefined();
      +    });
  • J4 — waitFor with async callback (high)
    • Same J4 issue as line 42: an async callback is passed to waitFor. Unhandled rejections from the async callback can silently pass, making the test non-deterministic. The callback should be synchronous.
    • Suggested fix in app/components/Views/confirmations/components/hero-token/hero-token.test.tsx:65:
      -    await waitFor(async () => {
      -      expect(queryByTestId('avatar-with-badge-avatar-token-ETH')).toBeTruthy();
      -      expect(getByText('0.0001 ETH')).toBeDefined();
      -      expect(getByText('$0.36')).toBeDefined();
      -    });
      +    await waitFor(() => {
      +      expect(queryByTestId('avatar-with-badge-avatar-token-ETH')).toBeOnTheScreen();
      +      expect(getByText('0.0001 ETH')).toBeDefined();
      +      expect(getByText('$0.36')).toBeDefined();
      +    });
  • J4 — waitFor with async callback (high)
    • Same J4 issue: async callback inside waitFor. The retry loop in waitFor does not correctly handle async callbacks — if the async function rejects, the error may be swallowed and the test may pass spuriously or fail non-deterministically depending on the microtask queue state.
    • Suggested fix in app/components/Views/confirmations/components/hero-token/hero-token.test.tsx:87:
      -    await waitFor(async () => {
      -      expect(getByText('Sending')).toBeDefined();
      -      expect(queryByTestId('avatar-with-badge-avatar-token-ETH')).toBeTruthy();
      -      expect(getByText('0.0556 ETH')).toBeDefined();
      -      expect(getByText('$199.79')).toBeDefined();
      -    });
      +    await waitFor(() => {
      +      expect(getByText('Sending')).toBeDefined();
      +      expect(queryByTestId('avatar-with-badge-avatar-token-ETH')).toBeOnTheScreen();
      +      expect(getByText('0.0556 ETH')).toBeDefined();
      +      expect(getByText('$199.79')).toBeDefined();
      +    });
  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The useParams mock is set up at module level with jest.fn().mockReturnValue(...) and there is no beforeEach block calling jest.clearAllMocks() or jest.resetAllMocks(). If any test overrides the mock return value (e.g. to test a different maxValueMode), that mutation will bleed into subsequent tests, causing order-dependent failures. Adding a beforeEach with jest.clearAllMocks() ensures each test starts with a clean mock state.
    • Suggested fix in app/components/Views/confirmations/components/hero-token/hero-token.test.tsx:17:
      -jest.mock('../../../../../util/navigation/navUtils', () => ({
      -  ...jest.requireActual('../../../../../util/navigation/navUtils'),
      -  useParams: jest.fn().mockReturnValue({
      -    params: {
      -      maxValueMode: false,
      -    },
      -  }),
      -}));
      -
      -describe('HeroToken', () => {
      +jest.mock('../../../../../util/navigation/navUtils', () => ({
      +  ...jest.requireActual('../../../../../util/navigation/navUtils'),
      +  useParams: jest.fn().mockReturnValue({
      +    params: {
      +      maxValueMode: false,
      +    },
      +  }),
      +}));
      +
      +describe('HeroToken', () => {
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +  });

app/components/Views/confirmations/legacy/components/AddressElement/AddressElement.test.tsx

  • J3 — Missing jest.clearAllMocks() / resetAllMocks() between tests (high)
    • The test file defines multiple top-level jest.mock() calls (including a complex Engine mock with inline jest.fn()s) but contains no beforeEach() to reset mock state between tests. This allows mock call histories and implementations to leak across tests, which can cause intermittent failures depending on test order (classic J3). The other 4 test files in this PR all explicitly call jest.clearAllMocks() or jest.resetAllMocks() in beforeEach. No historical flaky signal for this file.
    • Suggested fix in app/components/Views/confirmations/legacy/components/AddressElement/AddressElement.test.tsx:
      -describe('AddressElement', () => {
      -  it('should render correctly', () => {
      -    const { getByTestId } = renderComponent(initialState);
      -    expect(getByTestId('address-element')).toBeOnTheScreen();
      -  });
      -
      -  it('should render the address', () => {
      -    const address = '0xd018538C87232FF95acbCe4870629b75640a78E7';
      -    const { getByText } = renderComponent(initialState);
      -    const addressText = getByText(renderShortAddress(address));
      -    expect(addressText).toBeDefined();
      -  });
      -
      -  it('renders the network badge when displayNetworkBadge is true', () => {
      -    const { getByTestId } = renderComponent(
      -      {
      -        ...initialState,
      -        engine: {
      -          backgroundState: {
      -            ...backgroundState,
      -            NetworkController: {
      -              ...backgroundState.NetworkController,
      -              networkConfigurationsByChainId: {
      -                '0x1': {
      -                  name: 'Ethereum Mainnet',
      -                  chainId: '0x1',
      -                  blockExplorerUrls: [],
      -                  rpcEndpoints: [],
      -                  defaultRpcEndpointIndex: 0,
      -                  nativeCurrency: 'ETH',
      -                },
      -              },
      -            },
      -          },
      -        },
      -      },
      -      {
      -        displayNetworkBadge: true,
      -      },
      -    );
      -
      -    expect(getByTestId('address-element-network-badge')).toBeOnTheScreen();
      -  });
      -});
      +describe('AddressElement', () => {
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +  });
      +
      +  it('should render correctly', () => {
      +    const { getByTestId } = renderComponent(initialState);
      +    expect(getByTestId('address-element')).toBeOnTheScreen();
      +  });
      +
      +  it('should render the address', () => {
      +    const address = '0xd018538C87232FF95acbCe4870629b75640a78E7';
      +    const { getByText } = renderComponent(initialState);
      +    const addressText = getByText(renderShortAddress(address));
      +    expect(addressText).toBeDefined();
      +  });
      +
      +  it('renders the network badge when displayNetworkBadge is true', () => {
      +    const { getByTestId } = renderComponent(
      +      {
      +        ...initialState,
      +        engine: {
      +          backgroundState: {
      +            ...backgroundState,
      +            NetworkController: {
      +              ...backgroundState.NetworkController,
      +              networkConfigurationsByChainId: {
      +                '0x1': {
      +                  name: 'Ethereum Mainnet',
      +                  chainId: '0x1',
      +                  blockExplorerUrls: [],
      +                  rpcEndpoints: [],
      +                  defaultRpcEndpointIndex: 0,
      +                  nativeCurrency: 'ETH',
      +                },
      +              },
      +            },
      +          },
      +        },
      +      },
      +      {
      +        displayNetworkBadge: true,
      +      },
      +    );
      +
      +    expect(getByTestId('address-element-network-badge')).toBeOnTheScreen();
      +  });
      +});

This check is informational only and does not block merging.

brianacnguyen and others added 2 commits August 4, 2026 20:32
…types

Add AddressFrom and badge branch tests, tighten on-screen matchers, and cast network badge sources for BadgeNetwork.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added size-L and removed size-M labels Aug 5, 2026
@matthewwalsh0
matthewwalsh0 requested a review from OGPoyraz August 5, 2026 04:34
OGPoyraz
OGPoyraz previously approved these changes Aug 5, 2026
@github-actions github-actions Bot added risk:high AI analysis: high risk and removed risk:medium AI analysis: medium risk labels Aug 5, 2026
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:high AI analysis: high risk labels Aug 5, 2026
brianacnguyen and others added 3 commits August 5, 2026 08:59
BadgeNetwork is for BadgeWrapper overlays; use a sized network avatar to match sibling confirmation rows.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread app/components/UI/AccountFromToInfoCard/AddressFrom.test.tsx
brianacnguyen and others added 3 commits August 6, 2026 05:59
Pass network name to TokenIcon BadgeNetwork, gate badges on truthy image sources, and fix AddressFrom mock typing for missing images.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot 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.

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

Fix All in Cursor

❌ 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 e8de38a. Configure here.

brianacnguyen and others added 4 commits August 6, 2026 07:00
Satisfy tsc TS2352 for the missing networkImage mock return value.

Co-authored-by: Cursor <cursoragent@cursor.com>
…geSource

Keep BadgeNetwork name from useNetworkName without gating the image on Redux EVM network config.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.09%. Comparing base (3a56af7) to head (3c5a625).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ls-account-row/transaction-details-account-row.tsx 0.00% 0 Missing and 1 partial ⚠️
...ews/confirmations/components/hero-nft/hero-nft.tsx 0.00% 0 Missing and 1 partial ⚠️
...-network-badge/avatar-token-with-network-badge.tsx 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #34307   +/-   ##
=======================================
  Coverage   85.08%   85.09%           
=======================================
  Files        6332     6332           
  Lines      172506   172517   +11     
  Branches    42652    42660    +8     
=======================================
+ Hits       146784   146795   +11     
+ Misses      15691    15686    -5     
- Partials    10031    10036    +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations, SmokeSwap, SmokeStake, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR migrates badge/network badge components from the internal component library (Badge, BadgeWrapper from component-library/components/Badges/) to the new design system (BadgeNetwork, BadgeWrapper, BadgeWrapperPosition from @metamask/design-system-react-native). The changes are widespread across confirmation UI components.

Affected areas:

  1. Confirmations flow (SmokeConfirmations): Multiple components changed — token icons, NFT icons, hero tokens, hero NFTs, gas fee token icons, token conversion asset headers, transaction details account rows, address elements, approve transaction headers. These are all core confirmation UI elements. The testID values have changed (e.g., "token-network-badge", "nft-network-badge", "gas-fee-token-network-badge", "avatar-token-network-badge", "address-element-network-badge") which could affect E2E selectors.

  2. Swap flow (SmokeSwap): token-conversion-asset-header.tsx is changed — this component renders the input/output token display in swap confirmations. Also requires SmokeConfirmations per tag dependency rules.

  3. Staking flow (SmokeStake): staking-contract-interaction-details.tsx is changed — replaces Badge with AvatarNetwork for the network display in staking confirmations. Also requires SmokeConfirmations per tag dependency rules.

  4. Wallet platform / activity (SmokeWalletPlatform): transaction-details-account-row.tsx is changed — this is used in transaction history/activity display.

Risk assessment: Medium — these are UI component swaps that change the rendering implementation. While the visual output should be similar, the component API changes (props renamed: badgePositionposition, badgeElementbadge, imageSourcesrc) and new testIDs could affect E2E tests that rely on specific component behavior or test identifiers. The AccountBase component used in the "from" address display in confirmations is also changed.

Not selected: SmokeAccounts (account management flows not directly affected), SmokeNetworkAbstractions (network manager UI not changed), SmokeNetworkExpansion (Solana/multi-chain provider not changed), SmokeMoney (card/ramps not changed), SmokePerps (perps trading not changed), SmokeMultiChainAPI (CAIP-25 sessions not changed), SmokePredictions (prediction markets not changed), SmokeSeedlessOnboarding (social login not changed), SmokeBrowser (browser not changed), SmokeSnaps (snaps not changed), SmokeMMConnect (MM connect not changed).

Performance Test Selection:
The changes are purely UI component migrations (Badge → BadgeNetwork from design system). These are visual rendering changes that don't affect app performance metrics like launch time, login speed, asset loading, or swap execution times. No performance-sensitive code paths are modified.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

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

Labels

risk:medium AI analysis: medium risk size-L team-design-system All issues relating to design system in Mobile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants