-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Reorder collectibles #350
Reorder collectibles #350
Conversation
@estebanmino looks great! @cjeria OpenSea API ToS require us to mention them. We should do that to avoid getting in trouble, maybe a “powered by” + logo below the collectible description text? |
@brunobar79 @cjeria I forgot about the credit requirement from OpenSea I added it (is a button, going to OpenSea when clicking). We can move it I also deleted a lot of code. We were using different components to render assets in the lists, now we only use |
UPDATE
|
IMO The logo looks way too big but I’ll defer to @cjeria on that. Regarding position, i’d put it centered between address and the two bottom with a decent amount of margin bottom. Round borders for the kudos image would be nice. |
This is cool, but it requires a bit more design work. @bdresser can we add this the design repo todo? cc @estebanmino @brunobar79 |
yep - filed here MetaMask/website#351, added to design project & public beta milestone |
Made a restructuration of how collectibles are baing handled from GABA and also getting collectibles information from OpenSea MetaMask/core#57. With this just pushed some changes to be in line with your feedback @cjeria This GIF shows how collectibles work on mainnet, starting with auto-detection of a Kudos collectible. While the next one shows how collectibles on testnets are being handled. We can get at least name and symbol for these collectibles. If these contracts respect ERC721 Metadata interface, we are getting name and image as well (tested with Kudos skipping OpenSea) This PR is getting way big and also is blocking some other work that I could do with collectibles (as implement send collectibles finished MetaMask/website#331 or block users adding collectibles that they don't own). What do you think in merging this if functional requirements are fulfilled? Then when we get the full UX/UI we could add those changes. Thoughts? @cjeria @brunobar79 @bitpshr @bdresser |
@estebanmino i think it makes sense to merge and we can revisit design later (in #351 ) |
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.
👍
* render collectible in its own view * handle collectibles without name * wip opensea * WIP * rendering collectible contract list * flow collectible contract list then collectible contract then collectible * collectible overview * add AssetActionButtons * small refactor * some ui improvements * remove collectible contract when deleting collectible * snapshots * trigger onrefresh once and ui improvement * add collectible from collectible * open sea doc * add locale * handle fetch opensea errors * AssetElement to render assets list and avoid code duplication * delete TokenElement in favor of AssetElement * delete collectibleElement in favor of AssetElement * powered by opensea * snapshots * center collectible images * total_supply from opensea * add opensea logo * snapshots * add and remove collectible and collectibleContract from GABA * delete opensea utils * add collectible description * handle collectibles by network and account * add collectible in one method * do not handle remove collectible contract from ui * add collectible contract overview navbar * render collectible contract information on modal * handle collectibles without information * small ui changes * dont show opensea if testnet * snapshots * handle undefined collectibles attributes * snapshots * bump gaba * move opensea to collectible contract information * snapshots
Description
This PR reorders the way the app handles collectibles, requesting collectible contract information from OpenSea.
Following this MetaMask/website#331 (comment)
BLOCKER: MetaMask/core#56
Checklist
Issue
Resolves #342
cc @cjeria