[wallet-ext] Add OriginByte NFTs support #4621
Conversation
| @@ -29,14 +29,17 @@ function NFTDisplayCard({ | |||
| const { filePath, nftObjectID, nftFields, fileExtentionType } = | |||
| useNFTBasicData(nftobj); | |||
|
|
|||
| const name = nftFields?.name || nftFields?.metadata?.fields?.name; | |||
There was a problem hiding this comment.
Is there any special reason using an independent metadata field to store NFT name?
There was a problem hiding this comment.
The reasoning behind it is that we are trying to be as generic as possible, but I guess some basic fields do make sense to have. In general, which fields do you see being part of the NFT object rather than the metadata object?
There was a problem hiding this comment.
name, description, image, [optional]animation_url, [optional] attributes
you can take metaplex for an example here doc
There was a problem hiding this comment.
Having a number of "special" (field name, type) pairs is the approach we've taken so far--cc @666lcz for more details on what we currently support.
There was a problem hiding this comment.
Hi @sblackshear @666lcz @nmboavida, we also noticed ecosystem projects have totally different practices on the same thing inside the metadata of the objects. So we create a draft standard/best practice for the ecosystem projects, hoping to improve the forward compatibility and extendability.
Feel free to comment on our repo, we really hoping to push the ecosystem forward together.
https://std.suiet.app/
https://github.com/suiet/standard
| @@ -10,7 +10,7 @@ export default function useMediaUrl(objData: SuiData) { | |||
| const { fields } = (isSuiMoveObject(objData) && objData) || {}; | |||
| return useMemo(() => { | |||
| if (fields) { | |||
| const mediaUrl = fields.url; | |||
| const mediaUrl = fields.url || fields.metadata?.fields.uri; | |||
There was a problem hiding this comment.
we should also consider using url rather than uri, because we will need the specific protocols to access the media.
There was a problem hiding this comment.
We are changing the name of the field to url in this PR
There was a problem hiding this comment.
The PR has been merged, from version 0.3.0 onwards it is now url
There was a problem hiding this comment.
Field name has been changed as well.
| )} | ||
| {!!nftFields?.metadata?.fields?.attributes && ( | ||
| <> | ||
| {nftFields.metadata.fields.attributes.fields.keys.map( |
There was a problem hiding this comment.
qq: how big this list pf attributes can be? (or is usually?)
Do you mind adding some screenshots in the PR description?
cc: @mystie711
There was a problem hiding this comment.
@pchrysochoidis
Usually not more than 3-4 attributes/tags.
I've attached some video too.
There was a problem hiding this comment.
I would say the typical collection of NFTs will have around 5-10 attributes
|
Only just getting around to reviewing earlier pieces of the conversation. Agree that we should keep this moving. Based on at most "5-10 attributes" expectation, @pchrysochoidis i would suggest letting the attributes section accommodate up to 8 lines of attributes BEFORE the section becomes scrollable. |
Add support of OriginByte NFTs standard.
Screen.Recording.2022-09-14.at.12.00.07.mov