NFT Details - make it work based on only url params #1750
Conversation
| action: () => | ||
| navigate( | ||
| ROUTES.NFT_DETAILS(nft.collection.collection_id || '', nft.id), | ||
| ), | ||
| action: () => { | ||
| const [chainName, contractAddress, tokenId] = nft.fullUniqueId.split( | ||
| '_', | ||
| ) as [ChainName, Address, string]; | ||
| const chainId = chainNameToIdMapping[chainName]; | ||
| if (!chainId) return; | ||
| return navigate( | ||
| ROUTES.NFT_DETAILS(`${contractAddress}_${chainId}`, tokenId), | ||
| { state: { nft } }, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Am I missing something or was the fix to just pass nft into state here
There was a problem hiding this comment.
lol yes,
but since token details work like that I made it the same, this way we can search for nfts the users doesn't own
There was a problem hiding this comment.
Do you think there's any value in conditionally fetching the nft if the data doesn't exist? Presumably the data in the details screen is slower to populate now. Would be nice to support this, but use things like the images if we already have them.
There was a problem hiding this comment.
it shouldn't be slower is it? since we only fetch if we don't have the data, but currently we always have the data
I agree it's was not necessary and will try not to do stuff thinking we may need later like that in the future, I believe we should code what's needed when needed and overcomplicated here thinking of a future case
There was a problem hiding this comment.
"but currently we always have the data" <-- I was just checking that that was the case and asking your opinion on conditional fetching. Sounds like you've already thought through it.
"and will try not to do stuff thinking we may need later like that in the future" <-- I think this was a good idea. I appreciate the forethought. I was just asking for some clarification. This change wasn't properly documented.
|
@BrodyHughes Can you QA this? Thank u |
Fixes BX-####
Figma link (if any):
What changed (plus any additional context for devs)
try opening the bx in this route
/home/nft-details/0x529bb7c24fed03ad67b1c69012710c11af019c8c_8453/7879, it should work correctly, before it would be in a broken stateScreen recordings / screenshots
Screen.Recording.2024-10-31.at.06.45.06.mov
What to test