core
core copied to clipboard
Fix/keep nft when transaction fail
Keep nft when transaction fail -> when the NFT is being transacted, was needed some changes
- Was added two new props to the collectible interface in CollectibleController, for when a collectible is being transacted we have can know about it and change the UI according to that.
Description
Itemize the changes you have made into the categories below
-
CHANGED:
- Changed the Collectible Props to have more two optional props
-
ADDED:
- It was added 3 functions:
- One to get the collectible transaction status, it will return if the collectible is being transacted and the id of that transaction
- One to update the properties transactionId and isTransacting of the collectible
- One to reset the transaction status of the collectible that updates the prop isTransacting for false and the transactionId to an empty string. It returns true if have success, false if have not
Checklist
- [x] Tests are included if applicable
- [x] Any added code is fully documented
Issue
Resolves #https://github.com/MetaMask/metamask-mobile/issues/2784
It will have an impact on this PR of metamask mobile
Pinging @adonesky1 in case the Extension has the same issue as Mobile.
Issue: The app removes the collectible when the transaction is submitted instead of waiting for it to be confirmed
Pinging @adonesky1 in case the Extension has the same issue as Mobile. Issue: The app removes the collectible when the transaction is submitted instead of waiting for it to be confirmed
Sorry coming to this late... wondering if you had considered using the method checkAndUpdateSingleCollectibleOwnershipStatus. In the extension the way we update ownership of a collectible after an attempted send can be found here. Basically when we get a notification from the transaction controller that a transaction status has updated we check if it was a transferFrom token method and get the tokenId and contractAddress and send them to checkAndUpdateSingleCollectibleOwnershipStatus to see if the user still owns the collectible after the transaction and update the state accordingly.
@tommasini @gantunesr
Hey, @adonesky1 Thank you so much for the review and for bringing these methods up. I was thinking about how to implement those methods into our issue, but the methods that are added in this PR are to make be able to make some changes in the UI when the NFT is being transacted, so I would have always to update the collectible with the transactionId. To remove the collectible we use the method removeCollectible when the tx is confirmed
Here is one small demo of the final result of what we accomplish https://user-images.githubusercontent.com/46944231/167123888-30dc816d-d8d5-494b-9208-0c71c48f43ff.mov
@tommasini @Gudahtt I'm sorry I don't understand why we need to associate a transactionId with the collectible in the CollectiblesController state and these additional methods to achieve your goals here. Could we jump on a call and discuss sometime next week?
@tommasini added some comments/change requests! Let me know if you have any questions!