[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.
| @@ -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