core icon indicating copy to clipboard operation
core copied to clipboard

feat: Implement display nft media

Open tommasini opened this issue 2 years ago • 3 comments

References

relates to: https://github.com/MetaMask/mobile-planning/issues/1166

This PR aims to solve the implementation of display nft media being a user preference. It adds a handling error when the proxy or our third party fails, giving the possibility for the client to show a fallback image instead.

@metamask/preferences-controller

  • BREAKING: changed name of openSeaEnabled state property to displayNftMedia
  • BREAKING: changed name of setOpenSeaEnabled function to setDisplayNftMedia, this function is responsbile to update the value of displayNftMedia state property

@metamask/assets-controller/NftController

  • ADDED: Added error handling when fetch from third parties or the chain fails This property was added in order to register if the fetch from third parties, open sea, the chain, or our proxy fails and we can avoid keep calling to update it, avoiding a loop that way, This error can assume the value of 'Opensea import error', 'URI import error' and 'Both import failed'.

Checklist

  • [ ] I've updated the test suite for new or updated code as appropriate
  • [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate

tommasini avatar Oct 24 '23 11:10 tommasini

Could you update the PR to describe the error scenarios meant to be covered here? It wasn't totally clear from the linked planning issue (which seemed to be about a larger feature, not focused on error handling)

i.e. What is the "display NFT media" feature, and how does this PR relate to it? What problems are these changes meant to solve?

Gudahtt avatar Nov 09 '23 15:11 Gudahtt

Ah, it looks like at least one test is still failing

Gudahtt avatar Nov 24 '23 22:11 Gudahtt

~This PR needs to be recreated on main with the code with this latest commit having the needed for the displayNftMedia feature on this repo.~

Able to revert the commits that were wrong on this PR

tommasini avatar Feb 19 '24 21:02 tommasini