kodadot / nft-gallery

Generative Art Marketplace
https://kodadot.xyz
MIT License
605 stars 347 forks source link

feat: confirmation modal for mint #7241

Closed xiiiAtCn closed 8 months ago

xiiiAtCn commented 9 months ago

PR Type

Context

Did your issue had any of the "$" label on it?

Community participation

Screenshot πŸ“Έ

Copilot Summary

πŸ€– Generated by Copilot at ff3ebfe

This pull request adds confirmation modals for minting collections and tokens in the create section of the NFT gallery. It also adds new components and translation keys to display the minting information and fees. The files affected are CreateCollection.vue, CreateToken.vue, en.json, ConfirmMintItem.vue, MintConfirmModal.vue, and PriceItem.vue.

πŸ€– Generated by Copilot at ff3ebfe

Sing, O Muse, of the crafty code that the heroes wrought To mint the shining tokens of their skill and art, With ConfirmMintItem.vue and MintConfirmModal.vue they brought The fee and image of each NFT to the user's heart.

kodabot commented 9 months ago

SUCCESS @xiiiAtCn PR for issue #7169 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

netlify[bot] commented 9 months ago

Deploy Preview for koda-canary ready!

Name Link
Latest commit aa62e8add85a7631c10d9ec5c0193d20475c9a95
Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/6523e6d20ad3860008bc4784
Deploy Preview https://deploy-preview-7241--koda-canary.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

reviewpad[bot] commented 9 months ago

AI-Generated Summary: This pull request introduces a significant number of changes centered around the creation of new Vue components and the modification of existing ones. These new components ConfirmMintItem.vue, CreateCollection.vue, MintConfirmModal.vue, PriceItem.vue are related to minting process improvements, and handling the modal layout of certain user actions. Changes made in CreateToken.vue and locales/en.json provide additional functionalities and translations for different operations related to minting. Additionally, the patch shows the creation of multiple Vue templates with associated scripts and styles. The changes made in CreateCollection.vue and CreateToken.vue integrate the new components into the process of creating a collection and token respectively. The important modification made is enabling the confirmation of mint in the components. The MintConfirmModal.vue and PriceItem.vue components get updated with new functionalities and UI improvements. The update includes adding information about currency used in respective components, adding clickable chevrons for better UI/UX, handling the display of fees on the basis of certain conditions, and adding information icons for explaining certain functionalities.

exezbcz commented 9 months ago

hello, will leave the majority of the review for devs. Few bits from me:

xiiiAtCn commented 9 months ago
  • make the whole text with the dropdown clickable, plus the cursor pointer
  • tooltip should popup when you hover over the text as well - making the area bigger

okay. Work almost done. Understanding these fees cost much time for a newbie

exezbcz commented 9 months ago
  • make the whole text with the dropdown clickable, plus the cursor pointer
  • tooltip should popup when you hover over the text as well - making the area bigger

okay. Work almost done. Understanding these fees cost much time for a newbie

Yeah but at least he finally knows what is he paying and what is required

xiiiAtCn commented 9 months ago

Yeah but at least he finally knows what is he paying and what is required

Can't agree more. Cause I'm the oneπŸ˜ƒ

xiiiAtCn commented 9 months ago

@exezbcz hey, it' ready to test. I find some suspicious conditions but can't confirm:

  1. cover costs setting not work for creating collection
  2. on chain basilisk, network fee is much higher than I expect
exezbcz commented 8 months ago

@prury couuld you have a look at mint, collection + chains please πŸ‘€

prury commented 8 months ago

Let me break it down and see if I'm correct.

Collection Creation(Chain Fee): Ahp - 10 DOT Ahk - 0.1 KSM

NFT Creation(Chain Fee):

Ahp - 0.1 DOT Ahk - 0.001 KSM

Here in Ahp, for example, Existential deposit is being shown together with the amount needed to create a collection image

Edit: I'm still editing, submitted not to lose the info

exezbcz commented 8 months ago

Existential deposit is something that the user needs to have on the wallet/chain in order for its account not to be wiped,

yes, but that is not the same as the existential deposit displayed in the modal.

Collection Creation(Chain Fee):

prury commented 8 months ago

Existential deposit is something that the user needs to have on the wallet/chain in order for its account not to be wiped,

yes, but that is not the same as the existential deposit displayed in the modal.

Collection Creation(Chain Fee):

* this is the existential deposit for collection creation - its also refundable, when you burn your collection you get it back

ah, i see, better rename it to Collection/NFT existential deposit then, wdyt?

anyway, if the fee is 10 DOT, is still showing 10.3 DOT

exezbcz commented 8 months ago

anyway, if the fee is 10 DOT, is still showing 10.3 DOT

weird

ah, i see, better rename it to Collection/NFT existential deposit then, wdyt?

lets make it collection existential deposit, there is no deposit for nft afaik

prury commented 8 months ago

anyway, if the fee is 10 DOT, is still showing 10.3 DOT

weird

hmmm, its probably the metadata fee https://wiki.polkadot.network/docs/learn-guides-assets-create

prury commented 8 months ago

@xiiiAtCn can you change the existential deposit to Collection existential deposit?

xiiiAtCn commented 8 months ago

can you change the existential deposit to Collection existential deposit?

have fixed it.

there is no deposit for nft afaik

In Basilisk and AssetHub, NFTs need metadataDeposit and itemDeposit. You can check them in composables/useDeposit @prury

exezbcz commented 8 months ago

so is this one ready for dev review?

@kodadot/code-review-guild

daiagi commented 8 months ago

1.when fees are expanded, modal top is under the navbar

image

2. odd things happen below desktop size:

image

3. mobile:

image

4. USD value doesn't seem correct

image

vikiival commented 8 months ago

Collection Creation(Chain Fee):

It is not a chain fee, its unlockable deposit (in german pfand)

When you burn you collection the deposit is returned

daiagi commented 8 months ago

When you burn you collection the deposit is returned

btw @vikiival I was looking both in docs and in Polkadot UI extrinsic for a way to burn collection and didn't find any do you know how to?

vikiival commented 8 months ago

burn a collection and didn't find any do you know how to?

For ah*

Screenshot 2023-09-25 at 13 53 05
prury commented 8 months ago

Collection Creation(Chain Fee):

It is not a chain fee, its unlockable deposit (in german pfand)

When you burn you collection the deposit is returned

got it, ty!

daiagi commented 8 months ago

Header still flips color under 1024px

image

xiiiAtCn commented 8 months ago

Header still flips color under 1024px

image

this color change is followed by https://github.com/kodadot/nft-gallery/issues/6986. Shall we align them?

daiagi commented 8 months ago

this color change is followed by https://github.com/kodadot/nft-gallery/issues/6986. Shall we align them?

if I read correctly. the issue you mentioned is about burger menu and sidebars, not "center screen" modals like we have here

also, looking at the issue #7169 design, I don't see the header flipping color

so looks to me that the header shouldn't change it's color below 1024px

@exezbcz

exezbcz commented 8 months ago

so looks to me that the header shouldn't change it's color below 1024px

correct, the color change is only for mobile/tablet devices in burger menu

xiiiAtCn commented 8 months ago

so looks to me that the header shouldn't change it's color below 1024px

correct, the color change is only for mobile/tablet devices in burger menu

kk. I will solve it.

prury commented 8 months ago

Modal on desktop still white: image

test related: playwright here will fail since it is looking for the not enough funds button and it is now inside the modal

yangwao commented 8 months ago

hey @xiiiAtCn I'm keen to raise bounty for this little if you finish it in upcoming 48 hours to be mergeable :)

yangwao commented 8 months ago

Thank you for your work @xiiiAtCn, sending at least some fair payout pay 50 usd

Seems @xiiiAtCn is MIA Can you @preschian please take over, resolve conflicts and merge it? Thanks

yangwao commented 8 months ago

😍 Perfect, I’ve sent the payout πŸ’΅ $50 @ 3.86 USD/DOT ~ 12.953 $DOT πŸ§— 12jXHf7yYF4wTRjDPbwFB24V1nVRPRkuU2S8Ke5ZWTfvyBGW πŸ”— 0x384530c0fa22e20ea256fae4c39ea437711a58b88233a9fb107a52eb4eb8f3ad

πŸͺ… Let’s grab another issue and get rewarded! πŸͺ„ github.com/kodadot/nft-gallery/issues

codeclimate[bot] commented 8 months ago

Code Climate has analyzed commit aa62e8ad and detected 0 issues on this pull request.

View more on Code Climate.

prury commented 8 months ago

On KusamaHub it says that i don't have enough to mint, even though i have: image

same for Kusama, i believe its related to mint settings, since deactivating koda mint costs lets me proceed with the transaction

netlify[bot] commented 8 months ago

Deploy Preview for nuxt-kodadot ready!

Name Link
Latest commit 5531657011bf26860a083b7236d8bc3977a1989f
Latest deploy log https://app.netlify.com/sites/nuxt-kodadot/deploys/65251fafdab2f7000891d599
Deploy Preview https://deploy-preview-7241--nuxt-kodadot.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

preschian commented 8 months ago

On KusamaHub it says that i don't have enough to mint, even though i have: same for Kusama, i believe its related to mint settings, since deactivating koda mint costs lets me proceed with the transaction

updated

exezbcz commented 8 months ago

Should it not be "on" Kusama hub?

image

otherwise works for me! thanks for completing

sonarcloud[bot] commented 8 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information