Skip to content

fix: remove "Reset notifications" button#26641

Merged
oilnam merged 3 commits into
mainfrom
fix-remove-reset-notif-button
Feb 27, 2026
Merged

fix: remove "Reset notifications" button#26641
oilnam merged 3 commits into
mainfrom
fix-remove-reset-notif-button

Conversation

@oilnam

@oilnam oilnam commented Feb 26, 2026 •
edited by cursor Bot
Loading

Copy link
Copy Markdown
Contributor

Description

This PR removes the "Reset notifications" button which is not needed anymore, and it's currently broken anyway.
Translation strings are also removed.

image

Changelog

CHANGELOG entry: Removed "Reset notifications" button from notifications list

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

Before

After

Pre-merge author checklist

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

Low Risk
Primarily removes dead/broken UI and navigation wiring; main risk is missing a remaining reference to the removed route/string, but changes are otherwise non-functional.

Overview
Removes the broken “Reset notifications” entry from notification settings by deleting the ResetNotificationsButton, the ResetNotificationsModal bottom sheet, and their associated tests/snapshots.

Cleans up navigation by removing Routes.SHEET.RESET_NOTIFICATIONS and its route typing, and deletes related i18n strings across supported languages; also stabilizes useNotifications hook tests by properly mocking usePushNotificationsToggle and clearing mocks between tests.

Written by Cursor Bugbot for commit 3be27b9. This will update automatically on new commits. Configure here.

@oilnam oilnam requested review from a team as code owners February 26, 2026 17:35
@github-actions

github-actions Bot commented Feb 26, 2026 •
edited
Loading

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.

cursor[bot]
cursor Bot reviewed Feb 26, 2026
View reviewed changes

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

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread app/constants/navigation/Routes.ts
@oilnam oilnam force-pushed the fix-remove-reset-notif-button branch from 3c00064 to cdcf862 Compare February 26, 2026 18:16
@oilnam oilnam changed the title Remove "Reset notifications" button fix: remove "Reset notifications" button Feb 26, 2026
@oilnam

oilnam commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@oilnam oilnam requested a review from a team as a code owner February 27, 2026 08:52
@oilnam oilnam force-pushed the fix-remove-reset-notif-button branch from daf068d to 40d2e40 Compare February 27, 2026 08:53
@github-actions

github-actions Bot commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeNetworkAbstractions
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR removes the "Reset Notifications" feature entirely, which was a developer/debug feature. The changes include:

  1. Deleted Components: ResetNotificationsModal and ResetNotificationsButton with their tests
  2. Navigation Changes: Removed RESET_NOTIFICATIONS route from Routes.ts and App.tsx navigation stack
  3. Settings UI: Removed the reset button from NotificationsSettings view
  4. Localization: Removed related strings from all language files

The notification E2E tests (tagged with SmokeNetworkAbstractions) test the notification settings flow including:

  • Enabling/disabling notifications
  • Toggling push notifications
  • Toggling feature announcements
  • Toggling account notifications

These tests do NOT test the "Reset Notifications" button being removed, so they should pass. However, since the NotificationsSettings screen is modified (button removed), running SmokeNetworkAbstractions tests ensures the settings screen still renders and functions correctly after this removal.

This is a low-risk change because:

  • It's removing a feature, not modifying core functionality
  • The removed feature was a developer/debug tool
  • Core notification functionality (viewing, enabling, toggling) is unchanged
  • No E2E tests depend on the removed button

Performance Test Selection:
This PR removes a feature (Reset Notifications button and modal) without adding new functionality or modifying any rendering logic, data loading, or state management that could impact performance. The changes are purely subtractive - removing components, routes, and localization strings. No performance-sensitive code paths are affected.

View GitHub Actions results

@oilnam oilnam enabled auto-merge February 27, 2026 17:05
baptiste-marchand
baptiste-marchand approved these changes Feb 27, 2026
View reviewed changes

@baptiste-marchand baptiste-marchand 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

Cal-L
Cal-L approved these changes Feb 27, 2026
View reviewed changes

@Cal-L Cal-L 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.

Platform LGTM

@oilnam oilnam added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit e7499e0 Feb 27, 2026
216 of 232 checks passed
@oilnam oilnam deleted the fix-remove-reset-notif-button branch February 27, 2026 18:47
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 27, 2026
@metamaskbot metamaskbot added the release-7.69.0 Issue or pull request that will be included in release 7.69.0 label Feb 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@Cal-L Cal-L Cal-L approved these changes

@vinnyhoward vinnyhoward vinnyhoward approved these changes

@baptiste-marchand baptiste-marchand baptiste-marchand approved these changes

@Prithpal-Sooriya Prithpal-Sooriya Prithpal-Sooriya left review comments

Assignees

No one assigned

Labels

release-7.69.0 Issue or pull request that will be included in release 7.69.0 size-M team-evm-datalake

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@oilnam @Cal-L @Prithpal-Sooriya @vinnyhoward @baptiste-marchand @metamaskbot