Geo-Web-Project / cadastre

Map-based interface for claiming, buying, and managing Geo Web land parcels.
https://geoweb.land/
MIT License
25 stars 15 forks source link

Feat: Attach NFT media from connected wallet to Media Gallery #460

Closed jordang7 closed 11 months ago

jordang7 commented 11 months ago

Description

Secondary upload button on media gallery screen added, opens up a modal when pressed that displays all owned NFTs of that connected wallet through Alchemy NFT API. When the user selects an NFT, it takes the NFT image file and uploads to web3storage and populates the CID and the name.

Issue

fixes #449

dework task - https://app.dework.xyz/geo-web/cadastre?taskId=07cf1133-493e-4ef4-b986-14ecf1bba69e

Checklist:

Alert Reviewers

@codynhat @gravenp @tnrdd

jordang7 commented 11 months ago

Thanks for the feedback @gravenp.

  • The "Wallet Icon" on the new button on the Media Gallery page isn't loading for me
  • Looks like I forgot to commit the icon file. I will add it.
  • Could we add support to pull NFTs from Ethereum mainnet in addition to the network that the user is currently connected to? I think most users will have the bulk of their collections there.
  • Makes sense, I will look into a way to do this with the Alchemy sdk.
  • There's a noticeable delay between clicking Select on the "NFTs Held by..." page and returning to the "Media Gallery" page. Seems like there's some processing going on before navigation. Since there is already a spinner on the "Media Gallery" page, is it possible to navigate instantly and have all the processing take place with that pending spinner shown? If that's not appropriate, we should add a spinner to the Select button to let the user know something is going on. I found myself clicking multiple times thinking it didn't work.
  • Yes I agree with this. We can make it instantly navigate back to the main media gallery modal for a better user experience.

I will try to get these changes done by either tomorrow night or Monday.

jordang7 commented 11 months ago

Hey @gravenp , just added the changes you requested.

I modified the logic to have the modal close on the Select press and have the rest of the processing happen in the main Modal Gallery.

It pulls NFTs from both Optimism and Ethereum Mainnnet.

I can't test the specific error message you came across as I haven't been able to get the MultiFaucet to work. However I added some error handling for the cases of missing/no media attached to the asset, and if the request to fetch the image fails. Let me know if that resolves the issue you were having or not.

codynhat commented 11 months ago

Hey @jordang7. Thanks for the PR! We met on a call earlier today to discuss this. The main outstanding issue is that this currently assumes that NFT metadata are all IPFS URIs and attempts to re-upload assets to Web3.storage. To make this cleaner and easier, we think it is best to just take the URI from NFT metadata and insert directly into the content field of the MediaObject. There is no need to re-upload or check the URI at all. Browsers will read the URI and decide how to fetch the content.

codynhat commented 11 months ago

Hey @jordang7. Looks like I messed something up with the build settings for geo-web-cadastre-testnet. We are going to review this preview (https://bafybeieixxalspz732cp5a55lx4rnrdyfspp6m4suui537jd6a3ml2zuoe.on.fleek.co/) at geo-web-cadastre-staging, which uses mainnet config.