Skip to content

feat: enable sending ERC1155 NFTs from the NFT details screen#27800

Merged
juanmigdr merged 8 commits into
mainfrom
feat/support-sending-erc1155-from-nft-details
Mar 25, 2026
Merged

feat: enable sending ERC1155 NFTs from the NFT details screen#27800
juanmigdr merged 8 commits into
mainfrom
feat/support-sending-erc1155-from-nft-details

Conversation

@juanmigdr
Copy link
Copy Markdown
Member

@juanmigdr juanmigdr commented Mar 23, 2026

Description

ERC1155 NFTs were not sendable from the NFT details screen because the isTradable guard only allowed ERC721 tokens, even though the underlying send flow already supports ERC1155.

This PR:

  1. Enables the "Send" button for ERC1155 tokens in NftDetails by extending isTradable to accept both ERC721 and ERC1155 standards when isCurrentlyOwned is true.
  2. Adds a loading skeleton on the Amount screen while useEVMNfts asynchronously processes the NFT (resolves IPFS image URLs, fetches ERC1155 balance, computes networkBadgeSource). Without this, the screen briefly shows 0 units available and no image before the data arrives.
  3. Replaces the deprecated StyledButton with Button from @metamask/design-system-react-native on the NFT details send button, using its built-in isLoading and isDisabled props to give feedback during the network-switch step.

The loading state is propagated through useEVMNftsuseRouteParamsAmount, keeping the data flow explicit and avoiding any inference from data absence (which would cause the skeleton to hang if the NFT is not found).

Changelog

CHANGELOG entry: Added support for sending ERC1155 NFTs from the NFT details screen

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2908

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Screen.Recording.2026-03-23.at.14.04.49.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.

Note

Medium Risk
Moderate risk because it changes send entry conditions and refactors useEVMNfts to be async with a new loading state that affects multiple send screens; bugs could block NFT selection/sending or cause incorrect asset resolution.

Overview
Enables sending owned ERC1155 NFTs from NftDetails by widening the tradable guard to include ERC1155 (in addition to ERC721) and updating the Send CTA to use the design-system Button.

Improves Send Amount UX while NFT data resolves by adding an isLoading state to useEVMNfts, plumbing it through useRouteParams, and showing skeleton placeholders (including balance) on the Amount screen.

Tightens NFT resolution for send navigation by matching NFTs by address + chainId + tokenId (important for ERC1155), and updates mocks/snapshots/tests accordingly (including an ERC721 mock and new loading/error-path coverage).

Written by Cursor Bugbot for commit 068ef91. This will update automatically on new commits. Configure here.

@juanmigdr juanmigdr requested review from a team as code owners March 23, 2026 13:07
@github-actions github-actions Bot added size-M risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 23, 2026
Comment thread app/components/Views/confirmations/hooks/send/useNfts.ts Outdated
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
Comment thread app/components/Views/confirmations/hooks/send/useNfts.ts
Comment thread app/components/Views/confirmations/hooks/send/useRouteParams.ts
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
@juanmigdr juanmigdr enabled auto-merge March 24, 2026 10:06
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
Comment thread app/components/Views/NftDetails/NftDetails.tsx
sahar-fehri
sahar-fehri previously approved these changes Mar 24, 2026
Copy link
Copy Markdown
Member

@OGPoyraz OGPoyraz left a comment

Choose a reason for hiding this comment

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

I appreciate adding a recording to initiate ERC1155 send directly from the asset page. Can we also add full send flow example as well? (Click send from home screen, pick ERC1155 and confirm)

Comment thread app/components/Views/NftDetails/NftDetails.tsx Outdated
@juanmigdr juanmigdr dismissed stale reviews from sahar-fehri and Prithpal-Sooriya via 832055d March 25, 2026 07:36
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 25, 2026
Comment thread app/components/Views/NftDetails/NftDetails.tsx
@juanmigdr juanmigdr requested a review from OGPoyraz March 25, 2026 07:45
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 25, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are focused on the NFT send flow and confirmation UI:

  1. amount.tsx: Fixed isNFT check to include ERC721 (was only ERC1155 before - a bug fix). Added skeleton loading states using isNftLoading from useRouteParams. This directly affects the send amount screen in the confirmation flow.

  2. useNfts.ts: Added isLoading state tracking to useEVMNfts(), added cancellation pattern for async NFT processing, improved error handling. Returns { nfts, isLoading } instead of just nfts[].

  3. useRouteParams.ts: Destructures new isLoading from useEVMNfts(), adds tokenId to NFT matching logic (bug fix - previously could match wrong NFT if same address/chainId), returns isLoading state.

  4. asset.tsx: Minor update to destructure nfts from updated useEVMNfts() return type.

  5. NftDetails.tsx: Replaced StyledButton with Button from design system, removed manual network switching logic (now handled by the send flow itself), extended isTradable() to include ERC1155 (previously only ERC721 was tradable), simplified onSend callback.

SmokeConfirmations is the primary tag because:

  • The send flow (amount screen, asset selection) is directly modified
  • The ERC721/ERC1155 send flow is the core change
  • Send tests (send-erc20-token, send-native-token, etc.) exercise the same send infrastructure
  • The isNFT bug fix (ERC721 was not treated as NFT before) could affect existing send behavior

SmokeWalletPlatform was considered but not selected because:

  • NFT details view changes are UI-only (button component swap, no functional change to non-send flows)
  • No dedicated NFT E2E tests exist in the test suite
  • The core risk is in the send/confirmation flow

No performance tests needed - changes are UI skeleton loading states and bug fixes, not performance-sensitive code paths.

Performance Test Selection:
The changes add skeleton loading UI states and fix NFT matching logic. These are not performance-sensitive changes - no new data fetching, no list rendering changes, no state management overhaul. The skeleton loading is a UI improvement that doesn't impact measurable performance metrics.

View GitHub Actions results

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread app/components/Views/confirmations/hooks/send/useNfts.ts
@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
17 value mismatches detected (expected — fixture represents an existing user).
View details

@sonarqubecloud
Copy link
Copy Markdown

@juanmigdr juanmigdr requested review from Prithpal-Sooriya and sahar-fehri and removed request for OGPoyraz March 25, 2026 08:43
@juanmigdr juanmigdr added this pull request to the merge queue Mar 25, 2026
Merged via the queue into main with commit 46e0425 Mar 25, 2026
96 checks passed
@juanmigdr juanmigdr deleted the feat/support-sending-erc1155-from-nft-details branch March 25, 2026 09:56
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 25, 2026
@metamaskbot metamaskbot added the release-7.72.0 Issue or pull request that will be included in release 7.72.0 label Mar 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.72.0 Issue or pull request that will be included in release 7.72.0 risk-medium Moderate testing recommended · Possible bug introduction risk size-M team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants