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

feat: show default petnames for watched NFTs #23438

Merged
merged 4 commits into from Mar 15, 2024

Conversation

matthewwalsh0
Copy link
Member

@matthewwalsh0 matthewwalsh0 commented Mar 12, 2024

Description

The petnames component displays default names in scenarios where the address can be partially trusted despite no petname being saved.

This is currently the case for MetaMask contracts and ERC-20 tokens from the static token list or those recognised by the remote token API.

This PR also shows default names and icons for any watched NFT contracts from any of the user's accounts on the current chain.

Open in GitHub Codespaces

Related issues

Fixes: #2245

Manual testing steps

  1. Watch an NFT.
  2. Create a transaction targeting the same NFT contract.
  3. Verify the default petname matches the contract.
  4. Verify the correct icon is visible if the NFT is known by OpenSea.

Screenshots/Recordings

Before

Screenshot 2024-03-12 at 21 01 37

After

Screenshot 2024-03-12 at 21 00 20

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.

@matthewwalsh0 matthewwalsh0 added the team-confirmations-system DEPRECATED: please use "team-confirmations" label instead label Mar 12, 2024
Store addresses in lowercase.
Fix unit test.
Fix storybook.
@metamaskbot
Copy link
Collaborator

Builds ready [0dc6096]
Page Load Metrics (1011 ± 408 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint693951377134
domContentLoaded1183352311
load5618961011849408
domInteractive1183352311
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.74 KiB (0.02%)
  • common: 0 Bytes (0.00%)

@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review March 13, 2024 13:11
@matthewwalsh0 matthewwalsh0 requested a review from a team as a code owner March 13, 2024 13:11
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.76%. Comparing base (5a5805f) to head (54bd93d).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #23438      +/-   ##
===========================================
+ Coverage    68.73%   68.76%   +0.03%     
===========================================
  Files         1124     1125       +1     
  Lines        43617    43655      +38     
  Branches     11674    11683       +9     
===========================================
+ Hits         29976    30015      +39     
+ Misses       13641    13640       -1     

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

@metamaskbot
Copy link
Collaborator

Builds ready [54bd93d]
Page Load Metrics (869 ± 468 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint652491335627
domContentLoaded1092352713
load562694869974468
domInteractive1092352613
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.74 KiB (0.02%)
  • common: 0 Bytes (0.00%)

ui/selectors/nft.ts Show resolved Hide resolved
@sleepytanya
Copy link
Contributor

The default petname matches the contract:

Screenshot 2024-03-15 at 1 02 30 AM

The correct icon is visible for the NFT is known by OpenSea:
Screenshot 2024-03-15 at 1 20 19 AM

@matthewwalsh0 matthewwalsh0 merged commit 805c81e into develop Mar 15, 2024
70 checks passed
@matthewwalsh0 matthewwalsh0 deleted the feat/petnames-watched-nfts branch March 15, 2024 11:26
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2024
@metamaskbot metamaskbot added the release-11.14.0 Issue or pull request that will be included in release 11.14.0 label Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-11.14.0 Issue or pull request that will be included in release 11.14.0 team-confirmations-system DEPRECATED: please use "team-confirmations" label instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants