Skip to content

refactor(notification): migrate BaseNotification to TS and align with design-system tokens#30285

Open
vinnyhoward wants to merge 1 commit into
mainfrom
chore-tmcu-639-legacy-toast-component-updates
Open

refactor(notification): migrate BaseNotification to TS and align with design-system tokens#30285
vinnyhoward wants to merge 1 commit into
mainfrom
chore-tmcu-639-legacy-toast-component-updates

Conversation

@vinnyhoward
Copy link
Copy Markdown
Contributor

@vinnyhoward vinnyhoward commented May 15, 2026

Description

Part of the pure-black design-tokens audit. BaseNotification is the renderer behind every legacy NotificationManager.showSimpleNotification / showTransactionNotification call (deeplink approve, SDK Connect v2, Ramps, token add/remove, signature, asset details, password reset, and the tx-controller notification path).

It was previously using colors.overlay.* tokens for foreground text/icons and a double-background layer — under the pure-black dark theme the inner surface collapsed into the wallet background and contrast broke.

This PR:

  • Converts the component from .js.tsx, adds BaseNotification.types.ts (status union, props) and a co-located BaseNotification.styles.ts consumed via useStyles to match modern components.
  • Replaces overlay.* foreground misuse with the correct semantic tokens (text.default, icon.default) and uses background.section for the elevated surface so the notification stays visible against both stable dark and pure-black backgrounds.
  • Swaps Base/Text for the design-system Text (variant + color props). The remaining react-native-vector-icons usages keep a scoped eslint-disable with a TODO — DS Icon swaps shift glyph proportions at 36pt and need a separate design review.
  • Modernises the test file: renamed to .test.tsx, typed against BaseNotificationStatus, converted to it.each for clearer per-status failures, added coverage for previously untested statuses (speedup, import_success, simple_notification, simple_notification_rejected) and for onPress, autoDismiss, and data=undefined.

Changelog

CHANGELOG entry: null

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Note: the screenshots below were captured using a temporary on-screen trigger panel added to Wallet/index.tsx for audit purposes. That code is not part of this PR — it lives separately and is only used to exercise the notification surfaces during the pure-black audit.

toast_after.mov

Before

toast_before.mov

After

toast_after.mov

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
Touches the shared in-app notification renderer used across many flows; while mostly a UI/theming refactor, it changes styling/token usage and some title/icon logic which could affect notification rendering across the app.

Overview
Refactors BaseNotification from JS/PropTypes to typed TS (.tsx), extracting new co-located BaseNotification.types.ts (status/props) and BaseNotification.styles.ts consumed via useStyles.

Updates notification theming to use semantic design tokens (e.g., background.section, text.default, icon.default) and swaps legacy Base/Text for design-system Text variants, while keeping react-native-vector-icons with a scoped TODO.

Modernizes tests by converting to index.test.tsx, parameterizing status coverage (including additional statuses), and adding interaction/robustness coverage for onPress, autoDismiss close behavior, and data being undefined.

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

… design-system tokens

Convert BaseNotification to TypeScript with co-located styles via useStyles, swap overlay.* foreground misuse for text.default/icon.default plus background.section, and adopt DS Text. Test file moved to .tsx with it.each, full status coverage, and onPress/autoDismiss assertions.
@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.

@metamaskbotv2 metamaskbotv2 Bot added the team-mobile-ux Mobile UX team label May 15, 2026
@vinnyhoward vinnyhoward added the team-design-system All issues relating to design system in Mobile label May 15, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The changes are a TypeScript migration of the BaseNotification component (renamed from .js to .tsx) with the following notable behavioral/visual changes:

  1. Color token changes: floatingBackground changed from colors.background.default to colors.background.section, and icon colors changed from colors.overlay.inverse to colors.icon.default. The defaultFlashFloating background color (colors.overlay.alternative) was removed entirely. These visual changes affect how toast notifications appear across the app.

  2. Text component swap: Replaced the custom Base/Text component with @metamask/design-system-react-native Text component with explicit variants (BodyMd/BodySm) and colors (TextDefault).

  3. Nonce parsing fix: Improved nonce parsing for EIP-7702 transactions (no nonce) using Number.isNaN instead of isNaN.

  4. TypeScript migration: Added proper types, removed PropTypes.

Impact scope: BaseNotification is used by:

  • TransactionNotification (used in Nav/Main for transaction status toasts during send/swap/stake flows)
  • SimpleNotification (used across many views: Onboarding, DetectedTokens, AssetDetails, AddAsset, AssetOptions, Ramp, TokenDetails, confirmations)
  • Onboarding/index.tsx directly

This means the notification toasts shown during transaction confirmations, swaps, stakes, ramps/money flows, and wallet platform activities could be visually affected. Selected tags cover the main flows where these notifications appear:

  • SmokeConfirmations: Transaction send/confirmation flows trigger TransactionNotification
  • SmokeWalletPlatform: Transaction history and activity display, EVM provider events
  • SmokeMoney: Ramp flows use SimpleNotification extensively
  • SmokeSwap: Swap completion shows transaction notifications
  • SmokeStake: Stake/lending flows show transaction notifications

No performance tests needed as this is a UI component refactor with no data loading or rendering performance implications.

Performance Test Selection:
The changes are a TypeScript migration and visual refactoring of the BaseNotification component. There are no changes to data loading, list rendering, state management, or app initialization that would impact performance metrics. The component is a simple toast notification UI element.

View GitHub Actions results

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

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 2 potential issues.

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 be2ff7d. Configure here.

Comment thread app/components/UI/Notification/BaseNotification/index.test.tsx
Comment thread app/components/UI/Notification/BaseNotification/index.test.tsx
@sonarqubecloud
Copy link
Copy Markdown

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

Labels

size-L team-design-system All issues relating to design system in Mobile team-mobile-ux Mobile UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant