Skip to content
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

feat: marketplace links in NFT details page #111

Merged
merged 34 commits into from
Oct 2, 2023

Conversation

patricio0312rev
Copy link
Contributor

@patricio0312rev patricio0312rev commented Sep 20, 2023

[nfts] Add marketplace links to NFTs

Summary

  • Marketplaces' URLs have been added to NFT details page.
  • New URLs have been added to urls.php.
  • New Marketplaces component has been created, it supports the URL's for nfts and collections.
  • Tests for this new component.
Screen.Recording.2023-09-25.at.11.52.08.mov

Steps to reproduce

  1. Run the app in test_e2e mode. Make sure you have connected a wallet with collections for both ethereum and polygon.
  2. Go to /collections page.
  3. Select any collection and click on any of its NFTs.
  4. You can start clicking on the marketplace icons and see the magic ✨

Checklist

  • I checked my UI changes against the design and there are no notable differences
  • I checked my UI changes for any responsiveness issues
  • I checked my (code) changes for obvious issues, debug statements and commented code
  • I opened a corresponding card on Clickup for any remaining TODOs in my code
  • I added a short description on how to test this PR (if necessary)
  • I added a storybook entry for the component that was added (if necessary)
  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@patricio0312rev patricio0312rev marked this pull request as ready for review September 20, 2023 11:27
@patricio0312rev
Copy link
Contributor Author

patricio0312rev commented Sep 20, 2023

I found an issue regarding collections like ENS domains. While the URL works, there are some marketplaces like Rarible and Blur that will display a message saying that the item has been removed or simply no information has been found. This is an issue we cannot control from our end, however, this works with almost all the other collections.

I think we can add an extra field to our DB to check if we should display or not that marketplace icon, however, this would require us to check each collection and see if they are working correctly.

Another idea I have is to use our blacklist for some collections and just hide the marketplace icons for them.

Attaching proof of this issue:

Screen.Recording.2023-09-20.at.06.28.08.mov

Copy link

@samharperpittam samharperpittam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patricio0312rev - quick on this PR.

The task states " In case there isn't that collection listed there we can assume NFT isn't either so in that case we hide that marketplace from the view."

Though, I'm looking at a collection called hwape_com.

I have all marketplace links
image

However, the collection doesn't exist on OpenSea or LooksRare as far as I can tell.

Opensea
image

Looksrare
image

Is the logic for hiding marketplaces missing?

Other than that the PR looks nice

@samharperpittam samharperpittam marked this pull request as draft September 20, 2023 19:25
@ItsANameToo ItsANameToo added this to the 0.7.0 milestone Sep 21, 2023
@patricio0312rev patricio0312rev marked this pull request as ready for review September 25, 2023 15:19
@patricio0312rev
Copy link
Contributor Author

patricio0312rev commented Sep 25, 2023

#138 depends on this PR

Copy link
Contributor

@ItsANameToo ItsANameToo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important note: this requires to rerun the FetchCollectionsMetaData job for existing collections, otherwise they will not show any marketplace details until the next time it triggers

app/Data/Collections/CollectionDetailData.php Outdated Show resolved Hide resolved
@ItsANameToo ItsANameToo marked this pull request as draft September 26, 2023 10:14
@patricio0312rev patricio0312rev marked this pull request as ready for review September 26, 2023 11:25
alfonsobries added a commit that referenced this pull request Sep 29, 2023
@ItsANameToo
Copy link
Contributor

can you have a look at the conflicts @patricio0312rev

@ItsANameToo ItsANameToo merged commit 55c34ef into develop Oct 2, 2023
16 checks passed
@ItsANameToo ItsANameToo deleted the feat/marketplace-links-to-nfts branch October 2, 2023 15:28
@ItsANameToo ItsANameToo restored the feat/marketplace-links-to-nfts branch October 2, 2023 15:36
@ItsANameToo ItsANameToo deleted the feat/marketplace-links-to-nfts branch October 2, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants