Skip to content

refactor: present account screens as right-sliding cards - #33670

Merged
gantunesr merged 6 commits into
mainfrom
gar/refactor/accounts-nav
Jul 24, 2026
Merged

refactor: present account screens as right-sliding cards#33670
gantunesr merged 6 commits into
mainfrom
gar/refactor/accounts-nav

Conversation

@gantunesr

@gantunesr gantunesr commented Jul 22, 2026

Copy link
Copy Markdown
Member

Description

Account management screens (AccountSelector, AddWallet, AccountGroupDetails, import/hardware flows, and Reveal Secret Recovery Phrase) were presented as transparentModal, so on iOS the slide_from_right animation was ignored and they slid up from the bottom with no swipe-back gesture.

This converts them to presentation: 'card' with slide_from_right + full-screen gesture so they open from the right and support swipe-to-go-back. AccountSelector is also extracted out of ROOT_MODAL_FLOW into AppFlow as a direct screen, and its route is moved from the SHEET namespace to MULTICHAIN_ACCOUNTS since it is a view, not a sheet.

Changelog

CHANGELOG entry: Account, wallet, and import screens now open as right-sliding cards with swipe-back instead of bottom-up modals.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MUL-1773

Manual testing steps

Feature: Account screen navigation

  Scenario: user opens the account selector from the wallet
    Given the wallet home is visible

    When user taps the account picker
    Then the Accounts screen slides in from the right and swipe-back dismisses it

  Scenario: user opens account details from the account selector
    Given the Accounts screen is open

    When user taps an account
    Then the account details screen slides in from the right and swipe-back returns to Accounts

Screenshots/Recordings

Screen.Recording.2026-07-22.at.3.23.33.PM.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.

Made with Cursor

Convert AccountSelector and related account-management screens from
transparentModal presentations to card presentations with slide_from_right
animation and full-screen back gesture, so they open from the right and
support swipe-back.

- Extract AccountSelector out of ROOT_MODAL_FLOW into AppFlow as a direct card
- Move AddWallet into AppFlow (was only reachable via RootModalFlow)
- Move ACCOUNT_SELECTOR route from SHEET to MULTICHAIN_ACCOUNTS namespace
- Apply card/slide_from_right to AccountGroupDetails, ImportPrivateKeyView,
  ImportSRPView, ConnectHardwareWalletFlow, and RevealPrivateCredential

Co-authored-by: Cursor <cursoragent@cursor.com>
@metamask-ci metamask-ci Bot added the team-accounts-framework Accounts team label Jul 22, 2026
@metamask-ci

metamask-ci Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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

Warnings — informational, address before merging:

  • 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.

@gantunesr gantunesr changed the title refactor(nav): present account screens as right-sliding cards refactor: present account screens as right-sliding cards Jul 22, 2026
@gantunesr
gantunesr marked this pull request as ready for review July 22, 2026 19:39
@gantunesr
gantunesr requested review from a team as code owners July 22, 2026 19:39

@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 4256841. Configure here.

Comment thread tests/component-view/renderers/ramps.tsx Outdated
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jul 22, 2026
…allet

AddWallet now lives in AppFlow (same navigator as the HW flow), so the
previous navigate()+goBack() pattern dismissed the just-pushed HW flow
instead of AddWallet. Use StackActions.replace to swap AddWallet for the
HW flow so completion pop(2) still lands on AccountSelector.

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

github-actions Bot commented Jul 22, 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/Nav/App/App.test.tsx 0/143 0/215 0/299

AI-detected flaky patterns

app/components/Nav/App/App.test.tsx

  • J8 — jest.useFakeTimers() combined with waitFor() (high)
    • The outer describe block activates fake timers with jest.useFakeTimers(), while numerous tests (including performance tracing, seedless password checks, version handling, and route rendering tests) rely on await waitFor() with assertions inside. Fake timers block waitFor's internal real setTimeout polling, leading to intermittent timeouts, hangs, or flaky passes under varying CI load. This is explicitly listed in the skill reference as a known issue in App.test.tsx. Historical data showed no prior flakiness, so this was not used as a hint.
    • Suggested fix in app/components/Nav/App/App.test.tsx:
      -  describe('App', () => {
      -    jest.useFakeTimers();
      -    beforeEach(() => {
      -      jest.clearAllMocks();
      -      mockNavigate.mockClear();
      -    });
      -    afterEach(() => {
      -      cleanup();
      -      jest.runOnlyPendingTimers();
      -    });
      -    // ... many tests use await waitFor(() => expect(...))
      -  });
      +  describe('App', () => {
      +    beforeEach(() => {
      +      jest.clearAllMocks();
      +      mockNavigate.mockClear();
      +    });
      +    afterEach(() => {
      +      cleanup();
      +      jest.runOnlyPendingTimers();
      +      jest.useRealTimers();
      +    });
      +    // Individual tests that need timers should call jest.useFakeTimers() locally
      +    // and restore with jest.useRealTimers() or use act + runAllTimersAsync()
      +  });
  • J10 — jest.spyOn() without restoreAllMocks()/mockRestore() afterward (medium)
    • Multiple jest.spyOn calls target StorageWrapper.getItem/setItem and other modules inside it() blocks (e.g. version handling, error logging tests). The afterEach only performs cleanup() and runOnlyPendingTimers(); it never calls jest.restoreAllMocks() or mockRestore(). Spies therefore leak between tests, altering behavior of subsequent tests in non-deterministic ways depending on execution order. Matches J10 exactly. No historical flakiness signal was present.
    • Suggested fix in app/components/Nav/App/App.test.tsx:
      -  afterEach(() => {
      -    cleanup();
      -    jest.runOnlyPendingTimers();
      -  });
      -
      -  // later in version handling tests:
      -  const setItemSpy = jest
      -    .spyOn(StorageWrapper, 'setItem')
      -    .mockResolvedValue();
      -  jest.spyOn(StorageWrapper, 'getItem').mockImplementation(async (key) => {
      +  afterEach(() => {
      +    cleanup();
      +    jest.runOnlyPendingTimers();
      +    jest.restoreAllMocks();
      +  });

This check is informational only and does not block merging.

SelectHardwareWallet renders its own HeaderStandard, so hide the
native-stack default header in ConnectHardwareWalletFlow to avoid a
doubled header.

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

Copy link
Copy Markdown
Contributor

gantunesr and others added 2 commits July 22, 2026 16:19
createAccountSelectorNavDetails now navigates straight to
Routes.MULTICHAIN_ACCOUNTS.ACCOUNT_SELECTOR, so renderBuildQuoteWithRoutes
must register the picker at that route on the root stack instead of under
ROOT_MODAL_FLOW. Otherwise sell BuildQuote account switching cannot open
the accounts list.

Co-authored-by: Cursor <cursoragent@cursor.com>
AccountSelector now navigates straight to the AccountSelector route
with isEvmOnly/disableAddAccountButton params instead of routing through
RootModalFlow. Update the four affected assertions accordingly.

Co-authored-by: Cursor <cursoragent@cursor.com>
@gantunesr
gantunesr requested a review from a team as a code owner July 22, 2026 20:56
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps
  • Selected Performance tags: @PerformanceAccountList, @PerformanceOnboarding, @PerformanceLogin, @PerformanceSwaps, @PerformanceLaunch, @PerformanceAssetLoading, @PerformancePredict, @PerformancePreps
  • Risk Level: high
  • AI Confidence: %
click to see 🤖 AI reasoning details

E2E Test Selection:
Fallback: AI analysis did not complete successfully. Running all tests.

Performance Test Selection:
Fallback: AI analysis did not complete successfully. Running all performance tests.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

3 tests failed · 20 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

❌ Failed Tests (3)

@Accounts-team

Test Platform Device Reason Recording
Import SRP with +50 accounts, SRP 1, SRP 2, SRP 3 Android Google Pixel 8 Pro (v14.0) Test error 📹 Watch

@metamask-onboarding-team

Test Platform Device Reason Recording
Fresh SRP wallet creation performance Android Google Pixel 8 Pro (v14.0) Quality gates exceeded 📹 Watch

@mm-perps-engineering-team

Test Platform Device Reason Recording
Perps open position and close it Android Google Pixel 8 Pro (v14.0) Test error 📹 Watch
✅ Passed Tests (17)
Test Platform Device Duration Team Recording
Aggregated Balance Loading Time, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 8.38s @assets-dev-team 📹 Watch
Asset View, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 3.06s @assets-dev-team 📹 Watch
Cross-chain swap flow - ETH to SOL - 50+ accounts, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 4.85s @swap-bridge-dev-team 📹 Watch
Swap flow - ETH to LINK, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 1.97s @swap-bridge-dev-team 📹 Watch
Cold Start: Measure ColdStart To Login Screen Android Google Pixel 8 Pro (v14.0) 4.20s @metamask-mobile-platform 📹 Watch
Measure Warm Start: Login To Wallet Screen Android Google Pixel 8 Pro (v14.0) 2.02s @metamask-mobile-platform 📹 Watch
Measure Warm Start: Warm Start to Login Screen Android Google Pixel 8 Pro (v14.0) 1.21s @metamask-mobile-platform 📹 Watch
Perps add funds Android Google Pixel 8 Pro (v14.0) 11.31s @mm-perps-engineering-team 📹 Watch
Predict Available Balance - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 1.22s @team-predict 📹 Watch
Predict Deposit - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 10.15s @team-predict 📹 Watch
Predict Market Details - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 3.08s @team-predict 📹 Watch
Onboarding Import SRP with +50 accounts, SRP 3 Android Google Pixel 8 Pro (v14.0) 7.35s @metamask-onboarding-team 📹 Watch
Measure Cold Start To Onboarding Screen Android Google Pixel 8 Pro (v14.0) 3.37s @metamask-mobile-platform 📹 Watch
Cold Start after importing a wallet Android Google Pixel 8 Pro (v14.0) 1.24s @metamask-mobile-platform 📹 Watch
Seedless Onboarding: Apple Login New User Android Google Pixel 8 Pro (v14.0) 11.42s @metamask-onboarding-team 📹 Watch
Account creation after fresh install Android Google Pixel 8 Pro (v14.0) 3.55s @metamask-onboarding-team 📹 Watch
Seedless Onboarding: Google Login New User Android Google Pixel 8 Pro (v14.0) 9.83s @metamask-onboarding-team 📹 Watch

Branch: gar/refactor/accounts-nav · Build: Normal · Commit: 4eb3e99 · View full run

@ccharly ccharly 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.

LGTM (not tested, just checked the recording)

@gantunesr
gantunesr enabled auto-merge July 24, 2026 15:33
@gantunesr
gantunesr added this pull request to the merge queue Jul 24, 2026
Merged via the queue into main with commit 6e9860f Jul 24, 2026
215 checks passed
@gantunesr
gantunesr deleted the gar/refactor/accounts-nav branch July 24, 2026 17:15
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 24, 2026
@metamask-ci metamask-ci Bot added the release-8.6.0 Issue or pull request that will be included in release 8.6.0 label Jul 24, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-8.6.0 Issue or pull request that will be included in release 8.6.0 risk:medium AI analysis: medium risk size-M team-accounts-framework Accounts team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants