-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Multichain NFT import #14748
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
base: main
Are you sure you want to change the base?
feat: Multichain NFT import #14748
Conversation
…nature to removeAndIgnoreNft and addNft
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. |
…mask-mobile into feat/multichain-nft-import
|
|
|
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14748 +/- ##
==========================================
+ Coverage 67.50% 67.55% +0.04%
==========================================
Files 2307 2309 +2
Lines 49618 49648 +30
Branches 7198 7205 +7
==========================================
+ Hits 33496 33541 +45
+ Misses 14005 13986 -19
- Partials 2117 2121 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
|
@@ -197,6 +168,7 @@ const AddCustomCollectible = ({ | |||
// TODO: Replace "any" with type | |||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
const { NftController } = Engine.context as any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding comment here because i couldn't add a comment on the correct line ^^"
But there is a logic before adding NFT that validates collectible address validateCustomCollectible
and ownership
validateCollectibleOwnership
; I think those should be updated as well to pass the networkClientId of the network the user has selected.
@@ -282,6 +282,13 @@ const AddAsset = () => { | |||
<AddCustomCollectible | |||
navigation={navigation} | |||
collectibleContract={collectibleContract} | |||
selectedNetwork={ | |||
selectedNetwork | |||
? networkConfigurations?.[selectedNetwork as Hex]?.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pass the networkConfiguration object instead of just the name? and leave it to the component to display the name?
I think the AddCustomCollectible
would also need the networkclientId
from the networkConfiguration to pass it down to the controller fcts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
networkClientId
anduserAddress
signature toNftController.removeAndIgnoreNft
callsnetworkClientId
signature toNftController.addNft
callsRelated issues
Fixes:
Manual testing steps
Screenshots/Recordings
Screen.Recording.2025-04-18.at.5.21.36.PM.mov
Pre-merge author checklist
Pre-merge reviewer checklist