Skip to content

Keyboard navigation - #283

Merged
bordalix merged 17 commits into
masterfrom
keyboard_navigation
Dec 30, 2025
Merged

bordalix merged 17 commits into
masterfrom
keyboard_navigation

Conversation

@bordalix

@bordalix bordalix commented Dec 22, 2025 •

Copy link
Copy Markdown
Collaborator

Allow for keyboard navigation.

This is the first PR of a series of ARIA related PRs

TAB = go to next focusable element
SHIFT+TAB = go to previous focusable element

To enter the transaction list, focus on "Transaction history" and press Enter.
Press Esc to make transactions in list unfocusable again.
Same strategy for swap list in apps boltz

Summary by CodeRabbit

  • New Features

    • App-wide keyboard focusable interactions: Enter/Space activation, Escape to exit, Arrow keys to expand address lines, and keyboard navigation for lists, menus, cards, toggles and tab buttons.
  • Bug Fixes

    • Improved input focus behavior; removed a debug log; disabled Enter auto-submit on one amount input.
  • Style

    • Added focused-state styling and a theme variable for focused background.
  • Chores

    • Updated build base images.
  • Tests

    • E2E tests updated for new selectors and added explicit copy test IDs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 22, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds a new Focusable component and applies keyboard focus/activation across many UI elements; refactors input-focus effects to respond to a focus prop; unifies swap types into PendingSwap; adds focus CSS and ionic variable; introduces Shadow testId and small Docker base-image updates.

Changes

Cohort / File(s) Summary
Focusable + focus styles
src/components/Focusable.tsx, src/index.css, src/ionic.css
New Focusable component (tabIndex, onEnter/onEscape, role/aria). Global .focusable and *:focus-visible rules added; --ion-background-focused variable added and propagated to tab buttons.
App-level navigation & tabs
src/App.tsx
Global Escape key handler added (navigates to Wallet); bottom tab button contents wrapped with Focusable; padding adjustments to tab button inner layout.
Input focus refactor
src/components/InputAmount.tsx, src/components/InputPassword.tsx, src/components/InputWithScanner.tsx
Removed firstRun guards; focus effects now trigger on focus prop and use input refs; Max control wrapped in Focusable; IonInput ref/value consolidated in InputWithScanner.
Lists / focusable rows
src/components/ExpandAddresses.tsx, src/components/SwapsList.tsx, src/components/TransactionsList.tsx, src/components/Table.tsx, src/screens/Settings/Logs.tsx
Many lists and rows wrapped with Focusable; Enter/Escape handlers added to copy/activate rows; focus management scaffold added; SwapLine/TransactionLine signatures updated to accept onClick; SwapsList uses unified PendingSwap type and adds keyboard navigation.
Menus, settings, header, toggle, apps
src/components/Menu.tsx, src/screens/Settings/General.tsx, src/components/Header.tsx, src/components/Toggle.tsx, src/screens/Apps/Index.tsx
Menu rows, settings rows, header back/aux controls, toggles, and app cards wrapped with Focusable and wired to onEnter; minor layout/padding changes; SideButton signature simplified.
Swaps types & provider
src/lib/types.ts, src/providers/lightning.tsx, src/components/SwapsList.tsx
Added exported `PendingSwap = PendingReverseSwap
Forms & Receive behavior
src/screens/Wallet/Send/Form.tsx, src/screens/Wallet/Receive/Amount.tsx
Send form Enter-handling now conditions focus shifts on current focus; Receive Amount removed Enter-triggered proceed behavior.
Shadow/test IDs & minor cleanup
src/components/Shadow.tsx, src/components/Keyboard.tsx
Shadow gained optional testId?: string and emits data-testid; removed debug console.log in Keyboard.
E2E tests
src/test/e2e/nostr.test.ts, src/test/e2e/swap.test.ts
Updated selectors/test-ids (e.g., app-Boltz → app-boltz), added explicit copy test-ids and waits, and adjusted assertions to text-based SATS checks.
Icons & styling
src/icons/Settings.tsx
Settings icon stroke changed to currentColor to inherit text color.
Docker build updates
arkd.Dockerfile, arkdwallet.Dockerfile, nak.Dockerfile
Go builder image bumped from 1.25.3 → 1.25.5; nak.Dockerfile adds apk add --no-cache gcc musl-dev in builder stage.

Sequence Diagram(s)

(None — changes are pervasive UI accessibility additions and typing updates without a single new multi-component sequential flow that requires visualization.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Use SwapManager #265: Overlaps on swap typings and Lightning provider changes (migration to unified PendingSwap).
  • fix Playwright tests #244: Modifies the same e2e test files/selectors (nostr.test.ts, swap.test.ts) and test-id usage updated here.
  • new app card #220: Touches Shadow changes (adds/changes props like testId/border) and therefore overlaps with the Shadow testId addition.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Keyboard navigation' clearly and concisely summarizes the main objective of the changeset, which adds keyboard navigation support throughout the application.
✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

@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: 12

🧹 Nitpick comments (5)
src/screens/Wallet/Send/Form.tsx (1)

366-367: Smart focus management for keyboard navigation looks good!

The context-aware focus logic effectively guides users to missing fields based on their current position, which aligns well with the PR's keyboard accessibility goals.

💡 Optional: Make the amount check more explicit

The !amount check uses truthiness, which treats 0 as missing. While this works given that amounts < 1 are invalid anyway, being more explicit improves code clarity:

-    if (!amount && focus === 'recipient') setFocus('amount')
+    if ((amount === undefined || amount === 0) && focus === 'recipient') setFocus('amount')

Alternatively, if you want to keep it concise and only check for undefined:

-    if (!amount && focus === 'recipient') setFocus('amount')
+    if (amount === undefined && focus === 'recipient') setFocus('amount')
src/providers/navigation.tsx (1)

234-235: Consider using a more descriptive variable name.

The variable c is not very descriptive. Consider renaming it to mainElement or mainContent for better code readability.

🔎 Proposed refactor
-    const c = document.querySelector('[role="main"]') as HTMLElement | null
-    if (c) c.focus()
+    const mainElement = document.querySelector('[role="main"]') as HTMLElement | null
+    if (mainElement) mainElement.focus()
src/screens/Settings/General.tsx (1)

22-34: Consider consolidating keyboard and mouse handlers on a single element.

The current structure creates nested interactive elements: the outer div handles keyboard events while the inner FlexRow handles click events. This works but is not ideal for accessibility.

Consider consolidating both handlers on the same element to avoid nested interactivity and provide a simpler accessibility tree. You could add the onClick and onKeyDown handlers to the same element (either the div or the FlexRow).

🔎 Proposed refactor
   const Row = ({ option, value }: { option: SettingsOptions; value: string }) => (
-    <div className='focusable' tabIndex={0} onKeyDown={(e) => handleKeyDown(e, option)}>
-      <FlexRow between padding='0.8rem 0' onClick={() => setOption(option)}>
+    <div 
+      className='focusable' 
+      tabIndex={0} 
+      onKeyDown={(e) => handleKeyDown(e, option)}
+      onClick={() => setOption(option)}
+    >
+      <FlexRow between padding='0.8rem 0'>
         <Text capitalize thin>
           {option}
         </Text>
         <FlexRow end>
           <Text small thin color='dark50'>
             {value}
           </Text>
           <ArrowIcon />
         </FlexRow>
       </FlexRow>
     </div>
   )
src/components/ExpandAddresses.tsx (1)

37-56: Consider scoping the keyboard listener to avoid conflicts.

The global window keyboard listener for ArrowDown/ArrowUp may interfere with other components' keyboard navigation (e.g., scrolling lists, other arrow key handlers). Consider adding the listener only when the component is focused or checking if the target is within this component's DOM tree.

🔎 Suggested approach

Instead of a global window listener, consider adding the keyboard handler to a focused container:

-  useEffect(() => {
-    const handleArrowDown = (event: KeyboardEvent) => {
-      if (event.key === 'ArrowDown') {
-        if (!expand) {
-          setExpand(true)
-          bip21ref.current?.focus()
-        }
-      }
-      if (event.key === 'ArrowUp') {
-        if (expand) {
-          setExpand(false)
-        }
-      }
-    }
-    window.addEventListener('keydown', handleArrowDown)
-    return () => window.removeEventListener('keydown', handleArrowDown)
-  }, [expand])
+  const handleKeyDown = (event: React.KeyboardEvent) => {
+    if (event.key === 'ArrowDown' && !expand) {
+      setExpand(true)
+      bip21ref.current?.focus()
+    }
+    if (event.key === 'ArrowUp' && expand) {
+      setExpand(false)
+    }
+  }

Then attach onKeyDown={handleKeyDown} to the main container div.

src/components/SwapsList.tsx (1)

72-72: Consider renaming tabindex variable to tabIndex for consistency.

While functionally correct, the lowercase tabindex variable name (line 72) is inconsistent with React's tabIndex prop naming convention and may cause confusion.

🔎 Suggested fix
-  const tabindex = focusable ? 0 : -1
+  const tabIndexValue = focusable ? 0 : -1

And update the usage:

-      tabIndex={tabindex}
+      tabIndex={tabIndexValue}

Also applies to: 119-119

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e39fcfe and 36122c4.

📒 Files selected for processing (17)
  • src/App.tsx
  • src/components/ExpandAddresses.tsx
  • src/components/FlexCol.tsx
  • src/components/FlexRow.tsx
  • src/components/InputAmount.tsx
  • src/components/InputPassword.tsx
  • src/components/InputWithScanner.tsx
  • src/components/Menu.tsx
  • src/components/SwapsList.tsx
  • src/components/TransactionsList.tsx
  • src/index.css
  • src/ionic.css
  • src/providers/navigation.tsx
  • src/screens/Apps/Index.tsx
  • src/screens/Settings/General.tsx
  • src/screens/Wallet/Receive/Amount.tsx
  • src/screens/Wallet/Send/Form.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.

Applied to files:

  • src/components/InputPassword.tsx
  • src/components/InputAmount.tsx
  • src/screens/Wallet/Send/Form.tsx
  • src/components/InputWithScanner.tsx
📚 Learning: 2025-08-06T09:53:28.428Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 134
File: src/screens/Settings/Password.tsx:34-39
Timestamp: 2025-08-06T09:53:28.428Z
Learning: In src/screens/Settings/Password.tsx, the saveNewPassword function doesn't need error handling for getPrivateKey because it's only called after authentication is verified. The useEffect validates the oldPassword using isValidPassword() which internally calls getPrivateKey(), and only sets authenticated=true if successful. The saveNewPassword function has a guard clause that returns early if not authenticated, so getPrivateKey(oldPassword) is guaranteed to work when called.

Applied to files:

  • src/components/InputPassword.tsx
📚 Learning: 2025-12-20T10:09:41.030Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 278
File: src/providers/annoucements.tsx:32-50
Timestamp: 2025-12-20T10:09:41.030Z
Learning: In src/providers/annoucements.tsx, the useEffect hook depends on [config] (not just [configLoaded]) because it needs to react to config changes after nostr restoration. The seen.current ref guard prevents showing multiple announcements per session, even though the effect may re-run on config updates.

Applied to files:

  • src/components/InputPassword.tsx
  • src/components/InputAmount.tsx
  • src/components/ExpandAddresses.tsx
  • src/screens/Apps/Index.tsx
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.

Applied to files:

  • src/App.tsx
  • src/screens/Wallet/Send/Form.tsx
🧬 Code graph analysis (6)
src/App.tsx (3)
src/components/FlexCol.tsx (1)
  • FlexCol (17-48)
src/icons/Wallet.tsx (1)
  • WalletIcon (1-12)
src/icons/Apps.tsx (1)
  • AppsIcon (1-12)
src/components/ExpandAddresses.tsx (7)
src/components/FlexRow.tsx (1)
  • FlexRow (19-52)
src/components/FlexCol.tsx (1)
  • FlexCol (17-48)
src/components/Text.tsx (2)
  • TextSecondary (91-97)
  • Text (25-79)
src/lib/format.ts (1)
  • prettyLongText (76-83)
src/components/Shadow.tsx (1)
  • Shadow (18-55)
src/icons/CheckMark.tsx (1)
  • CheckMarkIcon (1-11)
src/icons/Copy.tsx (1)
  • CopyIcon (1-13)
src/components/SwapsList.tsx (2)
src/components/Empty.tsx (1)
  • EmptySwapList (46-54)
src/components/Text.tsx (1)
  • TextLabel (81-89)
src/components/TransactionsList.tsx (2)
src/lib/types.ts (1)
  • Tx (71-81)
src/components/Text.tsx (3)
  • Text (25-79)
  • TextSecondary (91-97)
  • TextLabel (81-89)
src/components/Menu.tsx (4)
src/components/FlexCol.tsx (1)
  • FlexCol (17-48)
src/components/FlexRow.tsx (1)
  • FlexRow (19-52)
src/components/Text.tsx (1)
  • Text (25-79)
src/icons/Arrow.tsx (1)
  • ArrowIcon (1-7)
src/screens/Settings/General.tsx (3)
src/components/FlexRow.tsx (1)
  • FlexRow (19-52)
src/components/Text.tsx (1)
  • Text (25-79)
src/icons/Arrow.tsx (1)
  • ArrowIcon (1-7)
🪛 Biome (2.1.2)
src/components/TransactionsList.tsx

[error] 58-58: Do not shadow the global "Date" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages: wallet-mutinynet
🔇 Additional comments (14)
src/components/TransactionsList.tsx (2)

1-1: LGTM!

The additional React hooks are correctly imported for the focus management implementation.


17-17: LGTM!

The signature change and tabindex computation correctly implement the focusable behavior for keyboard navigation.

Also applies to: 26-26

src/screens/Wallet/Receive/Amount.tsx (1)

119-119: LGTM! Good defensive programming.

The early return guard prevents proceeding without a valid amount, providing defense-in-depth alongside the existing disabled button logic.

src/index.css (2)

157-159: Verify that all focusable elements have visible focus indicators.

Removing the default outline for :focus-visible can harm keyboard accessibility if not all interactive elements provide alternative focus indicators. Ensure that all focusable elements (buttons, links, inputs, custom interactive elements) have visible focus states, either through the .focusable class or other styling mechanisms.


161-166: LGTM! Custom focus styling aligns with the design system.

The .focusable class provides consistent focus styling using the theme's focus background variable, supporting keyboard accessibility across the application.

src/screens/Settings/General.tsx (1)

17-19: LGTM! Good keyboard accessibility pattern.

The handleKeyDown function correctly handles Enter key activation, providing keyboard accessibility for the settings rows.

src/ionic.css (1)

30-30: LGTM! Focused background variables support keyboard navigation.

The new --ion-background-focused CSS variables provide consistent focus styling across light and dark themes, supporting the keyboard accessibility enhancements throughout the application.

Also applies to: 55-55, 146-146

src/components/FlexCol.tsx (1)

13-13: LGTM! Clean tabIndex prop forwarding.

The tabIndex prop is correctly added to the interface, destructured, and forwarded to the root div, enabling focus management for FlexCol components.

Also applies to: 27-27, 44-44

src/components/ExpandAddresses.tsx (1)

70-80: Good keyboard accessibility for ExpandLine.

The focusable wrapper with tabIndex={0} and onKeyDown handler enables proper keyboard interaction for copying addresses.

src/components/FlexRow.tsx (1)

15-15: LGTM! Clean tabIndex prop forwarding.

The tabIndex prop is correctly added and forwarded to the root div, enabling keyboard focus management for FlexRow components. This mirrors the FlexCol implementation.

Also applies to: 31-31, 48-48

src/components/Menu.tsx (1)

31-33: LGTM! Proper keyboard navigation implementation.

The focusable wrapper with Enter key handling enables keyboard navigation while preserving the existing click behavior. The pattern correctly separates focus management from the visual/interactive elements.

Also applies to: 38-48

src/components/SwapsList.tsx (1)

51-59: Excellent focus management implementation!

The implementation properly handles:

  • Focusable state toggle via Enter/Space/ArrowDown
  • Escape key to unfocus and return focus to the header
  • Conditional tabIndex based on focusable state
  • Ref management to restore focus

This aligns well with the PR objectives for keyboard navigation in transaction lists.

Also applies to: 109-112, 134-134, 137-137, 173-182, 188-193

src/components/InputWithScanner.tsx (1)

49-49: LGTM! Proper ref and value binding.

The ref and value props are correctly applied to the IonInput, enabling focus management and controlled input behavior.

Also applies to: 51-51

src/screens/Apps/Index.tsx (1)

53-53: Excellent keyboard navigation and focus management for app cards!

The implementation provides:

  • Keyboard activation via Enter key
  • Conditional tabIndex based on live status (preventing focus on non-live apps)
  • Auto-focus on the first app card on mount
  • Proper ref forwarding through the first prop

This aligns well with the PR's accessibility goals.

Also applies to: 68-70, 73-81, 106-110, 124-124

Comment thread src/App.tsx Resolved
Comment thread src/App.tsx Resolved
Comment thread src/components/ExpandAddresses.tsx Resolved
Comment thread src/components/ExpandAddresses.tsx Resolved
Comment thread src/components/InputAmount.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/ExpandAddresses.tsx Resolved
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Dec 23, 2025 •

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: b0b761f
Status: ✅  Deploy successful!
Preview URL: https://58a30ce5.wallet-bitcoin.pages.dev
Branch Preview URL: https://keyboard-navigation.wallet-bitcoin.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Dec 23, 2025 •

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: b0b761f
Status: ✅  Deploy successful!
Preview URL: https://d015efdf.arkade-wallet.pages.dev
Branch Preview URL: https://keyboard-navigation.arkade-wallet.pages.dev

View logs

@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: 3

🧹 Nitpick comments (1)
src/components/Focusable.tsx (1)

1-36: LGTM! Well-implemented keyboard accessibility wrapper.

The Focusable component correctly implements keyboard activation for interactive elements:

  • Properly handles Enter and Space keys
  • Uses appropriate ARIA attributes and semantic roles
  • Includes stopPropagation and preventDefault to prevent conflicts
  • Sets tabIndex={0} for natural keyboard navigation
💡 Optional: Consider more descriptive default aria-label

The default aria-label of "Focusable element" is quite generic. Consider making the ariaLabel prop required, or provide a more context-specific default. This would improve screen reader experience.

 export default function Focusable({ ariaLabel, role, children, fit, onKeyDown }: FocusableProps) {
   // ...
   return (
     <div
       tabIndex={0}
       style={style}
       className='focusable'
       role={role ?? 'button'}
       onKeyDown={handleKeyDown}
-      aria-label={ariaLabel ?? 'Focusable element'}
+      aria-label={ariaLabel ?? 'Interactive element'}
     >
       {children}
     </div>
   )
 }

Alternatively, make ariaLabel required to ensure each usage provides meaningful context.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36122c4 and 0da582d.

📒 Files selected for processing (15)
  • src/App.tsx
  • src/components/ExpandAddresses.tsx
  • src/components/Focusable.tsx
  • src/components/Header.tsx
  • src/components/InputAmount.tsx
  • src/components/Keyboard.tsx
  • src/components/Menu.tsx
  • src/components/SwapsList.tsx
  • src/components/Toggle.tsx
  • src/components/TransactionsList.tsx
  • src/index.css
  • src/ionic.css
  • src/screens/Apps/Index.tsx
  • src/screens/Settings/General.tsx
  • src/screens/Wallet/Receive/Amount.tsx
💤 Files with no reviewable changes (2)
  • src/components/Keyboard.tsx
  • src/screens/Wallet/Receive/Amount.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/index.css
  • src/ionic.css
  • src/components/TransactionsList.tsx
  • src/screens/Apps/Index.tsx
  • src/App.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.

Applied to files:

  • src/components/InputAmount.tsx
📚 Learning: 2025-12-20T10:09:41.030Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 278
File: src/providers/annoucements.tsx:32-50
Timestamp: 2025-12-20T10:09:41.030Z
Learning: In src/providers/annoucements.tsx, the useEffect hook depends on [config] (not just [configLoaded]) because it needs to react to config changes after nostr restoration. The seen.current ref guard prevents showing multiple announcements per session, even though the effect may re-run on config updates.

Applied to files:

  • src/components/InputAmount.tsx
  • src/components/ExpandAddresses.tsx
🧬 Code graph analysis (7)
src/components/InputAmount.tsx (1)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/components/Toggle.tsx (1)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/components/ExpandAddresses.tsx (5)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/Text.tsx (2)
  • TextSecondary (91-97)
  • Text (25-79)
src/lib/format.ts (1)
  • prettyLongText (76-83)
src/components/Shadow.tsx (1)
  • Shadow (18-55)
src/screens/Settings/General.tsx (6)
src/providers/config.tsx (1)
  • ConfigContext (33-43)
src/providers/options.tsx (1)
  • OptionsContext (125-131)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/Text.tsx (1)
  • Text (25-79)
src/icons/Arrow.tsx (1)
  • ArrowIcon (1-7)
src/components/Menu.tsx (4)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/Text.tsx (1)
  • Text (25-79)
src/icons/Arrow.tsx (1)
  • ArrowIcon (1-7)
src/components/SwapsList.tsx (5)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/FlexCol.tsx (1)
  • FlexCol (16-46)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/providers/lightning.tsx (1)
  • LightningContext (45-62)
src/components/Text.tsx (1)
  • TextLabel (81-89)
src/components/Header.tsx (2)
src/components/Focusable.tsx (1)
  • Focusable (9-36)
src/icons/Back.tsx (1)
  • BackIcon (1-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test
🔇 Additional comments (7)
src/screens/Settings/General.tsx (1)

19-31: LGTM! Keyboard accessibility properly implemented.

The Row component correctly wraps the settings option in a Focusable component, enabling both click and keyboard navigation while maintaining the existing visual structure and behavior.

src/components/Toggle.tsx (1)

20-22: LGTM! Toggle is now keyboard accessible.

The IonToggle is correctly wrapped in a Focusable component with the fit prop, enabling keyboard interaction via Enter/Space keys while preserving the existing click behavior.

src/components/InputAmount.tsx (2)

52-55: LGTM! Dependency array correctly updated.

The useEffect now only depends on [focus], which is correct since input.current changes don't trigger re-renders. The guard if (focus && input.current) safely handles null refs.


99-109: LGTM! Max button is now keyboard accessible.

The Max button is correctly wrapped in a Focusable component, enabling keyboard interaction while preserving the existing click behavior and styling.

src/components/Header.tsx (1)

40-44: LGTM! Back button is now keyboard accessible.

The back navigation control is correctly wrapped in a Focusable component with the fit prop, enabling keyboard interaction via Enter/Space keys.

src/components/Menu.tsx (1)

35-45: LGTM! Menu items are now keyboard accessible.

The menu rows are correctly wrapped in Focusable components, enabling keyboard navigation while preserving the existing click behavior and styling.

src/components/SwapsList.tsx (1)

155-157: Both SwapsList and TransactionsList already implement the same focusable pattern consistently.

Both components use identical Enter-to-focus behavior: the header Focusable sets focusable=true, which enables keyboard navigation on individual list items. However, neither component currently implements Escape-to-unfocus handlers. If unfocusing is required, add Escape key handlers to both components to complete the pattern.

Likely an incorrect or invalid review comment.

Comment on lines +36 to +47
useEffect(() => {
const handleArrowDown = (event: KeyboardEvent) => {
if (event.key === 'ArrowDown') {
if (!expand) setExpand(true)
}
if (event.key === 'ArrowUp') {
if (expand) setExpand(false)
}
}
window.addEventListener('keydown', handleArrowDown)
return () => window.removeEventListener('keydown', handleArrowDown)
}, [expand])

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.

⚠️ Potential issue | 🟠 Major

Global keyboard listener may cause conflicts.

The window-level keyboard listener for ArrowDown/ArrowUp will be active whenever this component is mounted, regardless of whether the component is focused or visible. This could interfere with other parts of the application that use these keys for navigation.

Consider one of these approaches:

  • Attach the listener to the component's container ref instead of window
  • Check if the component or one of its descendants has focus before handling the event
  • Add an event.target check to ensure the event originated from within this component
🔎 Proposed fix using a container ref
+import { useEffect, useRef, useState } from 'react'
-import { useEffect, useState } from 'react'

 export default function ExpandAddresses({
   bip21uri,
   boardingAddr,
   offchainAddr,
   invoice,
   onClick,
 }: ExpandAddressesProps) {
   const [copied, setCopied] = useState('')
   const [expand, setExpand] = useState(false)
+  const containerRef = useRef<HTMLDivElement>(null)

   const [present] = useIonToast()

   useEffect(() => {
     const handleArrowDown = (event: KeyboardEvent) => {
+      // Only handle if the event target is within this component
+      if (!containerRef.current?.contains(event.target as Node)) return
+      
       if (event.key === 'ArrowDown') {
         if (!expand) setExpand(true)
       }
       if (event.key === 'ArrowUp') {
         if (expand) setExpand(false)
       }
     }
     window.addEventListener('keydown', handleArrowDown)
     return () => window.removeEventListener('keydown', handleArrowDown)
   }, [expand])

   // ... rest of component

   return (
-    <div style={{ margin: '0 auto', maxWidth: '100%', width: '300px' }}>
+    <div ref={containerRef} style={{ margin: '0 auto', maxWidth: '100%', width: '300px' }}>
       {/* ... */}
     </div>
   )
 }
🤖 Prompt for AI Agents
In src/components/ExpandAddresses.tsx around lines 36-47, the current
window-level keydown listener for ArrowDown/ArrowUp runs whenever the component
is mounted and can conflict with other parts of the app; change the
implementation to use a container ref (create a ref for the component wrapper
and attach the keydown listener to ref.current instead of window), or—if you
prefer keeping a global listener—first check that the event originated inside
the component by verifying event.target is contained within the container ref
(or that document.activeElement is a descendant of the container) before
toggling expand, and ensure you add/remove the listener from the ref element on
mount/unmount and include the ref in the effect dependencies for proper cleanup.

Comment thread src/components/ExpandAddresses.tsx Resolved
Comment thread src/components/SwapsList.tsx Resolved

@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: 5

🧹 Nitpick comments (1)
src/components/Focusable.tsx (1)

1-8: Consider requiring ariaLabel or improving the default.

The default aria-label of "Focusable element" is generic and doesn't provide meaningful context for screen reader users. Since this component wraps diverse interactive elements (transaction rows, swap items, headers), each usage should provide a descriptive label.

🔎 Options to improve accessibility

Option 1: Make ariaLabel required

 interface FocusableProps {
   children: React.ReactNode
   onEscape?: () => void
   onEnter?: () => void
-  ariaLabel?: string
+  ariaLabel: string
   fit?: boolean
   role?: string
 }

Option 2: Add a console warning when ariaLabel is not provided (development only)

 export default function Focusable({ ariaLabel, role, children, fit, onEnter, onEscape }: FocusableProps) {
+  if (process.env.NODE_ENV === 'development' && !ariaLabel) {
+    console.warn('Focusable: ariaLabel should be provided for accessibility')
+  }
+
   const style: React.CSSProperties = {

Also applies to: 36-36

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0da582d and 1b9fc8f.

📒 Files selected for processing (12)
  • src/components/ExpandAddresses.tsx
  • src/components/Focusable.tsx
  • src/components/Header.tsx
  • src/components/InputAmount.tsx
  • src/components/Menu.tsx
  • src/components/SwapsList.tsx
  • src/components/Toggle.tsx
  • src/components/TransactionsList.tsx
  • src/lib/types.ts
  • src/providers/lightning.tsx
  • src/screens/Apps/Index.tsx
  • src/screens/Settings/General.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/screens/Settings/General.tsx
  • src/components/ExpandAddresses.tsx
  • src/components/Menu.tsx
  • src/screens/Apps/Index.tsx
  • src/components/InputAmount.tsx
  • src/components/Header.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.

Applied to files:

  • src/components/TransactionsList.tsx
🧬 Code graph analysis (3)
src/components/TransactionsList.tsx (5)
src/lib/types.ts (1)
  • Tx (74-84)
src/components/Text.tsx (3)
  • Text (25-79)
  • TextSecondary (91-97)
  • TextLabel (81-89)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/Focusable.tsx (1)
  • Focusable (10-41)
src/providers/wallet.tsx (1)
  • WalletContext (42-56)
src/components/SwapsList.tsx (6)
src/lib/types.ts (1)
  • PendingSwap (38-38)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/Focusable.tsx (1)
  • Focusable (10-41)
src/providers/lightning.tsx (1)
  • LightningContext (46-63)
src/components/Empty.tsx (1)
  • EmptySwapList (46-54)
src/components/Text.tsx (1)
  • TextLabel (81-89)
src/providers/lightning.tsx (1)
src/lib/types.ts (1)
  • PendingSwap (38-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test
🔇 Additional comments (6)
src/lib/types.ts (1)

1-1: LGTM!

The new PendingSwap type alias cleanly consolidates the union type for reuse across the codebase, improving type consistency.

Also applies to: 38-39

src/components/Toggle.tsx (1)

20-22: LGTM!

The Focusable wrapper correctly enables keyboard activation (Enter/Space) for the toggle, aligning with the broader keyboard navigation pattern introduced in this PR.

src/providers/lightning.tsx (1)

18-18: LGTM!

The return type update to Promise<PendingSwap[]> is consistent with the new type alias defined in src/lib/types.ts, improving type clarity.

Also applies to: 41-41, 191-194

src/components/Focusable.tsx (1)

16-27: LGTM!

The keyboard event handling is correct:

  • Uses ' ' (space character) instead of the string 'Space'
  • Properly calls preventDefault() and stopPropagation()
  • Handles both Enter and Escape keys appropriately
src/components/TransactionsList.tsx (1)

1-1: LGTM on the core keyboard navigation implementation!

The changes successfully address the previous review concerns:

  • Component renamed from Date to When (avoids shadowing global Date)
  • Keyboard event handling delegated to the Focusable component
  • Focus management properly implemented with state and callbacks

Also applies to: 14-14, 58-58, 124-124, 128-128

src/components/SwapsList.tsx (1)

14-15: LGTM on the type updates and focus management!

The changes properly integrate the new PendingSwap type and implement keyboard navigation with state management and callbacks, consistent with the pattern used in TransactionsList.tsx.

Also applies to: 53-53, 115-116, 150-150

Comment thread src/components/SwapsList.tsx Resolved
Comment thread src/components/SwapsList.tsx Resolved
Comment thread src/components/SwapsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved
Comment thread src/components/TransactionsList.tsx Resolved

@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: 0

🧹 Nitpick comments (3)
src/test/e2e/nostr.test.ts (1)

91-91: Consider investigating the need for the fixed timeout.

The 1000ms wait after wallet restoration might be masking an async timing issue. While this pattern exists elsewhere in the test suite (line 205), consider whether a more reliable wait condition (e.g., waitForSelector on a specific element) could replace the fixed timeout.

src/components/Focusable.tsx (2)

3-17: Consider adding ref forwarding and a disabled prop.

The component would benefit from:

  1. Ref forwarding to enable programmatic focus management (e.g., auto-focusing after navigation)
  2. A disabled prop to properly handle disabled states with aria-disabled and prevent keyboard activation
🔎 Example implementation
+import { ReactNode, forwardRef } from 'react'
-import { ReactNode } from 'react'

-export default function Focusable({
+const Focusable = forwardRef<HTMLDivElement, {
   ariaLabel?: string
   children: ReactNode
   onEscape?: () => void
   onEnter?: () => void
   role?: string
   fit?: boolean
-}: {
+  disabled?: boolean
+}>(({
   ariaLabel,
   children,
   onEscape,
   onEnter,
   role,
   fit,
-}: {
-  ariaLabel?: string
-  children: ReactNode
-  onEscape?: () => void
-  onEnter?: () => void
-  role?: string
-  fit?: boolean
-}) {
+  disabled = false,
+}, ref) => {
   // ... rest of implementation
   const handleKeyDown = (e: React.KeyboardEvent) => {
+    if (disabled) return
     // ... existing key handling
   }
   
   return (
     <div
+      ref={ref}
       tabIndex={0}
       // ... other props
+      aria-disabled={disabled}
     >
       {children}
     </div>
   )
-}
+})
+
+Focusable.displayName = 'Focusable'
+export default Focusable

43-43: Consider the generic default aria-label.

The default 'Focusable element' provides a fallback but isn't very descriptive for screen reader users. Consider documenting that consumers should provide meaningful ariaLabel values for better accessibility, especially when the role is 'button'.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23c4e0f and 36870cb.

📒 Files selected for processing (4)
  • src/components/Focusable.tsx
  • src/screens/Apps/Index.tsx
  • src/test/e2e/nostr.test.ts
  • src/test/e2e/swap.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/screens/Apps/Index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-07T14:41:35.991Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.

Applied to files:

  • src/test/e2e/nostr.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages: wallet-mutinynet
🔇 Additional comments (3)
src/test/e2e/swap.test.ts (1)

18-18: LGTM! Test selector updates are consistent.

The test-id updates from app-Boltz to app-boltz are consistent across all test cases and align with the UI component changes for keyboard navigation support.

Also applies to: 79-79, 142-142, 209-209

src/test/e2e/nostr.test.ts (1)

149-150: LGTM! Test assertions updated to match new UI structure.

The test-id updates and text-based assertions align with the Focusable component integration. The exact text matches (+ 1,992 SATS, - 1,001 SATS) correctly verify the swap amounts are displayed.

Also applies to: 180-181, 222-224

src/components/Focusable.tsx (1)

23-34: LGTM! Keyboard handler follows ARIA best practices.

The implementation correctly handles Enter and Space for activation (consistent with button role) and Escape for dismissal. The use of stopPropagation() and preventDefault() appropriately prevents default behaviors and event bubbling.

@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: 1

♻️ Duplicate comments (1)
src/components/ExpandAddresses.tsx (1)

36-47: Global keyboard listener may conflict with other UI elements.

The window-level keydown listener for ArrowDown/ArrowUp will be active whenever this component is mounted, regardless of whether it has focus. This could interfere with other parts of the application that use arrow keys for navigation (e.g., scrolling lists, dropdown menus).

🔎 Recommended fix: scope the listener to the component

Add a container ref and check if the component (or one of its descendants) has focus before handling the event:

+import { useEffect, useRef, useState } from 'react'
-import { useEffect, useState } from 'react'
 
 export default function ExpandAddresses({
   bip21uri,
   boardingAddr,
   offchainAddr,
   invoice,
   onClick,
 }: ExpandAddressesProps) {
   const [copied, setCopied] = useState('')
   const [expand, setExpand] = useState(false)
+  const containerRef = useRef<HTMLDivElement>(null)
 
   const [present] = useIonToast()
 
   useEffect(() => {
     const handleArrowDown = (event: KeyboardEvent) => {
+      // Only handle if focus is within this component
+      if (!containerRef.current?.contains(document.activeElement)) return
+      
       if (event.key === 'ArrowDown') {
         if (!expand) setExpand(true)
       }
       if (event.key === 'ArrowUp') {
         if (expand) setExpand(false)
       }
     }
     window.addEventListener('keydown', handleArrowDown)
     return () => window.removeEventListener('keydown', handleArrowDown)
   }, [expand])
 
   // ... rest of component
 
   return (
-    <div style={{ margin: '0 auto', maxWidth: '100%', width: '300px' }}>
+    <div ref={containerRef} style={{ margin: '0 auto', maxWidth: '100%', width: '300px' }}>
       {/* ... */}
     </div>
   )
 }
🧹 Nitpick comments (1)
src/test/e2e/nostr.test.ts (1)

91-91: Verify that the timeout addresses the underlying race condition.

The 1000ms wait after wallet restoration may be masking a timing issue. Consider whether the test could wait for a specific UI state (e.g., a loading indicator to disappear or a specific element to become visible) instead of using an arbitrary timeout.

If there's a specific element that signals the wallet is fully loaded, consider replacing the timeout with:

await page.waitForSelector('[data-testid="some-loaded-indicator"]', { state: 'visible' })
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36870cb and b912304.

📒 Files selected for processing (4)
  • src/components/ExpandAddresses.tsx
  • src/components/Shadow.tsx
  • src/test/e2e/nostr.test.ts
  • src/test/e2e/swap.test.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-07T14:41:35.991Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.

Applied to files:

  • src/test/e2e/swap.test.ts
  • src/test/e2e/nostr.test.ts
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.

Applied to files:

  • src/components/ExpandAddresses.tsx
📚 Learning: 2025-12-20T10:09:41.030Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 278
File: src/providers/annoucements.tsx:32-50
Timestamp: 2025-12-20T10:09:41.030Z
Learning: In src/providers/annoucements.tsx, the useEffect hook depends on [config] (not just [configLoaded]) because it needs to react to config changes after nostr restoration. The seen.current ref guard prevents showing multiple announcements per session, even though the effect may re-run on config updates.

Applied to files:

  • src/components/ExpandAddresses.tsx
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.

Applied to files:

  • src/test/e2e/nostr.test.ts
🧬 Code graph analysis (2)
src/test/e2e/swap.test.ts (1)
src/test/e2e/utils.ts (1)
  • readClipboard (3-16)
src/components/ExpandAddresses.tsx (6)
src/components/Focusable.tsx (1)
  • Focusable (3-48)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/lib/format.ts (1)
  • prettyLongText (76-83)
src/components/Shadow.tsx (1)
  • Shadow (19-57)
src/icons/CheckMark.tsx (1)
  • CheckMarkIcon (1-11)
src/icons/Copy.tsx (1)
  • CopyIcon (1-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages: wallet-mutinynet
🔇 Additional comments (4)
src/components/Shadow.tsx (1)

16-16: LGTM! Test ID support added cleanly.

The addition of the testId prop enables test hooks for the Shadow component, which aligns with the broader keyboard accessibility and testability improvements in this PR.

Also applies to: 32-32, 53-53

src/test/e2e/swap.test.ts (1)

18-18: LGTM! Test selectors updated consistently.

The updates to use lowercase app-boltz and explicit test IDs (ark-address-copy, invoice-address-copy) align well with the new UI structure and improve test reliability by replacing complex locator chains with direct test ID references.

Also applies to: 52-54, 75-75, 94-96, 136-136, 155-157, 200-200

src/test/e2e/nostr.test.ts (1)

127-129: LGTM! Consistent test selector updates.

The updates to use app-boltz, explicit test IDs, and exact SATS text matches improve test specificity and align with the broader keyboard accessibility changes in this PR.

Also applies to: 145-146, 176-177, 218-220

src/components/ExpandAddresses.tsx (1)

65-65: LGTM! Test IDs enable better test reliability.

The addition of testId props to ExpandLine and the propagation to Shadow components (with the -address-copy suffix) aligns perfectly with the test updates in the e2e files, enabling more reliable and maintainable tests.

Also applies to: 72-72, 92-95

Comment thread src/components/ExpandAddresses.tsx Outdated
Comment on lines +60 to +63
const handleKeyDown = (value: string) => {
handleCopy(value)
onClick(value)
}

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.

⚠️ Potential issue | 🟠 Major

Inconsistent behavior: keyboard Enter differs from mouse click.

The keyboard interaction creates different behavior than mouse clicks:

  • Pressing Enter on a Focusable line (line 66): calls handleKeyDown → copies and calls onClick
  • Clicking the FlexRow (line 67): only calls onClick
  • Clicking the Shadow copy icon (line 72): only calls handleCopy

This means keyboard users get a combined copy+click action, while mouse users can choose either action separately.

🔎 Recommended fix: align keyboard behavior with primary click action

Make the keyboard Enter key match the row click behavior (call onClick only), and keep the copy icon separate:

-  const handleKeyDown = (value: string) => {
-    handleCopy(value)
-    onClick(value)
-  }
-
-  const ExpandLine = ({ testId, title, value }: { testId: string; title: string; value: string }) => (
-    <Focusable onEnter={() => handleKeyDown(value)}>
+  const ExpandLine = ({ testId, title, value }: { testId: string; title: string; value: string }) => (
+    <Focusable onEnter={() => onClick(value)}>
       <FlexRow between onClick={() => onClick(value)}>
         <FlexCol gap='0'>
           <TextSecondary>{title}</TextSecondary>
           <Text>{prettyLongText(value, 12)}</Text>
         </FlexCol>
-        <Shadow flex onClick={() => handleCopy(value)} testId={testId + '-address-copy'}>
+        <Shadow 
+          flex 
+          onClick={(e) => {
+            e.stopPropagation()
+            handleCopy(value)
+          }} 
+          testId={testId + '-address-copy'}
+        >
           {copied === value ? <CheckMarkIcon /> : <CopyIcon />}
         </Shadow>
       </FlexRow>
     </Focusable>
   )

Note: Add e.stopPropagation() to prevent the copy icon click from also triggering the row's onClick.

Also applies to: 66-72

🤖 Prompt for AI Agents
In src/components/ExpandAddresses.tsx around lines 60 to 72, the Enter key
handler currently both copies and triggers onClick, causing inconsistent
behavior vs mouse interactions; change handleKeyDown to only call onClick
(remove the call to handleCopy) so keyboard Enter matches the row click
behavior, and ensure the Shadow copy icon's click handler calls handleCopy with
e.stopPropagation() so clicking the copy icon does not also trigger the row's
onClick; apply the same adjustment to the other instance mentioned (lines
66–72).

@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: 2

♻️ Duplicate comments (2)
src/components/ExpandAddresses.tsx (2)

60-76: Inconsistent behavior between keyboard and mouse activation.

The onEnter handler (line 60-63) performs both handleCopy(value) AND onClick(value), while:

  • Clicking the FlexRow (line 67) only calls onClick(value)
  • Clicking the Shadow copy icon (line 72) only calls handleCopy(value)

This means keyboard users get a combined action while mouse users can choose either action separately.

🔎 Proposed fix: align keyboard Enter with row click behavior
-  const onEnter = (value: string) => {
-    handleCopy(value)
-    onClick(value)
-  }
-
   const ExpandLine = ({ testId, title, value }: { testId: string; title: string; value: string }) => (
-    <Focusable onEnter={() => onEnter(value)}>
+    <Focusable onEnter={() => onClick(value)}>
       <FlexRow between onClick={() => onClick(value)}>

36-47: Global keyboard listener may cause conflicts with other components.

The window.addEventListener('keydown', ...) captures ArrowDown/ArrowUp globally regardless of whether this component has focus. This could interfere with other parts of the application.

Consider scoping the listener to only fire when the component (or a descendant) has focus:

🔎 Proposed fix using focus check
 useEffect(() => {
+  const containerRef = document.getElementById('expand-addresses-container')
   const handleArrowDown = (event: KeyboardEvent) => {
+    // Only handle if focus is within this component
+    if (!containerRef?.contains(document.activeElement)) return
+    
     if (event.key === 'ArrowDown') {
       if (!expand) setExpand(true)
     }
     if (event.key === 'ArrowUp') {
       if (expand) setExpand(false)
     }
   }
   window.addEventListener('keydown', handleArrowDown)
   return () => window.removeEventListener('keydown', handleArrowDown)
 }, [expand])

And add an id to the container:

-  <div style={{ margin: '0 auto', maxWidth: '100%', width: '300px' }}>
+  <div id='expand-addresses-container' style={{ margin: '0 auto', maxWidth: '100%', width: '300px' }}>
🧹 Nitpick comments (5)
src/components/Header.tsx (1)

50-58: Redundant onClick on outer div may cause confusion.

The outer div at line 50 has onClick={auxFunc}, but the inner Focusable wrapper handles keyboard activation via onEnter={auxFunc}. This creates an inconsistent structure where mouse clicks are handled at the outer level while keyboard activation is handled at the inner level.

Consider moving the click handling entirely to the Focusable wrapper or its children for consistency:

🔎 Proposed fix
-        <div style={style} onClick={auxFunc} aria-label={auxAriaLabel}>
+        <div style={style} aria-label={auxAriaLabel}>
           {auxText || auxIcon ? (
-            <Focusable onEnter={auxFunc} fit round>
-              {auxText ? SideButton(auxText) : <div style={{ padding: '0.5rem' }}>{auxIcon}</div>}
+            <Focusable onEnter={auxFunc} fit round>
+              <div onClick={auxFunc} style={{ cursor: 'pointer' }}>
+                {auxText ? SideButton(auxText) : <div style={{ padding: '0.5rem' }}>{auxIcon}</div>}
+              </div>
             </Focusable>
           ) : (
             '\u00A0'
           )}
         </div>
src/components/Table.tsx (1)

40-49: Hardcoded id='outer' may cause conflicts.

Using a hardcoded id='outer' could cause issues if:

  1. Multiple Table components are rendered on the same page
  2. Other components use the same id

Consider generating a unique id per table instance.

🔎 Proposed fix using useId
+import { useState, useId } from 'react'
 
 export default function Table({ data }: { data: any[][] }) {
+  const tableId = useId()
   const [focused, setFocused] = useState(false)
   
   // ...
   
   const focusOnOuterShell = () => {
     setFocused(false)
-    const outer = document.getElementById('outer') as HTMLElement
+    const outer = document.getElementById(`outer-${tableId}`) as HTMLElement
     if (outer) outer.focus()
   }
   
   // ...
   
   return (
-    <Focusable id='outer' inactive={focused} onEnter={focusOnFirstRow} ariaLabel={ariaLabel()}>
+    <Focusable id={`outer-${tableId}`} inactive={focused} onEnter={focusOnFirstRow} ariaLabel={ariaLabel()}>
src/screens/Settings/Logs.tsx (1)

48-52: Hardcoded id='outer' may cause conflicts with other components.

Same issue as in Table.tsx - if LogsTable is used alongside other components with id='outer', focus management will break.

🔎 Proposed fix using useId
+import { useEffect, useState, useId } from 'react'
 
 function LogsTable({ logs }: { logs: LogLine[] }) {
+  const tableId = useId()
   const [focused, setFocused] = useState(false)
   
   // ...
   
   const focusOnOuterShell = () => {
     setFocused(false)
-    const outer = document.getElementById('outer') as HTMLElement
+    const outer = document.getElementById(`outer-${tableId}`) as HTMLElement
     if (outer) outer.focus()
   }
src/components/TransactionsList.tsx (1)

140-155: Hardcoded id='outer' may conflict with other components.

This is the third component using id='outer'. If TransactionsList, Table, or LogsTable are rendered on the same page, focus management will break.

Consider using React's useId() hook to generate unique identifiers, or use component-specific prefixes like id='transactions-outer'.

src/components/SwapsList.tsx (1)

164-179: Hardcoded id='outer' may conflict with other list components.

Same issue as TransactionsList - consider using a unique identifier.

🔎 Proposed fix
+import { useContext, useEffect, useState, useId } from 'react'

 export default function SwapsList() {
+  const listId = useId()
   // ...
   
   const focusOnOuterShell = () => {
     setFocused(false)
-    const outer = document.getElementById('outer') as HTMLElement
+    const outer = document.getElementById(`swaps-outer-${listId}`) as HTMLElement
     if (outer) outer.focus()
   }
   
   // ...
   
   return (
     <div style={{ width: 'calc(100% + 2rem)', margin: '0 -1rem' }}>
       <TextLabel>Swap history</TextLabel>
-      <Focusable id='outer' inactive={focused} onEnter={focusOnFirstRow} ariaLabel={ariaLabel()}>
+      <Focusable id={`swaps-outer-${listId}`} inactive={focused} onEnter={focusOnFirstRow} ariaLabel={ariaLabel()}>
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b912304 and b92a36f.

📒 Files selected for processing (9)
  • src/components/ExpandAddresses.tsx
  • src/components/Focusable.tsx
  • src/components/Header.tsx
  • src/components/SwapsList.tsx
  • src/components/Table.tsx
  • src/components/Toggle.tsx
  • src/components/TransactionsList.tsx
  • src/icons/Settings.tsx
  • src/screens/Settings/Logs.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Toggle.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.

Applied to files:

  • src/components/ExpandAddresses.tsx
  • src/components/TransactionsList.tsx
📚 Learning: 2025-12-20T10:09:41.030Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 278
File: src/providers/annoucements.tsx:32-50
Timestamp: 2025-12-20T10:09:41.030Z
Learning: In src/providers/annoucements.tsx, the useEffect hook depends on [config] (not just [configLoaded]) because it needs to react to config changes after nostr restoration. The seen.current ref guard prevents showing multiple announcements per session, even though the effect may re-run on config updates.

Applied to files:

  • src/components/ExpandAddresses.tsx
🧬 Code graph analysis (4)
src/components/Header.tsx (3)
src/components/Shadow.tsx (1)
  • Shadow (19-57)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
src/icons/Back.tsx (1)
  • BackIcon (1-13)
src/components/ExpandAddresses.tsx (9)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
src/components/FlexRow.tsx (1)
  • FlexRow (18-50)
src/components/FlexCol.tsx (1)
  • FlexCol (16-46)
src/components/Text.tsx (1)
  • Text (25-79)
src/components/Shadow.tsx (1)
  • Shadow (19-57)
src/icons/CheckMark.tsx (1)
  • CheckMarkIcon (1-11)
src/icons/Copy.tsx (1)
  • CopyIcon (1-13)
src/icons/ChevronUp.tsx (1)
  • ChevronUpIcon (1-13)
src/icons/ChevronDown.tsx (1)
  • ChevronDownIcon (1-13)
src/components/SwapsList.tsx (4)
src/lib/types.ts (1)
  • PendingSwap (38-38)
src/providers/flow.tsx (1)
  • FlowContext (89-104)
src/providers/navigation.tsx (1)
  • NavigationContext (169-173)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
src/components/TransactionsList.tsx (5)
src/lib/types.ts (1)
  • Tx (74-84)
src/providers/flow.tsx (1)
  • FlowContext (89-104)
src/providers/navigation.tsx (1)
  • NavigationContext (169-173)
src/providers/wallet.tsx (1)
  • WalletContext (42-56)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages: wallet-mutinynet
🔇 Additional comments (7)
src/icons/Settings.tsx (1)

19-27: LGTM! Good accessibility improvement.

Using currentColor instead of hardcoded colors allows the icon to inherit styling from parent elements, which is essential for proper focus state visibility in the keyboard navigation context.

src/components/Header.tsx (1)

40-44: LGTM!

The back button correctly uses Focusable for keyboard activation while maintaining onClick on the inner element for mouse interaction.

src/components/Focusable.tsx (1)

1-55: Well-designed reusable accessibility component.

The implementation correctly:

  • Handles both Enter and Space keys for activation (standard button behavior)
  • Uses stopPropagation() and preventDefault() to prevent event bubbling
  • Controls focusability via tabIndex based on the inactive prop
  • Defaults to role="button" for proper semantic meaning
src/screens/Settings/Logs.tsx (1)

60-82: LGTM! Good keyboard navigation implementation for logs.

The focus management correctly:

  • Makes the outer shell focusable when not in navigation mode
  • Makes individual rows focusable when in navigation mode
  • Provides descriptive aria labels for screen readers
  • Handles Enter to copy and Escape to exit
src/components/TransactionsList.tsx (1)

18-103: Good refactoring of TransactionLine.

The component now accepts an onClick prop, cleanly separating the line rendering from navigation logic. This improves reusability and testability.

src/components/SwapsList.tsx (2)

53-94: Good consolidation of swap line rendering.

The SwapLine component is now cleaner with inline component definitions and properly accepts onClick prop for navigation delegation.


137-143: Early return guards against empty array access.

The check at line 137 if (swapHistory.length === 0) return <EmptySwapList /> ensures focusOnFirstRow is only callable when the list is non-empty, preventing the crash scenario seen in other components.

Comment thread src/components/Table.tsx
Comment on lines +21 to +26
const focusOnFirstRow = () => {
setFocused(true)
const title = data[0][0]
const first = document.getElementById(title) as HTMLElement
if (first) first.focus()
}

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.

⚠️ Potential issue | 🟠 Major

Potential crash when data array is empty.

focusOnFirstRow accesses data[0][0] without checking if data is empty. If called when data.length === 0, this will throw a runtime error.

🔎 Proposed fix
 const focusOnFirstRow = () => {
+  if (data.length === 0) return
   setFocused(true)
   const title = data[0][0]
   const first = document.getElementById(title) as HTMLElement
   if (first) first.focus()
 }
📝 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
const focusOnFirstRow = () => {
setFocused(true)
const title = data[0][0]
const first = document.getElementById(title) as HTMLElement
if (first) first.focus()
}
const focusOnFirstRow = () => {
if (data.length === 0) return
setFocused(true)
const title = data[0][0]
const first = document.getElementById(title) as HTMLElement
if (first) first.focus()
}
🤖 Prompt for AI Agents
In src/components/Table.tsx around lines 21 to 26, focusOnFirstRow
unconditionally reads data[0][0] which will throw if data is empty; add a guard
that checks data exists and data.length > 0 and that data[0] has at least one
cell before accessing [0][0]; if the guard fails, still call setFocused(true) if
desired (or return early) and do not attempt to query the DOM; after confirming
a non-empty title, query by id and focus as before.

Comment thread src/components/TransactionsList.tsx Resolved
@pietro909

Copy link
Copy Markdown
Contributor

Found a minor UI issue when sending funds: in the "amount" the "max" selector is now expanded.

Screenshot from 2025-12-30 16-05-18

@bordalix

Copy link
Copy Markdown
Collaborator Author

Found a minor UI issue when sending funds: in the "amount" the "max" selector is now expanded.

Nice catch.
Fixed by c3a4021

@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: 0

🧹 Nitpick comments (4)
src/screens/Settings/Logs.tsx (1)

66-81: Consider extracting the key to avoid duplicate computation.

The key() function is called twice for each log entry (lines 70 and 71). While this is not a performance issue for typical log sizes, extracting it to a variable would follow DRY principles.

🔎 Proposed refactor
         {[...logs].reverse().map(({ time, msg, level }) => {
+          const rowKey = key({ time, msg, level })
           return (
             <Focusable
               inactive={!focused}
               onEnter={() => copy(msg)}
               onEscape={focusOnOuterShell}
-              id={key({ time, msg, level })}
-              key={key({ time, msg, level })}
-              ariaLabel={ariaLabel({ time, msg, level })}
+              id={rowKey}
+              key={rowKey}
+              ariaLabel={ariaLabel({ time, msg, level })}
             >
src/components/Table.tsx (2)

35-37: Clarify the ariaLabel semantics.

The ariaLabel describes the value as "status" (line 37), but the Table component is generic and the value field may represent various types of data (not just status). Consider using a more generic description like "value" instead.

🔎 Proposed refactor
 const ariaLabel = (title?: string, value?: string) => {
   if (!title || !value) return 'Pressing Enter enables keyboard navigation of the table'
-  return `Title ${title} with status ${value}. Press Escape to exit keyboard navigation.`
+  return `Row with title ${title} and value ${value}. Press Escape to exit keyboard navigation.`
 }

44-51: Component design relies on caller to ensure unique titles; current usages are safe but consider refactoring to reduce fragility.

While the Table component uses title for both id and key attributes, all current usages (Details.tsx, About.tsx, Swap.tsx) construct tables with hardcoded static labels that are unique within each table. However, the component design doesn't enforce or validate title uniqueness, making it fragile for future usage. Consider either:

  • Using an index-based key and deriving IDs from a stable identifier, or
  • Requiring a unique id field separate from the display title.
src/components/TransactionsList.tsx (1)

111-132: Consider extracting the focus management pattern into a custom hook.

The focus management logic (focused state, focusOnFirstRow, focusOnOuterShell, ariaLabel) is repeated across TransactionsList, SwapsList, Logs, and Table components. Extracting this into a reusable useFocusableList hook would reduce duplication and improve maintainability.

Example hook structure
function useFocusableList<T>(
  items: T[],
  keyFn: (item: T) => string,
  ariaLabelFn: (item?: T) => string
) {
  const [focused, setFocused] = useState(false)
  
  const focusOnFirstRow = () => {
    setFocused(true)
    if (items.length === 0) return
    const id = keyFn(items[0])
    const first = document.getElementById(id) as HTMLElement
    if (first) first.focus()
  }
  
  const focusOnOuterShell = () => {
    setFocused(false)
    const outer = document.getElementById('outer') as HTMLElement
    if (outer) outer.focus()
  }
  
  return { focused, focusOnFirstRow, focusOnOuterShell, ariaLabel: ariaLabelFn }
}
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b92a36f and c3a4021.

📒 Files selected for processing (5)
  • src/components/InputAmount.tsx
  • src/components/SwapsList.tsx
  • src/components/Table.tsx
  • src/components/TransactionsList.tsx
  • src/screens/Settings/Logs.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/InputAmount.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.

Applied to files:

  • src/components/TransactionsList.tsx
🧬 Code graph analysis (3)
src/components/Table.tsx (5)
src/lib/clipboard.ts (1)
  • copyToClipboard (3-11)
src/lib/toast.ts (1)
  • copiedToClipboard (13-16)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
src/components/Text.tsx (1)
  • Text (25-79)
src/lib/format.ts (1)
  • prettyLongText (76-83)
src/components/TransactionsList.tsx (6)
src/lib/types.ts (1)
  • Tx (74-84)
src/components/Text.tsx (3)
  • Text (25-79)
  • TextSecondary (91-97)
  • TextLabel (81-89)
src/providers/flow.tsx (1)
  • FlowContext (89-104)
src/providers/navigation.tsx (1)
  • NavigationContext (169-173)
src/providers/wallet.tsx (1)
  • WalletContext (42-56)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
src/components/SwapsList.tsx (6)
src/lib/types.ts (1)
  • PendingSwap (38-38)
src/providers/flow.tsx (1)
  • FlowContext (89-104)
src/providers/navigation.tsx (1)
  • NavigationContext (169-173)
src/providers/lightning.tsx (1)
  • LightningContext (46-63)
src/components/Text.tsx (1)
  • TextLabel (81-89)
src/components/Focusable.tsx (1)
  • Focusable (3-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages: wallet-mutinynet

@bordalix
bordalix merged commit cc69620 into master Dec 30, 2025
5 checks passed
This was referenced Jan 5, 2026
This was referenced Feb 15, 2026
@bordalix
bordalix deleted the keyboard_navigation branch March 4, 2026 10:12
Kukks added a commit that referenced this pull request Mar 11, 2026
The app card redesign (#398) replaced the Focusable wrapper with a plain
div, losing the focus ring styling and keyboard nav flow from #283.
Re-wrap each App card in Focusable to restore focus indicator and
Enter/Space activation.
Kukks added a commit that referenced this pull request Mar 11, 2026
The app card redesign (#398) replaced the Focusable wrapper with a plain
div, losing the focus ring styling and keyboard nav flow from #283.
Re-wrap each App card in Focusable to restore focus indicator and
Enter/Space activation.
pietro909 added a commit that referenced this pull request Mar 12, 2026
* Use next branches

* Implements chain swaps

* fixes

* Use new SDK storage

* Update integration to Contract Manager

* App starting, payments received

* Use correct import

* Use new SDK storage

* wip

* SW working

* using ws

* Latest SDK

* workaround for build

* Fix build

* Rollback unneeeded changes, clean up code

* Address Rabbit comment

* Set flag in unsub

* Fix tx history rendering issue

* Update package.json, lock file, worker

* Run migration only if needed

* Update lock and worker

* Use next releases

* Clear setInterval on re-init to prevent leaking duplicate intervals

* Implements delegators UI (#319)

* implements delegators UI

* rabbit fixes

* dumb down + rename to delegates

* revert changes

* add unit tests

* add e2e test

* rabbit fixes

* fixes

* Upgrade latest sdk

* fixes

* Ignore md helpers

* Stop pinging wallet when locked

* Allow the page to force activation of a newly installed worker.

* Handle failure in getCommitmentTxIds

* Add defensive parsing of messages for stale service workers

* Upgrade sdk and boltz-swap

* fix Indexer constructor in wallet provider

* Use event.waitUntil

* update dependencies

* bug fix

* update dependencies

* default delegator

* fix URL

* delegator

* make delegator URL configurable

* Wire up delegates

* Centralize compressed-xonly conversion, hook in delegates

* Trigger reload on delegate on/off

* Delegates on by default

* Use persisted preference for delegates

* Fix type in wallet

* Remove workspace file

* Fix formatting

* Enforce use of delegate upon restore

* Use correct delegates port, update e2e test

* Remove log

* Update Fulmine 0.3.15 and port mapping

* Change fulmine port in setup script

* fix server persistency

* Use delegate url from network

* Remove unused VITE_DELEGATOR_URL

* fix delegate server

* feat: asset support (#318)

* Add assets integration design document

Design for integrating Arkade SDK asset system (ts-sdk PR #279) into the wallet,
covering import, receive, send, mint, reissue, and burn flows.

* Add assets integration implementation plan

16-task step-by-step plan covering SDK install, types, providers, navigation,
BIP21, all asset screens, send/receive modifications, and tx history updates.

* feat: upgrade @arkade-os/sdk to 0.3.13

Upgrade from 0.3.12 to 0.3.13, the latest published version.
The asset branch (github:arkade-os/ts-sdk#asset) does not exist yet,
so using the latest npm release as the foundation for asset integration.

* feat: add asset types and importedAssets to config

* feat: add asset flow state to FlowContext

* feat: add asset balances and metadata cache to WalletContext

* feat: register asset pages in navigation with placeholders

* feat: add BIP21 asset URI encoding and decoding

* feat: add Assets app card to Apps index

* feat: implement Assets list screen

* feat: implement asset import screen

* feat: implement asset detail screen

* feat: implement mint and mint success screens

* feat: implement reissue and burn screens

* feat: add asset support to receive flow

* feat: add asset support to send flow

* feat: show asset info in transaction history and coin control

* refactor: replace as-any casts with proper SDK types

* feat: install @arkade-os/sdk from asset branch (louisinger/wallet-sdk#asset)

* refactor: use proper SDK types for transaction assets

ArkTransaction now includes assets field from SDK changes,
removing the last as-any cast in asp.ts.

* chore: gitignore docs/plans

* format + ts-sdk from gh repo

* allow build

* remove useless tests

* fixes

* fixes

* UI fixes

* bump ts-sdk

* feat(mint): add optional control asset selector

Generated with Claude Code
via Happy

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>

* fix: display asset amounts with proper decimals and convert unit inputs to raw

- Add formatAssetAmount utility for raw→human-readable display
- Convert user-entered units to raw integers in Burn, Reissue, and Send flows
- Apply decimal formatting across all asset display surfaces

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>

* style: fix prettier formatting in Burn and Reissue

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>

* refactor: address code review issues across asset feature

- Skip BTC limits for asset receives in Amount screen
- Fix test mocks with missing context properties
- Format raw supply display in MintSuccess and Detail screens
- Type assetMetadataCache as Map<string, AssetDetails> instead of any
- Extract shared assetInputStyle to src/lib/styles.ts
- Extract AssetAvatar component to deduplicate avatar pattern (~10 locations)
- Fix stale closure in Detail.tsx useEffect
- Format asset amounts in Vtxos debug view with metadata lookup
- Fix default decimals from 0 to 8 in MintSuccess
- Fix Burn placeholder to show formatted balance

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>

* fix: apply decimal unit conversion to all asset amount inputs and displays

- Add formatAssetAmount() to format.ts for display (raw → human units)
- Mint: convert user units to raw via Decimal.mul(amount, 10^decimals)
- Send form: convert input to raw, display available balance in units
- Burn: convert input to raw, show balance in human-readable units
- Reissue: convert input to raw using decimals metadata
- All display locations (tx list, asset list, detail, send confirm/success)
  now apply formatAssetAmount to show correct decimal representation
- No trailing zeros thanks to Intl.NumberFormat maximumFractionDigits

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>

* persist asset metada

* clean

* various UI fixes

* add playwright tests

* fix asset e2e tests

* fixes

* fix invoice bip21 encoding

* fix Apps/Asset/receive

* simplify asset import

* UI fix + reuse Input component

* Merge remote-tracking branch origin/master into assets

* Show asset ticker instead of SATS on InputAmount component when receiving an asset

* Also fix amount when user has Fiat Only on Settings / Display Preferences

* revert changes to InputWithScanner

* new InputAssetId component

* dry code

* tests for lib/assets

* Mint: hide dropdown if knownAsset.length <= 0

* allow to mint control asset

* feat: add isIssuance helper to detect issuance txs

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

* feat: show issuance label and green assets in tx list

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

* feat: show issuance direction and positive asset amounts in tx detail

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

* test: add issuance transaction rendering test

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

* feat: add burn tx handling, remove asset prefix, move asset icon right

- Add isBurn helper (self-send with amount=0 and negative asset amounts)
- Update isIssuance to check for positive asset amounts
- Remove +/- prefix from asset display (SDK now returns signed amounts)
- Show "Burn" label and direction for burn transactions
- Move asset icon to right side of amount in tx list

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

* handle Issuance and Burn tx in transaction history

* Fix restoration

* fix test

* fix mint tests

* update to latest ts-sdk master

* fix playwright tests

* not:cloned-input selector

* verified | approved asset flow

* playwright: use reduced motion

* move upgrade logic to updateConfig

* simplify mint form

* fix send all btc

* remove ctrlDecimals

* remove fuji app

* show asset amount in Receive/Success

* receive/success fix

* rework receive/success

* flxShrink mint form

* add link to Asset app

* add assets settings to toggle service + hide assets in send if inactive (#353)

* add assets settings to toggle service + hide assets in send if inactive

* nit: 'enabled' instead of 'connected'

* add test + fix tests

* update boltz config

* fixes

* fix: revert boltz config and add missing enableAssets in reissue test

- Revert [[currencies.lnds]] back to [currencies.lnd] in boltz TOML
  config. The array syntax broke Boltz's LND connection, causing all
  Lightning swap e2e tests to fail.
- Add missing enableAssets() call to the reissue test, which was
  omitted when asset toggle support was added in ea958fd.

* fix: increase timeout for multi-swap e2e tests

The restore and nostr swap tests perform multiple swap operations
(reverse + submarine) plus wallet restore, exceeding the default 30s
test timeout. Increase to 90s to give these complex flows enough time.

* fix: increase action timeout and add balance wait for swap tests

The submarine swap creation is async and can take variable time,
especially after a reverse swap. Increase actionTimeout from 30s to
60s to accommodate slow swap creation, and add test.setTimeout(120s)
for multi-swap tests. Also verify wallet balance is visible before
attempting the send flow in the restore test.

* fix: revert actionTimeout to 30s and verify balance in nostr test

Increasing actionTimeout to 60s made the nostr swap test worse by
keeping failed swap connections alive longer, preventing retries from
getting a clean Boltz state. Revert to 30s and ensure balance is
verified by exact match before submarine swap.

* chore: retrigger CI

* ci: trigger workflows for PRs targeting next branch

* fix: revert Boltz LND config to singular [currencies.lnd] format

The boltz/boltz:ark image does not support the plural [[currencies.lnds]]
array syntax. This broke all Lightning swap tests (receive, send, refund).
Keep pull_policy: always to ensure fresh images.

* chore: retrigger CI

* fix: fund Boltz core wallet for chain swaps in regtest setup

The ARK-to-BTC chain swap direction requires Boltz to have BTC in its
core Bitcoin wallet. Without funding, createArkToBtcSwap hangs/fails,
breaking "send to onchain address" and "send to onchain via swap" tests.

* chore: format setup.mjs

* fix: create Bitcoin Core 'core' wallet before funding for Boltz chain swaps

* fix: create and fund Boltz core wallet before Boltz starts

Move wallet creation before Boltz backend startup so the wallet exists
and has funds when Boltz initializes. Mine an extra block to confirm.

* fix: fall back to collaborative exit when chain swap creation fails

When createArkToBtcSwap returns null or throws, navigate to SendDetails
without a pendingSwap. The Details page already handles this case by
using collaborativeExitWithFees as the fallback for BTC address sends.

* revert: remove Boltz wallet funding from setup (breaks Boltz startup)

The wallet funding caused Boltz to fail to start. Instead, the Form.tsx
fallback to collaborative exit handles onchain sends when chain swaps
are unavailable.

* fix: update onchain send test assertions for collaborative exit fees

The chain swap path (Boltz ARK→BTC) is unavailable in regtest, so the
app falls back to collaborative exit. This changes the fee from 275
(Boltz swap fee) to 200 (ASP onchain output fee from fees intent).
Also makes fees intent/clear calls synchronous to prevent race conditions.

* fix: remove unused exec import from send test

* fix: disable delegate in e2e tests (no delegator service in CI)

The delegate feature defaults to true but requires a delegator service
at localhost:7004. Since CI has no delegator, wallet initialization hangs
causing all tests to timeout. Use VITE_DELEGATE_ENABLED=false in the
Playwright config to disable delegation in the test environment.

* chore: format all files with prettier after merge

* ci: add workflow_dispatch trigger to enable manual CI runs

* fix: enable fulmine delegator and fix port mapping for e2e tests

- Add FULMINE_DELEGATOR_ENABLED=true and FULMINE_DELEGATOR_FEE=350
- Fix port mapping to expose delegator on port 7002
- Aligns fulmine config with next branch

* fix server persistency

---------

Co-authored-by: louisinger <louis@vulpem.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>

* Delete data on IndexedDB when reset (#366)

* fix delegate dev server

* fix tests

* update dependencies

* Use mutinynet server for testing

* Fix setup 7001

* E2E fixes, config, delegates url (#369)

* Fix e2e delegate test

* Log commit from SW to verify updates

* Ensure config has all the expected fields

* Fix delegates url resolution

* Missing type, harden config parsing

* reduce tests waiting for maybeLater

* skip swap restore test

* disable chain swap on receive

* Upgrade SDK, boltz, use 30s ServiceWorker timeout (#371)

* SDK Version next 8

* Use SW custom timeout

* Update SW

* Use sdk 0.4.0 and boltz-swap 0.3.0

* Update service worker

* Update service worker 2

* fix receive fractional assets (#373)

* fix receive fractional assets

* fix bip21 in asset receive

* disable mint if balance < dust

* New asset card (#374)

* New asset card

* lint

* General fixes (#382)

* dry code

* remove console.log

* fix throw with fractional sats

* refactor Asset send input amount

* fix keyboard to work with assets

* Add vmempool.space explorer links for offchain txs and assets (#380)

* Add vmempool.space explorer links for offchain txs and assets

- Add vmempool.space URL configuration with VITE_VMEMPOOL_URL env var
- Add helper functions for offchain tx and asset URLs in explorers.ts
- Update Table component to support clickable rows with onClick handlers
- Update Details component to accept wallet, assetId, and isOffchainTx props
- Make Transaction ID and Asset ID fields clickable in transaction details
- Open offchain txs (redeemTxid) in vmempool.space
- Open onchain txs (boardingTxid) in mempool.space
- Open assets in vmempool.space asset explorer
- Only show vmempool links for mainnet by default (configurable via env var)

* fix: keep tap-to-copy, add external link icon for explorer links

* fix: add aria-label to ExternalLinkIcon, hide link when URL unavailable

* fixes

---------

Co-authored-by: OpenClaw Agent <agent@openclaw.dev>
Co-authored-by: louisinger <louisinger@protonmail.com>
Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>

* fix max button with large amounts

* fix issuing of assets with new control asset

* Replace sleep with wait for new vtxo

* fix input amount

* Don't block app on swap init error (#383)

* Don't block the UX if swaps service isn't available and log errors

* Update SW

* Add tests

* Add tests

* lint

* fix nostr backup of swaps

* fix SVGs

* fix boltz env

* fix tests

* lint

* fix tests

* remove console.log

* update pnpm-lock

* refactor: simplify Hero component and remove LearnMore section (#395)

* fix(pwa): fix swipe-back navigation for iOS standalone mode

The iOS edge-swipe gesture was broken in the PWA: it navigated to wrong
screens, activated when it shouldn't (showing white pages), and caused
double animations. This was due to browser history entries being out of
sync with internal navigation state.

Replaces the old push-every-navigation model with 1:1 browser history
sync — browser entries only exist when there's real back navigation.
When the back stack is empty, iOS disables the swipe gesture entirely.

Key changes:
- navigation.tsx: stable popstate handler via refs (no phantom entries),
  root navigations clear browser history via history.go(-n), forward
  navigations push 1:1 to browser history
- options.tsx: Settings sub-navigation now pushes browser history entries
  so swipe-back works within Settings pages, coordinates with main nav
  via subNavHandler for cleanup on tab switches
- Distinguishes swipe vs back-button via isButtonBack flag to suppress
  Framer Motion animation on swipe (browser already animated it)
- Adds overscroll-behavior: none to prevent rubber-banding

Note: this is a pragmatic PWA workaround — relies on browser history
depth controlling iOS gesture activation. Will be properly solved when
migrating to Capacitor (native WKWebView has no Safari gestures).

* fix(nav): address CodeRabbit review feedback

- Pop blocked entries (InitConnect/Loading) from backStack to prevent
  infinite loop when user repeatedly presses back
- Guard historyDepth from going negative in Settings sub-navigation

* fix(nav): address second round of CodeRabbit feedback

- goBack delegates to subNavHandler when main backStack is empty
- navigateToOption resets historyDepth when returning to Menu

* update ts-sdk dependency

* fix double swap when claiming reverse swap

* fix(ui): match iOS/Android status bar color to app theme (#334)

* fix(ui): match iOS/Android status bar color to app theme

The status bar stayed white in dark mode because:
- The theme-color meta tag was hardcoded to #000000
- Safari doesn't reliably support media queries on theme-color
- The html element had no background-color, so the safe area defaulted to white

Fix: inline script in <head> reads the stored theme from localStorage
and sets theme-color + ion-palette-dark class before first paint.
applyTheme() keeps it in sync on manual toggle. Root element background
now follows --ion-background-color so the safe area fills correctly.

* fix(ui): add dev-only logging to theme bootstrap catch block

Address CodeRabbit nitpick: log theme init failures on localhost
so broken localStorage/config parsing is visible during development.

* fix(ios): status bar color matching and content overlap for iOS 26+ PWA

iOS 26+ ignores the theme-color meta tag — it derives the status bar
color from the actual rendered content at the top of the page. Add a
position:fixed body::before pseudo-element that sits behind the
transparent status bar and uses var(--ion-background-color), so iOS
samples the correct theme color instantly on theme switch.

Push .ion-page below the safe area with top: env(safe-area-inset-top)
using ion-app > .ion-page selector (higher specificity than Ionic's
core .ion-page { top: 0 } which loads later in the cascade).

Also fix manifest.json theme_color (#fff) and improve inline theme
bootstrap comments in index.html.

* style: fix prettier formatting in ionic.css

* fix(ui): match theme enum casing in bootstrap script

The Themes enum uses capitalized values ('Dark', 'Light', 'Auto') but
the inline bootstrap script compared against lowercase. This caused
the dark-mode flash prevention to fail when theme was explicitly set.

Also makes the iOS status bar comment version-agnostic per review.

---------

Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com>

* Upgrade ts-sdk 0.4.2 and boltz-swap 0.3.1 (#399)

* Fix send balance when sending btc with assets (#401)

* fix max balance when sending btc with assets

* fix keyboard when sending assets

* style: rebrand Assets to Arkade Mint + redesign app cards (#398)

* style(assets): rebrand Assets to Arkade Mint with pixel coin icon

- Rename "Assets" to "Arkade Mint" across all asset screens (list, settings, mint success)
- Replace placeholder "A" icon with Mario-style pixel coin in Arkade purple
- New icon matches size/format of other app icons (55px/78px)

* style(apps): redesign app cards with subtle surface treatment

Replace Shadow border+borderPurple cards with whisper-surface style:
- Subtle background tint instead of gray bg + harsh purple border
- Tactile press state with scale(0.97) + background darken
- Touch events matching Button component pattern (mouse + touch)
- Icon aligned to top-left, description nested under title
- Inline accessibility (role, tabIndex, keyboard handler)

src/screens/Apps/Index.tsx

* style: fix prettier formatting

* fix(test): update test selectors after Assets → Arkade Mint rename

The app card testId changed from 'app-assets' to 'app-arkade-mint' and
the header/settings text changed accordingly.

* fix(test): update 'Back to Assets' → 'Back to Arkade Mint' in tests

MintSuccess button text was renamed but tests still referenced old text.

---------

Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com>
Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>
Co-authored-by: Pietro Grandi <dev@pietro.uno>

* Upgrade ts-sdk@0.4.3 and boltz-swap@0.3.2 (#402)

* Upgrade ts-sdk@0.4.3 and boltz-swap@0.3.2

* Update SW

* feat: streamline onboarding — remove carousel, add dismissible PWA install banner (#375)

* feat(onboarding): remove onboarding carousel, skip straight to wallet

Remove the 4-step onboarding carousel (Onboard.tsx) and all supporting
components (StepBars, Bullet, AlertBox, Announcement, Onboard1-4 SVGs).
New users now go directly from Init → InitConnect → Wallet instead of
through the carousel. The announcement/PSA system is also removed as it
interrupted the flow without providing value.

Files removed:
- src/screens/Wallet/Onboard.tsx (carousel screen)
- src/components/AlertBox.tsx, Announcement.tsx, Bullet.tsx, StepBars.tsx
- src/icons/Onboard1-4.tsx (~10,800 lines of SVG illustrations)
- src/providers/announcements.tsx (PSA announcement system)
- Associated tests for removed components

Files modified:
- src/App.tsx: route to Init instead of Onboard for new wallets
- src/screens/Init/*.tsx: navigate to InitConnect instead of InitSuccess
- src/providers/navigation.tsx: remove Onboard from Pages enum
- src/index.tsx: remove AnnouncementProvider wrapper
- src/test/e2e/utils.ts: remove onboard steps from createWallet helper

* feat(wallet): add dismissible PWA install banner and password nudge

Add a new DismissibleBanner component used for both the PWA install
prompt and the password creation nudge. The banner features Framer Motion
enter/exit animations (EASE_OUT_QUINT, y:-24, scale:0.97), horizontal
swipe-to-dismiss gesture, layout animation for smooth sibling reflow,
and prefers-reduced-motion support with 44px touch targets.

PWA install banner:
- Mobile-only gating (isIOS || isAndroid) — hidden on desktop
- Platform-specific descriptions (iOS share sheet vs Android install)
- Module-level beforeinstallprompt capture for native install prompt
- Respects persistent dismissed state via config.dismissedBanners

Password nudge:
- Replaced old AlertBox with DismissibleBanner (expanded layout)
- Added nudgeVisible/nudgeCheckComplete to context for PWA banner
  priority coordination (PWA banner waits for nudge check)
- Fixed error handling: .catch/.finally on async password check

Config: replaced announcementsSeen with dismissedBanners (existing
configs handled gracefully via ?? [] fallbacks).

New files:
- src/components/DismissibleBanner.tsx

Modified:
- src/screens/Wallet/Index.tsx (PWA banner integration)
- src/providers/nudge.tsx (DismissibleBanner + error handling)
- src/lib/pwa.ts (usePwaInstalled, beforeinstallprompt, promptPwaInstall)
- src/lib/types.ts, src/providers/config.tsx (dismissedBanners)
- src/ionic.css (--purple10 color variable)
- src/test/setup.ts (matchMedia mock)

* fix: address CodeRabbit review feedback

- Password.tsx: reset restoring flag when navigating to InitConnect
  to prevent stale state from a previous restore attempt
- DismissibleBanner.tsx: disable layout animation and popLayout mode
  when prefers-reduced-motion is active
- Wallet/Index.tsx: only show Install action button when native
  prompt is available (Android); iOS shows instruction-only banner

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

* fix(wallet): clarify iOS PWA banner instruction text

Update the iOS PWA install banner description to reference "the share
icon in Safari's toolbar" instead of generic "the share button" so
users know exactly where to look.

* fix(pwa): return prompt outcome and auto-dismiss banner after install

promptPwaInstall() now returns 'accepted' | 'dismissed' | null instead
of void. On Android, the banner auto-dismisses after the user interacts
with the native install prompt, rather than staying visible.

* fix(banner): add haptic feedback to DismissibleBanner interactions

Fire hapticSubtle() on button taps (Dismiss, Install, Set password)
and on swipe-to-dismiss gesture, consistent with all other interactive
elements in the app.

* feat(onboarding): add animated SVG logo morph sequence with delight effects

Redesign the Init screen with a rich onboarding animation:

- OnboardingLogo: SVG cross-fade between Arcade, Space Invader, and Heart
  shapes with deep squash-stretch bounces using Framer Motion async
  animation controls. Final bounce flies logo to header position via FLIP.
- PixelSplash: burst of purple pixel particles on each bounce
- PixelSunrise: pixelated purple gradient at screen top (fixed position)
- Three bullet points with outlined icon components (Bolt, Globe, ShieldCheck)
  replacing the old paragraph text
- Reduced motion: all animations skip, content renders instantly

Files: OnboardingLogo.tsx, PixelSplash.tsx, PixelSunrise.tsx,
BoltOutline.tsx, GlobeOutline.tsx, ShieldCheckOutline.tsx,
Init.tsx, ionic.css

* style(onboarding): fix Prettier formatting in OnboardingLogo and Init

* style(onboarding): smooth radial gradient, layout + status bar fixes

- PixelSunrise: replace pixelated grid with smooth radial-gradient,
  remove all pixel dissolution code
- Init: move content to bottom of screen (flex-end + paddingBottom),
  stack logo above title with optical alignment to bullet icons
- Status bar: add apple-mobile-web-app-capable for PWA bleed-through,
  set theme-color to #8771BC (computed gradient top on white),
  fix manifest.json theme_color mismatch
- Dynamic theme-color useEffect on Init screen for Safari tinting

* style(onboarding): fix Prettier formatting in Init

* fix(onboarding): stable fly target, gradient portal, dark mode prep

- Always render content in DOM (hidden until ready) so logoTargetRef
  position is stable during fly animation instead of shifting on mount
- Portal OnboardingLogo and PixelSunrise to document.body so they
  escape IonPage's max-width: 640px container on desktop
- Add color-scheme light/dark to :root and .ion-palette-dark
- Add color-scheme meta tag and body background-color for Safari
- Sunrise animation synced with fly via onFlyStart callback
- Snappier bounce physics: deeper squash, higher overshoot, faster timings

* style(onboarding): fix Prettier formatting in Init.tsx

* feat(onboarding): pixel morph animation, spring physics, loading bypass

Logo morph animation:
- Rewrite OnboardingLogo to use pixel-shapes.ts grid system instead of
  opacity crossfade between SVG path components
- Arcade "A" always renders as clean SVG paths (never pixelated), while
  Invader and Heart shapes use pixel rects with CSS transform transitions
- Spring-based bounce recovery (stiffness: 600, damping: 18, mass: 0.6)
  replaces scripted multi-step keyframes for natural overshoot feel
- Morph + bounce happen simultaneously — shape swap at squash peak
- Faster timing: 180ms initial pause, 60ms between morphs, ~1s total
- Portal to document.body so logo escapes IonPage max-width on desktop

Pixel splash particles:
- Increase from 12-17 to 20-28 particles per burst
- Wider velocity range (25-105) and more size variation (2.5-6.5px)

Skip loading screen for new users:
- New users (no wallet in storage) bypass ASP + JS capability gates,
  going straight to Init where the logo morph serves as intro visual
- Wallet provider reads from localStorage synchronously via lazy
  useState initializer, eliminating one-frame Loading flash
- "Create wallet" button stays disabled until ASP resolves

Revert status bar changes:
- All theme-color, color-scheme, and apple-mobile-web-app changes from
  this branch reverted — will be handled in a separate PR
- Remove temp replay button and associated state

Files: OnboardingLogo.tsx, PixelSplash.tsx, Init.tsx, App.tsx,
       wallet.tsx, index.html, manifest.json, ionic.css, index.css

* fix(wallet): remove unused setWalletLoaded setter

walletLoaded is now always true (synchronous init), so the setter is
dead code. Replace useState with a constant to satisfy eslint.

* Apply suggestion from @tiero

Signed-off-by: Marco Argentieri <3596602+tiero@users.noreply.github.com>

* fix: use named export for PageTransition to fix Rollup tree-shaking

Rollup's module concatenation was dropping the default export definition
while keeping the JSX references, causing 'PageTransition is not defined'
at runtime. Named exports are handled more reliably.

* fix: update e2e helpers for new onboarding flow (no more InitSuccess page)

The onboarding flow now goes Init → InitConnect → Wallet directly,
skipping the InitSuccess page. The createWallet() and restoreWallet()
helpers were waiting for "Your new wallet is live!" / clicking
"Go to wallet" which no longer exist, causing all Playwright tests
to hang indefinitely.

---------

Signed-off-by: Marco Argentieri <3596602+tiero@users.noreply.github.com>
Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Marco Argentieri <3596602+tiero@users.noreply.github.com>
Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>

* perf(transactions): virtualise TransactionsList with @tanstack/react-virtual (#394)

* test: add smoke test for TransactionsList with large tx history

Reproduces the reported lag on wallets with several thousand transactions.
TransactionsList renders every row into the DOM at once with no windowing;
render time scales linearly (~1.8ms/tx in jsdom), so:
  100 txs  →   471ms  ✓
  500 txs  → 1,475ms  ✗
 1000 txs  → 2,258ms  ✗
 3000 txs  → 5,477ms  ✗
 5000 txs  → 9,206ms  ✗

The fix is to virtualise the list (e.g. @tanstack/react-virtual).
This test will serve as the regression gate once that is in place.

https://claude.ai/code/session_01LDxnFdhREuPcK3yPCkQUMU

* perf(transactions): virtualise TransactionsList with @tanstack/react-virtual

Fixes the wallet freeze / extreme lag reported for wallets with several
thousand transactions.

Root cause
----------
TransactionsList rendered every Tx into the DOM unconditionally via a plain
.map() call.  With no windowing, render time scaled linearly with the
transaction count (~1.8 ms/tx in jsdom, worse in a real browser due to
layout and paint):

  Before fix (all DOM nodes mounted):
    100 txs  →    471 ms
    500 txs  →  1 475 ms
   1000 txs  →  2 258 ms
   3000 txs  →  5 477 ms
   5000 txs  →  9 206 ms

Fix
---
Replace the plain map() with @tanstack/react-virtual's useVirtualizer.
Only the rows that fit in the viewport (+ a 5-row overscan buffer above
and below) are ever mounted in the DOM.  The total DOM node count is now
O(visible rows) ≈ 15–25 regardless of wallet history size.

  After fix (virtual rows only):
    100 txs  →  4 ms ✓
    500 txs  →  3 ms ✓
   1000 txs  →  4 ms ✓
   3000 txs  →  4 ms ✓
   5000 txs  →  3 ms ✓

The list renders in a self-contained scrollable container
(height: calc(100dvh - 380px), min-height: 200px) so the balance/buttons
section above it remains static — a common pattern in mobile wallet UIs.

Smoke test update
-----------------
- Added a DOM-count assertion: at most MAX_DOM_ROWS (20) rows may be
  mounted at any one time, ensuring virtualisation cannot regress silently.
- Retained the per-count render-time assertions (≤ 500 ms) as a belt-and-
  braces guard.

https://claude.ai/code/session_01LDxnFdhREuPcK3yPCkQUMU

* feat(TransactionsList): add ArrowUp/ArrowDown keyboard navigation with virtualizer scroll-sync

Add focusedIndex state and handleListKeyDown on the parentRef scroll
container. Arrow key presses call virtualizer.scrollToIndex(next) then
requestAnimationFrame to focus the target Focusable row once it is
mounted. Existing Enter → handleClick and Escape → focusOnOuterShell
behaviour is preserved; Tab is unchanged.

https://claude.ai/code/session_01LDxnFdhREuPcK3yPCkQUMU

* fix(TransactionsList): guard focusRow and focusOnFirstRow against empty/out-of-range indices

Add an early-return bounds check in focusRow (index < 0 || index >= txs.length)
to prevent virtualizer.scrollToIndex and key(txs[index]) from executing on
invalid indices. Move the txs.length === 0 guard before setFocused(true) in
focusOnFirstRow so focused is never set true when there are no rows to navigate.
The focusRow guard also covers the edge case in handleListKeyDown where
txs.length === 0 would produce a -1 next index.

https://claude.ai/code/session_01LDxnFdhREuPcK3yPCkQUMU

* remove transactions smoke test

https://claude.ai/code/session_01LDxnFdhREuPcK3yPCkQUMU

* fix(keyboard): sync focusedIndex with actual DOM focus on row focus events (#397)

* Initial plan

* fix(keyboard): sync focusedIndex with actual DOM focus on row focus events

Co-authored-by: tiero <3596602+tiero@users.noreply.github.com>

* chore: remove accidentally committed package-lock.json

Co-authored-by: tiero <3596602+tiero@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tiero <3596602+tiero@users.noreply.github.com>

* fix: hide virtualizer scrollbar and address review feedback

- Hide scrollbar on transaction list (reported by @Kukks)
- Remove package-lock.json from .gitignore (feedback from @pietro909)
- Remove redundant setFocusedIndex calls (CodeRabbit nitpick)

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>

* fix: restore keyboard navigation on Apps list (#404)

The app card redesign (#398) replaced the Focusable wrapper with a plain
div, losing the focus ring styling and keyboard nav flow from #283.
Re-wrap each App card in Focusable to restore focus indicator and
Enter/Space activation.

* fix: dynamic row height for virtualised transaction list (#405)

Rows with asset info (e.g. "-14.7 BB") are taller than the 61px
estimate, causing overlap. Enable measureElement so @tanstack/react-virtual
measures actual DOM heights instead of relying on the fixed estimate.

* Support undefined amounts and invalid invoices (#403)

* fix sending to onchain address when having assets (#406)

* remove tooltip from Text

* hotfix

* Fix coin list with assets (#407)

* remove tooltip from Text

* show amounts in separate line in coin list

* fix InfoLine flash show (#408)

* show loading in coin control (#409)

* remove link from assets app

* fix: dark mode banner button text invisible (#391998 on dark bg)

- Switch DismissibleBanner button color from --purple to --purpletext
- Add --purpletext dark mode override (#9678f7) for better contrast

* fix: transaction list requires two clicks to navigate

The onFocus handler on virtualised rows called setState (setFocused,
setFocusedIndex), triggering a React re-render that destroyed the
clicked DOM element before the click event completed. Convert both
to refs since they're only used in event handlers, not render output.

* Upgrade ts-sdk@0.4.4 and boltz-swap@0.3.3 (#411)

* Fix receive amount in fiat only

* Fix receive amount in fiat only

* Fix collab exit (#413)

---------

Signed-off-by: Marco Argentieri <3596602+tiero@users.noreply.github.com>
Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>
Co-authored-by: louisinger <louis@vulpem.com>
Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
Co-authored-by: louisinger <41042567+louisinger@users.noreply.github.com>
Co-authored-by: OpenClaw Agent <agent@openclaw.dev>
Co-authored-by: louisinger <louisinger@protonmail.com>
Co-authored-by: Marco Argentieri <3596602+tiero@users.noreply.github.com>
Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com>
Co-authored-by: Sahil Chaturvedi <sahilchaturvedi@protonmail.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
@coderabbitai coderabbitai Bot mentioned this pull request Mar 23, 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.

2 participants