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

Add new mintDrop function and useMintDrop hook #175

Merged
merged 19 commits into from
Sep 22, 2023
Merged

Conversation

ismailToyran
Copy link
Contributor

@ismailToyran ismailToyran commented Aug 11, 2023

Required for https://trello.com/c/FFkLOSNr/947-growth-hacking-drops

  • mintDrop function added to the core
  • useMintDrop hook added to the react

Hook added from https://github.com/liteflow-labs/o-mee/blob/main/hooks/useMintDrop.ts

If namings are good, might need to add to the documentation as well.

  • Add to the documentation

@ismailToyran ismailToyran requested a review from antho1404 August 11, 2023 09:01
@ismailToyran ismailToyran self-assigned this Aug 11, 2023
@changeset-bot
Copy link

changeset-bot bot commented Aug 11, 2023

🦋 Changeset detected

Latest commit: a5f1e92

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@liteflow/core Minor
@liteflow/react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@antho1404 antho1404 left a comment

Choose a reason for hiding this comment

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

One general feedback on this PR, a mint is a drop event, the goal is to create an asset similar to the mint and lazymint so it feels weird to have it as part of the exchange. I do understand the idea because we can optionally have to pay for the mint but the asset is not created yet thus this is not really an exchange in my opinion.

I would have put this in the asset.mintFromDrop with only the dropId as the chain and collection can be found from the drop

@ismailToyran
Copy link
Contributor Author

ismailToyran commented Aug 24, 2023

One question on this one,
dropId doesn't include chain and collection like assetId. So instead you want me to do another query with dropId to get related chain and collection?

@antho1404
Copy link
Member

Yes you can fetch the drop to get the info we are doing that in some places where we fetch an offer to get the asset

@ismailToyran
Copy link
Contributor Author

Have a look one last time Anthony.
Tested with npm pack.
Everything seems to be working as intended. 🎉

Copy link
Member

@antho1404 antho1404 left a comment

Choose a reason for hiding this comment

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

Except for one part, the rest looks good, I couldn't really test so please let me know when I can test properly and/or how to test properly

@antho1404 antho1404 merged commit 08a9c8a into main Sep 22, 2023
@antho1404 antho1404 deleted the feature/mint-drop branch September 22, 2023 03:11
@github-actions github-actions bot mentioned this pull request Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants