-
-
Notifications
You must be signed in to change notification settings - Fork 280
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
EVM NFT section #15467
EVM NFT section #15467
Conversation
92e75fd
to
4b72ec9
Compare
7a926bb
to
13d1ed5
Compare
packages/suite/src/components/wallet/WalletLayout/AccountTopPanel/AccountNavigation.tsx
Outdated
Show resolved
Hide resolved
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.
I understand that this is just a PR draft, but I would like to avoid duplicating code and making additional changes later.
1478947
to
1368894
Compare
1368894
to
a53d6c8
Compare
I appreciate this approach |
fc7faee
to
deb7eaf
Compare
deb7eaf
to
963015e
Compare
🚀 Expo preview is ready!
|
f1df078
to
3389b4c
Compare
3389b4c
to
ad6d987
Compare
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.
collections present
7aa29d0
to
7b7f14a
Compare
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.
LGTM 🚀
@trezor/qa NFT section is introduced as an experimental feature. You have to turn it on in the settings. Only for EVM based chains. StandardsIt works with 2 types of NFT standards: ERC-721 (Non-Fungible Token Standard): ERC-1155 (Multi-Token Standard): In the table you can't tell which is which, you can check it in the explorer. Empty collectionsIf a collection has been present in the account before, but now it's empty (because user sent all the nfts away using a third party wallet), it will be displayed in the "Empty collections" on the last row of the table. Recognized/unrecognized collectionsA collection can be recognized (based on information from coingecko), and unrecognized, in that case it's displayed in the hidden part. (It's similar as in tokens) Tips for testingFor testing I recommend you to buy some cheap recognized collection, on BSC or Polygon to avoid high gas fees. You don't have to worry about unrecognized collections because there's a ton of spam collections on QA seed. (on Polygon for sure) https://www.coingecko.com/en/nft/chains/binance-smart-chain you can check out the collections there and filter it by the lowest floor price. Then you can find it either on OpenSea or MagicEden (or another NFT marketplace of your choice). You'll have to connect using a third party wallet (can be Metamask or Rabby) I want to ask you to really take your time and test every possible scenario with recognized/unrecognized collections, with each standard as well as empty and hidden collections. Maybe on the L2s in debug mode as well. This is a big feature and as you can see in the amount of comments, it's easy to overlook something. May you have any questions, feel free to reach out to me |
@@ -545,7 +546,7 @@ export const networks = { | |||
decimals: 18, | |||
testnet: true, | |||
explorer: getExplorerUrls('https://holesky1.trezor.io', 'ethereum'), | |||
features: ['rbf', 'sign-verify', 'tokens', 'staking'], | |||
features: ['rbf', 'sign-verify', 'tokens', 'nfts', 'nft-definitions'], |
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.
removed staking
from ethereum holesky :(
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.
fixing here #16129
thank you for the catch
QA OK
Info:
|
Description
NFT section is introduced for EVM based chains, enabled through experimental features. Supportes NFTs are ERC721 and ERC1155.
Related Issue
Resolve #8004
Resolve #12165 (last issue)
Screenshots:
In experimental features: