Skip to content

fix: Makes NFT list properly wrap within the send modal #30036

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

Merged
merged 4 commits into from
Jan 31, 2025
Merged

Conversation

dbrans
Copy link
Contributor

@dbrans dbrans commented Jan 31, 2025

Description

Wraps the NFT list within the Send flow such that it will not overflow

Open in GitHub Codespaces

Related issues

Fixes: #29974

Manual testing steps

  1. Use an account with many NFTs
  2. Start send flow
  3. Click the AssetPicker, choose NFTs
  4. See the wrapped NFTs

Screenshots/Recordings

Before

After

SCR-20250130-hbwm

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.

@metamaskbot metamaskbot added the team-extension-platform Extension Platform team label Jan 31, 2025
@dbrans dbrans marked this pull request as ready for review January 31, 2025 17:57
@dbrans dbrans enabled auto-merge January 31, 2025 18:02
@httpJunkie
Copy link
Contributor

httpJunkie commented Jan 31, 2025

So, the resulting CSS will be:

.nft-items__wrapper {
  grid-template-columns: repeat(4, minmax(120px, 1fr));
}

.mm-modal-content .nft-items__wrapper {
  grid-template-columns: repeat(2, minmax(120px, 1fr));
}

What It Does:

  • By default, .nft-items__wrapper is a grid with 4 cols where each col has:
    • A minimum width of 120px
    • A maximum width that adjusts (1fr makes it flexible to take available space)
  • Inside .mm-modal-content (ie: in a modal window), the .nft-items__wrapper:
    • Changes to 2 columns instead of 4
    • This means the layout adapts based on whether it is inside a modal

Sorry couldn't not chime in on a pure SCSS change lol

@dbrans dbrans added this pull request to the merge queue Jan 31, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [b8489b6]
Page Load Metrics (1727 ± 103 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45523511662348167
domContentLoaded15122326170120699
load152123571727216103
domInteractive2393482512
backgroundConnect117126178
firstReactRender1675382512
getState596162210
initialActions01000
loadScripts10811685124716278
setupStore881222211
uiStartup173726541979263126
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Merged via the queue into main with commit c3acb6f Jan 31, 2025
112 of 115 checks passed
@dbrans dbrans deleted the pr-29974-nft-width branch January 31, 2025 19:00
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2025
@metamaskbot metamaskbot added the release-12.13.0 Issue or pull request that will be included in release 12.13.0 label Jan 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.13.0 Issue or pull request that will be included in release 12.13.0 team-extension-platform Extension Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: [Bug]: In the NFT view within the Send flow, the token assets are displayed only partially
6 participants