refactor: replace old notifications opt-in screen by CTA to notif settings [GE-244] by baptiste-marchand · Pull Request #30440 · MetaMask/metamask-mobile

baptiste-marchand · 2026-05-20T13:07:48Z

Description

Simplifies notifications onboarding by removing the OptIn screen/stack entirely (routes, navigation registration, hooks, and tests) and updating callers (e.g., AccountsMenu, BasicFunctionalityModal) to go directly to Routes.NOTIFICATIONS.VIEW / notification settings.

Changelog

CHANGELOG entry:

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/GE-244

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

Before

After

Simulator Screenshot - iPhone 15 Pro - 2026-05-21 at 11 25 05

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
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
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
- See trace() for usage and addToken for an example

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
Changes notifications navigation and onboarding by removing the dedicated OptIn screens/routes and adjusting entry points to show a disabled-state prompt with a CTA to settings. Moderate risk due to altered navigation paths and deleted E2E/page-object coverage, though changes are contained to notifications UI/routing.

Overview
Simplifies notifications onboarding by removing the OptIn screen/stack entirely (routes, navigation registration, hooks, and tests) and updating callers (e.g., AccountsMenu, BasicFunctionalityModal) to go directly to Routes.NOTIFICATIONS.VIEW / notification settings.

Updates NotificationsView to render a new DisabledNotifications empty-state when notifications are turned off, with a CTA that navigates to Routes.SETTINGS.NOTIFICATIONS, and adds new strings and selector test IDs to support this UI.

Cleans up related tests and test infrastructure (route constant expectations, navigator registration tests, removed notifications opt-in page object and smoke spec) and modernizes the existing notifications empty component to use the design system components/variants.

^{Reviewed by Cursor Bugbot for commit 74225e7. Bugbot is set up for automated code reviews on this repo. Configure here.}

github-actions · 2026-05-20T13:08:00Z

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.

…tings

codecov-commenter · 2026-05-20T14:58:00Z

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.03%. Comparing base (3ee5c69) to head (2637305).
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
...asicFunctionalityModal/BasicFunctionalityModal.tsx	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #30440    +/-   ##
========================================
  Coverage   82.03%   82.03%            
========================================
  Files        5454     5460     +6     
  Lines      145830   146297   +467     
  Branches    33411    33567   +156     
========================================
+ Hits       119629   120013   +384     
- Misses      18016    18054    +38     
- Partials     8185     8230    +45

☔ View full report in Codecov by Sentry.
📢 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.

cursor

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

^{❌ 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 7f0de4a. Configure here.}

… notifications state

github-actions · 2026-05-21T09:31:08Z

🔍 Smart E2E Test Selection

Selected E2E tags: SmokeNetworkAbstractions, SmokeAccounts, SmokeWalletPlatform
Selected Performance tags: None (no tests recommended)
Risk Level: medium
AI Confidence: 82%

click to see 🤖 AI reasoning details

E2E Test Selection:

Changes Overview:
This PR removes the Notifications OptIn flow (screen, stack, hooks, page objects, and E2E spec) and replaces it with a simpler DisabledNotifications component embedded in the NotificationsView. Key changes:

Routes removed: Routes.NOTIFICATIONS.OPT_IN and Routes.NOTIFICATIONS.OPT_IN_STACK removed from Routes.ts and NavigationService/types.ts
MainNavigator.js: NotificationsOptInStack navigator removed, OptIn screen removed from NotificationsModeView, OPT_IN_STACK screen removed from root navigator
AccountsMenu.tsx: Navigation target changed from Routes.NOTIFICATIONS.OPT_IN_STACK to Routes.NOTIFICATIONS.VIEW — this directly affects the notification flow triggered from the accounts menu
BasicFunctionalityModal.tsx: Removed OPT_IN caller handling
New DisabledNotifications component: Replaces Empty component in NotificationsView when notifications are disabled
Test infrastructure: EnableNotificationsModal.ts and NotificationSettingsView.ts page objects deleted; notification-settings-flow.spec.ts deleted (was already skipped)

Tag Selection Reasoning:

SmokeNetworkAbstractions: The existing enable-notifications-after-onboarding.spec.ts test uses this tag and tests the notification flow via AccountsMenu.tapNotifications(). The AccountsMenu.tsx change (navigation target change) directly affects this test. This is the primary tag to validate.
SmokeAccounts: The AccountsMenu.tsx is part of the accounts management UI. Changes to navigation from the accounts menu could affect account-related flows. The accounts menu is used in account management tests.
SmokeWalletPlatform: The MainNavigator.js changes affect the navigation structure. The notification bell/menu is accessed via the accounts menu which is part of the wallet platform. Navigation structure changes can have broader impacts on wallet platform tests.

Performance Tests: No performance-sensitive changes — this is a UI navigation refactor removing a screen and replacing a component. No impact on rendering performance, data loading, or critical user flows that would warrant performance testing.

Performance Test Selection:
The changes are a UI navigation refactor — removing the OptIn screen/stack and replacing the Empty component with DisabledNotifications. No performance-sensitive code paths are affected (no list rendering changes, no data loading changes, no state management changes, no app startup changes). Performance tests are not warranted.

View GitHub Actions results

sonarqubecloud · 2026-05-21T09:50:10Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

christopherferreira9

Looks good for QA

Cal-L

LGTM

baptiste-marchand requested review from a team as code owners May 20, 2026 13:07

metamaskbotv2 Bot added the team-engagement label May 20, 2026

github-actions Bot added pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. size-XL labels May 20, 2026

Base automatically changed from GE-13-integrating-AUS-with-notification-setting to main May 20, 2026 13:12

baptiste-marchand added 2 commits May 20, 2026 15:34

refactor: replace old notifications opt-in screen by CTA to notif set…

7355f01

…tings

style: format

3bb688a

baptiste-marchand force-pushed the refactor/remove-opt-in-screen branch from 643f45b to 3bb688a Compare May 20, 2026 13:34

test: remove obsolete smoke spec

2637305

baptiste-marchand changed the title ~~refactor: replace old notifications opt-in screen by CTA to notif settings~~ refactor: replace old notifications opt-in screen by CTA to notif settings [GE-244] May 20, 2026

baptiste-marchand added no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed and removed pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. labels May 20, 2026

refactor: update Empty component styles and improve notification view

7f0de4a

cursor Bot reviewed May 20, 2026

View reviewed changes

Comment thread app/components/UI/Notification/Empty/styles.ts Outdated

feat: introduce DisabledNotifications component for handling disabled…

74225e7

… notifications state

oilnam approved these changes May 21, 2026

View reviewed changes

christopherferreira9 approved these changes May 21, 2026

View reviewed changes

baptiste-marchand enabled auto-merge May 21, 2026 11:35

vinnyhoward approved these changes May 21, 2026

View reviewed changes

Cal-L approved these changes May 21, 2026

View reviewed changes

baptiste-marchand added this pull request to the merge queue May 21, 2026

Merged via the queue into main with commit 053037f May 21, 2026
181 of 202 checks passed

baptiste-marchand deleted the refactor/remove-opt-in-screen branch May 21, 2026 18:21

github-actions Bot locked and limited conversation to collaborators May 21, 2026

metamaskbotv2 Bot added the release-7.79.0 Issue or pull request that will be included in release 7.79.0 label May 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: replace old notifications opt-in screen by CTA to notif settings [GE-244]#30440

refactor: replace old notifications opt-in screen by CTA to notif settings [GE-244]#30440
baptiste-marchand merged 5 commits into
mainfrom
refactor/remove-opt-in-screen

baptiste-marchand commented May 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

codecov-commenter commented May 20, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

sonarqubecloud Bot commented May 21, 2026

Uh oh!

christopherferreira9 left a comment

Uh oh!

Cal-L left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

baptiste-marchand commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

Pre-merge reviewer checklist

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

codecov-commenter commented May 20, 2026

Codecov Report

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 21, 2026

🔍 Smart E2E Test Selection

Uh oh!

sonarqubecloud Bot commented May 21, 2026

Quality Gate passed

Uh oh!

christopherferreira9 left a comment

Choose a reason for hiding this comment

Uh oh!

Cal-L left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

baptiste-marchand commented May 20, 2026 •

edited

Loading