search

Toniq-Labs / stoic-wallet

MIT License
47 stars 32 forks source link

Integrate DAB with Stoic Wallet #13

Open tarek-eg opened 2 years ago

tarek-eg commented 2 years ago

Link to forum https://forum.dfinity.org/t/icdevs-org-bounty-integrate-dab-with-stoic-wallet-6-100-icp/9824

tarek-eg commented 2 years ago

Open questions: Stoic collections has the following properties but DAB doesn't, what should we default to in these cases?

   comaddress ? 
   commission ?
   route ?
tarek-eg commented 2 years ago

An update on the progress so far. Query DAB Nfts is working fine. A little issue with the thumbnails in DAB, since the returned NFTS could be of many types we need to check first the content_type and based on the that we should know how to render, currently I am falling back to the collection icon. A link to discussion on DAB discord

Transfer function: I am a bit blocked on the transfer function since there's a mismatch between agent-js versions used by DAB-js and stoic that causing an error. Stoic is using 0.8.9 and we need to upgrade to at least version 0.9.3 which has a breaking changes. I will be spending sometime with the upgrade but the changes could grow out of proportion.

tarek-eg commented 2 years ago

Ready for review

stephenandrews commented 2 years ago

Thanks will review and test locally, thanks for this!

stephenandrews commented 2 years ago

Open questions: Stoic collections has the following properties but DAB doesn't, what should we default to in these cases?

   comaddress ? 
   commission ?
   route ?

We will pull from a second API (entrepot specific) to pull these in - these are entrepot specific/marketplace variables, but soon will be stored in-canister

stephenandrews commented 2 years ago

Tried to run locally and got the following, please advise:

./src/components/SendNFTForm.js
Module not found: Can't resolve '@dfinity/principal' in 'C:\Users\Stephen Andrews\Documents\GitHub\stoic-wallet\src\components'
tarek-eg commented 2 years ago

/components/SendNFTForm.js

Can you delete your node_modules and reinstall again? I suspect there's a conflict in @dfinity/principal versions.

tarek-eg commented 2 years ago

Probably you will also need to follow this in order to install DAB https://docs.dab.ooo/nft-list/getting-started/#0-preparing-your-environment

To pull and install from @Psychedelic via the NPM CLI, you'll need:

A Github account
A Github personal access token
The personal access token with the correct scopes, repo and read:packages to be granted access to the GitHub Package Registry.
Authentication via npm login, using your Github email for the username and the personal access token as your password:

Once you have those ready, run:
npm login --registry=https://npm.pkg.github.com --scope=@Psychedelic
stephenandrews commented 2 years ago

Merged, built and tested locally - some issues:

  1. NFTs are displaying the wrong image - seems to be the same image for all NFTs of the same collection. Clicking the image does go to the right image though
  2. For some reason it is showing the wrong NFT count - I'm seeing the same number of NFTs for every wallet, which is wrong
tarek-eg commented 2 years ago

Merged, built and tested locally - some issues:

  1. NFTs are displaying the wrong image - seems to be the same image for all NFTs of the same collection. Clicking the image does go to the right image though
  2. For some reason it is showing the wrong NFT count - I'm seeing the same number of NFTs for every wallet, which is wrong

For the first issue this happens because DAB NFTS can be of types other than image, in order to get the correct image we will have to make a http request for each image to figure out how to display. @letmejustputthishere has a opened a PR that is targeting the same issue.

We can merge his PR after merging this one or I can integrate his PR in this one what do you think.

For the second issue how can we debug this? Can you give more details on how to reproduce this? Is it different wallets with different identity or is it the same wallet with different account (sub account)?

tarek-eg commented 2 years ago

I have fixed the thumbnail display issue, now it's showing the actual image for each NFT.

tarek-eg commented 2 years ago

Merged, built and tested locally - some issues:

  1. For some reason it is showing the wrong NFT count - I'm seeing the same number of NFTs for every wallet, which is wrong

DAB connects NFTS to the principal id by using sub-account 0, so for each principal you will see the same NFT count for each subaccount, maybe that's what you meant or did you mean a different identity has the same NFT count?