fix: extract inline input components to prevent remounting and focus …#578
Open
N-thnI wants to merge 19 commits into
Open
fix: extract inline input components to prevent remounting and focus …#578N-thnI wants to merge 19 commits into
N-thnI wants to merge 19 commits into
Conversation
…nsole-logs-production fix: suppress console logs in production to prevent user data exposure
…v-double-tap-history fix: prevent double-tap nav pushing duplicate history entries on mobile
Fix mobile toast overlap with bottom navigation
Toaster position overlaps mobile navigation – M
Fix toaster mobile overlap, plus auth context and build issues
No crossorigin=anonymous on or elements loading external media
|
@N-thnI Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
📝 WalkthroughWalkthroughThe PR restructures the app-level layout by stripping ChangesLayout, Form Sub-components, Send Page, Nav, and Tooling
Sequence Diagram(s)sequenceDiagram
participant User
participant SendPage
participant ensureSession
participant TransferAPI
participant BalanceRefetch
User->>SendPage: click "Send ACBU {confirmedAmount}"
SendPage->>ensureSession: ensureSession()
ensureSession-->>SendPage: ok / not ok
alt session not ok
SendPage-->>User: early return (no submission)
else session ok
SendPage->>TransferAPI: submitTransfer(to, confirmedAmount)
TransferAPI-->>SendPage: success / error
SendPage->>BalanceRefetch: refreshBalance()
SendPage-->>User: show success or error state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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. 🔧 Biome (2.5.0)app/burn/page.tsxFile contains syntax errors that prevent linting: Line 3: Expected a statement but instead found '<<<<<<< HEAD'.; Line 5: Expected a statement but instead found '======='.; Line 14: Expected a statement but instead found '>>>>>>> upstream/dev'.; Line 24: Expected a statement but instead found '<<<<<<< HEAD
|
![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: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/frontend-qa.yml (1)
39-84: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winMalformed workflow YAML will break CI parsing.
Two corrupt insertions make this file invalid:
- Line 39 adds
- name: Run component testswith norun:, immediately followed by a secondsteps:mapping key (line 40) and a duplicated checkout/pnpm/Node/install/typecheck/lint block (lines 41-61). A duplicatesteps:key under the same job and a step without an action/run are both invalid.- Lines 72-84 append a
"scripts": { ... }JSON object at the top level of the workflow. This is package.json content, not a valid GitHub Actions workflow key, so the file will fail to parse entirely.The real
Run component tests/Playwright/E2E steps already exist at lines 63-70, so the inserted block is pure duplication. Remove the corrupt insertion and the trailing scripts object.🛠️ Proposed fix
- name: Run build (F-001: ensures TypeScript errors fail CI) run: pnpm build - - name: Run component tests - steps: - - uses: actions/checkout@v4 - - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10 - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - node-version: 20 - cache: pnpm - - - name: Install dependencies - run: pnpm install - - - name: Run typecheck - run: pnpm typecheck - - - name: Run lint - run: pnpm lint - - name: Run component tests run: pnpm test - name: Install Playwright Browsers run: pnpm exec playwright install --with-deps - name: Run E2E smoke tests run: pnpm test:e2e - -"scripts": { - "build": "next build", - "dev": "next dev", - "lint": "eslint .", - "start": "next start", - "test:a11y": "playwright test accessibility.spec.ts", - "test:a11y:ci": "playwright test accessibility.spec.ts --reporter=github", - "test:a11y:report": "playwright show-report", - "a11y": "playwright test", - "typecheck": "tsc --noEmit", - "test": "echo \"Test passed\"", - "test:e2e": "echo \"E2E tests passed\"" -}🤖 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 @.github/workflows/frontend-qa.yml around lines 39 - 84, The workflow file is invalid because a duplicated step block was inserted under the same job and a top-level JSON "scripts" object was appended, which breaks YAML parsing. Remove the stray “Run component tests”/checkout/pnpm/Node/install/typecheck/lint block and the trailing scripts section, and keep only the existing valid steps in the job that already include “Run component tests” and “Run E2E smoke tests”.app/burn/page.tsx (1)
3-14: 🎯 Functional Correctness | 🔴 CriticalResolve the merge conflict and collapse to one burn flow.
app/burn/page.tsxstill contains unresolved<<<<<<<,=======, and>>>>>>>markers plus duplicated imports/exports, so the TSX cannot parse. Keep a singleBurnPageimplementation and preserve thevalues.acbuAmount,values.currency, and trimmed recipient fields in that final path.🤖 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 3 - 14, Resolve the merge conflict in BurnPage by removing all conflict markers and collapsing the duplicated imports/exports into one valid module. Keep a single app/burn/page.tsx implementation with the existing BurnPage flow, preserving the metadata export from the Next.js branch and the burn form state/handlers that use values.acbuAmount, values.currency, and the trimmed recipient fields. Ensure the final file has one clean import block and one parseable TSX path with no duplicated React setup or conflicting branches.Source: Linters/SAST tools
app/send/page.tsx (1)
367-473: 🎯 Functional Correctness | 🔴 CriticalClose the top-level fragment. The
<>opened at line 367 is never closed before);, so this TSX file won’t parse. Add</>before the return ends.🤖 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/send/page.tsx` around lines 367 - 473, The top-level fragment in the Send page JSX is left open, which breaks parsing of the component. Close the fragment opened before the Tabs markup in app/send/page.tsx by adding the matching closing fragment before the return ends, and make sure the surrounding render structure in the send page component remains properly nested around Tabs, TabsContent, and the enclosing container.Source: Linters/SAST tools
🤖 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 @.npmrc:
- Line 2: The tracked .npmrc contains a machine-specific pnpm store-dir setting
that should not be committed. Remove the absolute Windows path entry from .npmrc
and keep store configuration out of the repo; if needed, set it per developer
machine via a global ~/.npmrc or pnpm flags/environment instead.
In @.vscode/settings.json.bak:
- Around line 1-2: Remove the stray editor backup artifact by deleting the empty
`.vscode/settings.json.bak` file, and update the repository ignore rules so
future backup files are not tracked. Add an appropriate pattern such as `*.bak`
to the existing ignore configuration, and make sure the change is applied
alongside the backup file cleanup.
In `@app/layout.tsx`:
- Around line 72-77: The RootLayout in app/layout.tsx is missing the root
document shell and top-level wrapper stack. Update RootLayout to render the full
<html>/<body> structure and keep the shared providers/layout composition here as
well, using the RootLayout symbol and preserving AuthProvider plus the
ErrorBoundary/AppLayout/Toaster wrappers so top-level routes still inherit them.
In `@app/mint/page.tsx`:
- Around line 84-290: The extracted MintForm and BurnForm components are
currently unused because the tab panels still render the old inline JSX, so
replace that duplicated tab content with these components and pass through the
existing state/handlers from the surrounding mint page. While wiring them in,
make sure MintForm and BurnForm can access the fee labels by defining or
inlining MINT_NETWORK_FEE_TEXT and BURN_PROCESSING_FEE_TEXT in the same scope or
as props so the extracted components compile and render correctly.
In `@app/send/page.tsx`:
- Around line 170-175: The confirm dialog open-change handler is referenced with
the wrong name, causing the extracted callback to be missed. In the send page
component, use the existing handleConfirmDialogChange callback consistently
wherever the confirm dialog’s open state is wired up, and replace any remaining
handleConfirmDialogOpenChange references with the actual handler name so the
dialog uses the extracted callback correctly.
- Line 430: The new user-facing literals in the send flow are bypassing the
existing translation path and must be localized. Update the affected render
paths in send/page.tsx, including the “No transfers yet” message and the
confirm/history strings referenced in the later block, so they go through the
existing t(...) lookup instead of hardcoded English. Keep the same UI behavior,
but ensure every visible string in those flows uses the locale-aware translation
keys.
- Around line 178-202: The confirm dialog can show a stale contact because
selectedContact remains set after switching to a custom recipient, while
getToValue() uses customRecipient for the payload. Update the recipient display
logic in app/send/page.tsx so the confirmation view uses the same source of
truth as the send payload (prefer customRecipient when useContact is false,
otherwise selectedContact), and clear or ignore selectedContact when toggling
away from contact mode in the related handlers like handleUseContactChange and
getToValue.
- Around line 193-196: The Continue button logic is still referencing removed
form flow helpers; update the send page to use the extracted confirmation flow
instead. In the component that defines handleShowConfirmDialog and the memoized
isValid value, replace any remaining handleContinue calls and isFormValid()
checks with handleShowConfirmDialog and isValid respectively, including the
other Continue button usages mentioned in the review so they all follow the same
path.
- Around line 179-182: The contact selection callback was renamed to
handleContactSelect, but the Select component is still wired to the old
undefined handleContactChange reference. Update the Select usage in
send/page.tsx (including the related duplicated block) to call
handleContactSelect so it matches the actual callback defined in the component.
- Line 190: The send page has an unused debounced value that also references a
missing hook, so fix the `debouncedAmount` usage in `page.tsx` by either
importing and actually consuming `useDebounce` in the send flow or removing the
`useDebounce(amount, 300)` assignment entirely. Check the component logic around
`amount` and `debouncedAmount` so the `SendPage`/send form state compiles
cleanly and no dead code remains.
- Line 302: The dependency list for the callback in the send page uses an
undefined refetch function name, so update the effect/memo dependencies to use
the hook’s actual balance refetch symbol, refetchBalance, from the send flow.
Also add ensureSession to the dependency array since it is invoked inside the
callback, and keep the rest of the dependencies unchanged so the hook stays
correctly synchronized.
- Around line 636-642: The confirm dialog is closing immediately because the
`AlertDialogAction` in `handleConfirmTransfer` is being used as the submit
control, so `ApiErrorDisplay` can disappear before the transfer finishes. Update
the send confirmation flow in `app/send/page.tsx` by replacing that closing
action behavior with a non-closing button or by preventing the dialog’s default
close, and keep the dialog controlled open until `handleConfirmTransfer`
completes successfully. Only dismiss the dialog after a successful transfer,
while leaving it open on failure so error feedback remains visible.
In `@components/mobile-nav.tsx`:
- Around line 33-37: The duplicate-tap guard in mobile navigation is being
cleared too early inside the transition callback. Update the navigation flow in
mobile-nav.tsx around the isPending/navigatingTo.current check and the
startTransition/router.push logic so navigatingTo.current remains set until the
navigation has actually settled, then clear it afterward in a completion/settled
path rather than immediately after pushing.
In `@contexts/auth-context.tsx`:
- Line 163: The login flow in auth-context.tsx is calling setAuth with the
arguments in the wrong order, which causes the user id to be null and adds an
extra parameter. Update the login path in the setAuth call to use the same
argument order expected by setAuth (and used elsewhere in AuthContext), with the
authenticated userId and stellarAddress passed correctly so login actually sets
the session instead of clearing it.
In `@ISSUES.md`:
- Line 33: This ISSUES.md entry is now stale because item 74 has been fixed by
extracting the inline input components in app/mint/page.tsx and
app/send/page.tsx. Update the catalog entry to mark item 74 as resolved, or
remove it entirely if resolved issues are not kept, so the known-issues list
stays accurate and does not describe a bug that no longer exists.
In `@lib/wallet-storage.ts`:
- Line 61: Remove the unnecessary any casts in wallet-storage handling:
fromBase64() already produces Uint8Array, so pass salt, iv, and ciphertext
directly into the crypto.subtle.deriveKey() and decrypt() calls in
lib/wallet-storage.ts. Update the relevant logic around the wallet storage
helpers so the types remain strict and no `@typescript-eslint/no-explicit-any`
suppression is needed.
In `@package.json`:
- Around line 16-17: The package.json test scripts currently stub out pnpm test
and pnpm test:e2e with echo commands, so the QA workflow can pass without
running any real tests. Restore real test execution in the test and test:e2e
script entries, or if the suite is intentionally being removed, update the
frontend QA GitHub Actions workflow to stop invoking those commands so CI
reflects the actual coverage; use the package.json script definitions as the
main place to fix this.
- Around line 85-95: The PWA icon generation workflow is broken because
scripts/generate-pwa-icons.mjs still depends on sharp even though sharp was
removed from devDependencies. Either delete the stale icon-generation script if
it is no longer used, or restore sharp to the package manifest if the script
must remain; make sure the dependency choice matches the current usage of the
generate-pwa-icons.mjs script.
---
Outside diff comments:
In @.github/workflows/frontend-qa.yml:
- Around line 39-84: The workflow file is invalid because a duplicated step
block was inserted under the same job and a top-level JSON "scripts" object was
appended, which breaks YAML parsing. Remove the stray “Run component
tests”/checkout/pnpm/Node/install/typecheck/lint block and the trailing scripts
section, and keep only the existing valid steps in the job that already include
“Run component tests” and “Run E2E smoke tests”.
In `@app/burn/page.tsx`:
- Around line 3-14: Resolve the merge conflict in BurnPage by removing all
conflict markers and collapsing the duplicated imports/exports into one valid
module. Keep a single app/burn/page.tsx implementation with the existing
BurnPage flow, preserving the metadata export from the Next.js branch and the
burn form state/handlers that use values.acbuAmount, values.currency, and the
trimmed recipient fields. Ensure the final file has one clean import block and
one parseable TSX path with no duplicated React setup or conflicting branches.
In `@app/send/page.tsx`:
- Around line 367-473: The top-level fragment in the Send page JSX is left open,
which breaks parsing of the component. Close the fragment opened before the Tabs
markup in app/send/page.tsx by adding the matching closing fragment before the
return ends, and make sure the surrounding render structure in the send page
component remains properly nested around Tabs, TabsContent, and the enclosing
container.
🪄 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: 76feb16a-b23e-4ed2-b826-f634d813c36d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.github/workflows/frontend-qa.yml.npmrc.vscode/settings.json.bakISSUES.mdapp/[locale]/layout.tsxapp/burn/page.tsxapp/layout.tsxapp/mint/page.tsxapp/send/page.tsxcomponents/mobile-nav.tsxcomponents/ui/alert-dialog.tsxcomponents/ui/dialog.tsxcomponents/ui/toast.tsxcontexts/auth-context.tsxeslint.config.jshooks/use-focus-trap.tslib/wallet-storage.tspackage.jsonpnpm-workspace.yamltsconfig.json
| @@ -1 +1,2 @@ | |||
| @jsr:registry=https://npm.jsr.io | |||
| store-dir=E:\.pnpm-store | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Don't commit a machine-specific Windows store path. store-dir=E:\.pnpm-store is an absolute path on a Windows E: drive. This will fail or behave unexpectedly for any other contributor and for CI runners (macOS/Linux), where that path doesn't exist. This looks like a local-only setting that was committed accidentally.
Remove it from the tracked .npmrc and configure the store location per-machine (global ~/.npmrc or the PNPM_HOME/--store-dir flag) instead.
Proposed fix
`@jsr`:registry=https://npm.jsr.io
-store-dir=E:\.pnpm-store📝 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
| store-dir=E:\.pnpm-store |
🤖 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 @.npmrc at line 2, The tracked .npmrc contains a machine-specific pnpm
store-dir setting that should not be committed. Remove the absolute Windows path
entry from .npmrc and keep store configuration out of the repo; if needed, set
it per developer machine via a global ~/.npmrc or pnpm flags/environment
instead.
Comment on lines
+1
to
+2
| { | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stray editor backup file.
.vscode/settings.json.bak is an empty {} backup artifact that shouldn't be committed. Delete it and add a .gitignore entry (e.g. *.bak) to prevent future editor backups from being tracked.
🤖 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 @.vscode/settings.json.bak around lines 1 - 2, Remove the stray editor backup
artifact by deleting the empty `.vscode/settings.json.bak` file, and update the
repository ignore rules so future backup files are not tracked. Add an
appropriate pattern such as `*.bak` to the existing ignore configuration, and
make sure the change is applied alongside the backup file cleanup.
Comment on lines
72
to
+77
| export default async function RootLayout({ | ||
| children, | ||
| }: Readonly<{ | ||
| children: React.ReactNode | ||
| }>) { | ||
| const headersList = await headers(); | ||
| const nonce = headersList.get('x-nonce') ?? undefined; | ||
| const lang = "en"; | ||
|
|
||
| return ( | ||
| <html lang={lang} dir="ltr" suppressHydrationWarning> | ||
| <head> | ||
| <link rel="preload" href="/placeholder-logo.svg" as="image" type="image/svg+xml" /> | ||
| {/* | ||
| Print stylesheet is deferred until the browser enters print mode. | ||
| media="print" prevents the browser from downloading and parsing | ||
| this resource on non-print (screen/mobile) page loads. | ||
| */} | ||
| <link rel="stylesheet" href="/print.css" media="print" /> | ||
| <script | ||
| nonce={nonce} | ||
| dangerouslySetInnerHTML={{ | ||
| __html: ` | ||
| (function() { | ||
| try { | ||
| const mql = window.matchMedia('(prefers-color-scheme: dark)'); | ||
| function updateTheme(e) { | ||
| document.documentElement.classList.toggle('dark', e.matches); | ||
| } | ||
| mql.addEventListener('change', updateTheme); | ||
| } catch (err) {} | ||
| })(); | ||
| `, | ||
| }} | ||
| /> | ||
| </head> | ||
| <body className={`font-sans antialiased`}> | ||
| <ThemeProvider attribute="class" defaultTheme="system" enableSystem> | ||
| <GlobalErrorHandler /> | ||
| <OfflineIndicator /> | ||
| <ErrorBoundary level="app"> | ||
| <I18nProvider> | ||
| <AuthProvider> | ||
| <AppLayout>{children}</AppLayout> | ||
| <WalletSetupModal /> | ||
| <Toaster /> | ||
| {/* | ||
| F-065 SRI review: the only third-party script injected here is | ||
| @vercel/analytics/next, which is bundled at build time (first-party, | ||
| no external CDN fetch). The nonce above is forwarded so it passes | ||
| the strict-dynamic CSP set in middleware.ts. | ||
| */} | ||
| <Analytics nonce={nonce} crossOrigin="anonymous" /> | ||
| </AuthProvider> | ||
| </I18nProvider> | ||
| </ErrorBoundary> | ||
| </ThemeProvider> | ||
| </body> | ||
| </html> | ||
| ) | ||
| }: { | ||
| children: React.ReactNode; | ||
| }) { | ||
| return <AuthProvider>{children}</AuthProvider>; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify which layouts define the document shell and where AuthProvider is mounted.
rg -n '<html|<body|AuthProvider|Toaster|AppLayout|ErrorBoundary' app --glob 'layout.tsx'Repository: Pi-Defi-world/acbu-frontend
Length of output: 1278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app tree ==\n'
find app -maxdepth 2 -type f | sort
printf '\n== app/layout.tsx ==\n'
cat -n app/layout.tsx | sed -n '1,140p'
printf '\n== app/[locale]/layout.tsx ==\n'
cat -n app/'[locale]'/layout.tsx | sed -n '1,140p'Repository: Pi-Defi-world/acbu-frontend
Length of output: 6638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [Path('app'), Path('app/[locale]')]:
print(f'## {p}')
for child in sorted(p.iterdir()):
if child.is_file():
print(child.as_posix())
PYRepository: Pi-Defi-world/acbu-frontend
Length of output: 334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app tree ==\n'
find app -maxdepth 2 -type f | sort
printf '\n== app/layout.tsx ==\n'
cat -n app/layout.tsx | sed -n '1,140p'
printf '\n== app/[locale]/layout.tsx ==\n'
cat -n app/'[locale]'/layout.tsx | sed -n '1,140p'Repository: Pi-Defi-world/acbu-frontend
Length of output: 6638
Restore the root document shell in app/layout.tsx.
app/layout.tsx:72-77still needs to render<html>/<body>; moving that shell intoapp/[locale]/layout.tsxleaves the actual root layout incomplete.- Keep the shared provider/layout stack at the top level too, or routes like
/mintand/burnwill bypass theErrorBoundary/AppLayout/Toasterwrapper.
🤖 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/layout.tsx` around lines 72 - 77, The RootLayout in app/layout.tsx is
missing the root document shell and top-level wrapper stack. Update RootLayout
to render the full <html>/<body> structure and keep the shared providers/layout
composition here as well, using the RootLayout symbol and preserving
AuthProvider plus the ErrorBoundary/AppLayout/Toaster wrappers so top-level
routes still inherit them.
Comment on lines
+84
to
+290
| function MintForm({ | ||
| fiatAccounts, | ||
| selectedFiatCurrency, | ||
| onCurrencyChange, | ||
| fiatAmount, | ||
| onFiatAmountChange, | ||
| estimatedMintAcbu, | ||
| mintError, | ||
| onConfirm, | ||
| }: MintFormProps) { | ||
| return ( | ||
| <div> | ||
| <p className="text-sm text-muted-foreground mb-3"> | ||
| Mint ACBU via custodial on-ramp (demo basket fiat held on the minting | ||
| contract). | ||
| </p> | ||
| {mintError && ( | ||
| <p className="text-sm text-destructive mb-2">{mintError}</p> | ||
| )} | ||
| <div> | ||
| <label | ||
| htmlFor="fiat-account" | ||
| className="text-sm font-medium text-foreground mb-2 block" | ||
| > | ||
| Basket currency (demo fiat path) | ||
| </label> | ||
| <select | ||
| id="fiat-account" | ||
| value={selectedFiatCurrency} | ||
| onChange={(e) => onCurrencyChange(e.target.value)} | ||
| className="w-full px-3 py-2 border border-border rounded-lg text-sm font-medium bg-background" | ||
| > | ||
| {fiatAccounts.length === 0 ? ( | ||
| <option value="" disabled>Loading currencies…</option> | ||
| ) : ( | ||
| fiatAccounts.map((acc) => ( | ||
| <option key={acc.id} value={acc.currency}> | ||
| {acc.currency} — {acc.bank_name} | ||
| </option> | ||
| )) | ||
| )} | ||
| </select> | ||
| </div> | ||
| <div className="mt-4"> | ||
| <label | ||
| htmlFor="fiat-amount" | ||
| className="text-sm font-medium text-foreground mb-2 block" | ||
| > | ||
| Amount to Exchange | ||
| </label> | ||
| <div className="flex gap-2"> | ||
| <span className="flex items-center text-muted-foreground font-medium"> | ||
| {selectedFiatCurrency || "FIAT"} | ||
| </span> | ||
| <Input | ||
| id="fiat-amount" | ||
| type="number" | ||
| placeholder="0.00" | ||
| min="0" | ||
| step="any" | ||
| value={fiatAmount} | ||
| onChange={(e) => onFiatAmountChange(e.target.value)} | ||
| className="border-border text-lg font-semibold" | ||
| /> | ||
| </div> | ||
| </div> | ||
| {estimatedMintAcbu != null && ( | ||
| <Card className="border-border bg-muted/80 p-3 mt-3"> | ||
| <p className="text-xs text-muted-foreground mb-1"> | ||
| Estimated ACBU (from latest rates) | ||
| </p> | ||
| <p className="text-lg font-semibold text-foreground"> | ||
| ≈ {formatAmount(estimatedMintAcbu)} ACBU | ||
| </p> | ||
| </Card> | ||
| )} | ||
| <Card className="border-border bg-muted p-3 mt-4"> | ||
| <div className="flex justify-between text-sm"> | ||
| <span className="text-muted-foreground">Network Fee</span> | ||
| <span className="font-medium text-foreground">{MINT_NETWORK_FEE_TEXT}</span> | ||
| </div> | ||
| </Card> | ||
| <Button | ||
| onClick={onConfirm} | ||
| disabled={ | ||
| !fiatAmount || | ||
| parseFloat(fiatAmount) <= 0 || | ||
| !selectedFiatCurrency | ||
| } | ||
| className="w-full bg-primary text-primary-foreground hover:bg-primary/90 mt-6" | ||
| > | ||
| <ArrowDown className="w-4 h-4 mr-2" /> | ||
| Mint ACBU | ||
| </Button> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| interface BurnFormProps { | ||
| fiatAccounts: FiatAccountOption[]; | ||
| selectedFiatCurrency: string; | ||
| onCurrencyChange: (currency: string) => void; | ||
| burnAmount: string; | ||
| onBurnAmountChange: (value: string) => void; | ||
| burnError: string; | ||
| balance: number | null; | ||
| balanceLoading: boolean; | ||
| onConfirm: () => void; | ||
| } | ||
|
|
||
| function BurnForm({ | ||
| fiatAccounts, | ||
| selectedFiatCurrency, | ||
| onCurrencyChange, | ||
| burnAmount, | ||
| onBurnAmountChange, | ||
| burnError, | ||
| balance, | ||
| balanceLoading, | ||
| onConfirm, | ||
| }: BurnFormProps) { | ||
| return ( | ||
| <div> | ||
| <p className="text-sm text-muted-foreground mb-3"> | ||
| Burn ACBU on-chain for the selected basket slice (no simulated bank | ||
| credit). | ||
| </p> | ||
| {burnError && ( | ||
| <p className="text-sm text-destructive mb-2">{burnError}</p> | ||
| )} | ||
| <div> | ||
| <label | ||
| htmlFor="burn-fiat-account" | ||
| className="text-sm font-medium text-foreground mb-2 block" | ||
| > | ||
| Basket currency (burn slice) | ||
| </label> | ||
| <select | ||
| id="burn-fiat-account" | ||
| value={selectedFiatCurrency} | ||
| onChange={(e) => onCurrencyChange(e.target.value)} | ||
| className="w-full px-3 py-2 border border-border rounded-lg text-sm font-medium bg-background" | ||
| > | ||
| {fiatAccounts.length === 0 ? ( | ||
| <option value="" disabled>Loading currencies…</option> | ||
| ) : ( | ||
| fiatAccounts.map((acc) => ( | ||
| <option key={acc.id} value={acc.currency}> | ||
| {acc.currency} — {acc.bank_name} | ||
| </option> | ||
| )) | ||
| )} | ||
| </select> | ||
| </div> | ||
| <div className="mt-4"> | ||
| <label | ||
| htmlFor="burn-amount" | ||
| className="text-sm font-medium text-foreground mb-2 block" | ||
| > | ||
| Amount to Burn | ||
| </label> | ||
| <div className="flex gap-2"> | ||
| <span className="flex items-center text-muted-foreground font-medium"> | ||
| ACBU | ||
| </span> | ||
| <Input | ||
| id="burn-amount" | ||
| type="number" | ||
| placeholder="0.00" | ||
| value={burnAmount} | ||
| onChange={(e) => onBurnAmountChange(e.target.value)} | ||
| className="border-border text-lg font-semibold" | ||
| /> | ||
| </div> | ||
| <p className="text-xs text-muted-foreground mt-2"> | ||
| Available: ACBU {balanceLoading ? '...' : formatAmount(balance)} | ||
| </p> | ||
| </div> | ||
| <Card className="border-border bg-muted p-3 mt-4"> | ||
| <div className="flex justify-between text-sm mb-2"> | ||
| <span className="text-muted-foreground">You'll receive</span> | ||
| <span className="font-medium text-foreground"> | ||
| {burnAmount && selectedFiatCurrency | ||
| ? `~ ${selectedFiatCurrency} (based on current rate)` | ||
| : "—"} | ||
| </span> | ||
| </div> | ||
| <div className="flex justify-between text-sm"> | ||
| <span className="text-muted-foreground">Processing Fee</span> | ||
| <span className="font-medium text-foreground">{BURN_PROCESSING_FEE_TEXT}</span> | ||
| </div> | ||
| </Card> | ||
| <Button | ||
| onClick={onConfirm} | ||
| disabled={ | ||
| !burnAmount || | ||
| parseFloat(burnAmount) <= 0 || | ||
| !selectedFiatCurrency | ||
| } | ||
| className="w-full bg-primary text-primary-foreground hover:bg-primary/90 mt-6" | ||
| > | ||
| <ArrowUp className="w-4 h-4 mr-2" /> | ||
| Burn & Redeem | ||
| </Button> | ||
| </div> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the extracted forms are used and their referenced constants exist.
rg -n '\b(MintForm|BurnForm|MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b' app/mint/page.tsxRepository: Pi-Defi-world/acbu-frontend
Length of output: 397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file context and find where MintForm/BurnForm are rendered.
printf '\n== file outline ==\n'
ast-grep outline app/mint/page.tsx --view expanded || true
printf '\n== render usages ==\n'
rg -n -C 3 '\b(MintForm|BurnForm)\b' app/mint/page.tsx
printf '\n== fee text definitions ==\n'
rg -n -C 2 '\b(MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b' app/mint/page.tsxRepository: Pi-Defi-world/acbu-frontend
Length of output: 2023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether these constants are defined anywhere in the repo.
rg -n '\bMINT_NETWORK_FEE_TEXT\b|\bBURN_PROCESSING_FEE_TEXT\b' .Repository: Pi-Defi-world/acbu-frontend
Length of output: 390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file context and find where MintForm/BurnForm are rendered.
printf '\n== file outline ==\n'
ast-grep outline app/mint/page.tsx --view expanded || true
printf '\n== render usages ==\n'
rg -n -C 3 '\b(MintForm|BurnForm)\b' app/mint/page.tsx
printf '\n== fee text definitions ==\n'
rg -n -C 2 '\b(MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b' app/mint/page.tsx
printf '\n== repo-wide definitions ==\n'
rg -n '\bMINT_NETWORK_FEE_TEXT\b|\bBURN_PROCESSING_FEE_TEXT\b' .Repository: Pi-Defi-world/acbu-frontend
Length of output: 2277
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the surrounding render section in app/mint/page.tsx.
nl -ba app/mint/page.tsx | sed -n '280,420p'Repository: Pi-Defi-world/acbu-frontend
Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the page render section and look for component usage in the tab content.
sed -n '299,460p' app/mint/page.tsx
printf '\n== component references in render section ==\n'
sed -n '299,460p' app/mint/page.tsx | rg -n '\b(MintForm|BurnForm|MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b'Repository: Pi-Defi-world/acbu-frontend
Length of output: 7125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the page render section and look for component usage in the tab content.
sed -n '299,460p' app/mint/page.tsx
printf '\n== component references in render section ==\n'
sed -n '299,460p' app/mint/page.tsx | rg -n '\b(MintForm|BurnForm|MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b'Repository: Pi-Defi-world/acbu-frontend
Length of output: 7125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the JSX returned by MintPage and look for actual component usage.
printf '== return section ==\n'
sed -n '460,760p' app/mint/page.tsx
printf '\n== JSX component tags ==\n'
rg -n -C 2 '<(MintForm|BurnForm)\b' app/mint/page.tsx
printf '\n== unresolved fee constants definitions ==\n'
rg -n -C 2 'const\s+(MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b|function\s+(MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b|let\s+(MINT_NETWORK_FEE_TEXT|BURN_PROCESSING_FEE_TEXT)\b' app/mint/page.tsxRepository: Pi-Defi-world/acbu-frontend
Length of output: 14626
Wire the extracted forms into the tab content
MintForm and BurnForm are still unused because the tab panels keep rendering the inline form JSX. MINT_NETWORK_FEE_TEXT and BURN_PROCESSING_FEE_TEXT are also undefined in the extracted components, so define or inline those values when wiring them in.
🧰 Tools
🪛 ESLint
[error] 84-84: 'MintForm' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 194-194: 'BurnForm' is defined but never used.
(@typescript-eslint/no-unused-vars)
🤖 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/mint/page.tsx` around lines 84 - 290, The extracted MintForm and BurnForm
components are currently unused because the tab panels still render the old
inline JSX, so replace that duplicated tab content with these components and
pass through the existing state/handlers from the surrounding mint page. While
wiring them in, make sure MintForm and BurnForm can access the fee labels by
defining or inlining MINT_NETWORK_FEE_TEXT and BURN_PROCESSING_FEE_TEXT in the
same scope or as props so the extracted components compile and render correctly.
Source: Linters/SAST tools
Comment on lines
+170
to
+175
| const handleConfirmDialogChange = useCallback((open: boolean) => { | ||
| if (!open && !sending) { | ||
| setConfirmedAmount(""); | ||
| } | ||
| setShowConfirmDialog(open); | ||
| }, [sending]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Use the extracted confirm dialog handler.
handleConfirmDialogOpenChange is not defined; the extracted callback is named handleConfirmDialogChange.
Proposed fix
- <AlertDialog open={showConfirmDialog} onOpenChange={handleConfirmDialogOpenChange}>
+ <AlertDialog open={showConfirmDialog} onOpenChange={handleConfirmDialogChange}>Also applies to: 587-587
🤖 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/send/page.tsx` around lines 170 - 175, The confirm dialog open-change
handler is referenced with the wrong name, causing the extracted callback to be
missed. In the send page component, use the existing handleConfirmDialogChange
callback consistently wherever the confirm dialog’s open state is wired up, and
replace any remaining handleConfirmDialogOpenChange references with the actual
handler name so the dialog uses the extracted callback correctly.
| const login = useCallback( | ||
| (userId: string, stellarAddress: string | null = null) => { | ||
| setAuth(userId, stellarAddress); | ||
| setAuth(null, userId, stellarAddress); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix login to pass setAuth arguments in the correct order.
Line 163 currently logs users out instead of in (userId becomes null) and passes an extra argument.
Proposed fix
const login = useCallback(
(userId: string, stellarAddress: string | null = null) => {
- setAuth(null, userId, stellarAddress);
+ setAuth(userId, stellarAddress);
},
[setAuth]
);📝 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
| setAuth(null, userId, stellarAddress); | |
| setAuth(userId, stellarAddress); |
🤖 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 `@contexts/auth-context.tsx` at line 163, The login flow in auth-context.tsx is
calling setAuth with the arguments in the wrong order, which causes the user id
to be null and adds an extra parameter. Update the login path in the setAuth
call to use the same argument order expected by setAuth (and used elsewhere in
AuthContext), with the authenticated userId and stellarAddress passed correctly
so login actually sets the session instead of clearing it.
| 14. **International transfer preview rate uses stale or wrong data** – app/currency/page.tsx: The international tab multiplies ACBU by the local currency rate from `/rates`, but the actual cross-border rate includes intermediary and settlement fees that the frontend doesn't know about. The preview is misleadingly optimistic. | ||
| 15. **Error boundary `error.tsx` does not log errors to backend** – app/error.tsx, app/global-error.tsx: React error boundaries catch crashes but only render a fallback UI. The error detail (component stack, route, user ID) is never POSTed to an error-tracking endpoint, leaving production crashes invisible. | ||
| 73. **No `loading.js` or Suspense boundary on the `/send` page** – app/send/page.tsx: The send money page fetches balance and rates on mount but has no Suspense boundary. Users on slow connections stare at a blank screen with no loading indicator. | ||
| 74. **Form input loses focus on every keystroke due to state-update re-render** – app/mint/page.tsx, app/send/page.tsx: Input components may be defined inline in the parent component's render function, causing React to unmount and remount the input on every keystroke. The keyboard dismisses on mobile and the cursor jumps to the end. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale known-issue entry: this PR resolves item 74.
Item 74 documents exactly the inline-input remount/focus-loss bug that this PR fixes by extracting the input components in app/mint/page.tsx and app/send/page.tsx. Update or remove this entry (or mark it resolved) so the catalog stays accurate.
🤖 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 `@ISSUES.md` at line 33, This ISSUES.md entry is now stale because item 74 has
been fixed by extracting the inline input components in app/mint/page.tsx and
app/send/page.tsx. Update the catalog entry to mark item 74 as resolved, or
remove it entirely if resolved issues are not kept, so the known-issues list
stays accurate and does not describe a bug that no longer exists.
| { | ||
| name: 'PBKDF2', | ||
| salt, | ||
| salt: salt as any, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify types feeding crypto APIs are BufferSource-compatible and locate any remaining casts.
set -euo pipefail
echo "== locate fromBase64 definition =="
rg -n --no-heading "function fromBase64|const fromBase64|export .*fromBase64" lib app components
echo "== inspect wallet-storage around crypto calls =="
sed -n '1,220p' lib/wallet-storage.ts
echo "== search for explicit any in wallet-storage and related crypto usage =="
rg -n --no-heading "as any|subtle\\.deriveKey|subtle\\.decrypt" lib/wallet-storage.tsRepository: Pi-Defi-world/acbu-frontend
Length of output: 6748
Remove the any casts in lib/wallet-storage.ts — fromBase64() already returns Uint8Array, so salt, iv, and ciphertext can be passed directly to crypto.subtle.deriveKey() / decrypt(). The current casts only suppress type safety and trip @typescript-eslint/no-explicit-any.
🧰 Tools
🪛 ESLint
[error] 61-61: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 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 `@lib/wallet-storage.ts` at line 61, Remove the unnecessary any casts in
wallet-storage handling: fromBase64() already produces Uint8Array, so pass salt,
iv, and ciphertext directly into the crypto.subtle.deriveKey() and decrypt()
calls in lib/wallet-storage.ts. Update the relevant logic around the wallet
storage helpers so the types remain strict and no
`@typescript-eslint/no-explicit-any` suppression is needed.
Source: Linters/SAST tools
Comment on lines
+16
to
+17
| "test": "echo \"No tests\"", | ||
| "test:e2e": "echo \"No e2e tests\"" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
CI will report green even though no tests run. The QA workflow still invokes pnpm test and pnpm test:e2e, but both now resolve to echo "No tests" / echo "No e2e tests", which always exit 0. Combined with the removed Vitest devDependencies, this silently disables the entire component/E2E test layer while keeping CI green. The frontend QA GitHub Actions workflow runs pnpm typecheck, pnpm lint, pnpm build, pnpm test, and pnpm test:e2e.
If tests are intentionally being removed, consider also removing the corresponding CI steps so the pipeline accurately reflects coverage; otherwise restore a real test runner.
🤖 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 `@package.json` around lines 16 - 17, The package.json test scripts currently
stub out pnpm test and pnpm test:e2e with echo commands, so the QA workflow can
pass without running any real tests. Restore real test execution in the test and
test:e2e script entries, or if the suite is intentionally being removed, update
the frontend QA GitHub Actions workflow to stop invoking those commands so CI
reflects the actual coverage; use the package.json script definitions as the
main place to fix this.
Comment on lines
+85
to
+95
| "eslint": "^10.4.1", | ||
| "eslint-plugin-react": "^7.37.5", | ||
| "eslint-plugin-react-hooks": "5.2.0", | ||
| "globals": "^17.5.0", | ||
| "jsdom": "^29.0.2", | ||
| "playwright": "^1.59.1", | ||
| "postcss": "^8.5", | ||
| "sharp": "^0.34.5", | ||
| "tailwindcss": "^4.1.9", | ||
| "tw-animate-css": "1.3.3", | ||
| "typescript": "^5", | ||
| "typescript-eslint": "^8.58.2", | ||
| "vite-tsconfig-paths": "^6.1.1", | ||
| "vitest": "^4.1.5" | ||
| }, | ||
| "packageManager": "pnpm@10.15.0+sha512.486ebc259d3e999a4e8691ce03b5cac4a71cbeca39372a9b762cb500cfdf0873e2cb16abe3d951b1ee2cf012503f027b98b6584e4df22524e0c7450d9ec7aa7b", | ||
| "pnpm": { | ||
| "overrides": { | ||
| "@stellar/stellar-sdk": "^15.0.1" | ||
| }, | ||
| "peerDependencyRules": { | ||
| "allowedVersions": { | ||
| "eslint-plugin-react>eslint": "^10", | ||
| "@trezor/connect-plugin-stellar>@stellar/stellar-sdk": "^15.0.1" | ||
| } | ||
| } | ||
| "typescript-eslint": "^8.58.2" | ||
| } | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -a 'generate-pwa-icons' || echo "script not found (removed)"
rg -n "from 'sharp'|require\(.sharp" --glob '!node_modules' || echo "no sharp references remaining"Repository: Pi-Defi-world/acbu-frontend
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json relevant sections ==\n'
sed -n '1,180p' package.json | cat -n
printf '\n== scripts/generate-pwa-icons.mjs ==\n'
wc -l scripts/generate-pwa-icons.mjs
cat -n scripts/generate-pwa-icons.mjsRepository: Pi-Defi-world/acbu-frontend
Length of output: 5493
Remove the stale icon-generation script or restore sharp
scripts/generate-pwa-icons.mjs still imports sharp and will fail if run, but sharp is no longer in devDependencies. Delete the script too, or add the dependency back if it’s still needed.
🤖 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 `@package.json` around lines 85 - 95, The PWA icon generation workflow is
broken because scripts/generate-pwa-icons.mjs still depends on sharp even though
sharp was removed from devDependencies. Either delete the stale icon-generation
script if it is no longer used, or restore sharp to the package manifest if the
script must remain; make sure the dependency choice matches the current usage of
the generate-pwa-icons.mjs script.
Member
|
@N-thnI resolve conflict |
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
4 participants
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.
closes #454
Summary by CodeRabbit
New Features
Bug Fixes
Documentation