Skip to content

Remove unused notifications, their messages, and images#22678

Merged
HowardBraham merged 3 commits intodevelopfrom
remove-unused-notifications
Feb 13, 2024
Merged

Remove unused notifications, their messages, and images#22678
HowardBraham merged 3 commits intodevelopfrom
remove-unused-notifications

Conversation

@HowardBraham
Copy link
Copy Markdown
Contributor

@HowardBraham HowardBraham commented Jan 27, 2024

Description

Continues the work that @legobeat was doing in #20703 and #21533

All old numbered notifications except 8, 20, and 24 were permanently turned off, but their code, their messages.json, and most importantly their images were still present in the Extension.

By removing all this, we reduce the size of the zipped Extension by 949kB, and the unzipped Extension by 1,852kB.

IMPORTANT: This is NOT well-tested from a QA perspective, and QA should do some good testing on it before we merge.

Related issues

Followup to: #20703
Followup to: #21533

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • 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.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@HowardBraham HowardBraham added DO-NOT-MERGE Pull requests that should not be merged team-extension-platform Extension Platform team labels Jan 27, 2024
@HowardBraham HowardBraham self-assigned this Jan 27, 2024
@HowardBraham HowardBraham requested a review from a team as a code owner January 27, 2024 05:26
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 27, 2024

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.

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 27, 2024

Builds ready [321dc85]
Page Load Metrics (780 ± 21 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint85131109115
domContentLoaded9261742
load7218997804321
domInteractive9261742
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -118 Bytes (-0.00%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham requested a review from legobeat January 29, 2024 23:37
@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from 321dc85 to 65b7d0b Compare January 30, 2024 00:18
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 30, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b0bd096) 68.47% compared to head (17137c7) 68.54%.
Report is 2 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #22678      +/-   ##
===========================================
+ Coverage    68.47%   68.54%   +0.07%     
===========================================
  Files         1088     1088              
  Lines        42956    42900      -56     
  Branches     11436    11417      -19     
===========================================
- Hits         29410    29403       -7     
+ Misses       13546    13497      -49

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 30, 2024

Builds ready [65b7d0b]
Page Load Metrics (823 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint92157112199
domContentLoaded9231542
load727113382310148
domInteractive9231542
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -118 Bytes (-0.00%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from 65b7d0b to 6ef3df8 Compare January 31, 2024 17:53
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 31, 2024

Builds ready [6ef3df8]
Page Load Metrics (811 ± 24 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint96148114147
domContentLoaded10451884
load7339178114924
domInteractive10451884
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -118 Bytes (-0.00%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham added needs-qa Label will automate into QA workspace needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Feb 1, 2024
@sleepytanya
Copy link
Copy Markdown
Contributor

sleepytanya commented Feb 3, 2024 •
edited
Loading

Manual test cases passed on Chrome 121.0.6167.139 and Firefox 122.0:

  • Send native token MM
  • Send native token dapp
  • Check incoming transactions history is present in the Activity list
  • Send ERC20 token MM
  • Send ERC20 token dapp
  • Send ERC721 MM
  • Send ERC721 dapp
  • Import ERC 20 MM
  • Import ERC20 dapp
  • Import ERC721 MM
  • Import ERC721 dapp
  • Import ERC1155 MM
  • Import ERC1155 dapp
  • Contract interaction flows
  • Approve ERC20
  • Approve ERC721
  • Approve ERC1155
  • Cancel transaction
  • Speed up cancellation
  • Speed up transaction
  • Queue multiple transactions, cancel / speed up
  • Custom nonce
  • Token autodetect
  • NFT autodetect
  • Add / remove networks / custom networks
  • Personal sign
  • Sign typed data
  • Sign typed data v3
  • Sign typed data v4
  • Swap
  • Smart swap

@sleepytanya
Copy link
Copy Markdown
Contributor

sleepytanya commented Feb 3, 2024

  1. 'Watch NFT' and 'Watch all NFTs' is not responsive in Chrome, works as expected on Firefox.
  2. ENS name resolution on Chrome and Firefox:
    doesn't work on Linea Goerli and Sepolia
lineaGoerliEns works on Goerli goerliEns On mainnet ENS name is initially recognized then 'ens lookup failed' message pops up
ens.mov
  1. Errors in the background when cancelling a swap:
    Firefox
swapCancel.mov

Chrome
Screenshot 2024-02-02 at 8 34 44 PM
I wasn't able to cancel any swap (performed about 10 swaps).

@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from 1296466 to 32113b5 Compare February 3, 2024 08:41
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 3, 2024

Builds ready [32113b5]
Page Load Metrics (755 ± 15 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint90130108105
domContentLoaded9351752
load7078267553115
domInteractive9341752
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

davidmurdoch
davidmurdoch previously approved these changes Feb 6, 2024
View reviewed changes
@davidmurdoch davidmurdoch self-requested a review February 6, 2024 19:22
@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from 9319de5 to d2954e9 Compare February 6, 2024 19:27
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 6, 2024

Builds ready [d2954e9]
Page Load Metrics (791 ± 26 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint84150113178
domContentLoaded95623147
load7229097915426
domInteractive95623147
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from d2954e9 to c62f986 Compare February 8, 2024 01:19
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 8, 2024

Builds ready [c62f986]
Page Load Metrics (707 ± 12 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7412289115
domContentLoaded9171421
load6727517072412
domInteractive9171421
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

legobeat
legobeat reviewed Feb 8, 2024
View reviewed changes
@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from c62f986 to b02ec91 Compare February 9, 2024 10:41
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 9, 2024

Builds ready [b02ec91]
Page Load Metrics (763 ± 17 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7913796157
domContentLoaded9211442
load7128457633617
domInteractive9211442
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from b02ec91 to b1d94c3 Compare February 9, 2024 20:05
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 9, 2024

Builds ready [b1d94c3]
Page Load Metrics (810 ± 32 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint812761134421
domContentLoaded9197274019
load71410548106732
domInteractive9197274019
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from b1d94c3 to e6f0390 Compare February 12, 2024 23:27
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 12, 2024

Builds ready [e6f0390]
Page Load Metrics (1105 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint942211673617
domContentLoaded9142353215
load8501572110517785
domInteractive9142353215
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

@chloeYue chloeYue added QA Passed and removed needs-qa Label will automate into QA workspace labels Feb 13, 2024
@chloeYue
Copy link
Copy Markdown
Contributor

chloeYue commented Feb 13, 2024

I did some exploratory testing for notifications and compared the notifications between the PR branch and the develop branch. QA passed. ✔️ @HowardBraham

@HowardBraham HowardBraham removed the DO-NOT-MERGE Pull requests that should not be merged label Feb 13, 2024
@HowardBraham HowardBraham force-pushed the remove-unused-notifications branch from e6f0390 to 17137c7 Compare February 13, 2024 17:51
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 13, 2024

Builds ready [17137c7]
Page Load Metrics (1030 ± 109 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1162951894421
domContentLoaded986302411
load81618191030228109
domInteractive986302411
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: -1.33 KiB (-0.02%)
  • common: -14.19 KiB (-0.28%)

@HowardBraham HowardBraham merged commit 34df597 into develop Feb 13, 2024
@HowardBraham HowardBraham deleted the remove-unused-notifications branch February 13, 2024 18:59
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Feb 13, 2024
@metamaskbot metamaskbot added the release-11.12.0 Issue or pull request that will be included in release 11.12.0 label Feb 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@chloeYue chloeYue chloeYue approved these changes

@davidmurdoch davidmurdoch Awaiting requested review from davidmurdoch

+1 more reviewer

@legobeat legobeat legobeat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@HowardBraham HowardBraham

Labels

QA Passed release-11.12.0 Issue or pull request that will be included in release 11.12.0 team-extension-platform Extension Platform team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@HowardBraham @metamaskbot @sleepytanya @chloeYue @davidmurdoch @legobeat