[sahilc0]

Summary

BLOCKED BY: Kukks' LNURL work. This PR redesigns the receive flow to show the QR code immediately (skipping the amount screen), but we should not merge until we have zero-invoice Lightning via LNURL. Without that, showing the QR first means no Lightning option unless the user manually sets an amount. The full UX improvement depends on LNURL landing first.

• Receive layout: QR centered vertically with actions pinned to bottom for balanced spacing
• Sheet modal (Vaul-inspired): Visual elevation via Ionic CSS custom properties (shadow, border, overflow), wider pill handle, no close button
• Toast (Sonner-inspired): Custom toast system replacing useIonToast across all call sites — dark bg, top-positioned, 250ms ease-out animations, reduced-motion compliant
• Copy sheet: Reordered addresses (Unified > Lightning > Arkade > Bitcoin), proper header styling, accessible 44px copy buttons with aria-labels
• Accessibility: :focus-visible instead of :focus (no purple flash on tap), semantic <button> elements, touch-action: manipulation

Dependency

This PR is blocked by Kukks' LNURL implementation. The current receive flow shows the QR immediately without an amount, which means Lightning is unavailable until the user sets an amount (500+ sats). Once LNURL lands, we'll have zero-invoice Lightning support and the "show QR first" UX will work end-to-end.

Do not merge until LNURL is available.

Base branch

This PR is based on wt-navbar-root-only-20260325 (#474 — navbar only on root pages) to avoid navbar interference with the receive sheet modals.

Test plan

• QR centered vertically, actions pinned to bottom on various screen heights
• Sheet modals show visible elevation (shadow + border)
• Toast appears at top with dark bg, auto-dismisses in 2s
• Copy buttons have 44px tap targets, no purple focus flash on tap
• Address order in copy sheet: Unified, Lightning (when available), Arkade, Bitcoin
• Amount sheet header matches copy sheet styling
• Dark mode: toast slightly lighter gray, sheet has stronger shadow
• Reduced motion: no animation on toast enter/exit

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

• next-version

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 946e4e59-f1f7-4cac-b4ad-21d8aa44f616

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch wt-receive-on-navbar-20260326

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sahilc0]

⚠️ Blocked: depends on LNURL

@Kukks — this PR redesigns the receive flow to show the QR code immediately (no amount screen first). However, this UX only makes sense once we have zero-invoice Lightning via LNURL.

Right now, without an amount set, there's no Lightning invoice in the QR — just Arkade + on-chain. That's a worse experience than the current flow where we prompt for amount first.

This should not be merged until LNURL support lands. Once it does, the receive flow will show a unified QR that supports Lightning without requiring the user to enter an amount upfront.

Keeping this as draft until then.

[cloudflare-workers-and-pages]

Deploying wallet-bitcoin with    Cloudflare Pages

Latest commit:	ab0fe64
Status:	✅  Deploy successful!
Preview URL:	https://56737e0b.wallet-bitcoin.pages.dev
Branch Preview URL:	https://wt-receive-on-navbar-2026032.wallet-bitcoin.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	ab0fe64
Status:	✅  Deploy successful!
Preview URL:	https://87a2318d.arkade-wallet.pages.dev
Branch Preview URL:	https://wt-receive-on-navbar-2026032.arkade-wallet.pages.dev

View logs

[ghost]

🔍 Review — feat(receive): redesign receive flow with improved sheets and toasts

Large PR: +11,336/-587 across 85 files. Absorbs multiple merged PRs from master (Docker, LNURL, Branta, error boundary, SW recovery, loading status, etc.) plus the receive redesign itself.

---

Architecture

The PR layers on wt-navbar-root-only-20260325 (#474) and integrates master, so it carries a lot of merged-in work. The receive-specific changes are well-structured:

• useLnurlSession hook for SSE-based LNURL amountless receives
• SheetModal redesign with Vaul-inspired elevation
• Custom Toast system replacing useIonToast
• QrCode component rewrite with SVG rendering and staggered animations

Security Review

LNURL session (useLnurlSession.ts) — Overall solid:

• ✅ Auth token stored from session_created event, sent as Bearer header on invoice POSTs
• ✅ Abort controller properly cancels on unmount
• ✅ amountMsat validated as positive number before creating swap
• ✅ Errors sent back to lnurl-server so payers get immediate feedback
• ✅ SSE parser wraps JSON.parse in try/catch
• ✅ eventType persisted outside while loop to handle cross-chunk SSE boundaries

One concern: tokenRef.current is never cleared between sessions except on full unmount (the finally block sets it to null). If the SSE stream reconnects (e.g. network blip) within the same mount, a stale token could be sent. This is low risk since the component unmounts on navigation, but worth noting.

Service worker hardening (merged from #487):

• ✅ Zombie SW detection via PING/PONG before init
• ✅ SW unregistration after exhausted retries
• ✅ MessageBus timeout increased from 5s to 30s (appropriate for cold starts)
• ✅ navigator.serviceWorker access guarded behind feature check

LNURL reverse swap flow in QrCode.tsx:

• ✅ isAmountlessLnurl properly gated on: no amount set, no asset, lnurl server configured, connected, swaps initialized, no swap error
• ✅ waitAndClaim runs in background with error logging
• ⚠️ The handleInvoiceRequest callback captures recvInfo in its closure via the dependency array. If recvInfo changes between the initial render and when the invoice request arrives, the spread { ...recvInfo } may overwrite newer state. Consider using a ref for recvInfo in this callback, similar to how onInvoiceRequestRef is used in the hook itself.

ErrorBoundary (merged from #496):

• ✅ Captures to Sentry with component stack
• ✅ User-friendly fallback UI

UX Observations

• BLOCKED BY LNURL — PR description clearly states this shouldn't merge until LNURL lands. Since the LNURL work (Add LNURL support for amountless Lightning receives #482) is now merged into this branch, the blocker appears resolved assuming the lnurl-server is deployed.
• QR-first flow means Lightning is unavailable without amount unless LNURL session is active — the isAmountlessLnurl check handles this correctly.
• Address ordering (Unified > Lightning > Arkade > Bitcoin) in copy sheet matches the hierarchy of payment rails.
• package-lock.json (+8555 lines) — this is a npm lockfile in a pnpm project. Intentional? It's listed in the diff and would add significant repo bloat.

Cross-repo Impact

• Depends on @arkade-os/sdk IVtxoManager interface and VtxoManager.getExpiringVtxos() — ensure ts-sdk exports are stable before merging.
• @arkade-os/boltz-swap integration for LNURL reverse swaps uses the existing createReverseSwap flow — no new boltz-swap API surface needed.

Dev auto-init (VITE_DEV_NSEC)

The final commits fix race conditions where dev auto-init conflicted with boot animation and auth state checks. The approach (skip boot animation and auth check when VITE_DEV_NSEC is active) is pragmatic. Ensure VITE_DEV_NSEC is never set in production builds — the Vite env var prefix means it's compile-time, so this should be safe as long as .env.production doesn't define it.

Summary

Area	Status
LNURL security	✅ Token auth, validation, abort handling
SW recovery	✅ Zombie detection, retry limits, timeout tuning
Error handling	✅ ErrorBoundary, BootError, Sentry filtering
Receive UX	✅ QR-first, sheet modals, accessibility
State management	⚠️ recvInfo closure in handleInvoiceRequest
package-lock.json	⚠️ npm lockfile in pnpm project — likely unintended

Verdict: Well-structured feature PR with good security practices on the LNURL flow. Two minor issues flagged. The npm lockfile should be removed before merge.

[sahilc0]

@Kukks fixed the merge conflict — should be good to go now.

[ghost]

🔍 Iterative Review — Merge Commit d718a15

This merge integrates wt-navbar-root-only-20260325 (the base branch) into the PR, bringing ~30 changed files. Key observations on the new delta:

Changes Worth Noting

Input.onChange type coercion — Input now calls onChange(Number(v)) for type="number" inputs. This means onChange callers receive a number instead of a string. All asset screens (Burn, Mint, Reissue) have been updated to use useState<number> accordingly. This is a nice cleanup, but note that Number("") returns 0 — clearing the input field will silently produce 0 rather than undefined/empty. Verify this doesnt cause UX issues where a blank field should be distinguishable from 0.

prettyNumber robustness — Now correctly handles undefined/null/NaN (returns "0") and adds minimumFractionDigits. The old !num guard would treat 0 as falsy and return "0", which was technically correct but for the wrong reason. The new explicit check is better.

InputAmount asset guard — Changed from if (asset) return to if (asset?.assetId) return. This is a correctness fix — an object with no assetId would previously short-circuit the fiat conversion effect.

LoadingLogo triple-tap reveal — Status text and background are now hidden behind a 3-tap gesture. Clean implementation with proper timer cleanup. The 600ms window and 3-tap threshold feel reasonable.

Success screen navigation — All success screens (Send, Receive, Notes) now have a "Sounds good" button to navigate back to Pages.Wallet. Previously users had no explicit way to dismiss these screens. Good UX improvement.

Send form re-validation — parseRecipient effect now depends on [recipient, isAssetSend] instead of just [recipient]. This correctly re-validates when switching between BTC and asset sends, catching the "Assets can only be sent to Arkade addresses" case. Also, amount/textValue are now cleared before the asset-specific branch, fixing a potential state ordering issue.

Mint control asset amount — Changed from unitsToCents(parseFloat(ctrlAmount) || 0, 0) to parseInt(ctrlAmount.toString()). Since control assets have 0 decimals, this is equivalent, but parseInt without a radix parameter defaults to base-10 for decimal strings — consider explicitly passing radix 10: parseInt(ctrlAmount.toString(), 10).

No Security Concerns

No key handling, signing, or protocol-level changes in this merge delta. All changes are UI/UX layer.

[bordalix]

Issue: don't use hardcoded 500 sats for min lightning, use value received from provider.
Fixed by 92f9499

[ghost]

🔍 Arkana PR Review

Scope: UI redesign of the receive flow — QR code, sheet modals, toast system, accessibility.

Overview

Solid UX polish. The SVG QR code with animated dots and logo overlay is a nice upgrade from the GIF encoder. Custom toast system is cleaner than useIonToast. Sheet modal with pill handle matches modern mobile patterns. Good accessibility attention throughout.

Observations

QR Code Component (QrCode.tsx)

• 👍 SVG rendering with rounded finder patterns and animated dot entrance looks great. prefers-reduced-motion respected properly.
• ⚠️ The prevMatrixRef / renderCountRef pattern for animation diffing is clever but the useMemo depends on prefersReduced but mutates prevMatrixRef inside the memo. This works because refs are stable, but be aware that if prefersReduced changes, you get a re-render that treats all dots as "new" (animation replays). Minor, not blocking.
• ⚠️ isLogoZone uses Math.ceil(size * 0.2) for logo modules. For larger QR versions (long URIs), verify the error correction headroom at ecc: "medium" — clearing center modules with a large payload could push past the ECC limit and make the QR unscannable on some readers.

Toast System (Toast.tsx)

• 👍 Clean context-based approach. Timer cleanup on unmount is correct.
• Nit: nextId is a module-level counter — fine for this use case, but in strict mode with double-mount (React 18 dev), IDs will skip. Not a real issue in production.
• Nit: Consider using crypto.randomUUID() or Date.now() if ID uniqueness across hot reloads matters.

SheetModal (SheetModal.tsx)

• 👍 Haptic feedback on close is a nice touch.
• The handleAreaStyle has cursor: grab — on mobile this is irrelevant (no cursor), but on desktop web it sets the right expectation. Fine.
• The pill handle area is tap-to-close. Consider whether users might accidentally tap it. With initialBreakpoint={1} and backdropBreakpoint={0}, the Ionic modal already handles swipe-to-dismiss, so the tap-close on the handle area adds redundancy (which is fine for discoverability).

Receive Page

• 👍 Address order in copy sheet follows the PR description (Unified > Lightning > Arkade > Bitcoin). Logical priority.
• 👍 44px minimum tap targets on copy buttons with touch-action: manipulation — good mobile practice.
• The minSwapAllowed() hint for Lightning is helpful UX. The "Lightning temporarily unavailable" warning when swaps time out is a good degradation pattern.

package-lock.json

• This is a new 8,555-line file being added. Is the repo switching from pnpm-lock to npm lockfile? If intentional, might be worth noting in the PR description.

Content.tsx — noFade prop

• Clean addition. The no-content-fade class presumably disables a scroll-fade effect. Make sure the CSS class is defined (should be in ionic.css based on the diff).

Security

No concerns — this is purely UI/UX. No key material, no transaction logic, no external API changes.

Verdict

Clean, well-structured UI work. The QR ECC headroom question is the only thing worth verifying before merge. Everything else is polish-tier.

Reminder: PR is explicitly blocked on LNURL landing first — do not merge until that dependency is met.

[bordalix]

Issue: when user selects/copies one address, the qrcode should change to that address.

Scenario: paying wallet only reads lightning invoices, user wants to have a qrcode with only the bolt11 to show to the paying wallet.

Fixed by 74ca2b7

[bordalix]

Incorrect buttons spacing

Fixed by 8e5667d

[ghost]

🔍 Arkana Review (followup — new commits)

Previous review covered the base. This followup addresses changes since 92f94999.

New Changes Observed

QR Code component rewrite (QrCode.tsx):

• Moved from GIF-based QR (via encodeQR → gif blob URL) to inline SVG with rounded dot modules, styled finder patterns, and a centered Ark logo. Good visual upgrade.
• Ripple animation on dot appearance (distance-based delay) with prefers-reduced-motion respect — well implemented.
• prevMatrixRef for diff-based animation on value changes — clever, avoids full re-render animation when only data changes.
• Potential issue: logoColor uses var(--logo-color) but I don't see this CSS variable defined in the diff. Verify it's set somewhere (or falls back gracefully).

Toast system (Toast.tsx):

• Custom toast replaces all useIonToast call sites (Text, Table, ExpandAddresses, Backup, Logs, ReceiveQRCode).
• Top-positioned, dark bg, auto-dismiss in 2s, 250ms ease-out animations with reduced-motion fallback.
• Timer cleanup on unmount — good.
• nextId is module-scoped (not in state) — fine for this use case, but note it persists across hot reloads in dev.

SheetModal redesign:

• Removed close button, added drag handle (pill). Visual elevation via Ionic CSS custom properties.
• Now calls hapticLight() on dismiss — nice touch.
• Uses NavigationContext to detect if navbar is present and adjusts bottom padding.

Receive flow restructure (ReceiveQRCode):

• Now fetches addresses on mount (previously expected them from a prior screen). This enables skipping the amount screen.
• Amount and copy are bottom sheets instead of separate pages.
• BIP21 URI built even before QR display is triggered — the useEffect dependency on addressesLoaded is correct.
• Address order in copy sheet: Unified > Lightning > Arkade > Bitcoin — matches the PR description.
• minSwapAllowed() hint below QR for Lightning minimum — helpful UX.
• Dev fast-path: VITE_DEV_NSEC skips boot animation and auth state race — setAuthState('authenticated') called before initWallet to prevent race.

CSS changes:

• ion-modal now has explicit z-index: 200 !important — verify this doesn't conflict with other overlays (action sheets, alerts).
• Dark mode gets distinct --toast-bg, --toast-color, --sheet-bg — good.
• .no-content-fade class to opt out of scroll fade mask — used on receive page where the QR needs full visibility.
• :focus-visible replacing :focus on .focusable — correct, prevents purple flash on tap while maintaining keyboard accessibility.

Concerns

1. ion-modal z-index !important: This could cause stacking issues if multiple modals or action sheets are open simultaneously. Ionic manages z-index internally — overriding it globally might cause the amount sheet to appear behind an action sheet. Consider scoping this to .sheet-modal class instead.

2. 9,342 additions: The bulk is package-lock.json changes. The actual code diff is reasonable, but the lock file update should be reviewed for unexpected dependency additions (I see framer-motion added — is it used? The animation in QrCode.tsx is CSS-based).

3. Missing --logo-color variable: The QR code SVG references var(--logo-color) but the CSS changes only add --sheet-bg, --toast-bg, --toast-color. If --logo-color is undefined, the logo paths will render as the initial value (typically transparent or inherited).

4. useReducedMotion hook: Referenced in QrCode.tsx but the hook file isn't in the diff. Confirm it exists in the codebase.

Blocked Status

PR body clearly states this is blocked by LNURL implementation. The "show QR first without amount" UX only works end-to-end once zero-invoice Lightning is available. Do not merge until that lands.

[ghost]

Followup review (8e5667d — fix button alignment)

Good fix. Moves the action buttons (Amount / Copy / Share) out of the scrollable content area into <ButtonsOnBottom>, which pins them properly regardless of QR code + warning box height. The WarningBox stays inline with the QR content where it belongs.

Also picks up the re-imported ButtonsOnBottom that was previously dropped. Clean separation of scrollable content vs fixed actions.

No new concerns.

[bordalix]

Broken: when receiving assets it says "sats" instead of the asset token
In this case it should say "Requesting 12 BB"

Fixed by 4955070

[Kukks]

a QR code with no border margin makes reading by scanners very hard.

[ghost]

🔍 Arkana PR Review — wallet#480

PR: feat(receive): redesign receive flow with improved sheets and toasts
Author: sahilc0 | Base: wt-navbar-root-only-20260325 (#474)

---

Summary

Major UX overhaul of the receive flow: QR shown immediately (skipping amount screen), new SVG QR renderer with animated dots and embedded Arkade logo, custom Toast system replacing useIonToast, redesigned SheetModal with drag handle, and new bottom sheets for amount/copy on the receive page. Blocked on LNURL zero-invoice support.

What looks good

• Custom Toast system (Toast.tsx): Clean context-based design, proper cleanup of timers on unmount, prefers-reduced-motion respected. Well-structured.
• QR SVG renderer (QrCode.tsx): Nice upgrade from GIF to SVG. Finder pattern rendering with rounded corners, logo zone clearing with proper ecc: "medium" to compensate for removed modules. Reduced-motion compliance for dot animations.
• Accessibility improvements: :focus-visible instead of :focus (no purple flash on tap), 44px minimum tap targets on copy buttons, aria-label on interactive elements, touch-action: manipulation.
• SheetModal refactor: Drag handle instead of close button is a better mobile pattern. Haptic feedback on dismiss.
• Address ordering in copy sheet: Unified → Lightning → Arkade → Bitcoin — correct priority order.
• Dev workflow: VITE_DEV_NSEC guard to skip boot animation race is a good DX improvement.

Questions / Concerns

1. useEffect dependency arrays in ReceiveQRCode.tsx — Several effects use stale closure patterns:

   • Line ~83: useEffect depends on [svcWallet] but reads recvInfo from closure. If recvInfo changes between renders, the setRecvInfo call will overwrite those changes. Consider using the functional form: setRecvInfo(prev => ({ ...prev, boardingAddr, offchainAddr, ... })).
   • Same pattern in the payment listener effect (also [svcWallet]) — setRecvInfo({ ...recvInfo, satoshis: sats, receivedAssets }) captures a stale recvInfo.

2. QR code useMemo dependency — renderCountRef.current is mutated inside useMemo (line ~97). While refs don't trigger re-renders, mutating state inside useMemo is a React anti-pattern. The memo could be re-evaluated at unexpected times in concurrent mode. Consider moving the counter increment outside the memo.

3. prevMatrixRef comparison — The animation logic compares previous and current QR matrices. Since encodeQR with the same input produces the same matrix, the animation will only trigger on actual value changes — good. But the isUpdate check (prevMatrix !== null && prevMatrix.length === size) means the first render never animates. Is that intentional? Might want an entrance animation on mount.

4. Toast nextId as module-level variable — let nextId = 0 at module scope means IDs reset on hot reload in dev but persist across component unmount/remount. This is fine for production but could cause confusion in dev with HMR. Minor — just noting it.

5. ion-modal z-index override — z-index: 200 !important in ionic.css is a broad override. If other Ionic modals or overlays are used elsewhere, this could cause stacking context issues. Worth documenting why 200 was chosen.

6. Missing cleanup for createBtcToArkSwap promise — In the satoshis effect, createBtcAddress() calls createBtcToArkSwap which returns a promise. If the component unmounts before it resolves, there's no abort mechanism. The Promise.allSettled will still run its .then() and call state setters on an unmounted component. Consider an abort flag.

7. Sheet modal onClick on handle area — The handle area div has cursor: grab but uses onClick for dismiss. This is slightly confusing UX — grab implies drag. The Ionic modal already handles swipe-to-dismiss via initialBreakpoint, so the click-to-dismiss on the handle might be unexpected.

Security

• ✅ No key material exposure — addresses are fetched via existing getReceivingAddresses API
• ✅ QR encoding uses ecc: "medium" — appropriate error correction for logo overlay
• ✅ No new external dependencies (SVG rendering is all inline)
• ✅ BIP21 URI construction unchanged from existing logic
• ✅ DOMPurify already in deps — no raw HTML injection vectors

Cross-repo impact

• None — all changes are wallet-internal UI. No SDK or protocol changes.

Verdict

Solid UX improvement. The stale closure issues in useEffect dependencies (#1) are the main concern — they could cause subtle bugs where address/amount state gets overwritten. The rest are minor polish items. Blocked on LNURL as noted in the PR description.

[bordalix]

Mobile asset receive is totally broken

Screen.Recording.2026-04-03.at.14.58.31.mov

Fixed by eaefd87

Screen.Recording.2026-04-03.at.16.35.53.mov

[ghost]

🔍 PR Review — feat(receive): redesign receive flow with improved sheets and toasts

Reviewer: Arkana (automated)
Commit: eaefd87

---

Summary

Large UX refactor: replaces the amount-first receive flow with a QR-first approach, adds custom toast system, redesigned sheet modals with drag handle, and a sophisticated SVG QR code renderer with animated dot transitions. Good accessibility improvements throughout.

✅ Positives

• :focus-visible over :focus — correct fix, eliminates the purple flash on mobile tap while keeping keyboard a11y
• Custom toast system — clean context-based approach, reduced-motion compliant, proper cleanup of timers
• QR code SVG renderer — nice touch with the animated dots, finder pattern rendering, and logo overlay. The ecc: "medium" error correction compensates well for the center logo zone
• Sheet modal redesign — drag handle UX is much better than close button for bottom sheets on mobile
• 44px touch targets on copy buttons — good adherence to WCAG touch target guidelines
• Address ordering (Unified > Lightning > Arkade > Bitcoin) makes sense for progressive enhancement

⚠️ Issues & Suggestions

1. Memory leak: QR GIF blob URLs never revoked (removed code)

The old code created blob URLs via URL.createObjectURL() without ever calling URL.revokeObjectURL(). The new SVG approach avoids this entirely — good.

2. package-lock.json committed — intentional?

This PR adds a new package-lock.json (8555 lines). The wallet repo typically uses pnpm (pnpm-lock.yaml). Having both lockfiles can cause confusion. If the project uses pnpm exclusively, this file should likely be .gitignored.

3. createBip21 called inside useEffect without being in deps (QrCode.tsx lines ~235, ~310)

createBip21 is defined as a regular function inside the component, so it captures stale closures. It references recvInfo, swapAddress, invoice, lnurlSession, and several context values. The second useEffect that calls createBip21(amountInput) only has [amountInput] in its deps — it will use stale invoice, swapAddress, etc. Either wrap createBip21 in useCallback with proper deps, or inline the logic.

4. useEffect on [svcWallet] re-fetches addresses on every wallet change

useEffect(() => {
  if (!svcWallet) return
  if (boardingAddr && offchainAddr) { setAddressesLoaded(true); return }
  getReceivingAddresses(svcWallet)...
}, [svcWallet])

The early return guards against re-fetching, but boardingAddr and offchainAddr aren't in the dependency array. If svcWallet reference changes but addresses are already set, this works. But if addresses are cleared and svcWallet doesn't change, it won't re-fetch. Minor — just noting the implicit coupling.

5. Toast nextId is module-scoped global

let nextId = 0

This works fine in practice (single page app), but if ToastProvider is ever mounted/unmounted/remounted, IDs continue incrementing (never reset). Not a bug, just a design note — IDs only need uniqueness, not sequential continuity.

6. QR animation: renderCountRef used as React key suffix

key={`d-${row}-${col}-${renderCountRef.current}`}

This forces React to unmount/remount animated circles on every value change (even if the dot was already present). Intentional for re-triggering animation, but it means all new dots animate even if they were present before — the animation is "QR changed" not "this specific dot is new." The isNew check partially mitigates this, but the key change forces remount regardless. Consider using a stable key when !isNew.

7. setAuthState('authenticated') before initWallet completes (wallet.tsx)

setAuthState('authenticated')
await initWallet(privateKey)

In dev mode, auth state is set to authenticated before the wallet is actually initialized. If initWallet fails, the app thinks it's authenticated but has no wallet. The error is caught and logged, but authState isn't reverted.

8. Navigation hardcoded to Pages.Wallet

<Header text='Receive' back={() => navigate(Pages.Wallet)} />

Previously used the generic back prop. If ReceiveQRCode is reached from the asset detail page, back now always goes to Wallet instead of back to the asset. This changes the navigation behavior.

9. Sheet modal z-index override

ion-modal {
  z-index: 200 !important;
}

The !important on z-index is aggressive. If other modals (action sheets, alerts) use Ionic's default z-indexing, this could cause stacking issues. Consider scoping this to a specific class.

Security

• ✅ No private key handling changes (dev-mode NSEC is gated behind import.meta.env.DEV)
• ✅ No changes to transaction signing, VTXO handling, or exit paths
• ✅ copyToClipboard usage is consistent and doesn't expose sensitive data
• ✅ QR code generation uses the same encodeBip21 / encodeBip21Asset functions

Verdict

Solid UX improvement. The QR-first flow is the right direction, and the accessibility work is appreciated. The stale closure in createBip21 (issue #3) and the hardcoded back navigation (issue #8) are the most impactful items to address. The package-lock.json addition should be clarified.

PR is explicitly blocked on LNURL implementation — makes sense, the amountless QR is a degraded experience without zero-invoice Lightning.

[bordalix]

ReceiveAmount page is no longer used, can be removed.
Fixed by 0062dad

[ghost]

🔍 Arkana PR Review — arkade-os/wallet#480

Scope: UI/UX redesign of the receive flow — new QR code component, sheet modals, toast system, and flow simplification.

Overview

This PR removes the ReceiveAmount intermediate screen and goes directly to ReceiveQRCode, which now handles address fetching, amount setting (via sheet modal), and address copying (via sheet modal) all in one place. A new custom Toast system replaces useIonToast across the codebase. The QR code renderer is rebuilt from scratch as an SVG with animated dots, rounded finder patterns, and an embedded logo.

Positive Observations

1. Accessibility improvements are solid: :focus-visible instead of :focus, 44px tap targets on copy buttons, aria-label attributes, reduced-motion compliance across both QR and toast animations.
2. The toast system (Toast.tsx) is clean — context-based, handles enter/exit animations, auto-dismisses, and properly cleans up timers on unmount.
3. QR code component is significantly better than the GIF-based approach — SVG is sharper, scalable, and the dot-in animation with radial delay is a nice touch.
4. Sheet modal redesign with the drag handle (Vaul-inspired) is more intuitive than the close button approach.
5. Dev workflow improvement — VITE_DEV_NSEC skip for boot animation and auth state fast-path is practical.

Issues & Suggestions

Medium Priority

1. package-lock.json committed to a pnpm project — The wallet uses pnpm (engines field says pnpm >= 8), so committing a package-lock.json (npm lockfile) is likely unintentional and could cause confusion. If the project uses pnpm, this file should probably be in .gitignore. This adds ~8500 lines of noise.

2. QrCode.tsx — prevMatrixRef never resets on unmount — The ref accumulates state across mounts. If the component unmounts and remounts with the same value, prevMatrix will match and no animation plays. Consider resetting prevMatrixRef.current = null in a cleanup effect, or accepting this as intended behavior for same-value re-renders.

3. QrCode.tsx — renderCountRef monotonically increases — Used as a key suffix for animated dots (d-${row}-${col}-${renderCountRef.current}). This is fine for uniqueness, but React will never reuse these keys. Not a bug, just worth noting for React reconciliation awareness.

4. Toast nextId is a module-level variable — If multiple ToastProvider instances are ever mounted (unlikely but possible in tests or nested routing), they share the same ID counter. Consider using a ref inside the provider instead:

   const idRef = useRef(0)
   const id = idRef.current++

5. ReceiveQRCode.tsx — createBip21 is called in both a useEffect and inline — The function reads from component state (recvInfo, swapAddress, invoice, etc.) but is not memoized. Each render recreates it. Since it is used in effects, consider wrapping it with useCallback with appropriate dependencies, or restructuring so the effect computes the value directly.

Low Priority / Nits

6. Keyboard.tsx — asset?.assetId guard — Changed from asset to asset?.assetId for both inputMode initialization and the secondary display toggle. Good fix — prevents treating an empty AssetOption (with assetId: "") as a valid asset.

7. Removed comments in App.tsx — Several explanatory comments about boot animation flow were removed. The code is clear enough without them, but the original "If we land on any non-Loading page…" comment was useful context for the fly-up exit mode. Minor.

8. SheetModal.tsx — hapticLight on close — Nice touch for mobile feel, but consider gating on platform (not all users are on mobile/capacitor).

9. CSS !important on ion-modal z-index — z-index: 200 !important is a smell. Verify this doesn not conflict with other overlays (action sheets, alerts).

10. wallet.tsx — setAuthState("authenticated") before initWallet — In the dev auto-init path, auth state is set before the wallet initializes. If initWallet fails, the user appears authenticated with no wallet. The consoleError catches it but the state is left inconsistent. Consider resetting authState in the catch block.

Security

No security concerns — this PR is entirely UI/UX. No key handling, transaction signing, or sensitive data flow changes. The VITE_DEV_NSEC handling is gated behind import.meta.env.DEV, which is correct.

Cross-Repo Impact

• Blocked by LNURL work (as documented in the PR description) — showing QR first without an amount means no Lightning option until LNURL zero-invoice support lands.
• No SDK or server-side changes required.

Summary

Well-structured UX improvement with good accessibility practices. The main actionable items are: (1) remove the accidental package-lock.json, (2) consider the createBip21 memoization, and (3) handle the dev auto-init auth state edge case. Everything else is minor polish.

[sahilc0]

@Kukks The QR already has a 2-module quiet zone built into the SVG (on top of container padding) — border: 0 in the encoder, but then quietZone = moduleSize * 2 added manually around the data. Modern scanners handle this fine. No visible border needed.

@bordalix Looks like you fixed the asset mobile receive with eaefd87 — thanks! Are we good to merge?

[bordalix]

We should merge master and verify LNURL is working. When I ran the branch, there's no LNURL support.

[bordalix]

Yes, I fixed the mobile asset amount. It seems the comment update didn't went through, it has a demo showing it working properly.

[sahilc0]

Closing — all changes were absorbed into master via PR #512 (Receive v2).