Skip to content

chore: remove token and nft detection modals#26403

Merged
bergeron merged 5 commits intodevelopfrom
brian/token-detection-remove-modals
Aug 19, 2024
Merged

chore: remove token and nft detection modals#26403
bergeron merged 5 commits intodevelopfrom
brian/token-detection-remove-modals

Conversation

@bergeron
Copy link
Copy Markdown
Contributor

@bergeron bergeron commented Aug 14, 2024 •
edited
Loading

Description

Removes the modals prompting users to enable token + nft detection.

Open in GitHub Codespaces

Related issues

Manual testing steps

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 14, 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.

@bergeron bergeron changed the title remove token and nft detection modals chore: remove token and nft detection modals Aug 14, 2024
@bergeron bergeron added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Aug 14, 2024
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Aug 14, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

@bergeron bergeron marked this pull request as ready for review August 14, 2024 02:12
@bergeron bergeron requested a review from a team as a code owner August 14, 2024 02:12
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Aug 14, 2024

Builds ready [b1681b7]
Page Load Metrics (236 ± 246 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint78130100157
domContentLoaded105823126
load471883236512246
domInteractive105823126
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -372 Bytes (-0.01%)
  • ui: -10.05 KiB (-0.14%)
  • common: -4.21 KiB (-0.06%)

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.17%. Comparing base (b3c2323) to head (b1681b7).
Report is 19 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #26403      +/-   ##
===========================================
+ Coverage    70.13%   70.17%   +0.04%     
===========================================
  Files         1435     1431       -4     
  Lines        50309    50255      -54     
  Branches     13897    13895       -2     
===========================================
- Hits         35283    35265      -18     
+ Misses       15026    14990      -36

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

bergeron
bergeron commented Aug 14, 2024
View reviewed changes
app/scripts/controllers/preferences.js
Comment on lines -95 to -96
showTokenAutodetectModal: null,
showNftAutodetectModal: null, // null because we want to show the modal only the first time
Copy link
Copy Markdown
Contributor Author

@bergeron bergeron Aug 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on which version this goes in, these may need to be removed from state in a migration.

If it goes in before showNftAutodetectModal reaches users, that won't need to be removed since it won't have entered state yet.

@gambinish gambinish self-requested a review August 14, 2024 17:15
@bergeron bergeron merged commit 1b22343 into develop Aug 19, 2024
@bergeron bergeron deleted the brian/token-detection-remove-modals branch August 19, 2024 01:16
@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 19, 2024
@metamaskbot metamaskbot added release-12.2.0 Issue or pull request that will be included in release 12.2.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Aug 28, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Aug 28, 2024

Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.5.0), as PR was cherry-picked in branch 12.2.0.

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

Reviewers

@salimtb salimtb salimtb approved these changes

@gambinish gambinish gambinish approved these changes

Assignees

No one assigned

Labels

needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-12.2.0 Issue or pull request that will be included in release 12.2.0 team-assets

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bergeron @metamaskbot @salimtb @gambinish