feat: implement rich notification setting config using AUS#30106
feat: implement rich notification setting config using AUS#30106baptiste-marchand wants to merge 14 commits into
Conversation
|
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. |
…s and introducing a new section for wallet activity
…nts and enhancing the notification preferences UI
…fication-services-controller dependencies to latest versions
baptiste-marchand
changed the title
baptiste-marchand
force-pushed
the
GE-13-integrating-AUS-with-notification-setting
branch
from
e017e79 to
8993554
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…t and associated tests
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Performance Test Selection: |
There was a problem hiding this comment.
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 8c759d9. Configure here.
| withHorizontalPadding={false} | ||
| /> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
Conflicting write paths silently revert socialAI push toggle
High Severity
When type is socialAI, the parent NotificationSettingsSection manages push/in-app toggles via useNotificationStoragePreferences().updatePreference, while the child SocialAINotificationPreferencesContent independently writes the entire socialAI slice via useNotificationPreferences().applyChange → updatePreferencesSection('socialAI', nextSocialAI). If a user toggles a trader (creating an optimistic overlay in the child with stale pushNotificationsEnabled: true), then flips the parent's push toggle off, the child's overlay retains the old push value. When the child's queued persist runs, it overwrites the parent's change — silently reverting the push toggle back to true on the server.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8c759d9. Configure here.
|
| const getStatusText = (prefs?: NotificationPreferenceStatus | null) => { | ||
| const active = []; | ||
| if (prefs?.pushNotificationsEnabled) { | ||
| active.push('Push'); |
There was a problem hiding this comment.
question (non-blocking): do these need to be localized?
There was a problem hiding this comment.
Yes definitely! I'll make sure I didn't forget any other string
| title={strings('app_settings.notifications_opts.marketing_title')} | ||
| status={getStatusText(preferences?.marketing)} | ||
| iconName={IconName.Campaign} | ||
| onPress={() => |
There was a problem hiding this comment.
question: For the marketing section, should the In-app toggle also keep the existing feature-announcement controller flag in sync?
It looks like core still gates product-announcement notifications on NotificationServicesController.state.isFeatureAnnouncementsEnabled here:
https://github.com/MetaMask/core/blob/main/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts#L1260-L1263
Turning off Marketing > In-app updates AUS but I think it still might allow Contentful productAnnouncement items in the notification list.
| item={item} | ||
| evmAddress={evmAddress} | ||
| icon={accountAvatarType} | ||
| disabledSwitch={shouldDisableSwitches} |
There was a problem hiding this comment.
thought (non-blocking): the social AI section disables the secondary options when push notifications is disabled. Should we do something similar here? If the user does not have wallet push or in-app notifications on, then we disable the accounts options.
2 participants
Description
Revamps the notifications settings UX into section-based configuration (wallet activity, perps, Social AI, marketing) with a new NotificationSettingsSection screen and updated navigators to route into it.
Changelog
CHANGELOG entry: Revamp notification settings and enrich them with several sections
Related issues
Core PR: MetaMask/core#8784
Fixes: https://consensyssoftware.atlassian.net/browse/GE-13
Manual testing steps
Screenshots/Recordings
Before
After
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2026-05-15.at.17.16.52.mov
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2026-05-15.at.17.17.52.mov
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
Moderate risk: introduces new AUS-backed preference read/merge/write flows and rewires navigation/UI for notification settings, which could impact preference persistence and routing if edge cases (missing prefs/loading) are mishandled.
Overview
Revamps notification settings into section-based configuration (wallet activity, perps, Social AI, marketing) by adding a new
NotificationSettingsSectionscreen and updating navigators to route into it.Adds AUS-backed preference fetching/updating via
useNotificationStoragePreferences(optimistic cache update + read/merge/write persistence) and wires Social AI preferences to this shared storage; Social leaderboard entry points now deep-link into the new settings section or fall back toNotificationsSettingswhen no AUS row exists.Updates the notifications list UI to use
HeaderCompactStandardwith explicit close/settings actions, and removes legacy components/paths (SettingsNotification,resetNotifications, and the oldNotificationPreferencesView).Reviewed by Cursor Bugbot for commit 8c759d9. Bugbot is set up for automated code reviews on this repo. Configure here.