fix: skip confirm step on back navigation (#356)#447
Open
Emmanex01 wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR updates navigation in mint and burn pages to use Next.js ChangesMulti-step Flow Navigation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/burn/page.tsx (1)
136-204:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Malformed code structure with duplicates and unreachable code.
The
elsebranch handling external wallet connections has severe structural issues:
- Lines 153-159: Code after
resolve(address)inside the callback is unreachable- Lines 160-187: Duplicate wallet modal logic (repeats lines 137-151)
- Lines 189-196: Misplaced
try-catch-finallyblock that conflicts with the outertryat line 108- Lines 197-204: Duplicate
submitBurnRedeemSingleClientcall outside anytryblockThis appears to be a merge conflict or copy-paste error that will cause compilation errors and incorrect execution.
🔧 Suggested fix for the external wallet path
The external wallet path should be:
} else { if (!kit) { - throw new Error( - "Your wallet secret isn't available on this device and the wallet connector isn't ready yet. Please wait a moment and retry.", - ); + throw new Error("Wallet connector not ready"); } const address = await new Promise<string>((resolve, reject) => { kit .openModal({ onWalletSelected: async (selectedOption: { id: string }) => { try { kit.setWallet(selectedOption.id); const { address } = await kit.getAddress(); resolve(address); } catch (err) { reject(err); } }, }) .catch(reject); }); - const submit = await submitBurnRedeemSingleClient({ - userAddress: stellarAddress, - amountAcbu: values.acbuAmount, - currency: values.currency, - userSecret: secret, - }); - burnTxHash = submit.transactionHash; - } else { - if (!kit) throw new Error("Wallet connector not ready"); - const address = await new Promise<string>((resolve, reject) => { - kit - .openModal({ - onWalletSelected: async (selectedOption: { id: string }) => { - try { - kit.setWallet(selectedOption.id); - const { address } = await kit.getAddress(); - resolve(address); - } catch (err) { - reject(err); - } - }, - }) - .catch(reject); - }); - if (stellarAddress && address !== stellarAddress) { - throw new Error("Connected wallet doesn't match linked account"); + if (stellarAddress && address !== stellarAddress) { + throw new Error("Connected wallet doesn't match linked account"); } const submit = await submitBurnRedeemSingleClient({ - userAddress: stellarAddress, - amountAcbu: values.acbuAmount, - currency: values.currency, - external: { kit, address }, + userAddress: stellarAddress, + amountAcbu: values.acbuAmount, + currency: values.currency, + external: { kit, address }, }); burnTxHash = submit.transactionHash; - } - - const res = await burnApi.burnAcbu(values.acbuAmount, values.currency, recipientAccount, opts, burnTxHash); - setTxId(res.transaction_id); - reset({ ...values, acbuAmount: "" }); - } catch (e) { - setApiError(e); - } finally { - setLoading(false); - } - const submit = await submitBurnRedeemSingleClient({ - userAddress: stellarAddress, - amountAcbu: values.acbuAmount, - currency: values.currency, - external: { kit, address }, - }); - burnTxHash = submit.transactionHash; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/burn/page.tsx` around lines 136 - 204, The external-wallet branch contains duplicated and misplaced code (unreachable statements after resolve in the kit.openModal onWalletSelected handler, a repeated kit.openModal block, an extraneous try/catch/finally, and a stray submitBurnRedeemSingleClient call) — clean this up by removing the duplicated kit.openModal block and the duplicate submitBurnRedeemSingleClient outside the try, move the submitBurnRedeemSingleClient call into the correct branch after resolving address, ensure no code exists after resolve(address) inside onWalletSelected, keep one consistent flow that calls submitBurnRedeemSingleClient({ userAddress, amountAcbu: values.acbuAmount, currency: values.currency, external: { kit, address } }) for external wallets, then call burnApi.burnAcbu(...), setTxId(res.transaction_id), reset(...), and maintain the existing outer try/catch/finally that uses setApiError and setLoading.
🧹 Nitpick comments (1)
app/burn/page.tsx (1)
223-223: 💤 Low valueRemove duplicate declaration.
This line duplicates the
currencydeclaration from line 101. The variable is already available in the component scope.- const currency = form.watch("currency"); - return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/burn/page.tsx` at line 223, The variable currency is declared twice; remove the redundant declaration at the later occurrence (the line calling form.watch("currency") around where currency is re-declared) so the component uses the single currency variable already defined in the component scope; keep the original declaration (the one at the top ~line 101) and delete the duplicate form.watch("currency") declaration to avoid shadowing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/burn/page.tsx`:
- Around line 213-215: The success message never appears because after calling
setTxId(...) the code immediately calls router.replace('/mint'), so the success
dialog (which reads txId) cannot render; update the flow to show the success UI
before navigating: introduce a local dialog state (e.g., isBurnSuccessOpen) or
reuse the existing mint success dialog pattern, open the success dialog when
setTxId(...) runs, call form.reset(...) as needed, and only call
router.replace('/mint') after the user closes the dialog (or after a short
timeout if you prefer a delay), ensuring the success component can read txId and
render properly instead of being immediately redirected.
---
Outside diff comments:
In `@app/burn/page.tsx`:
- Around line 136-204: The external-wallet branch contains duplicated and
misplaced code (unreachable statements after resolve in the kit.openModal
onWalletSelected handler, a repeated kit.openModal block, an extraneous
try/catch/finally, and a stray submitBurnRedeemSingleClient call) — clean this
up by removing the duplicated kit.openModal block and the duplicate
submitBurnRedeemSingleClient outside the try, move the
submitBurnRedeemSingleClient call into the correct branch after resolving
address, ensure no code exists after resolve(address) inside onWalletSelected,
keep one consistent flow that calls submitBurnRedeemSingleClient({ userAddress,
amountAcbu: values.acbuAmount, currency: values.currency, external: { kit,
address } }) for external wallets, then call burnApi.burnAcbu(...),
setTxId(res.transaction_id), reset(...), and maintain the existing outer
try/catch/finally that uses setApiError and setLoading.
---
Nitpick comments:
In `@app/burn/page.tsx`:
- Line 223: The variable currency is declared twice; remove the redundant
declaration at the later occurrence (the line calling form.watch("currency")
around where currency is re-declared) so the component uses the single currency
variable already defined in the component scope; keep the original declaration
(the one at the top ~line 101) and delete the duplicate form.watch("currency")
declaration to avoid shadowing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db34bc6e-c8c5-4550-89f8-f5a4253447ba
📒 Files selected for processing (2)
app/burn/page.tsxapp/mint/page.tsx
Comment on lines
213
to
+215
| setTxId(res.transaction_id); | ||
| form.reset({ ...values, acbuAmount: "" }); | ||
| router.replace('/mint'); |
Contributor
There was a problem hiding this comment.
Major: Success message not visible to user.
After setting the transaction ID (line 213), the code immediately navigates away to /mint (line 215). This prevents the success message component (lines 247-254) from rendering, leaving users without confirmation of their burn transaction.
This creates an inconsistent experience compared to the mint flow, which displays a success dialog before returning to the input state.
Consider one of these approaches:
- Add a delay before navigation to let the success message display briefly
- Show a success dialog similar to the mint flow (requires adding dialog state)
- Pass txId as a query parameter to
/mintand show the success message there - Remove the automatic redirect and add a manual "Back to Mint" button
⏱️ Example: Add delay before navigation
setTxId(res.transaction_id);
form.reset({ ...values, acbuAmount: "" });
- router.replace('/mint');
+ // Show success message briefly before navigating back
+ setTimeout(() => {
+ router.replace('/mint');
+ }, 3000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| setTxId(res.transaction_id); | |
| form.reset({ ...values, acbuAmount: "" }); | |
| router.replace('/mint'); | |
| setTxId(res.transaction_id); | |
| form.reset({ ...values, acbuAmount: "" }); | |
| // Show success message briefly before navigating back | |
| setTimeout(() => { | |
| router.replace('/mint'); | |
| }, 3000); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/burn/page.tsx` around lines 213 - 215, The success message never appears
because after calling setTxId(...) the code immediately calls
router.replace('/mint'), so the success dialog (which reads txId) cannot render;
update the flow to show the success UI before navigating: introduce a local
dialog state (e.g., isBurnSuccessOpen) or reuse the existing mint success dialog
pattern, open the success dialog when setTxId(...) runs, call form.reset(...) as
needed, and only call router.replace('/mint') after the user closes the dialog
(or after a short timeout if you prefer a delay), ensuring the success component
can read txId and render properly instead of being immediately redirected.
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
1 participant
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.
Summary by CodeRabbit
Bug Fixes
Refactor