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: NFT details new design #25524

Merged
merged 45 commits into from
Jul 16, 2024
Merged

feat: NFT details new design #25524

merged 45 commits into from
Jul 16, 2024

Conversation

sahar-fehri
Copy link
Contributor

@sahar-fehri sahar-fehri commented Jun 26, 2024

Description

PR that updates the design for the NFT page.
Designs are here: https://www.figma.com/design/TfVzSMJA8KwpWX8TTWQ2iO/Asset-list-and-details?node-id=3717-47944&m=dev

Open in GitHub Codespaces

Related issues

Fixes:
Related: MetaMask/core#4443

Manual testing steps

  1. Go to NFT tab
  2. Click on any NFT you have
  3. You should be able to see the new NFT page with no errors even if some data is not available.

Screenshots/Recordings

Before

Screen.Recording.2024-06-28.at.10.58.45.mov

After

Screen.Recording.2024-06-28.at.10.56.41.mov

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.

@sahar-fehri sahar-fehri changed the title Feat/nft details new design feat: NFT details new design Jun 27, 2024
Copy link

codecov bot commented Jun 28, 2024

Codecov Report

Attention: Patch coverage is 48.67925% with 136 lines in your changes missing coverage. Please review.

Project coverage is 69.80%. Comparing base (7b3450a) to head (9d75a7f).

Files Patch % Lines
ui/components/app/nft-details/nft-details.tsx 55.50% 85 Missing ⚠️
ui/components/app/nft-details/nft-full-image.tsx 0.00% 23 Missing ⚠️
ui/helpers/utils/util.js 0.00% 17 Missing ⚠️
...ponents/app/nft-details/nft-detail-description.tsx 71.43% 4 Missing ⚠️
...nts/app/nft-details/nft-detail-information-row.tsx 63.64% 4 Missing ⚠️
...s/app/nft-details/nft-detail-information-frame.tsx 71.43% 2 Missing ⚠️
ui/pages/asset/util.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25524      +/-   ##
===========================================
- Coverage    69.98%   69.80%   -0.18%     
===========================================
  Files         1394     1398       +4     
  Lines        48945    49130     +185     
  Branches     13467    13565      +98     
===========================================
+ Hits         34250    34293      +43     
- Misses       14695    14837     +142     

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

@metamaskbot
Copy link
Collaborator

Builds ready [14b3062]
Page Load Metrics (256 ± 256 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint642311214119
domContentLoaded98030189
load391902256533256
domInteractive98030189
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 7.43 KiB (0.11%)
  • common: 2.21 KiB (0.03%)

@sahar-fehri sahar-fehri marked this pull request as ready for review June 28, 2024 09:00
@sahar-fehri sahar-fehri requested a review from a team as a code owner June 28, 2024 09:00
@sahar-fehri sahar-fehri self-assigned this Jun 28, 2024
@sahar-fehri sahar-fehri added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Jun 28, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [676f670]
Page Load Metrics (239 ± 198 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6459713511656
domContentLoaded95395711354
load411428239412198
domInteractive95395711354
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 7.43 KiB (0.11%)
  • common: 2.42 KiB (0.04%)

@sahar-fehri sahar-fehri requested a review from a team as a code owner July 1, 2024 16:43
@sahar-fehri
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated

@metamaskbot metamaskbot requested a review from a team as a code owner July 15, 2024 10:10
@metamaskbot
Copy link
Collaborator

Builds ready [4f413ba]
Page Load Metrics (158 ± 147 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint613451268239
domContentLoaded9148323015
load441469158307147
domInteractive9148323015
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 15.37 KiB (0.22%)
  • common: 2.51 KiB (0.04%)

@sahar-fehri sahar-fehri requested a review from salimtb July 15, 2024 13:00
salimtb
salimtb previously approved these changes Jul 15, 2024
bergeron
bergeron previously approved these changes Jul 15, 2024
Copy link
Contributor

@bergeron bergeron left a comment

Choose a reason for hiding this comment

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

tested a bunch of my NFTs and looked great to me. Left 1 datetime localization nitpick

@sahar-fehri sahar-fehri dismissed stale reviews from bergeron and salimtb via 1a00cc1 July 15, 2024 21:50
bergeron
bergeron previously approved these changes Jul 15, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [eba470b]
Page Load Metrics (402 ± 320 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint823971236833
domContentLoaded108229199
load481997402666320
domInteractive108229199
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 16.23 KiB (0.23%)
  • common: 2.51 KiB (0.04%)

Copy link

sonarcloud bot commented Jul 16, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [9d75a7f]
Page Load Metrics (70 ± 8 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6912498178
domContentLoaded95424115
load409770178
domInteractive95324115
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 16.23 KiB (0.22%)
  • common: 2.51 KiB (0.04%)

@sahar-fehri sahar-fehri merged commit b9de828 into develop Jul 16, 2024
79 checks passed
@sahar-fehri sahar-fehri deleted the feat/NFT-details-new-design branch July 16, 2024 08:52
@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2024
@metamaskbot metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-assets
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants