Feature/collectible detail #2451
Conversation
| ); | ||
| return { | ||
| collectionAddress: collection.address, | ||
| collectionName: collection.name, |
There was a problem hiding this comment.
adding these keys to the collectible object to make displaying details a little simpler. Mobile follows a similar data model
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| @@ -0,0 +1,41 @@ | |||
| import * as React from "react"; | |||
There was a problem hiding this comment.
implementing Popover from shadcn as a test run 🎉
There was a problem hiding this comment.
Picked up directly from here: https://ui.shadcn.com/docs/components/popover
| ) : ( | ||
| <View.Content> | ||
| <div className="CollectibleDetail__content"> | ||
| {collectible.metadata?.image && ( |
There was a problem hiding this comment.
we don't have any control of collectible metadata (as it's hosted by the NFT creator), so I'm using defensive checks for all the pieces of data we want to show. I'm working under the assumption some (or all) of these fields may not have been configured properly
| <div className="CollectibleDetail__header__right-button__popover-content__item__label"> | ||
| {t("Hide collectible")} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
this button functionality will be implemented in the next PR
| tokenId: selectedCollectible.tokenId, | ||
| tokenUri: collectible.tokenUri || "", | ||
| }); | ||
| setIsPopoverOpen(false); |
There was a problem hiding this comment.
TODO: we don't have an error state designed for what to do if refreshing metadata fails. For now, this fails silently but we should provide the user with some feedback if it failed @sdfcharles
| collections[resolvedData?.networkDetails?.network || ""]?.[ | ||
| resolvedData?.publicKey || "" | ||
| ] || [] | ||
| } |
There was a problem hiding this comment.
use our value from redux so when we refresh metadata, the change is reflected in all views
extension/e2e-tests/helpers/stubs.ts
Outdated
| }, | ||
| ]; | ||
| } | ||
| console.log(tokenMetadataCount); |
There was a problem hiding this comment.
leftover debug log
| title={name} | ||
| customBackAction={handleItemClose} | ||
| rightButton={ | ||
| <Popover open={isPopoverOpen} onOpenChange={setIsPopoverOpen}> |
There was a problem hiding this comment.
A couple of things I noticed -
The detail view should be a bottom slider but the Popover is for modals, should this use the sheet instead?
Our bottom sheets have the x icon for the dismiss action, here we use the back arrow.
There was a problem hiding this comment.
Ah yes, thanks for bringing that up. I did see that the mobile app uses the bottom sheet for this view, but I opted to re-use the style we use for the asset detail subview for consistency.
@sdfcharles What do you think? I can implement the shadcn bottom sheet, but I'd imagine we'd want to do this for the asset detail subview, as well
There was a problem hiding this comment.
After chatting with Charles, we decided to move forward with implementing shadcn's sheet. It is a sizable change so I opened a new PR for this work to make it easier to review: #2463
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Nice! Good work on making the "refresh metadata" fetch the metadata for a single collectible, we should probably do the same on mobile
piyalbasu
force-pushed
the
feature/collectible-detail
branch
from
aee6ae5 to
d3daa5a
Compare
* add CollectibleDetail UI * add popover and tests * adds additional testing * fetch only the metadata for the current detail * use state from hook * fix missing translations helpers * rm log * fix tests * test failing due to copy change * rm empty dir and fix test due to copy change
* add CollectibleDetail UI * add popover and tests * adds additional testing * fetch only the metadata for the current detail * use state from hook * fix missing translations helpers * rm log * fix tests * test failing due to copy change * rm empty dir and fix test due to copy change
Closes #2326
Adds Collectible Details UI
refresh metadatawill re-fetch just the metadata for only the selected collectible. The new metadata is then added to redux to make sure our redux store stays in sync while minimizing the amount of API's calls we need to makeExample of changing metadata for a collectible:
Screen.Recording.2025-12-09.at.5.06.49.PM.mov