Skip to content

chore(pure-black): delete useElevatedSurface shim from themeUtils (TMCU-1051)#33165

Draft
georgewrmarshall wants to merge 3 commits into
mainfrom
cursor/delete-use-elevated-surface-shim-fb7d
Draft

chore(pure-black): delete useElevatedSurface shim from themeUtils (TMCU-1051)#33165
georgewrmarshall wants to merge 3 commits into
mainfrom
cursor/delete-use-elevated-surface-shim-fb7d

Conversation

@georgewrmarshall

@georgewrmarshall georgewrmarshall commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

MMDS BottomSheet now handles pure-black elevated surface styling internally. This PR completes the platform cleanup by deleting the stopgap useElevatedSurface() hook from themeUtils.ts and removing all remaining app callsites.

Changes:

  • Deleted useElevatedSurface() from app/util/theme/themeUtils.ts and removed its unit tests
  • Removed twClassName={surfaceClass} / useElevatedSurface() from 30+ MMDS BottomSheet callsites (Bridge, Confirmations, Perps, Accounts, Money, Core UX, etc.)
  • Removed row-level surface fill from ActionListItem, CustomNetworkSelector, and NetworkMultiSelectorList
  • Inlined elevated surface class logic in TradeWalletActions custom popup (non-MMDS component)
  • Kept getElevatedSurfaceColor() with documented legacy-only consumers (BottomSheetDialog, Input, ModalConfirmation, ActionModal, NetworkManager, etc.)
  • Dropped obsolete useElevatedSurface mocks from affected test files

Changelog

CHANGELOG entry: null

Related issues

Fixes: TMCU-1051

Manual testing steps

Feature: Pure Black bottom sheets after useElevatedSurface removal

  Scenario: MMDS BottomSheet surfaces render correctly in Pure Black dark mode
    Given Pure Black preview is enabled (MM_PURE_BLACK_PREVIEW=true) and the app is in Dark mode

    When the user opens any MMDS BottomSheet (e.g. Fund Action Menu, Bridge modals, Perps action sheet, Account Selector)
    Then the BottomSheet displays a uniform elevated surface without visible black bands or mismatched row backgrounds

  Scenario: Trade Wallet Actions popup renders correctly in Pure Black dark mode
    Given Pure Black preview is enabled and the app is in Dark mode
    And the user is on the wallet asset overview

    When the user taps the Trade button to open Trade Wallet Actions
    Then the custom popup menu displays with correct elevated surface and border styling

N/A for automated cloud-agent environment — visual QA on device/simulator recommended.

Screenshots/Recordings

Before

N/A — removal-only cleanup; no visual regression expected since MMDS BottomSheet owns surface styling.

After

N/A — visual QA on device/simulator with MM_PURE_BLACK_PREVIEW=true recommended.

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 (N/A — removal-only change; legacy consumer list documented in themeUtils.ts)
  • 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
    • Ideally on a mid-range device; emulator is acceptable (N/A — styling-only change)
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens (N/A — no performance-sensitive paths touched)
  • I've instrumented key operations with Sentry traces for production performance metrics
    • See trace() for usage and addToken for an example (N/A — no new traced operations)

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.
Open in Web Open in Cursor 

…CU-1051)

Remove the stopgap useElevatedSurface() hook now that MMDS BottomSheet
handles pure-black elevation internally. All app callsites that passed
twClassName={surfaceClass} to MMDS BottomSheet components have been
cleaned up.

- Delete useElevatedSurface from themeUtils.ts and its unit tests
- Keep getElevatedSurfaceColor for documented legacy-only consumers
- Inline elevated surface class logic in TradeWalletActions custom popup
- Remove row-level surface fill from ActionListItem and network selectors
- Drop obsolete useElevatedSurface mocks from affected test files

Co-authored-by: George Marshall <georgewrmarshall@users.noreply.github.com>
@mm-token-exchange-service mm-token-exchange-service Bot added the team-design-system All issues relating to design system in Mobile label Jul 10, 2026
Co-authored-by: George Marshall <georgewrmarshall@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeSwap, SmokeConfirmations, SmokeMoney, SmokeNetworkAbstractions, SmokeAccounts, SmokeWalletPlatform, SmokeSeedlessOnboarding, SmokePerps, SmokePredictions, SmokeNetworkExpansion, SmokeMultiChainAPI
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR removes the useElevatedSurface hook from themeUtils.ts and cleans up all its consumers across the codebase. The changes are a theming/styling refactor — removing twClassName={surfaceClass} from BottomSheet components and deleting the deprecated hook. The functional behavior is preserved (TradeWalletActions inlines the equivalent logic; all other components simply use the BottomSheet default styling).

Despite being a refactor, the changes touch BottomSheet components across many critical user flows, so we need to verify that BottomSheet rendering still works correctly in each affected area:

  1. SmokeSwap: Bridge BatchSell modals (DestinationTokenSelector, FinalReview, MinimumReceived, NetworkFee, QuoteDetails), HighRateAlert, MissingPrice, PriceImpact modals all had twClassName removed.
  2. SmokeConfirmations: AccountSelector, AccountPickerRow, PayWithBottomSheet had twClassName removed — these are core confirmation flow components.
  3. SmokeMoney: MoneyUnlinkCardSheet and FundActionMenu had twClassName removed.
  4. SmokeNetworkAbstractions: CustomNetworkSelector, NetworkMultiSelectorList, NetworkListBottomSheet, DeleteNetworkModal had styling changes.
  5. SmokeAccounts + SmokeWalletPlatform: SelectSRPBottomSheet and AddressSelector had twClassName removed.
  6. SmokeSeedlessOnboarding: OnboardingSheet had twClassName removed.
  7. SmokePerps + SmokeWalletPlatform: PerpsActionSheet had twClassName removed.
  8. SmokePredictions: QuickBuyRoot (SocialLeaderboard) had twClassName removed.
  9. SmokeNetworkExpansion: LearnMoreBottomSheet (MultichainAccounts) had twClassName removed.
  10. SmokeMultiChainAPI: Included per dependency chain from SmokeNetworkAbstractions + SmokeNetworkExpansion.

The ActionListItem component also had surfaceClass removed from its style computation. This component is used in many list-based UIs.

The risk is medium because while the changes are refactors, they touch BottomSheet styling across many flows — a regression in BottomSheet rendering could affect multiple user journeys.

Performance Test Selection:
The changes are purely theming/styling refactors — removing a deprecated useElevatedSurface hook and its twClassName prop from BottomSheet components. These changes do not affect rendering performance, data fetching, state management, or any performance-sensitive code paths. No performance test tags are warranted.

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/util/theme/themeUtils.test.tsx 0/50 0/216 0/388

AI-detected flaky patterns

app/util/theme/themeUtils.test.tsx

  • J9 — Module-level mutable let binding not reset in beforeEach (high)
    • The module-level let mockIsPureBlackEnabled = false is mutated to true inside two test bodies within the getElevatedSurfaceColor describe block (lines 49 and 56). The beforeEach that resets it back to false is scoped only to that describe block — it does NOT run for the isPureBlackEnabled re-export describe block.

When Jest runs tests in randomized order (--randomize), a mutation from the getElevatedSurfaceColor tests can bleed into the isPureBlackEnabled re-export describe, leaving mockIsPureBlackEnabled = true when that describe's tests execute. The current assertion (typeof isPureBlackEnabled === 'boolean') is immune to the value, but any future assertion about the actual value of isPureBlackEnabled in that describe would be order-dependent and intermittently fail.

The fix is to promote the reset to a top-level beforeEach so it runs before every test in the file regardless of describe nesting, and remove the now-redundant nested beforeEach.

  • Suggested fix in app/util/theme/themeUtils.test.tsx:4:
    -let mockIsPureBlackEnabled = false;
    -
    -jest.mock('./pureBlackPreview', () => ({
    -  get isPureBlackEnabled() {
    -    return mockIsPureBlackEnabled;
    -  },
    -}));
    -
    -import { getElevatedSurfaceColor, isPureBlackEnabled } from './themeUtils';
    -
    -// ...
    -
    -describe('isPureBlackEnabled re-export', () => {
    -  it('re-exports the pure black preview flag as a boolean', () => {
    -    expect(typeof isPureBlackEnabled).toBe('boolean');
    -  });
    -});
    -
    -describe('getElevatedSurfaceColor', () => {
    -  beforeEach(() => {
    -    mockIsPureBlackEnabled = false;
    -  });
    -
    -  it('returns alternative background for dark mode when pure black preview is on', () => {
    -    mockIsPureBlackEnabled = true;
    -    // ...
    -  });
    -
    -  it('returns default background for light mode when pure black preview is on', () => {
    -    mockIsPureBlackEnabled = true;
    -    // ...
    -  });
    -});
    +let mockIsPureBlackEnabled = false;
    +
    +jest.mock('./pureBlackPreview', () => ({
    +  get isPureBlackEnabled() {
    +    return mockIsPureBlackEnabled;
    +  },
    +}));
    +
    +import { getElevatedSurfaceColor, isPureBlackEnabled } from './themeUtils';
    +
    +// Top-level reset covers ALL describe blocks in this file.
    +beforeEach(() => {
    +  mockIsPureBlackEnabled = false;
    +});
    +
    +describe('isPureBlackEnabled re-export', () => {
    +  it('re-exports the pure black preview flag as a boolean', () => {
    +    expect(typeof isPureBlackEnabled).toBe('boolean');
    +  });
    +});
    +
    +describe('getElevatedSurfaceColor', () => {
    +  // Nested beforeEach removed — top-level beforeEach above handles the reset.
    +
    +  it('returns default background when pure black preview is off', () => {
    +    const theme = createTheme(AppThemeKey.dark);
    +
    +    const result = getElevatedSurfaceColor(theme);
    +
    +    expect(result).toBe(theme.colors.background.default);
    +  });
    +
    +  it('returns alternative background for dark mode when pure black preview is on', () => {
    +    mockIsPureBlackEnabled = true;
    +    const theme = createTheme(AppThemeKey.dark);
    +
    +    const result = getElevatedSurfaceColor(theme);
    +
    +    expect(result).toBe(theme.colors.background.alternative);
    +  });
    +
    +  it('returns default background for light mode when pure black preview is on', () => {
    +    mockIsPureBlackEnabled = true;
    +    const theme = createTheme(AppThemeKey.light);
    +
    +    const result = getElevatedSurfaceColor(theme);
    +
    +    expect(result).toBe(theme.colors.background.default);
    +  });
    +});

This check is informational only and does not block merging.

- Drop unused useTailwind/tw from CustomNetworkSelector after surfaceClass removal
- Move mockIsPureBlackEnabled reset to top-level beforeEach in themeUtils tests

Co-authored-by: George Marshall <georgewrmarshall@users.noreply.github.com>
@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-M team-design-system All issues relating to design system in Mobile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants