Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove alert settings #27230

Closed
wants to merge 12 commits into from

Conversation

vinnyhoward
Copy link
Contributor

@vinnyhoward vinnyhoward commented Sep 17, 2024

Description

The "Alerts" settings section is no longer needed and needs to be removed.

I am not sure if we should remove the Alerts controller and related redux, there may be implications we don't know of when doing so. I purpose we just remove the tests, UI and possibly creating a redux migration to reset the state to default. Create a second follow up ticket to remove the controller and related redux

Open in GitHub Codespaces

Related issues

Fixes: #3212

Manual testing steps

  1. Goto settings page by clicking on the more icon which are the three dots in the top right corner, and then click on settings.
  2. Check to see if the "Alerts" settings are gone
  3. Search for "Alerts" in the settings search and make sure nothing appears other than "Security & Privacy > Security Alerts" which is unrelated to the "Alerts" section being removed

Screenshots/Recordings

NA

Before

Menu

before_menu

Search

before_search

After

Menu

after_menu

Search

after_search

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.

Copy link
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.

Copy link

sonarcloud bot commented Sep 17, 2024

@vinnyhoward vinnyhoward force-pushed the chore-3212-remove-alert-settings branch from 5dd1080 to 6e1ec3d Compare September 25, 2024 01:06
@vinnyhoward vinnyhoward marked this pull request as ready for review September 25, 2024 01:39
@vinnyhoward vinnyhoward requested review from kumavis and a team as code owners September 25, 2024 01:39
@vinnyhoward vinnyhoward requested review from a team as code owners September 25, 2024 20:42
@vinnyhoward vinnyhoward force-pushed the chore-3212-remove-alert-settings branch from 48c0d13 to 1b93100 Compare September 25, 2024 20:48
@metamaskbot
Copy link
Collaborator

Builds ready [1ad343b]
Page Load Metrics (2033 ± 145 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint38026681879575276
domContentLoaded167826591984281135
load169626722033302145
domInteractive227443157
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -4.86 KiB (-0.07%)
  • common: -664 Bytes (-0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [4d5195e]
Page Load Metrics (1608 ± 110 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint36219511454391188
domContentLoaded134723021595232111
load135423071608230110
domInteractive188336209
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 1.22 KiB (0.03%)
  • ui: -4.86 KiB (-0.07%)
  • common: -664 Bytes (-0.01%)

Copy link

sonarcloud bot commented Oct 7, 2024

@vinnyhoward vinnyhoward closed this Oct 8, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2024
@vinnyhoward
Copy link
Contributor Author

Closing because a past commit had triggered many code owners but since had been reverted but the codeowners still persist. Recreating a new PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants