feat: massmint flow refactor and redesign#816
Merged
Conversation
|
This PR was not deployed automatically as @Jarsen136 does not have access to the Railway project. In order to get automatic PR deploys, please add @Jarsen136 to your workspace on Railway. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved onboarding pages/components and replaced the Mass Mint experience with a five-step wizard: new Stepper UI, upload step component, template utilities, parser hardening, navbar routing change to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MassMint as MassMint.vue
participant Stepper as Stepper.vue
participant Upload as MassMintUploadStep.vue
participant Utils as utils.ts
User->>MassMint: Open /massmint
MassMint->>Stepper: render(currentStep, stepConfig)
User->>MassMint: select collection/settings
User->>Stepper: click step 2
Stepper->>MassMint: stepClick(2)
MassMint->>MassMint: goToStep(2) (canNavigateTo)
User->>Upload: upload / reorder media
Upload->>MassMint: mediaZipLoaded / mediaZipReordered
MassMint->>MassMint: convertNftsToMap, update NFTs state
User->>MassMint: Next to Metadata
MassMint->>Utils: generateTemplateContent(format, nfts)
Utils-->>User: download template
User->>MassMint: review / apply shared description
MassMint->>MassMint: calculate deposits & totals
User->>MassMint: confirm Mint
MassMint-->>User: mint result/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying app with
|
| Latest commit: |
d9ffb25
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://534b275e.app-bzd.pages.dev |
| Branch Preview URL: | https://feat-mass-mint-new-flow.app-bzd.pages.dev |
Collaborator
|
so, we wont have the studio? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
app/components/massmint/utils.ts (1)
1-1: Use~/alias for app-local imports.Please switch this new import to the project alias convention.
As per coding guidelines,
**/*.{ts,tsx,vue}: Use~/alias for app imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/utils.ts` at line 1, Replace the local relative import for NFT in utils.ts to use the project alias: change the import that reads "import type { NFT } from './types'" to use the app alias (e.g., import type { NFT } from '~/components/massmint/types') so the module NFT referenced in utils.ts uses the ~/ app-local import convention.app/components/common/Stepper.vue (1)
44-70: Add explicit accessibility attributes for step buttons.On small screens the visible label is hidden, so buttons may announce as generic numeric/icon controls. Add
aria-labelandaria-currentfor better screen-reader navigation.Suggested fix
<button + type="button" + :aria-label="step.label" + :aria-current="isCurrent(index) ? 'step' : undefined" class="flex items-center gap-2 px-3 py-1.5 rounded-full text-sm font-medium transition-all duration-200" :class="{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/common/Stepper.vue` around lines 44 - 70, Update the step button in Stepper.vue to include explicit accessibility attributes: add an aria-label computed from step.label (fallback to the numeric label like `${index + 1}`) so screen readers announce each step, set aria-current="step" when isCurrent(index) is true, and set aria-disabled="true" when the existing :disabled condition (!isClickable(index) && !isCurrent(index)) is true; keep existing visual logic (isCurrent, isCompleted, isReachable, isClickable) and existing click handler handleClick(index) unchanged so behavior remains the same.app/components/massmint/MassMint.vue (1)
3-15: Use~/alias for newly added app-local imports.Please align these new imports with the repo alias convention.
As per coding guidelines,
**/*.{ts,tsx,vue}: Use~/alias for app imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 3 - 15, The new app-local imports (EditPanel, MassMintUploadStep, OverviewTable, UploadDescription, and the utils exports convertNftsToMap/generateTemplateContent) use relative paths; update them to use the repo’s "~/..." alias convention consistent with other imports (e.g., CommonStepper) so they become app-alias imports instead of "./" relative imports—replace the import sources for EditPanel, MassMintUploadStep, OverviewTable, UploadDescription, and the utils module accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 209-212: onMediaZipLoaded currently ignores the areAllFilesValid
flag and always sets mediaLoaded true; update onMediaZipLoaded to respect
data.areAllFilesValid by only setting mediaLoaded.value = true when
data.areAllFilesValid is true (otherwise set mediaLoaded.value = false and
handle the invalid case such as preventing progression or setting an error
state), keeping the existing assignment to NFTS via convertNftsToMap; refer to
symbols onMediaZipLoaded, data.areAllFilesValid, mediaLoaded, NFTS, and
convertNftsToMap to locate and implement the change.
- Around line 330-347: The function applySharedDescriptionToAll is overwriting
every NFT.description; change its loop to only set description when the target
NFT has no existing description (e.g., check if nft.description is falsy or
nft.description.trim() === ''), update next[nft.id] only for those items, and
compute the applied count accordingly before calling successMessage; keep using
sharedDescription.value, NFTS.value, warningMessage and successMessage as in the
current implementation.
In `@app/components/massmint/MassMintUploadStep.vue`:
- Line 63: The emit call for 'mediaZipReordered' currently hardcodes
areAllFilesValid=true and ignores real validation state; change it to compute
areAllFilesValid from the reorderedMap contents (e.g., inspect
Object.values(reorderedMap) and ensure every file entry meets the validation
flag or criteria used elsewhere) and pass that computed boolean in the emit
payload instead of true so downstream steps receive the correct validation
status.
- Around line 117-138: Add accessible names to the icon-only buttons by adding
aria-label attributes: for the view toggles update the buttons that call
setFileViewMode('grid') and setFileViewMode('list') to include aria-label="Grid
view" and aria-label="List view" respectively (and consider aria-pressed bound
to fileViewMode === 'grid' or 'list' for stateful indication); likewise add
aria-labels to the delete icon buttons (the delete control handlers) with
descriptive text like aria-label="Delete file" or aria-label="Delete selected
files". Ensure labels are concise and match the action names used in the
component.
- Line 14: Change the emit payload type for the mediaZipLoaded event from any[]
to NFT[] so it matches the parallel mediaZipReordered emit and the parent
component MassMint.vue; locate the tuple signature for (e: 'mediaZipLoaded',
payload: { validFiles: any[], areAllFilesValid: boolean }): void in
MassMintUploadStep.vue and replace any[] with NFT[] to ensure the emitted
validFiles conforms to the NFT type expected by the parent.
In `@app/components/massmint/utils.ts`:
- Around line 26-35: The CSV builder in rows (nfts.map) concatenates raw fields
(file, name, description, attributes, price) and uses attributesCsvExample
directly, which will break when values contain commas or quotes; add a small
CSV-escape helper (e.g., escapeCsvField) and use it for every field before
composing each row (apply to file, the "Art `#1`" name, description text,
attributesCsvExample, and price) so fields with commas or quotes are wrapped and
internal quotes doubled, then join the escaped fields with commas in the rows
generation logic.
- Around line 58-60: convertNftsToMap currently uses .map and .reduce with
object spreads causing the accumulator to be copied on every iteration; change
it to build the map in-place (e.g., use a for loop or forEach) to avoid repeated
spreads: iterate over items, assign id = index+1 to each item and set result[id]
= item (or a shallow-cloned item if immutability is required) and return the
resulting Record; update references to convertNftsToMap accordingly so the
function still returns Record<string, T & { id: number }>.
In `@app/composables/massmint/parsers/parseTxt.ts`:
- Around line 73-75: The JSON.parse call in parseTxt.ts (within the branch
checking fieldName === 'attributes') can throw and abort the import; wrap the
parse in a try/catch around JSON.parse(fieldValue) so a malformed attributes
value does not crash processing — on catch, handle gracefully (e.g., set
entry.attributes to {} or null and optionally record/log the parsing error) and
continue processing the rest of the file; ensure you reference the same
variables (fieldName, fieldValue, entry.attributes) and keep behavior consistent
with other field parsing.
---
Nitpick comments:
In `@app/components/common/Stepper.vue`:
- Around line 44-70: Update the step button in Stepper.vue to include explicit
accessibility attributes: add an aria-label computed from step.label (fallback
to the numeric label like `${index + 1}`) so screen readers announce each step,
set aria-current="step" when isCurrent(index) is true, and set
aria-disabled="true" when the existing :disabled condition (!isClickable(index)
&& !isCurrent(index)) is true; keep existing visual logic (isCurrent,
isCompleted, isReachable, isClickable) and existing click handler
handleClick(index) unchanged so behavior remains the same.
In `@app/components/massmint/MassMint.vue`:
- Around line 3-15: The new app-local imports (EditPanel, MassMintUploadStep,
OverviewTable, UploadDescription, and the utils exports
convertNftsToMap/generateTemplateContent) use relative paths; update them to use
the repo’s "~/..." alias convention consistent with other imports (e.g.,
CommonStepper) so they become app-alias imports instead of "./" relative
imports—replace the import sources for EditPanel, MassMintUploadStep,
OverviewTable, UploadDescription, and the utils module accordingly.
In `@app/components/massmint/utils.ts`:
- Line 1: Replace the local relative import for NFT in utils.ts to use the
project alias: change the import that reads "import type { NFT } from './types'"
to use the app alias (e.g., import type { NFT } from
'~/components/massmint/types') so the module NFT referenced in utils.ts uses the
~/ app-local import convention.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/components/Navbar.vueapp/components/common/Stepper.vueapp/components/massmint/MassMint.vueapp/components/massmint/MassMintUploadStep.vueapp/components/massmint/OnBoardingCard.vueapp/components/massmint/Onboarding.vueapp/components/massmint/index.tsapp/components/massmint/utils.tsapp/composables/massmint/parsers/parseTxt.tsapp/i18n/locales/en.jsonapp/pages/massmint/index.vueapp/pages/massmint/onboarding.vue
💤 Files with no reviewable changes (5)
- app/components/massmint/OnBoardingCard.vue
- app/components/massmint/Onboarding.vue
- app/i18n/locales/en.json
- app/components/massmint/index.ts
- app/pages/massmint/onboarding.vue
Contributor
Author
Yess, when I look into the original MR with the studio in #791, I found that most of the new studio-related pages are not working functionally. For example studio/xx/details and studio/xx/items page only shows part of the collection details and nfts items, but it's unavailable to edit the collection in the end, neither make any batch action to the nft item. It does not seem to make sense to me, to build a stutio path but only include massmint feature. while we have already a seperate massmint page for it. I would make it focus on the new mass minting flow in the current MR. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/components/massmint/MassMint.vue (1)
327-348:⚠️ Potential issue | 🟠 Major
applySharedDescriptionToAllstill overwrites existing descriptions.Line 344 applies the shared text to all items, which contradicts the stated behavior and can destroy edited metadata.
🐛 Proposed fix
function applySharedDescriptionToAll() { const text = sharedDescription.value?.trim() if (!text) { warningMessage('Enter a shared description first.') return } const nfts = Object.values(NFTS.value) as NFT[] if (nfts.length === 0) { return } const next: { [id: string]: NFT } = { ...NFTS.value } - nfts.forEach((nft) => { - next[nft.id] = { ...nft, description: text } - }) + let applied = 0 + nfts.forEach((nft) => { + if (!nft.description?.trim()) { + next[nft.id] = { ...nft, description: text } + applied++ + } + }) NFTS.value = next - successMessage(`Applied shared description to ${nfts.length} item(s).`) + successMessage(`Applied shared description to ${applied} item(s).`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 327 - 348, The applySharedDescriptionToAll function currently overwrites every NFT.description; update it so it only sets description on items that lack an individual description (e.g., where nft.description is falsy or empty after trimming). In function applySharedDescriptionToAll, compute the shared text from sharedDescription.value, filter Object.values(NFTS.value) to only include NFTs with no existing description, apply the text only to those filtered NFTs when building next (leave others unchanged), and update the successMessage to report the number of items actually modified; keep references to NFTS, NFT and sharedDescription unchanged.
🧹 Nitpick comments (1)
app/components/massmint/MassMint.vue (1)
79-109: Consolidate step metadata to one source of truth.
stepsandstepConfigduplicate flow structure, which can desync navigation and UI labels over time.♻️ Proposed refactor
-const steps = [ - { id: 1, title: 'Collection', description: 'Choose chain and collection' }, - { id: 2, title: 'Upload', description: 'Upload media files' }, - { id: 3, title: 'Metadata', description: 'Prepare and upload metadata file' }, - { id: 4, title: 'Review', description: 'Check items and estimated cost' }, - { id: 5, title: 'Mint', description: 'Submit your mass mint transaction' }, -] as const - const stepConfig: StepConfig[] = [ { label: 'Collection', icon: 'i-heroicons-folder' }, { label: 'Upload', icon: 'i-heroicons-arrow-up-tray' }, { label: 'Metadata', icon: 'i-heroicons-document-text' }, { label: 'Review', icon: 'i-heroicons-clipboard-document-list' }, { label: 'Mint', icon: 'i-heroicons-sparkles' }, ] @@ - for (let i = 0; i < steps.length; i++) { + for (let i = 0; i < stepConfig.length; i++) { @@ - if (stepId < 1 || stepId > steps.length) { + if (stepId < 1 || stepId > stepConfig.length) {Also applies to: 146-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 79 - 109, The steps and stepConfig arrays duplicate the same flow metadata causing potential desync; consolidate into a single source (e.g., merge into one const like steps or stepDefinitions) and derive any UI-only structures from it instead of maintaining stepConfig separately. Update all usages: replace references to stepConfig with derived values from the unified steps (labels/icons), ensure stepperCompletedSteps and stepperMaxStepReached use the unified array (steps or stepDefinitions), and keep canNavigateTo/currentStep logic unchanged but referencing the single source; remove the redundant stepConfig declaration to avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 675-677: The hardcoded "DOT" in the Deposit info paragraph in
MassMint.vue should be made dynamic: replace the literal "DOT" in the <p> text
with a token symbol derived from the current chain (e.g. a computed prop or
store getter such as tokenSymbol or chainTokenSymbol) or use chain-agnostic
wording. Update the template where the paragraph is rendered and add a computed
property (or use an existing prop/store value like chain, currentChain, or
api.chainInfo) that returns the correct symbol for Kusama/Polkadot/others, then
interpolate that symbol into the paragraph (or reword to "network token") so the
message shows the proper token per chain (ensure you update any tests or i18n
keys if present).
---
Duplicate comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 327-348: The applySharedDescriptionToAll function currently
overwrites every NFT.description; update it so it only sets description on items
that lack an individual description (e.g., where nft.description is falsy or
empty after trimming). In function applySharedDescriptionToAll, compute the
shared text from sharedDescription.value, filter Object.values(NFTS.value) to
only include NFTs with no existing description, apply the text only to those
filtered NFTs when building next (leave others unchanged), and update the
successMessage to report the number of items actually modified; keep references
to NFTS, NFT and sharedDescription unchanged.
---
Nitpick comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 79-109: The steps and stepConfig arrays duplicate the same flow
metadata causing potential desync; consolidate into a single source (e.g., merge
into one const like steps or stepDefinitions) and derive any UI-only structures
from it instead of maintaining stepConfig separately. Update all usages: replace
references to stepConfig with derived values from the unified steps
(labels/icons), ensure stepperCompletedSteps and stepperMaxStepReached use the
unified array (steps or stepDefinitions), and keep canNavigateTo/currentStep
logic unchanged but referencing the single source; remove the redundant
stepConfig declaration to avoid future drift.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/components/common/Stepper.vueapp/components/massmint/MassMint.vueapp/components/massmint/MassMintUploadStep.vueapp/components/massmint/utils.tsapp/composables/massmint/parsers/parseTxt.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/massmint/utils.ts
- app/components/massmint/MassMintUploadStep.vue
This reverts commit 822af45.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/components/massmint/MassMint.vue (2)
675-677:⚠️ Potential issue | 🟡 MinorChain-specific fee text is hardcoded to DOT.
Line 676 displays "~0.001 DOT" which is incorrect when minting on Kusama or other chains. Make this token dynamic or use chain-agnostic wording.
Minimal safe wording fix
- Deposits are refundable when items are burned. Network fees (~0.001 DOT) are charged separately per transaction. + Deposits are refundable when items are burned. Network fees are charged separately per transaction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 675 - 677, The displayed fee text in the template paragraph is hardcoded to "DOT"; update it to use the current chain's native token symbol instead of a literal. Replace the static "~0.001 DOT" string in MassMint.vue with a dynamic value derived from your component state (e.g., a prop/computed like selectedChain.symbol, chain.nativeSymbol, or network.tokenSymbol) or fall back to chain-agnostic wording (e.g., "~0.001 (network token)") if the symbol is unavailable; ensure the template binds to that computed/prop so the correct symbol shows for Kusama, Polkadot, etc.
327-348:⚠️ Potential issue | 🟠 Major
applySharedDescriptionToAlloverwrites existing item descriptions.The comment on line 327 states the function "applies only to items without an individual description," but the implementation applies to every NFT, which can destroy user-edited metadata.
Suggested fix
function applySharedDescriptionToAll() { const text = sharedDescription.value?.trim() if (!text) { warningMessage('Enter a shared description first.') return } const nfts = Object.values(NFTS.value) as NFT[] if (nfts.length === 0) { return } const next: { [id: string]: NFT } = { ...NFTS.value } + let applied = 0 nfts.forEach((nft) => { - next[nft.id] = { ...nft, description: text } + if (!nft.description?.trim()) { + next[nft.id] = { ...nft, description: text } + applied++ + } }) NFTS.value = next - successMessage(`Applied shared description to ${nfts.length} item(s).`) + successMessage(`Applied shared description to ${applied} item(s).`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 327 - 348, applySharedDescriptionToAll currently overwrites every NFT.description but should only set the sharedDescription on items that lack a per-item description; update applySharedDescriptionToAll to iterate NFTs (from NFTS.value), check each nft.description (trimmed) and only replace when it's empty/null, build the next map updating only those ids, track how many were changed, then assign NFTS.value and call successMessage with the count of affected items; reference sharedDescription, applySharedDescriptionToAll, NFTS, NFT, warningMessage, and successMessage when making the change.
🧹 Nitpick comments (2)
app/components/massmint/MassMint.vue (2)
80-94: Consider consolidating step definitions.There are two separate step configuration arrays:
steps(lines 80-86) andstepConfig(lines 88-94). Since both represent the same five steps, consider merging them into a single source of truth to avoid potential drift.Example consolidation
const steps = [ { id: 1, title: 'Collection', description: 'Choose chain and collection', label: 'Collection', icon: 'i-heroicons-folder' }, { id: 2, title: 'Upload', description: 'Upload media files', label: 'Upload', icon: 'i-heroicons-arrow-up-tray' }, // ... etc ] as const const stepConfig = computed(() => steps.map(s => ({ label: s.label, icon: s.icon })))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 80 - 94, The two arrays steps and stepConfig are duplicate representations of the same wizard steps; consolidate into one source of truth by adding the label and icon fields to the existing steps constant (the steps array) and remove the separate stepConfig constant, then derive the StepConfig shape where needed by mapping steps (e.g. create stepConfig via steps.map or a computed mapping) so all consumers use the single steps definition (update any references to stepConfig to use the derived mapping and keep the StepConfig type usage intact).
36-44: Incomplete transaction calculation.The comment on line 38 indicates this is temporary placeholder logic. The
txvariable is set toitemDepositbut should include actual transaction fee calculation for accurate cost estimation.Would you like me to help implement the proper transaction fee calculation, or should this be tracked as a follow-up issue?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 36 - 44, The tx placeholder in the computed total (inside the total computed using NFTS, attributeDeposit, itemDeposit, metadataDeposit) must be replaced with a real transaction fee calculation: implement or call an estimator (e.g., estimateTxFee or calculateTxFeeForBatch) and compute tx as the estimated fee for minting the current NFT or batch (for example: per-NFT fee * numberOfNFTs or an estimator that uses NFT size/attributes to return a fee), then use that value in the summed return expression instead of itemDeposit.value; update the total computed to call this estimator (or loop to accumulate per-NFT fees) so totals.reduce includes accurate transaction fees.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 675-677: The displayed fee text in the template paragraph is
hardcoded to "DOT"; update it to use the current chain's native token symbol
instead of a literal. Replace the static "~0.001 DOT" string in MassMint.vue
with a dynamic value derived from your component state (e.g., a prop/computed
like selectedChain.symbol, chain.nativeSymbol, or network.tokenSymbol) or fall
back to chain-agnostic wording (e.g., "~0.001 (network token)") if the symbol is
unavailable; ensure the template binds to that computed/prop so the correct
symbol shows for Kusama, Polkadot, etc.
- Around line 327-348: applySharedDescriptionToAll currently overwrites every
NFT.description but should only set the sharedDescription on items that lack a
per-item description; update applySharedDescriptionToAll to iterate NFTs (from
NFTS.value), check each nft.description (trimmed) and only replace when it's
empty/null, build the next map updating only those ids, track how many were
changed, then assign NFTS.value and call successMessage with the count of
affected items; reference sharedDescription, applySharedDescriptionToAll, NFTS,
NFT, warningMessage, and successMessage when making the change.
---
Nitpick comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 80-94: The two arrays steps and stepConfig are duplicate
representations of the same wizard steps; consolidate into one source of truth
by adding the label and icon fields to the existing steps constant (the steps
array) and remove the separate stepConfig constant, then derive the StepConfig
shape where needed by mapping steps (e.g. create stepConfig via steps.map or a
computed mapping) so all consumers use the single steps definition (update any
references to stepConfig to use the derived mapping and keep the StepConfig type
usage intact).
- Around line 36-44: The tx placeholder in the computed total (inside the total
computed using NFTS, attributeDeposit, itemDeposit, metadataDeposit) must be
replaced with a real transaction fee calculation: implement or call an estimator
(e.g., estimateTxFee or calculateTxFeeForBatch) and compute tx as the estimated
fee for minting the current NFT or batch (for example: per-NFT fee *
numberOfNFTs or an estimator that uses NFT size/attributes to return a fee),
then use that value in the summed return expression instead of
itemDeposit.value; update the total computed to call this estimator (or loop to
accumulate per-NFT fees) so totals.reduce includes accurate transaction fees.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/components/common/Stepper.vueapp/components/massmint/MassMint.vueapp/components/massmint/MassMintUploadStep.vueapp/components/massmint/utils.tsapp/composables/massmint/parsers/parseTxt.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/massmint/MassMintUploadStep.vue
- app/composables/massmint/parsers/parseTxt.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/massmint/MassMint.vue (1)
36-44:⚠️ Potential issue | 🟠 MajorItem deposit appears to be double-counted in
totalcalculation.Line 38 sets
tx = itemDeposit.valueand line 40 adds bothitemDeposit.valueandtxto each NFT's cost. This effectively counts the item deposit twice per NFT. The comment "tmp add real tx calculation" suggests this is a placeholder, but it currently inflates the estimated cost.Suggested fix (if tx fee should be separate)
const total = computed(() => { const totals = Object.values(NFTS.value).map((nft) => { - const tx = itemDeposit.value // tmp add real tx calcualtion - - return ((nft.attributes?.length || 0) * attributeDeposit.value) + itemDeposit.value + metadataDeposit.value + tx + return ((nft.attributes?.length || 0) * attributeDeposit.value) + itemDeposit.value + metadataDeposit.value }) - return totals.reduce((acc, curr) => acc + curr, 0) + return totals.reduce((acc, curr) => acc + curr, 0) + transactionFee.value })Or remove
txentirely if a real transaction fee estimate isn't available yet.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 36 - 44, The computed total is double-counting the item deposit because tx is set to itemDeposit.value and then you add both itemDeposit.value and tx per NFT; update the total computation in the total computed so that tx is either removed or set to a proper separate fee (e.g., 0 until real tx calculation is implemented) and only add itemDeposit.value once per NFT — locate the total computed, the tx assignment and the return expression that uses NFTS.value, attributeDeposit.value, metadataDeposit.value, and itemDeposit.value to make this change.
♻️ Duplicate comments (1)
app/components/massmint/MassMint.vue (1)
331-349:⚠️ Potential issue | 🟠 Major
applySharedDescriptionToAlloverwrites existing item descriptions.The function applies the shared description to every NFT, including those that already have an individual description. This can destroy user-edited metadata unexpectedly. The comment on line 328 states it "applies only to items without an individual description" but the implementation doesn't respect that.
Suggested fix
function applySharedDescriptionToAll() { const text = sharedDescription.value?.trim() if (!text) { warningMessage('Enter a shared description first.') return } const nfts = Object.values(NFTS.value) as NFT[] if (nfts.length === 0) { return } const next: { [id: string]: NFT } = { ...NFTS.value } + let applied = 0 nfts.forEach((nft) => { - next[nft.id] = { ...nft, description: text } + if (!nft.description?.trim()) { + next[nft.id] = { ...nft, description: text } + applied++ + } }) NFTS.value = next - successMessage(`Applied shared description to ${nfts.length} item(s).`) + successMessage(`Applied shared description to ${applied} item(s).`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 331 - 349, applySharedDescriptionToAll currently overwrites every NFT description; change it to only set description when an NFT has no individual description (empty/null/whitespace). Inside applySharedDescriptionToAll, iterate over Object.values(NFTS.value) and for each nft check whether nft.description is falsy after trimming (e.g., !nft.description || nft.description.trim() === ''), and only then copy the NFT with description: text into the next map; leave others unchanged. Also update the successMessage to reflect the number of items actually updated (count only NFTs that received the shared description).
🧹 Nitpick comments (1)
app/components/massmint/MassMint.vue (1)
215-215: Avoidanyin function parameter type.The
entriesparameter usesRecord<string, any>which bypasses type safety. Consider defining a proper type for the parsed entry structure.Suggested fix
-function onDescriptionLoaded(entries: Record<string, any>) { +function onDescriptionLoaded(entries: Record<string, Partial<Entry>>) {Where
Entryis imported from~/composables/massmint/parsers/commonor a similar type is defined locally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` at line 215, The parameter type for onDescriptionLoaded currently uses Record<string, any>; change it to a strongly-typed structure (e.g. Record<string, Entry> or a specific interface) by importing the Entry type from ~/composables/massmint/parsers/common (or define a local Entry/interface matching the parsed entry shape), update the function signature to onDescriptionLoaded(entries: Record<string, Entry>) and adjust any property access inside onDescriptionLoaded to match the Entry fields so TypeScript type checks correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 36-44: The computed total is double-counting the item deposit
because tx is set to itemDeposit.value and then you add both itemDeposit.value
and tx per NFT; update the total computation in the total computed so that tx is
either removed or set to a proper separate fee (e.g., 0 until real tx
calculation is implemented) and only add itemDeposit.value once per NFT — locate
the total computed, the tx assignment and the return expression that uses
NFTS.value, attributeDeposit.value, metadataDeposit.value, and itemDeposit.value
to make this change.
---
Duplicate comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 331-349: applySharedDescriptionToAll currently overwrites every
NFT description; change it to only set description when an NFT has no individual
description (empty/null/whitespace). Inside applySharedDescriptionToAll, iterate
over Object.values(NFTS.value) and for each nft check whether nft.description is
falsy after trimming (e.g., !nft.description || nft.description.trim() === ''),
and only then copy the NFT with description: text into the next map; leave
others unchanged. Also update the successMessage to reflect the number of items
actually updated (count only NFTs that received the shared description).
---
Nitpick comments:
In `@app/components/massmint/MassMint.vue`:
- Line 215: The parameter type for onDescriptionLoaded currently uses
Record<string, any>; change it to a strongly-typed structure (e.g.
Record<string, Entry> or a specific interface) by importing the Entry type from
~/composables/massmint/parsers/common (or define a local Entry/interface
matching the parsed entry shape), update the function signature to
onDescriptionLoaded(entries: Record<string, Entry>) and adjust any property
access inside onDescriptionLoaded to match the Entry fields so TypeScript type
checks correctly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/common/Stepper.vueapp/components/massmint/MassMint.vueapp/components/massmint/MassMintUploadStep.vueapp/composables/massmint/parsers/parseTxt.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Implements a redesigned multi-step Mass Mint flow (collection → upload → metadata → review → mint), replacing the old onboarding-based entrypoint and adding template generation + richer upload/review UI.
Changes:
- Removed Mass Mint onboarding page/components and routed Mass Mint directly to
/massmint. - Refactored
MassMint.vueinto a step-based wizard with a shared stepper component and a dedicated upload step UI. - Added client-side metadata template generation (JSON/CSV/TXT) and improved TXT parsing for attributes.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/pages/massmint/onboarding.vue | Removes onboarding page entrypoint. |
| app/pages/massmint/index.vue | Updates Mass Mint page layout and removes onboarding navigation wiring. |
| app/i18n/locales/en.json | Removes onboarding-related i18n strings. |
| app/composables/massmint/parsers/parseTxt.ts | Adds safer TXT attribute parsing (now needs adjustment for empty/invalid values). |
| app/components/massmint/utils.ts | Adds template generation + convertNftsToMap utility. |
| app/components/massmint/index.ts | Removes onboarding exports from massmint barrel. |
| app/components/massmint/Onboarding.vue | Deletes onboarding component. |
| app/components/massmint/OnBoardingCard.vue | Deletes onboarding card component. |
| app/components/massmint/MassMintUploadStep.vue | New upload step with grid/list view + drag reorder. |
| app/components/massmint/MassMint.vue | Major refactor into 5-step wizard with cost breakdown + template download + shared description. |
| app/components/common/Stepper.vue | Introduces reusable stepper component. |
| app/components/Navbar.vue | Routes “Mass Mint” directly to /massmint. |
Comments suppressed due to low confidence (1)
app/composables/massmint/parsers/parseTxt.ts:84
attributeshandling makes TXT entries invalid when the field is empty or JSON parsing fails. WithfieldValue === '', the code falls through toentry[fieldName] = fieldValueand setsattributesto an empty string; on parse failure it sets{}, which failsisValidEntry(expects an array). Consider treating empty/invalid attributes asundefined/[](and not setting the field at all when empty) so the rest of the entry can still be considered valid.
else if (fieldName === 'attributes' && fieldValue) {
try {
entry.attributes = JSON.parse(fieldValue)
}
catch {
console.error('Invalid attributes JSON in TXT metadata block')
entry.attributes = {}
}
}
else {
entry[fieldName] = fieldValue
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/massmint/MassMint.vue (1)
36-44: Placeholder transaction fee calculation needs follow-up.The
txvariable is set toitemDeposit.value(noted by the "tmp" comment), which causesitemDepositto be counted twice per NFT. While the UI breakdown and totals are internally consistent, this placeholder should be replaced with actual transaction fee estimation before release.Also, typo in comment: "calcualtion" → "calculation".
Suggested clarification
const total = computed(() => { const totals = Object.values(NFTS.value).map((nft) => { - const tx = itemDeposit.value // tmp add real tx calcualtion + // TODO: Replace with actual transaction fee calculation + const estimatedTxFee = itemDeposit.value - return ((nft.attributes?.length || 0) * attributeDeposit.value) + itemDeposit.value + metadataDeposit.value + tx + return ((nft.attributes?.length || 0) * attributeDeposit.value) + itemDeposit.value + metadataDeposit.value + estimatedTxFee }) return totals.reduce((acc, curr) => acc + curr, 0) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/massmint/MassMint.vue` around lines 36 - 44, The computed total currently uses a placeholder tx set to itemDeposit.value which double-counts itemDeposit for each NFT; update the total computation in the total computed property so tx is derived from a real transaction-fee estimation function or prop (e.g., call a new getTransactionFee(nft) or use a provided transactionFee.value) instead of reusing itemDeposit.value, and correct the inline comment "tmp add real tx calcualtion" to "tmp: add real tx calculation" (or remove the tmp comment once implemented); ensure the returned sum uses ((nft.attributes?.length || 0) * attributeDeposit.value) + itemDeposit.value + metadataDeposit.value + tx for each nft without duplicating itemDeposit elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 210-213: onMediaZipLoaded currently sets mediaLoaded.value = true
even when data.areAllFilesValid is false or data.validFiles is empty; update
onMediaZipLoaded to only set mediaLoaded.value = true when data.areAllFilesValid
is true and data.validFiles has at least one entry (after using convertNftsToMap
to populate NFTS.value), or alternatively add a guard to canGoToMetadataStep
that checks NFTS has at least one NFT and data.areAllFilesValid is true before
allowing the metadata step; reference functions/vars: onMediaZipLoaded,
convertNftsToMap, NFTS, mediaLoaded, areAllFilesValid, and canGoToMetadataStep
to locate and apply the guard.
---
Nitpick comments:
In `@app/components/massmint/MassMint.vue`:
- Around line 36-44: The computed total currently uses a placeholder tx set to
itemDeposit.value which double-counts itemDeposit for each NFT; update the total
computation in the total computed property so tx is derived from a real
transaction-fee estimation function or prop (e.g., call a new
getTransactionFee(nft) or use a provided transactionFee.value) instead of
reusing itemDeposit.value, and correct the inline comment "tmp add real tx
calcualtion" to "tmp: add real tx calculation" (or remove the tmp comment once
implemented); ensure the returned sum uses ((nft.attributes?.length || 0) *
attributeDeposit.value) + itemDeposit.value + metadataDeposit.value + tx for
each nft without duplicating itemDeposit elsewhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Inspired by @exezbcz 's original design and new minting flow presented from MR #791.
I created this MR to implement the new mass minting flow with all available functionality.
Feel free to test it on multiple chain includingthe testing net. @chaotic-art/internal @chaotic-art/dev
closes chaotic-art/planning#17
closes chaotic-art/planning#18
some screenshot:
Summary by CodeRabbit
New Features
Bug Fixes
Chores