Skip to content

feat: multichain manual import#14400

Merged
salimtb merged 27 commits into
mainfrom
salim/multichain-import
Apr 11, 2025
Merged

feat: multichain manual import#14400
salimtb merged 27 commits into
mainfrom
salim/multichain-import

Conversation

@salimtb

@salimtb salimtb commented Apr 3, 2025

Copy link
Copy Markdown
Contributor

Description

the goal of this PR is to make the manual import token multichain
design:
https://www.figma.com/design/CvBEDNwQqfW4Wbil40Bhr1/Substitute-Global-Network-selector?t=7VlJYKwbd0ONMpnl-0

Related issues

Fixes:

Manual testing steps

  1. Go to import token
  2. Import token using search
  3. Import token using contract address

Screenshots/Recordings

Before

After

search.mov
custom.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.

@github-actions

github-actions Bot commented Apr 3, 2025

Copy link
Copy Markdown
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.

@salimtb salimtb force-pushed the salim/multichain-import branch from bd74ffb to 4a4d9a4 Compare April 4, 2025 18:45
@salimtb salimtb changed the title Fix: test feat: multichain manual import Apr 4, 2025
@salimtb salimtb force-pushed the salim/multichain-import branch 4 times, most recently from 556ca3c to 4f509d5 Compare April 6, 2025 14:44
@github-actions

github-actions Bot commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 4f509d57ba07038139dcde6651c690d673901951
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9d5345b6-2305-4793-9e95-358d80521dc1

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@salimtb salimtb force-pushed the salim/multichain-import branch 2 times, most recently from 657d56e to 5d9093d Compare April 6, 2025 19:55
@github-actions

github-actions Bot commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4209f421b620302e213840a5ba49167f619123f3
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d8c4565e-b85e-40f7-864a-df99920d2a5b

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@salimtb salimtb force-pushed the salim/multichain-import branch from dd26967 to 5e3e0d2 Compare April 6, 2025 22:19
@github-actions

github-actions Bot commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 127f0c3
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d5819a60-dbb8-4e5e-a9e7-4c1e46cce61f

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@salimtb salimtb added team-assets needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. labels Apr 7, 2025
@salimtb salimtb marked this pull request as ready for review April 7, 2025 20:48
@salimtb salimtb requested review from a team as code owners April 7, 2025 20:48
@salimtb salimtb added the INVALID-PR-TEMPLATE PR's body doesn't match template label Apr 10, 2025
@metamaskbot metamaskbot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Apr 10, 2025
Comment thread app/components/UI/AddCustomToken/index.js
Comment thread app/components/UI/AddCustomToken/index.js
Comment thread app/components/UI/AddCustomToken/index.js
Comment thread app/components/UI/AddCustomToken/index.test.tsx
Comment thread app/components/UI/AddCustomToken/index.test.tsx
Comment thread app/components/UI/AddCustomToken/index.test.tsx
Comment thread app/components/UI/AddCustomToken/index.test.tsx
Comment thread app/components/UI/AssetSearch/index.tsx
Comment thread app/components/UI/ConfirmAddAsset/ConfirmAddAsset.test.tsx
Comment thread app/components/UI/NetworkImages/index.test.tsx
Comment thread app/components/UI/NetworkImages/index.test.tsx
Comment thread app/components/UI/NetworkImages/index.tsx
Comment thread app/components/Views/AddAsset/AddAsset.tsx
@gambinish gambinish added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Apr 10, 2025
github-merge-queue Bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Apr 10, 2025
## **Description**

Fixes a bug introduced into 12.16.0 RC in this PR:
MetaMask/metamask-mobile#14400

Bug was that users could enter into a state where they could send NFTs
on a different network than they belonged on. This is because we are now
showing NFTs across all chains that a user has NFTs on, rather than the
constrained to the GNS currentChain.

Users could enter into NFT send flow in two ways:

1. Via the NFT detail screen. We proactively change the network on the
users behalf if they enter the send flow this way (press Send from the
detail screen of a NFT from a different chain)

2. Via the Send button on the main NFT Grid view. This is where the bug
was caught. When entering into the send flow in this way, users could
enter into a state where they could accidentally send an NFT on a
different chain than intended.

View recording below to visualize this difference.

This fix filters out NFTs in the second flow so that they only ever show
NFTs from the globally selected chain.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31850?quickstart=1)

## **Related issues**

Fixes: #31858

## **Manual testing steps**

1. Add account with NFTs on a network
2. Enter into Send flow as specified in flow 2
3. NFTs should only be listed on network that they are on.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/user-attachments/assets/b35ec707-fe42-4212-8343-446a048758fc

### **After**


https://github.com/user-attachments/assets/eafe05fd-9950-4446-b79e-0d5773e724d1

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: Nicholas Gambino <nicholas.gambino@consensys.net>
gambinish added a commit to MetaMask/metamask-extension that referenced this pull request Apr 10, 2025
## **Description**

Fixes a bug introduced into 12.16.0 RC in this PR:
MetaMask/metamask-mobile#14400

Bug was that users could enter into a state where they could send NFTs
on a different network than they belonged on. This is because we are now
showing NFTs across all chains that a user has NFTs on, rather than the
constrained to the GNS currentChain.

Users could enter into NFT send flow in two ways:

1. Via the NFT detail screen. We proactively change the network on the
users behalf if they enter the send flow this way (press Send from the
detail screen of a NFT from a different chain)

2. Via the Send button on the main NFT Grid view. This is where the bug
was caught. When entering into the send flow in this way, users could
enter into a state where they could accidentally send an NFT on a
different chain than intended.

View recording below to visualize this difference.

This fix filters out NFTs in the second flow so that they only ever show
NFTs from the globally selected chain.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31850?quickstart=1)

## **Related issues**

Fixes: #31858

## **Manual testing steps**

1. Add account with NFTs on a network
2. Enter into Send flow as specified in flow 2
3. NFTs should only be listed on network that they are on.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/user-attachments/assets/b35ec707-fe42-4212-8343-446a048758fc

### **After**


https://github.com/user-attachments/assets/eafe05fd-9950-4446-b79e-0d5773e724d1

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: Nicholas Gambino <nicholas.gambino@consensys.net>
@salimtb salimtb added this pull request to the merge queue Apr 11, 2025
Merged via the queue into main with commit 6ba5ae4 Apr 11, 2025
@salimtb salimtb deleted the salim/multichain-import branch April 11, 2025 08:43
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 11, 2025
@metamaskbot metamaskbot added the release-7.46.0 Issue or pull request that will be included in release 7.46.0 label Apr 11, 2025
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. no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-7.46.0 Issue or pull request that will be included in release 7.46.0 team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants