Skip to content

feat(wallet): redesign home and settings entry points - #627

Merged
bordalix merged 111 commits into
masterfrom
wt-wallet-redesign-followups-20260520
Jul 7, 2026
Merged

bordalix merged 111 commits into
masterfrom
wt-wallet-redesign-followups-20260520

Conversation

@sahilc0

@sahilc0 sahilc0 commented May 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Brings over the scoped wallet home redesign foundation and composition from the draft branch: heading typography, shared haptics, motion constants, sticky home header, portfolio hero, quick actions, real wallet assets, upsells, and recent activity preview.
  • Keeps the home backed by real wallet data, without hardcoded demo assets, and hides the homepage activity preview when there are no transactions.
  • Removes the global bottom nav for the redesigned flow and ports the settings redesign, including Arkade Mint under Settings > General with back navigation returning to settings.
  • Adds the “Swaps are coming soon” drawer for the home Swap action and keeps Scan deep-linking into the send scanner.

QA notes

  • Boltz receive/swap QA still needs manual coverage.
  • Arkade Mint needs manual QA now that it lives in Settings, including verifying newly minted assets appear in the wallet.

Testing

  • bun run format:check
  • bun run lint
  • bun run build
  • bun run test:unit -- src/test/App.test.tsx src/test/screens/wallet/index.test.tsx
  • Browser smoke: opened the local app, verified wallet home loads, Activity header alignment, and full Activity refresh showing received rows.

Build passed with the existing Vite warnings about publicDir, mnemonic import chunking, and bundle size.

Summary by CodeRabbit

  • New Features

    • Haptic feedback for interactive actions
    • Privacy controls to mask balances and fiat amounts
    • Redesigned wallet home: header, portfolio hero, quick actions (Receive, Send, Swap, Scan), assets & upsell sections
    • Dedicated Activity screen and compact Recent Activity preview
    • Token logos for common tickers and improved currency formatting
  • Improvements

    • Smoother animations and reduced-motion handling
    • Improved navigation/back-stack behavior and settings flow
    • Updated typography, icon color behavior, and UI styling consistency

Review Change Stack

sahilc0 added 3 commits May 20, 2026 13:54
Port the scoped home redesign foundation and bitcoin-only wallet home from the draft branch. Adds shared motion and haptics helpers, redesigned home header/hero/actions/assets/upsells/activity preview, hides bottom navigation for home-launched wallet flows, and wires scan/swap interactions without pulling in passkeys or prototype asset accounts.
Remove the bottom nav from the redesigned wallet flow and align the settings screens with the draft redesign, including the Arkade Mint entry under General.

Refresh the full Activity page before showing an empty state, keep the home activity preview hidden when empty, and use the shared secondary/drawer styling for the swap coming-soon sheet.
@sahilc0
sahilc0 requested review from bordalix and pietro909 May 20, 2026 21:34
@coderabbitai

coderabbitai Bot commented May 20, 2026 •

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Refactors the wallet home and portfolio UI, adds privacy-masked balance components and portfolio hooks, migrates haptics to web-haptics, introduces token logos and icon updates, adds replace()-based navigation and dev auto-init boot/watchdog plumbing, updates animations and styles, and centralizes E2E navigation helpers and tests.

Changes

Wallet Home & Portfolio Redesign

Layer / File(s) Summary
Dev Auto-Init Boot Flow & Service Worker
src/index.tsx, src/providers/wallet.tsx, src/App.tsx
Service worker reset with sessionStorage guard; WalletProvider dev-auto-init watchdog and devAutoInitFailed flag; App redirect gating, loading rendering, and animation suppression when dev auto-init is active.
Formatting, Haptics, Animations
src/lib/format.ts, src/lib/haptics.ts, src/lib/animations.ts, package.json
Added configurable fiat/asset formatting APIs, migrated haptics to web-haptics with named patterns and reduced-motion gating, and adjusted Framer Motion stagger/duration variants. web-haptics dependency added.
Portfolio Data Hooks
src/hooks/usePortfolioFiat.ts, src/hooks/usePortfolioBalanceDisplay.ts
Hooks build PortfolioRow entries (including synthetic BTC row), compute total fiat, and format/mask total portfolio balance for UI consumption.
Privacy & Balance Masking
src/components/PrivacyAmount.tsx, src/components/AssetCard.tsx, src/lib/format.ts
PrivacyAmount component (interactive toggle + animations), maskedFiat helper, AssetCard refactor to use PrivacyAmount and prettyCurrencyAssetAmount, fiatText masking, and token-logo selection.
Token Logos & Icon System
src/components/TokenLogo.tsx, src/icons/*
TokenLogoTicker and TokenLogo with specific coin/flag SVG components; multiple icon components added/updated to use currentColor and size props; Settings/Close replaced inline SVGs.
Home Composition: Header, Hero, Quick Actions
src/screens/Wallet/HomeHeader.tsx, src/screens/Wallet/PortfolioHero.tsx, src/screens/Wallet/HomeQuickActions.tsx
HomeHeader with masked balance preview and navigation actions, PortfolioHero with collapse-driven transforms, HomeQuickActions toolbar and SwapComingSoonSheet modal.
Home Sections: Assets, Upsells, Recent Activity
src/screens/Wallet/AssetsSection.tsx, src/screens/Wallet/UpsellsSection.tsx, src/screens/Wallet/RecentActivitySection.tsx
AssetsSection maps portfolio rows to AssetCard with fiatText; UpsellsSection renders upsell cards navigating to apps; RecentActivitySection shows a 3-item TransactionsList preview.
Activity & Transactions List
src/screens/Wallet/Activity.tsx, src/components/TransactionsList.tsx
Activity screen with guarded wallet refresh; TransactionsList now supports static/virtual modes, asset filtering, privacy amounts, token logos, and updated virtualization keys/size estimates.
Wallet Screen Integration
src/screens/Wallet/Index.tsx
Wallet screen refactored to staged home sections, scroll-driven collapse progress for header/hero, PWA banner gating, and replaced legacy transactions/balance rendering.
Settings UI & Components
src/components/Menu.tsx, src/components/Select.tsx, src/components/Toggle.tsx, src/components/Content.tsx, src/components/WalletLoadIn.tsx, src/components/ui/drawer.tsx
Menu switched to native buttons with haptic selection and ArkadeMint routing; Select/Toggle rewrites to settings-* classes; Content/WalletLoadIn accept className; drawer Content className adjusted.
Settings Screens & Menu Flow
src/screens/Settings/*
Settings pages wrapped in settings-page/section containers, General refactored to native button rows, SettingsMenu accepts optional backFunc, Index passes menuBack when in wallet-launched settings flow.
Navigation: Pages & replace()
src/providers/navigation.tsx
Pages enum extended (Activity, WalletSettings); navigate memoized; new replace(page, backTo?) action with back-stack truncation; dev E2E hook exposes ARKADE_E2E_NAVIGATE; Provider value includes replace.
Asset Screen Navigation (replace flows)
src/screens/Apps/Assets/*, src/screens/Apps/Dfx/Index.tsx, src/screens/Apps/Lendasat/Index.tsx
Asset screens adopt replace() for exit flows with back-stack context; headers simplified to boolean back prop; some app screens remove direct NavigationContext dependency.
Types & Options
src/lib/types.ts, src/providers/options.tsx, src/providers/flow.tsx
Added SettingsOptions.ArkadeMint; options list updated with icons (CoinsIcon, HashIcon, ServerIcon, LockIcon); SendInfo gains optional scan flag.
Styles & Tokens
src/app.css, src/index.css
Heading typography token mapping updated to --font-family-heading/weight 500; text-heading utilities added; large home and settings stylesheet additions and control-secondary token usage; header divider shadow introduced.
E2E Navigation Helpers & Tests
src/test/e2e/utils.ts, src/test/*, src/test/screens/mocks.ts
navigateToSettings exported and rewritten; navigateHome and navigateToBoltz added; wallet creation/fund/send/receive helpers updated; many E2E tests refactored to use helpers and adjusted assertions; mockNavigationContextValue includes replace stub.
Send Form QR Scanner Flow
src/screens/Wallet/Send/Form.tsx
Added useEffect to react to sendInfo.scan: clears flag, resets keys, opens scanner overlay, persists updated sendInfo.

Estimated code review effort: 🎯 4 (Complex) | ⏱️ ~75 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wt-wallet-redesign-followups-20260520

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

Comment @coderabbitai help to get the list of available commands.

@sahilc0

sahilc0 commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@bordalix heads up for QA: we need a manual pass on Boltz swaps to make sure receives still work, and on Arkade Mint now that it lives in Settings. Please verify Arkade Mint works properly and that assets show up in the wallet after they are minted.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 20, 2026 •

Copy link
Copy Markdown

Deploying tmp-boltz-upstream-mainnet-arkade-wallet with  Cloudflare Pages  Cloudflare Pages

Latest commit: ccb4f80
Status: ✅  Deploy successful!
Preview URL: https://b88bc2fa.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev
Branch Preview URL: https://wt-wallet-redesign-followups.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 20, 2026 •

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: ccb4f80
Status: ✅  Deploy successful!
Preview URL: https://2a0d830d.wallet-bitcoin.pages.dev
Branch Preview URL: https://wt-wallet-redesign-followups.wallet-bitcoin.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 20, 2026 •

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: ccb4f80
Status: ✅  Deploy successful!
Preview URL: https://b105f70d.arkade-wallet.pages.dev
Branch Preview URL: https://wt-wallet-redesign-followups.arkade-wallet.pages.dev

View logs

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arkana Code Review — PR #627

Verdict: Approve — This is a UI-only redesign (home screen, settings, nav). No protocol-critical code (VTXO handling, signing, forfeit, round lifecycle, exit paths) is modified. SDK bumps (0.4.24→0.4.27, boltz 0.3.28→0.3.32) are backward-compatible (HD receive rotation, WS resilience, VTXO reconciliation fixes).

Issues (non-blocking)

1. Stale closure risk in Send/Form.tsx scan effect
src/screens/Wallet/Send/Form.tsx:122-129 — The useEffect depends on [sendInfo.scan] but spreads the full sendInfo object. If other sendInfo fields change between renders without scan changing, the spread captures stale state. In practice this fires once on mount then deletes the flag, so it's low-risk, but the ESLint exhaustive-deps rule will flag it. Consider destructuring or adding sendInfo to deps.

2. SVG clipPath ID collision in TokenLogo.tsx
src/components/TokenLogo.tsx:63,80 — UnitedStatesFlagLogo and BrazilFlagLogo use hardcoded clipPath IDs (us-flag-circle, br-flag-circle). If multiple instances of the same flag render on screen (e.g. two USD asset rows), the duplicate IDs will cause the second SVG to reference the first's clip region. Fix: use useId() or inline the clip path.

3. BigInt conversion fragility in AssetCard.tsx
src/components/AssetCard.tsx:31 — BigInt(balance) will throw for non-integer numbers. The balance prop accepts bigint | number but BigInt() rejects floats. If any consumer passes a fractional number, this crashes. Consider BigInt(Math.round(balance)) or tightening the type.

4. DOM query for scroll tracking is fragile
src/screens/Wallet/Index.tsx:82 — document.querySelector('.wallet-home-content') couples scroll tracking to a CSS class name. A ref passed to <Content> would be more resilient to refactors.

5. web-haptics is v0.0.6 (pre-1.0)
src/lib/haptics.ts — The library is very new. The .catch(() => {}) on trigger() is good defensive coding, but pin the version in package.json (currently ^0.0.6 allows any 0.0.x bump). Pre-1.0 semver means even patches can break.

What looks good

  • Clean removal of PillNavbarOverlay and radix-ui dependency tree — good dependency hygiene.
  • PrivacyAmount component is well-structured with proper aria-label/aria-pressed for accessibility.
  • Dev auto-init watchdog with session-storage reload guard is solid DX improvement.
  • matchesAssetFilter in TransactionsList handles edge cases correctly.
  • Tests updated to match the new nav behavior (navbar removal assertions flipped).
  • Vtxos.tsx restructuring with WaitingForRound is a cleaner separation of concerns.

SDK bumps — no action needed

  • @arkade-os/sdk 0.4.27: HD receive rotation via contract repo, VTXO reconciliation by outpoint, pre-flight VTXO validation. All backward-compatible.
  • @arkade-os/boltz-swap 0.3.32: WS fallback polling, SW restart recovery, default referral ID (wallet overrides with arkade-money). No breaking changes.

LGTM for merge after human sign-off.

Update Playwright helpers and specs to navigate through the redesigned home header, settings entry points, and Boltz deep link instead of the removed bottom nav.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/providers/wallet.tsx (1)

263-302: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add missing dependencies to effect dependency array.

The effect at line 263 uses isDevAutoInit, devAutoInitFailed, and initialized in its conditional logic but only includes wallet.pubkey in the dependency array. If devAutoInitFailed changes to true without a pubkey change, the effect won't re-run and authState can become stale.

🔧 Proposed fix
-  }, [wallet.pubkey])
+  }, [wallet.pubkey, isDevAutoInit, devAutoInitFailed, initialized])
🤖 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 `@src/providers/wallet.tsx` around lines 263 - 302, The effect in the hook uses
isDevAutoInit, devAutoInitFailed and initialized (and also the helper functions
like hasMnemonic/getMnemonic/noUserDefinedPassword/defaultPassword) but only
lists wallet.pubkey in the dependency array, so update the useEffect dependency
list to include isDevAutoInit, devAutoInitFailed, initialized (and any stable
helper refs if they are not stable) so the effect re-runs when those values
change; locate the effect surrounding detectPasswordState in the wallet provider
(the useEffect block that sets authState) and add the missing dependencies to
the dependency array accordingly.
🤖 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 `@src/components/AssetCard.tsx`:
- Around line 38-39: The component is calling BigInt(balance) without validating
balance which can throw on non-integer/NaN/Infinity values; update the logic
around rawBalance and prettyBalance in AssetCard (symbols: balance, rawBalance,
prettyBalance, prettyCurrencyAssetAmount, decimals, tokenTick) to first validate
balance (e.g., allow bigint, numeric finite integers, or integer-like numeric
strings), perform safe coercion inside a guarded block or try/catch, and fall
back to a safe value (such as BigInt(0) or null/placeholder) when validation
fails so rendering won't crash; ensure the chosen fallback is propagated into
prettyCurrencyAssetAmount or handled so the component renders gracefully.

In `@src/components/PrivacyAmount.tsx`:
- Around line 51-53: The aria-pressed value on the visibility toggle is
inverted: in the PrivacyAmount component set aria-pressed to reflect the current
"balances visible" state (visible) instead of its negation; update the toggle
rendering where aria-pressed is used so it reads aria-pressed={visible} (keep
the existing aria-label logic and className/cn usage intact).

In `@src/components/TokenLogo.tsx`:
- Line 4: The current conditional in TokenLogo.tsx returns UnitedStatesFlagLogo
for USD, USDT, and USDC; change it so only 'USD' returns UnitedStatesFlagLogo
and add explicit branches for 'USDT' and 'USDC' that return their dedicated
token logo components (e.g., USDTLogo/TetherLogo and USDCLogo/UsdcLogo) instead
of the flag; update the conditional or switch in the TokenLogo component to
check ticker === 'USD' -> UnitedStatesFlagLogo, ticker === 'USDT' -> USDT token
logo component, and ticker === 'USDC' -> USDC token logo component so each asset
displays the correct logo.

In `@src/components/TransactionsList.tsx`:
- Line 185: The current assignment for txs uses a truthy check so limit=0 is
treated as "no limit"; change the condition to explicitly test for a numeric
limit (e.g., typeof limit === 'number' or Number.isFinite(limit') and optionally
limit >= 0) so that when normalizedMode === 'static' and limit is 0 you return
visibleTxs.slice(0, 0) (empty) instead of the full list; update the expression
that defines txs (currently using normalizedMode, limit, visibleTxs)
accordingly.
- Around line 247-261: In the static-mode branch (where normalizedMode ===
'static') the TransactionLine entries are clickable but not keyboard-focusable;
update the rendered TransactionLine (or its wrapper div) so each row is
focusable (tabIndex=0), has an appropriate role (e.g., role="button"), and
handles keyboard activation by wiring onKeyDown to call handleClick(tx) when
Enter or Space is pressed; ensure the key generation (key(tx, index)), txs.map,
TransactionLine, handleClick, and normalizedMode are the identifiers you modify
so keyboard users can focus and activate rows the same way as mouse users.

In `@src/components/WaitingForRound.tsx`:
- Around line 23-32: The current useEffect uses sleep(2000).then(...) so if the
component unmounts before the 2s elapses the interval can still start and leak;
replace the sleep promise with a cancellable timer: declare both timeout and
interval handles (e.g. let timeoutId: NodeJS.Timeout, intervalId:
NodeJS.Timeout), use timeoutId = setTimeout(() => { intervalId = setInterval(()
=> setLogLength(getInfoLogsLength()), 500) }, 2000), and in the cleanup call
clearTimeout(timeoutId) and clearInterval(intervalId) to ensure setInterval
never starts after unmount; keep references to the same identifiers used in this
file (useEffect, setLogLength, getInfoLogsLength).

In `@src/index.css`:
- Around line 607-620: Keyboard focus is currently invisible for the new
interactive controls because .home-section-action (and the other interactive
control selectors added in this diff) have no :focus-visible styles and global
outline reset hides focus; add a clear, accessible focus style by defining
.home-section-action:focus-visible (and the same :focus-visible rule for the
other interactive selectors in this change) to restore a visible
outline/box-shadow (e.g., a 2px solid or ring-like box-shadow using the existing
theme color variable like var(--purple-700) or a high-contrast color), include a
border-radius/padding-safe offset so it doesn't shift layout, and add a fallback
.home-section-action:focus for browsers without :focus-visible support so
keyboard users always see a visible focus indicator.
- Around line 1645-1647: Update the .settings-page padding shorthand to use the
CSS variable --pill-navbar-footer-clearance with a fallback so the declaration
doesn't become invalid when the variable is unset; replace the current third
padding value with a var() expression that provides a safe fallback (e.g., 0)
for --pill-navbar-footer-clearance so the top/right/bottom/left padding remains
valid.

In `@src/lib/format.ts`:
- Around line 84-87: The code converts Decimal/BigInt to JS number (e.g.
Decimal.div(...).toNumber(), Number(amount), Number(centsToUnits(...))) which
loses precision for large values; instead preserve the Decimal/BigInt domain and
format as strings: keep unitAmount as a Decimal (remove .toNumber()), and update
calls into prettyNumber to accept a Decimal/string (or pass
Decimal.toFixed(fiatDecimals) to produce a precise string), and replace
Number(amount)/Number(centsToUnits(...)) with BigInt/Decimal-safe string
conversions; modify prettyNumber (or add a wrapper) to use Intl.NumberFormat on
the string or format grouping via string manipulation so no intermediate number
is created. Ensure you update references to fiatDecimalsFor, useGrouping and
centsToUnits accordingly so formatting consumes precise string/Decimal inputs
rather than JS numbers.

In `@src/screens/Settings/General.tsx`:
- Around line 17-21: The button in General.tsx is overriding the accessible name
via aria-label which hides the visible row text/value from screen readers;
remove the aria-label on the settings-row button (the JSX button with className
'settings-row settings-row--value') so the browser uses the visible content as
the accessible name, or alternatively implement aria-labelledby that references
the label and value element IDs so both label and current value are announced
(ensure those elements have stable ids and update the button to use
aria-labelledby if choosing that approach).

---

Outside diff comments:
In `@src/providers/wallet.tsx`:
- Around line 263-302: The effect in the hook uses isDevAutoInit,
devAutoInitFailed and initialized (and also the helper functions like
hasMnemonic/getMnemonic/noUserDefinedPassword/defaultPassword) but only lists
wallet.pubkey in the dependency array, so update the useEffect dependency list
to include isDevAutoInit, devAutoInitFailed, initialized (and any stable helper
refs if they are not stable) so the effect re-runs when those values change;
locate the effect surrounding detectPasswordState in the wallet provider (the
useEffect block that sets authState) and add the missing dependencies to the
dependency array accordingly.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e690bbcb-efb3-4203-b2e5-84d917f6a2b6

📥 Commits

Reviewing files that changed from the base of the PR and between 9848c02 and 9fb989d.

⛔ Files ignored due to path filters (2)
  • bun.lock is excluded by !**/*.lock
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (72)
  • package.json
  • src/App.tsx
  • src/app.css
  • src/components/AssetCard.tsx
  • src/components/Content.tsx
  • src/components/Menu.tsx
  • src/components/PrivacyAmount.tsx
  • src/components/Select.tsx
  • src/components/Toggle.tsx
  • src/components/TokenLogo.tsx
  • src/components/TransactionsList.tsx
  • src/components/WaitingForRound.tsx
  • src/components/WalletLoadIn.tsx
  • src/components/ui/drawer.tsx
  • src/hooks/usePortfolioBalanceDisplay.ts
  • src/hooks/usePortfolioFiat.ts
  • src/icons/Coins.tsx
  • src/icons/History.tsx
  • src/icons/Landmark.tsx
  • src/icons/Logo.tsx
  • src/icons/Preconfirmed.tsx
  • src/icons/Receive.tsx
  • src/icons/Received.tsx
  • src/icons/Scan.tsx
  • src/icons/Send.tsx
  • src/icons/Settings.tsx
  • src/index.css
  • src/index.tsx
  • src/lib/animations.ts
  • src/lib/format.ts
  • src/lib/haptics.ts
  • src/lib/types.ts
  • src/providers/flow.tsx
  • src/providers/navigation.tsx
  • src/providers/options.tsx
  • src/providers/wallet.tsx
  • src/screens/Apps/Assets/Index.tsx
  • src/screens/Apps/Dfx/Index.tsx
  • src/screens/Apps/Lendasat/Index.tsx
  • src/screens/Settings/Advanced.tsx
  • src/screens/Settings/Display.tsx
  • src/screens/Settings/Fiat.tsx
  • src/screens/Settings/General.tsx
  • src/screens/Settings/Haptics.tsx
  • src/screens/Settings/Index.tsx
  • src/screens/Settings/Menu.tsx
  • src/screens/Settings/Theme.tsx
  • src/screens/Settings/Vtxos.tsx
  • src/screens/Wallet/Activity.tsx
  • src/screens/Wallet/AssetsSection.tsx
  • src/screens/Wallet/HomeHeader.tsx
  • src/screens/Wallet/HomeQuickActions.tsx
  • src/screens/Wallet/Index.tsx
  • src/screens/Wallet/PortfolioHero.tsx
  • src/screens/Wallet/RecentActivitySection.tsx
  • src/screens/Wallet/Send/Form.tsx
  • src/screens/Wallet/UpsellsSection.tsx
  • src/test/App.test.tsx
  • src/test/e2e/backup.test.ts
  • src/test/e2e/bip21.test.ts
  • src/test/e2e/delegate.test.ts
  • src/test/e2e/init.test.ts
  • src/test/e2e/keyboard.test.ts
  • src/test/e2e/lnurl.test.ts
  • src/test/e2e/lock.test.ts
  • src/test/e2e/nostr.test.ts
  • src/test/e2e/receive.test.ts
  • src/test/e2e/restore.test.ts
  • src/test/e2e/send.test.ts
  • src/test/e2e/swap.test.ts
  • src/test/e2e/utils.ts
  • src/test/screens/wallet/index.test.tsx
💤 Files with no reviewable changes (1)
  • src/icons/Scan.tsx

Comment thread src/components/AssetCard.tsx Resolved
Comment thread src/components/PrivacyAmount.tsx Resolved
Comment thread src/components/TokenLogo.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/WaitingForRound.tsx Resolved
Comment thread src/index.css Resolved
Comment thread src/index.css Resolved
Comment thread src/lib/format.ts Resolved
Comment thread src/screens/Settings/General.tsx Resolved
Use the settings header text rendered by the shared Header component and assert the redesigned zero-balance home through stable test ids.

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arkana Code Review — PR #627 (incremental, post-push)

Scope of new commits (9fb989d1, 8df89a2c): E2E test refactoring only — adapts all Playwright specs to the redesigned navigation (bottom tab bar removed, replaced with header icon buttons).

Reviewed:

  1. navigateHome() — Bounded back-button loop (max 8 iterations) with page.goto('/') fallback. Sentinel: home-action-receive testid. Clean and robust.

  2. navigateToSettings() — Early-return if "Settings" heading already visible, otherwise navigates home first then clicks top-right-settings. Correct.

  3. navigateToBoltz() — Direct hash navigation page.goto('/#app+boltz'). Clean workaround for removed Apps tab.

  4. navigateToAssets() — Now routes through Settings → "Arkade Mint" to match the UI relocation. Consistent with the main PR changes.

  5. init.test.ts — Assertions correctly flipped: tab-wallet/tab-apps/tab-settings are now not.toBeVisible(), replaced by top-right-activity/top-right-settings. No transactions yet correctly hidden (activity preview absent when empty).

  6. All other test files — Mechanical tab-* → helper replacements. No logic changes.

No protocol-critical code. No security concerns. LGTM.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arkana Code Review — PR #627 (incremental, commit 5811a172)

Scope: Adds a dev-only __ARKADE_E2E_NAVIGATE__ window hook so Playwright tests can navigate to app screens (e.g. Boltz) without page.goto() hash hacks, plus timeout bumps for mintAsset.

Reviewed:

  1. navigate wrapped in useCallback(…, []) — All dependencies are refs (previousPage, backStack, screenRef, skipNextPopstate), module-level constants (ROOT_PAGES, pageTab, subNavHandler), or stable state setters (setDirection, setScreen, setTab). Empty dep array is correct.

  2. Dev-only useEffect — Guarded by import.meta.env.DEV, tree-shaken from production builds. Cleanup deletes the window property. [navigate] dep is correct (stable due to useCallback).

  3. navigateToBoltz() test helper — Replaces brittle page.goto('/#app+boltz') hash navigation with page.evaluate() calling the new hook. Throws explicitly if hook is unavailable. Clean.

  4. mintAsset timeout bumps (3s→10s, 30s→60s) — Reduces flaky test failures. No-op in production.

No protocol-critical code. No security concerns. LGTM.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arkana Code Review — PR #627 (incremental, commit 5811a172)

Scope: Adds a dev-only __ARKADE_E2E_NAVIGATE__ window hook so Playwright tests can navigate to app screens (e.g. Boltz) without page.goto() hash hacks, plus timeout bumps for mintAsset.

Reviewed:

  1. navigate wrapped in useCallback(…, []) — All dependencies are refs (previousPage, backStack, screenRef, skipNextPopstate), module-level constants (ROOT_PAGES, pageTab, subNavHandler), or stable state setters (setDirection, setScreen, setTab). Empty dep array is correct.

  2. Dev-only useEffect — Guarded by import.meta.env.DEV, tree-shaken from production builds. Cleanup deletes the window property. [navigate] dep is correct (stable due to useCallback).

  3. navigateToBoltz() test helper — Replaces brittle page.goto('/#app+boltz') hash navigation with page.evaluate() calling the new hook. Throws explicitly if hook is unavailable. Clean.

  4. mintAsset timeout bumps (3s→10s, 30s→60s) — Reduces flaky test failures. No-op in production.

No protocol-critical code. No security concerns. LGTM.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/test/e2e/utils.ts (1)

69-72: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reuse waitForWalletPage() after the hard-reload fallback.

Line 70 reloads the app, but this path skips the BootError / “Continue anyway” handling that already lives in waitForWalletPage(). If the fallback hits that overlay, navigateHome() just waits for home-action-receive until it times out.

Proposed fix
   if (!(await homeReceive.isVisible().catch(() => false))) {
     await page.goto('/')
+    await waitForWalletPage(page)
   }
   await homeReceive.waitFor({ state: 'visible', timeout: 30000 })
🤖 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 `@src/test/e2e/utils.ts` around lines 69 - 72, The hard-reload fallback in
navigateHome() currently does page.goto('/') then waits directly for homeReceive
which skips BootError / “Continue anyway” handling; after calling page.goto('/')
replace the direct waitFor({ state: 'visible' ... }) on homeReceive with a call
to waitForWalletPage() so the existing overlay handling runs (i.e., invoke await
waitForWalletPage() after the reload and remove or replace the
homeReceive.waitFor(...) line).
🧹 Nitpick comments (1)
src/providers/navigation.tsx (1)

341-348: ⚡ Quick win

Don’t rely on import.meta.env.DEV for the E2E navigation hook.

With the current Playwright setup (webServer.command runs pnpm start, i.e. Vite dev server), import.meta.env.DEV should be true so window.__ARKADE_E2E_NAVIGATE__ exists during normal test:e2e runs. Still, the setup is brittle: any switch to a production-like server (vite build + vite preview, etc.) would make DEV false and cause navigateToBoltz() to hard-fail when the hook is missing. Gate the hook behind an explicit E2E flag/env var (or add a UI fallback when the hook isn’t present).

🤖 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 `@src/providers/navigation.tsx` around lines 341 - 348, The effect currently
registers window.__ARKADE_E2E_NAVIGATE__ only when import.meta.env.DEV is true
which is brittle; change the guard to rely on an explicit E2E flag (e.g.
import.meta.env.VITE_E2E === 'true' or a runtime window flag) and/or always
register a no-op safe hook so callers like navigateToBoltz() never hard-fail;
specifically update the useEffect that defines __ARKADE_E2E_NAVIGATE__ to check
import.meta.env.VITE_E2E (or a runtime window.__ARKADE_E2E_ENABLED__) instead of
import.meta.env.DEV and ensure navigate(Pages[page]) is only invoked when
navigate and Pages are available (or provide a fallback no-op implementation) so
tests work regardless of dev vs preview server.
🤖 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.

Outside diff comments:
In `@src/test/e2e/utils.ts`:
- Around line 69-72: The hard-reload fallback in navigateHome() currently does
page.goto('/') then waits directly for homeReceive which skips BootError /
“Continue anyway” handling; after calling page.goto('/') replace the direct
waitFor({ state: 'visible' ... }) on homeReceive with a call to
waitForWalletPage() so the existing overlay handling runs (i.e., invoke await
waitForWalletPage() after the reload and remove or replace the
homeReceive.waitFor(...) line).

---

Nitpick comments:
In `@src/providers/navigation.tsx`:
- Around line 341-348: The effect currently registers
window.__ARKADE_E2E_NAVIGATE__ only when import.meta.env.DEV is true which is
brittle; change the guard to rely on an explicit E2E flag (e.g.
import.meta.env.VITE_E2E === 'true' or a runtime window flag) and/or always
register a no-op safe hook so callers like navigateToBoltz() never hard-fail;
specifically update the useEffect that defines __ARKADE_E2E_NAVIGATE__ to check
import.meta.env.VITE_E2E (or a runtime window.__ARKADE_E2E_ENABLED__) instead of
import.meta.env.DEV and ensure navigate(Pages[page]) is only invoked when
navigate and Pages are available (or provide a fallback no-op implementation) so
tests work regardless of dev vs preview server.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c7716bfb-24ba-4920-bcbc-b71a5d02dfae

📥 Commits

Reviewing files that changed from the base of the PR and between 9fb989d and 5811a17.

📒 Files selected for processing (3)
  • src/providers/navigation.tsx
  • src/test/e2e/init.test.ts
  • src/test/e2e/utils.ts

@bordalix

Copy link
Copy Markdown
Collaborator

@sahilc0 homepage balance doesn't obey to "sats only" option, i.e. balance is always in fiat, even when the user selects to see sats only values.

@bordalix

Copy link
Copy Markdown
Collaborator

Eternal loop between asset mint and asset.
To exit the only solution is to reload the app.

Screen.Recording.2026-05-21.at.14.52.49.mov

@bordalix

Copy link
Copy Markdown
Collaborator

@sahilc0 I think we should use the asset ticker instead of name here

Screenshot 2026-05-21 at 15 08 26

@bordalix

Copy link
Copy Markdown
Collaborator

@sahilc0 WaitingForRound.tsx‎ is back?

@tiero

tiero commented Jul 1, 2026

Copy link
Copy Markdown
Member

Can we hide non-verified assets by default (ie. exists in this github page https://github.com/ArkLabsHQ/asset-registry)? They can still be shown in Arkade Mint eventually

image

@bordalix
bordalix merged commit 830b0bf into master Jul 7, 2026
6 checks passed
This was referenced Jul 24, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants