Repository navigation
feat(ui): animated page transitions, wallet stagger, keyboard overlays - #329
Merged
Merged
Conversation
Add smooth, directional page transitions for sub-page navigation: - Forward navigation slides content left, new content enters from right - Back navigation reverses the slide direction - Tab switches (Wallet/Apps/Settings) remain instant, no animation - Settings sub-pages get the same slide transitions - Wallet screen gets a staggered cascade load-in on initial load - All animations respect prefers-reduced-motion Animation specs follow Emil Kowalski's principles: - ease-out quint easing, 300ms entrance / 240ms exit - Only transform + opacity animated (GPU-composited) New files: - src/lib/animations.ts (variants, easing, timing constants) - src/hooks/useReducedMotion.ts (accessibility hook) - src/components/PageTransition.tsx (directional slide wrapper) - src/components/WalletLoadIn.tsx (staggered cascade wrapper) - src/components/SettingsPageTransition.tsx (settings sub-page transitions) Modified files: - src/providers/navigation.tsx (direction tracking, isInitialLoad flag) - src/providers/options.tsx (settings direction tracking) - src/App.tsx (AnimatePresence per tab with PageTransition) - src/screens/Settings/Index.tsx (animated sub-page switching) - src/screens/Wallet/Index.tsx (staggered load-in on first load) - vite.config.ts (allowedHosts for cloudflare tunnel mobile testing)
… overlays
Page transitions:
- Directional slide + opacity animations using Framer Motion's custom
prop for dynamic forward/back direction
- Unified pageTransitionVariants replaces separate pageSlideForward,
pageSlideBack, and noAnimation variants
- AnimatePresence mode changed from popLayout to sync for smoother
concurrent enter/exit
- page-transition-container with overflow containment and CSS paint
isolation
Wallet entrance stagger:
- Sequential fade-in of all wallet elements on initial load: logo,
balance, buttons, "Transaction history" label, individual tx rows
- Bypass AnimatePresence initial={false} propagation using state-driven
animate prop in WalletStaggerContainer
- Delay navigation from Unlock to Wallet until dataReady flag is set,
eliminating skeleton→data flash
- TransactionsList accepts ItemWrapper prop for per-row stagger
Keyboard/scanner overlays:
- Send and Receive numpad/scanner slide up from bottom using
keyboardOverlay variants instead of replacing the page
Navigation:
- Add direction state and goBack() to NavigationContext
- Header back prop accepts boolean (uses goBack) or function
- isInitialLoad checks tab membership instead of specific page
- 10+ screens simplified from back={() => navigate(Pages.X)} to back
Accessibility:
- All animations respect prefers-reduced-motion via useReducedMotion
- Reduced motion renders static content without motion wrappers
Cleanup:
- Remove unused Skeleton.tsx, loadingExitVariants, noAnimation variants
- Remove orphaned fade-in and shimmer CSS keyframes
Deploying wallet-bitcoin with
|
| Latest commit: |
4abf59b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://16feca03.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://feat-page-transitions.wallet-bitcoin.pages.dev |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Framer Motion animations (page/overlay/stagger), reduced-motion support, new animation components/hooks/variants, provider state for navigation/options/wallet (direction, goBack, isInitialLoad, dataReady), widens Header.back typing, and updates many screens to use transitions, overlays, and staggered load-ins. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant NavigationContext
participant useReducedMotion
participant AnimatePresence
participant PageTransition
User->>App: trigger navigation / select tab
App->>NavigationContext: navigate()
NavigationContext-->>App: direction (forward/back/none)
App->>useReducedMotion: query prefersReduced
useReducedMotion-->>App: prefersReduced
App->>AnimatePresence: render tab children with custom direction
AnimatePresence->>PageTransition: mount pageKey with custom direction
PageTransition->>User: animate entry/exit or skip if prefersReduced
sequenceDiagram
participant User
participant UnlockScreen
participant WalletContext
participant NavigationContext
participant WalletScreen
participant WalletStaggerContainer
User->>UnlockScreen: submit credentials
UnlockScreen->>UnlockScreen: set unlocked = true
WalletContext->>WalletContext: load data -> set dataReady = true
UnlockScreen->>NavigationContext: navigate() (when unlocked && dataReady)
NavigationContext-->>WalletScreen: render (isInitialLoad flag)
WalletScreen->>WalletStaggerContainer: animate children if initial
WalletStaggerContainer->>User: show staggered UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying wallet-mutinynet with
|
| Latest commit: |
4abf59b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://97b33a9d.arkade-wallet.pages.dev |
| Branch Preview URL: | https://feat-page-transitions.arkade-wallet.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Send/Form.tsx (1)
2-35:⚠️ Potential issue | 🟡 MinorRespect reduced‑motion for keyboard/scanner overlays.
These overlays always animate, even when users prefer reduced motion. Consider bypassing the overlay animation when reduced motion is enabled to align with the rest of the app’s motion handling.♿ Suggested adjustment
-import { AnimatePresence, motion } from 'framer-motion' +import { AnimatePresence, motion } from 'framer-motion' +import { useReducedMotion } from '../../../hooks/useReducedMotion' @@ const { balance, svcWallet } = useContext(WalletContext) + const prefersReduced = useReducedMotion() @@ - const overlayStyle = { + const overlayVariants = prefersReduced + ? { initial: { y: 0 }, animate: { y: 0, transition: { duration: 0 } }, exit: { y: 0, transition: { duration: 0 } } } + : keyboardOverlay + const overlayStyle = { position: 'absolute' as const, inset: 0, zIndex: 10, display: 'flex', flexDirection: 'column' as const, background: 'var(--ion-background-color)', } @@ - variants={keyboardOverlay} + variants={overlayVariants} @@ - variants={keyboardOverlay} + variants={overlayVariants}Also applies to: 413-505
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Send/Form.tsx` around lines 2 - 35, The keyboard/scanner overlay animations (e.g., the keyboardOverlay animation used with AnimatePresence around Keyboard and Scanner) ignore users' reduced-motion preference; update the show/hide logic to detect reduced motion (use framer-motion's useReducedMotion or prefers-reduced-motion) and when reduced motion is enabled, render the overlays without animation (bypass keyboardOverlay variants and remove motion wrappers or set instantaneous transitions). Apply the same change for the other overlay usage around Scanner/Keyboard later in the file (the block referenced at lines ~413-505) so both overlays respect reduced motion.
🧹 Nitpick comments (12)
src/screens/Wallet/Unlock.tsx (2)
34-36:navigate(andinitWalletin the first effect) are missing from theiruseEffectdependency arrays.Both effects call context-provided functions without listing them as dependencies. While these functions are likely stable references, omitting them violates the exhaustive-deps rule and could cause stale-closure bugs if the context ever provides new references (e.g., after re-initialization).
♻️ Proposed fix
useEffect(() => { const pass = password ? password : defaultPassword getPrivateKey(pass) .then(initWallet) .then(() => setUnlocked(true)) .catch((err) => { setTried(true) if (password) { consoleError(err, 'error unlocking wallet') setError('Invalid password') } }) - }, [password]) + }, [password, initWallet]) useEffect(() => { if (unlocked && dataReady) navigate(Pages.Wallet) - }, [unlocked, dataReady]) + }, [unlocked, dataReady, navigate])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Unlock.tsx` around lines 34 - 36, The effect hooks reference context functions that aren’t listed in their dependency arrays — add the context functions to the deps so the effects react to changed references: include initWallet in the dependency array of the useEffect that calls initWallet, and include navigate in the dependency array of the useEffect that calls navigate (or alternatively memoize these context functions where provided); update the dependency arrays for the effects that call initWallet and navigate to include those identifiers to satisfy exhaustive-deps and avoid stale-closure bugs.
20-32: No cancellation guard —setUnlocked(true)may fire on an unmounted component.If the user navigates away (e.g., hard refresh or back) while
getPrivateKey/initWalletis still in flight, the.then(() => setUnlocked(true))callback will still execute. Add an ignore flag to guard against the update.♻️ Proposed fix
useEffect(() => { + let ignore = false const pass = password ? password : defaultPassword getPrivateKey(pass) .then(initWallet) - .then(() => setUnlocked(true)) + .then(() => { if (!ignore) setUnlocked(true) }) .catch((err) => { setTried(true) if (password) { consoleError(err, 'error unlocking wallet') setError('Invalid password') } }) + return () => { ignore = true } }, [password, initWallet])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Unlock.tsx` around lines 20 - 32, The effect calling getPrivateKey(pass).then(initWallet)... needs an "ignore/unmounted" guard to avoid calling setUnlocked (and other state setters) after the component unmounts; inside the useEffect create a local let cancelled = false (or isMounted = true), check cancelled before calling setUnlocked(true), setTried(true), and setError(...) in the .then/.catch handlers, and flip cancelled to true in the effect cleanup function so in-flight promises don't update state; reference the existing useEffect, getPrivateKey, initWallet, setUnlocked, setTried, and setError when making the change.vite.config.ts (1)
17-17: Consider keepingallowedHostsout of the committed config.The PR notes this was added for Cloudflare tunnel testing. Committing
.trycloudflare.comas an allowed host means the dev server is exposed to anyone with a Cloudflare tunnel on every developer's machine. Consider using a localvite.config.local.tsoverride or an environment variable guard instead.♻️ Proposed alternative using an env guard
- allowedHosts: ['.trycloudflare.com'], + ...(process.env.ALLOW_CLOUDFLARE_TUNNEL ? { allowedHosts: ['.trycloudflare.com'] } : {}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.ts` at line 17, The committed vite.config.ts adds allowedHosts: ['.trycloudflare.com'] which exposes the dev server; remove this from the main config and guard it behind a developer-only mechanism instead — either load an uncommitted vite.config.local.ts override or set allowedHosts conditionally in the exported Vite config (the defineConfig export in vite.config.ts) only when an env var like ENABLE_CF_TUNNEL==='true' is present; ensure the symbol allowedHosts is only set in that guarded branch and document the env/local file so developers can opt into Cloudflare tunnel testing without committing the host list.package.json (1)
21-21:framer-motion ^12.34.3is valid — consider migrating to the canonicalmotionpackage.
framer-motionversion12.34.3is the latest published release on npm. However, there are no breaking API changes in version 12 for React usage, and the Motion team now recommends uninstallingframer-motionand installingmotioninstead. Adoptingmotion(the renamed canonical package) will keep the project aligned with the library's active development path going forward.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 21, Replace the dependency "framer-motion": "^12.34.3" in package.json with the canonical package name "motion" (e.g., "motion": "^12.34.3" or the current motion semver), run package install, and update all imports that reference 'framer-motion' to import from 'motion' instead (search for string "framer-motion" across the repo and change to "motion"); after updating, rebuild and run tests to ensure there are no breaking import/API differences.src/providers/navigation.tsx (1)
251-260:navigateis not wrapped inuseCallback, causing a new reference on every render.Every re-render of
NavigationProviderproduces a newnavigatefunction, which can cause unnecessary re-renders in all context consumers that includenavigatein their dependency arrays. Wrapping it inuseCallback(with[tab, screen]or no deps if stable) would eliminate this churn.♻️ Proposed fix
- const navigate = (page: Pages) => { + const navigate = useCallback((page: Pages) => { const nextTab = pageTab[page] const isTabSwitch = nextTab !== tab && ROOT_PAGES.has(page) && ROOT_PAGES.has(screen) previousPage.current = screen push(page) setDirection(isTabSwitch ? 'none' : 'forward') setScreen(page) setTab(nextTab) - } + }, [tab, screen])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/navigation.tsx` around lines 251 - 260, The navigate function is recreated on every render causing unnecessary re-renders in consumers; wrap navigate in React.useCallback to memoize it and supply the correct dependency array (include tab, screen and any setter/utility references used such as push, setDirection, setScreen, setTab, and ROOT_PAGES if it can change) so its reference stays stable, keeping the existing logic (compute nextTab from pageTab, determine isTabSwitch using ROOT_PAGES and screen, set previousPage.current, call push(page), setDirection, setScreen, setTab).src/components/TransactionsList.tsx (2)
152-167: Inconsistent use ofWrapvs. inline conditional for transaction rows.
TextLabelis wrapped via<Wrap>(line 147–149), but each transaction row re-evaluatesItemWrapper ?inline (line 166) instead of reusingWrap. After hoisting the fallback (see above), usingWrapconsistently removes the redundant conditional and keeps the rendering pattern uniform.♻️ Proposed cleanup
- {txs.map((tx) => { - const k = key(tx) - const focusable = ( - <Focusable - id={k} - key={k} - inactive={!focused} - onEnter={() => handleClick(tx)} - onEscape={focusOnOuterShell} - ariaLabel={ariaLabel(tx)} - > - <TransactionLine onClick={() => handleClick(tx)} tx={tx} /> - </Focusable> - ) - return ItemWrapper ? <ItemWrapper key={k}>{focusable}</ItemWrapper> : focusable - })} + {txs.map((tx) => { + const k = key(tx) + return ( + <Wrap key={k}> + <Focusable + id={k} + inactive={!focused} + onEnter={() => handleClick(tx)} + onEscape={focusOnOuterShell} + ariaLabel={ariaLabel(tx)} + > + <TransactionLine onClick={() => handleClick(tx)} tx={tx} /> + </Focusable> + </Wrap> + ) + })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TransactionsList.tsx` around lines 152 - 167, The txs.map block creates a focusable variable and then uses an inline conditional ItemWrapper ? ... : focusable; instead hoist the fallback by ensuring Wrap is defined (the previously introduced Wrap fallback should be reused) and always render <Wrap key={k}>{focusable}</Wrap> (or the Wrap equivalent) so the map no longer contains the inline ternary; update the mapping to reference key(tx) / k, the Focusable instance, and keep props like onEnter={() => handleClick(tx)}, onEscape={focusOnOuterShell}, ariaLabel={ariaLabel(tx)} and TransactionLine intact while removing the ItemWrapper ? ... : focusable conditional.
143-143: Inline anonymous component definition causes unnecessary remounts.
const Wrap = ItemWrapper ?? (({ children }) => <>{children}</>)creates a new function reference on every render whenItemWrapperisundefined. React treats it as a new component type, unmounting and remounting children on each render cycle.♻️ Proposed fix — hoist the fallback outside the component
+const DefaultWrapper = ({ children }: { children: ReactNode }) => <>{children}</> export default function TransactionsList({ ItemWrapper }: TransactionsListProps) { ... - const Wrap = ItemWrapper ?? (({ children }: { children: ReactNode }) => <>{children}</>) + const Wrap = ItemWrapper ?? DefaultWrapper🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TransactionsList.tsx` at line 143, The inline anonymous fallback component causes remounts because a new function is created on every render; fix by hoisting a stable fallback component out of the component render scope (e.g., create a module-level DefaultWrapper component) and then use const Wrap = ItemWrapper ?? DefaultWrapper so Wrap is a stable reference when ItemWrapper is undefined; update references to Wrap to use the hoisted DefaultWrapper name to avoid recreating the function on each render.src/hooks/useReducedMotion.ts (2)
1-17: Consider delegating to framer-motion's built-inuseReducedMotioninstead of reimplementing it.Since framer-motion is already a dependency in this PR, it exports a
useReducedMotionhook that returnstrueif the current device has Reduced Motion enabled, and actively responds to changes, re-rendering components with the latest setting. Rolling a custom one adds maintenance surface for identical behavior.♻️ Suggested simplification
-import { useState, useEffect } from 'react' +import { useReducedMotion as useFramerReducedMotion } from 'framer-motion' -export function useReducedMotion(): boolean { - const [prefersReduced, setPrefersReduced] = useState(() => { - if (typeof window === 'undefined') return false - return window.matchMedia('(prefers-reduced-motion: reduce)').matches - }) - - useEffect(() => { - const mq = window.matchMedia('(prefers-reduced-motion: reduce)') - const handler = (e: MediaQueryListEvent) => setPrefersReduced(e.matches) - mq.addEventListener('change', handler) - return () => mq.removeEventListener('change', handler) - }, []) - - return prefersReduced -} +// Delegates to framer-motion's built-in hook; ?? false handles the initial null on SSR +export function useReducedMotion(): boolean { + return useFramerReducedMotion() ?? false +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useReducedMotion.ts` around lines 1 - 17, Replace the custom implementation of useReducedMotion with framer-motion's built-in hook: remove the local useReducedMotion function body and instead re-export or forward to framer-motion's useReducedMotion so the hook is identical to the dependency already in the project; update the file to import/export the symbol from 'framer-motion' (keep the exported name useReducedMotion so callers don't change).
1-17: Consider using framer-motion's built-inuseReducedMotioninstead of reimplementing it.Since framer-motion is already a dependency in this PR, it ships its own
useReducedMotionhook that handles the same SSR-safe media query subscription and updates on changes. Rolling a custom one adds maintenance surface for identical behavior.♻️ Suggested simplification
-import { useState, useEffect } from 'react' +import { useReducedMotion } from 'motion/react' -export function useReducedMotion(): boolean { - const [prefersReduced, setPrefersReduced] = useState(() => { - if (typeof window === 'undefined') return false - return window.matchMedia('(prefers-reduced-motion: reduce)').matches - }) - - useEffect(() => { - const mq = window.matchMedia('(prefers-reduced-motion: reduce)') - const handler = (e: MediaQueryListEvent) => setPrefersReduced(e.matches) - mq.addEventListener('change', handler) - return () => mq.removeEventListener('change', handler) - }, []) - - return prefersReduced -} +export function useReducedMotion(): boolean { + return useReducedMotion() +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useReducedMotion.ts` around lines 1 - 17, Replace the custom useReducedMotion implementation with framer-motion's built-in hook: remove the local useReducedMotion function body and import useReducedMotion from 'framer-motion' (or re-export it) so consumers use the library's SSR-safe implementation; ensure any code referencing the local useReducedMotion keeps the same boolean return shape and remove the MediaQueryList listener logic from this file.src/providers/options.tsx (1)
153-163:optionSectionmissing fromgoBackdependency array — safe but worth noting
goBackcallsoptionSection(defined at Lines 141–143) but doesn't list it in theuseCallbackdependency array. This is safe becauseoptionSectionis a pure function that only reads the module-leveloptionsconstant (no state/props captured), so it never goes stale. IfoptionSectionever starts capturing component-level state or props, this will silently break. Extracting it as a module-level helper (outside the component) would make the intent explicit and eliminate the lint-escape-hatch concern.♻️ Move `optionSection` outside the component
+const optionSection = (opt: SettingsOptions): SettingsSections => { + return options.find((o) => o.option === opt)?.section || SettingsSections.General +} + export const OptionsProvider = ({ children }: { children: ReactNode }) => { const [option, setOption] = useState(SettingsOptions.Menu) const [direction, setDirection] = useState<SettingsDirection>('forward') - const optionSection = (opt: SettingsOptions): SettingsSections => { - return options.find((o) => o.option === opt)?.section || SettingsSections.General - } - const navigateToOption = useCallback(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/options.tsx` around lines 153 - 163, The goBack callback references optionSection but doesn't include it in the useCallback dependency array; to make this explicit and future-proof, move optionSection out of the component as a module-level helper (so it no longer closes over component scope) or add optionSection to the dependency array of useCallback; update the implementation around goBack, optionSection, and the useCallback import (useCallback) so that goBack either depends on a stable module-level function or lists optionSection in its dependencies (preserving existing behavior with SettingsSections and SettingsOptions).src/screens/Settings/Index.tsx (1)
23-62:settingsContentdefault case silently swallows unknown optionsThe
default: return <></>case renders nothing without logging a warning, which could silently hide bugs if a newSettingsOptionsvalue is added without a corresponding case. Aconsole.warnorassertUnreachableguard would catch omissions at development time.♻️ Proposed: add a dev-time warning in the default case
default: + if (process.env.NODE_ENV !== 'production') { + console.warn(`settingsContent: unhandled option "${option}"`) + } return <></>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Settings/Index.tsx` around lines 23 - 62, The default branch of settingsContent currently returns an empty fragment and can silently hide missing cases for the SettingsOptions enum; update the default to surface a dev-time warning or guard by logging the unexpected option (e.g., console.warn with the option value) or calling an assertUnreachable helper so developers are alerted when a new SettingsOptions value lacks a corresponding case in settingsContent; make the change inside the settingsContent function and reference the SettingsOptions value passed in to include it in the diagnostic message.src/screens/Wallet/Index.tsx (1)
77-77: EmptyItemwrapper consumes a stagger slot when bothnudgeandpsaMessageare falsyWhen
nudgeisnullandpsaMessageis empty/falsy,<Item>{null}</Item>renders an emptyWalletStaggerChild— amotion.divthat participates in the stagger sequence but is invisible. This shifts the timing of subsequent animated children without any visible benefit.♻️ Proposed fix: conditionally render the Item
- <Item>{nudge ? nudge : psaMessage ? <InfoBox html={psaMessage} /> : null}</Item> + {(nudge || psaMessage) && ( + <Item>{nudge ? nudge : <InfoBox html={psaMessage} />}</Item> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Index.tsx` at line 77, The empty <Item> wrapper causes a hidden WalletStaggerChild to take a stagger slot when both nudge and psaMessage are falsy; update the JSX in the Wallet screen so that <Item> is only rendered when either nudge or psaMessage is truthy (i.e., render <Item>{nudge ?? <InfoBox html={psaMessage} />}</Item> or use a conditional like { (nudge || psaMessage) && <Item>...</Item> }), referencing the existing Item, nudge, psaMessage and InfoBox symbols to locate and change the line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 21: Update the framer-motion dependency version in package.json from the
non-existent "12.34.3" to a published release by replacing the version string
for "framer-motion" with "^12.34.0"; ensure the package.json dependency entry
"framer-motion" is updated and then run your package manager (npm/yarn/pnpm) to
refresh lockfile and verify installation.
In `@src/providers/wallet.tsx`:
- Around line 73-76: Reset the "dataReady" flag and the "hasLoadedOnce" ref when
the wallet is locked or reset so stale UI data can't flash on re-unlock: in the
implementations of lockWallet and resetWallet, explicitly call
setDataReady(false) and set hasLoadedOnce.current = false (references:
dataReady, setDataReady, hasLoadedOnce, lockWallet, resetWallet); this ensures
initWallet → svcWallet → reloadWallet will re-run the first-load guard and let
dataReady transition false→true only after fresh reload completes.
In `@src/screens/Wallet/Index.tsx`:
- Around line 51-89: The current pattern swaps JSX component types (Container
and Item) between WalletStaggerContainer/WalletStaggerChild and Passthrough
based on shouldStagger, causing React to unmount/remount the whole subtree when
isInitialLoad flips; fix it by always rendering the same component types and
threading shouldStagger as an animate prop: stop assigning Container/Item to
different component types in Index.tsx and instead render the stable
WalletStaggerContainer and WalletStaggerChild (or a single stable wrapper) with
an animate={shouldStagger} prop; update the implementations in
src/components/WalletLoadIn.tsx (WalletStaggerContainer and WalletStaggerChild)
to accept an animate boolean and internally no-op (render children) when animate
is false so animation behavior is toggled without changing element types.
In `@src/screens/Wallet/Receive/Amount.tsx`:
- Around line 181-201: The keyboard overlay currently always animates using the
keyboardOverlay variants; wrap or gate its animation with the same
reduced-motion check used elsewhere: call useReducedMotion (or reuse the
existing hook) in this component and when reduced motion is requested, render
the motion.div with no variants/animations (or set initial and animate to the
same state) so the Keyboard overlay (the AnimatePresence + motion.div that uses
keyboardOverlay and the Keyboard component) does not perform the slide-up
animation; keep the same DOM/props but disable motion when reduced motion is
true.
---
Outside diff comments:
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 2-35: The keyboard/scanner overlay animations (e.g., the
keyboardOverlay animation used with AnimatePresence around Keyboard and Scanner)
ignore users' reduced-motion preference; update the show/hide logic to detect
reduced motion (use framer-motion's useReducedMotion or prefers-reduced-motion)
and when reduced motion is enabled, render the overlays without animation
(bypass keyboardOverlay variants and remove motion wrappers or set instantaneous
transitions). Apply the same change for the other overlay usage around
Scanner/Keyboard later in the file (the block referenced at lines ~413-505) so
both overlays respect reduced motion.
---
Nitpick comments:
In `@package.json`:
- Line 21: Replace the dependency "framer-motion": "^12.34.3" in package.json
with the canonical package name "motion" (e.g., "motion": "^12.34.3" or the
current motion semver), run package install, and update all imports that
reference 'framer-motion' to import from 'motion' instead (search for string
"framer-motion" across the repo and change to "motion"); after updating, rebuild
and run tests to ensure there are no breaking import/API differences.
In `@src/components/TransactionsList.tsx`:
- Around line 152-167: The txs.map block creates a focusable variable and then
uses an inline conditional ItemWrapper ? ... : focusable; instead hoist the
fallback by ensuring Wrap is defined (the previously introduced Wrap fallback
should be reused) and always render <Wrap key={k}>{focusable}</Wrap> (or the
Wrap equivalent) so the map no longer contains the inline ternary; update the
mapping to reference key(tx) / k, the Focusable instance, and keep props like
onEnter={() => handleClick(tx)}, onEscape={focusOnOuterShell},
ariaLabel={ariaLabel(tx)} and TransactionLine intact while removing the
ItemWrapper ? ... : focusable conditional.
- Line 143: The inline anonymous fallback component causes remounts because a
new function is created on every render; fix by hoisting a stable fallback
component out of the component render scope (e.g., create a module-level
DefaultWrapper component) and then use const Wrap = ItemWrapper ??
DefaultWrapper so Wrap is a stable reference when ItemWrapper is undefined;
update references to Wrap to use the hoisted DefaultWrapper name to avoid
recreating the function on each render.
In `@src/hooks/useReducedMotion.ts`:
- Around line 1-17: Replace the custom implementation of useReducedMotion with
framer-motion's built-in hook: remove the local useReducedMotion function body
and instead re-export or forward to framer-motion's useReducedMotion so the hook
is identical to the dependency already in the project; update the file to
import/export the symbol from 'framer-motion' (keep the exported name
useReducedMotion so callers don't change).
- Around line 1-17: Replace the custom useReducedMotion implementation with
framer-motion's built-in hook: remove the local useReducedMotion function body
and import useReducedMotion from 'framer-motion' (or re-export it) so consumers
use the library's SSR-safe implementation; ensure any code referencing the local
useReducedMotion keeps the same boolean return shape and remove the
MediaQueryList listener logic from this file.
In `@src/providers/navigation.tsx`:
- Around line 251-260: The navigate function is recreated on every render
causing unnecessary re-renders in consumers; wrap navigate in React.useCallback
to memoize it and supply the correct dependency array (include tab, screen and
any setter/utility references used such as push, setDirection, setScreen,
setTab, and ROOT_PAGES if it can change) so its reference stays stable, keeping
the existing logic (compute nextTab from pageTab, determine isTabSwitch using
ROOT_PAGES and screen, set previousPage.current, call push(page), setDirection,
setScreen, setTab).
In `@src/providers/options.tsx`:
- Around line 153-163: The goBack callback references optionSection but doesn't
include it in the useCallback dependency array; to make this explicit and
future-proof, move optionSection out of the component as a module-level helper
(so it no longer closes over component scope) or add optionSection to the
dependency array of useCallback; update the implementation around goBack,
optionSection, and the useCallback import (useCallback) so that goBack either
depends on a stable module-level function or lists optionSection in its
dependencies (preserving existing behavior with SettingsSections and
SettingsOptions).
In `@src/screens/Settings/Index.tsx`:
- Around line 23-62: The default branch of settingsContent currently returns an
empty fragment and can silently hide missing cases for the SettingsOptions enum;
update the default to surface a dev-time warning or guard by logging the
unexpected option (e.g., console.warn with the option value) or calling an
assertUnreachable helper so developers are alerted when a new SettingsOptions
value lacks a corresponding case in settingsContent; make the change inside the
settingsContent function and reference the SettingsOptions value passed in to
include it in the diagnostic message.
In `@src/screens/Wallet/Index.tsx`:
- Line 77: The empty <Item> wrapper causes a hidden WalletStaggerChild to take a
stagger slot when both nudge and psaMessage are falsy; update the JSX in the
Wallet screen so that <Item> is only rendered when either nudge or psaMessage is
truthy (i.e., render <Item>{nudge ?? <InfoBox html={psaMessage} />}</Item> or
use a conditional like { (nudge || psaMessage) && <Item>...</Item> }),
referencing the existing Item, nudge, psaMessage and InfoBox symbols to locate
and change the line.
In `@src/screens/Wallet/Unlock.tsx`:
- Around line 34-36: The effect hooks reference context functions that aren’t
listed in their dependency arrays — add the context functions to the deps so the
effects react to changed references: include initWallet in the dependency array
of the useEffect that calls initWallet, and include navigate in the dependency
array of the useEffect that calls navigate (or alternatively memoize these
context functions where provided); update the dependency arrays for the effects
that call initWallet and navigate to include those identifiers to satisfy
exhaustive-deps and avoid stale-closure bugs.
- Around line 20-32: The effect calling getPrivateKey(pass).then(initWallet)...
needs an "ignore/unmounted" guard to avoid calling setUnlocked (and other state
setters) after the component unmounts; inside the useEffect create a local let
cancelled = false (or isMounted = true), check cancelled before calling
setUnlocked(true), setTried(true), and setError(...) in the .then/.catch
handlers, and flip cancelled to true in the effect cleanup function so in-flight
promises don't update state; reference the existing useEffect, getPrivateKey,
initWallet, setUnlocked, setTried, and setError when making the change.
In `@vite.config.ts`:
- Line 17: The committed vite.config.ts adds allowedHosts:
['.trycloudflare.com'] which exposes the dev server; remove this from the main
config and guard it behind a developer-only mechanism instead — either load an
uncommitted vite.config.local.ts override or set allowedHosts conditionally in
the exported Vite config (the defineConfig export in vite.config.ts) only when
an env var like ENABLE_CF_TUNNEL==='true' is present; ensure the symbol
allowedHosts is only set in that guarded branch and document the env/local file
so developers can opt into Cloudflare tunnel testing without committing the host
list.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
package.jsonsrc/App.tsxsrc/components/Header.tsxsrc/components/PageTransition.tsxsrc/components/SettingsPageTransition.tsxsrc/components/TransactionsList.tsxsrc/components/WalletLoadIn.tsxsrc/hooks/useReducedMotion.tssrc/index.csssrc/lib/animations.tssrc/providers/navigation.tsxsrc/providers/options.tsxsrc/providers/wallet.tsxsrc/screens/Apps/Boltz/Index.tsxsrc/screens/Apps/Boltz/Settings.tsxsrc/screens/Apps/Boltz/Swap.tsxsrc/screens/Apps/Lendasat/Index.tsxsrc/screens/Apps/Lendaswap/Index.tsxsrc/screens/Init/Password.tsxsrc/screens/Init/Restore.tsxsrc/screens/Settings/Index.tsxsrc/screens/Wallet/Index.tsxsrc/screens/Wallet/Receive/Amount.tsxsrc/screens/Wallet/Receive/QrCode.tsxsrc/screens/Wallet/Send/Details.tsxsrc/screens/Wallet/Send/Form.tsxsrc/screens/Wallet/Transaction.tsxsrc/screens/Wallet/Unlock.tsxvite.config.ts
| "@sentry/react": "^9.15.0", | ||
| "decimal.js": "^10.5.0", | ||
| "dompurify": "^3.2.6", | ||
| "framer-motion": "^12.34.3", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
framer-motion latest version npm 2026
💡 Result:
As of February 24, 2026, the latest stable npm version of framer-motion is 12.34.0 (released Feb 9, 2026). [1][2][3]
Install:
npm i framer-motion@12.34.0Fix framer-motion version to use an actual published release.
Version 12.34.3 does not exist on npm—the latest stable release is 12.34.0 (released Feb 9, 2026). Change to ^12.34.0 to resolve a dependency error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 21, Update the framer-motion dependency version in
package.json from the non-existent "12.34.3" to a published release by replacing
the version string for "framer-motion" with "^12.34.0"; ensure the package.json
dependency entry "framer-motion" is updated and then run your package manager
(npm/yarn/pnpm) to refresh lockfile and verify installation.
- Reset dataReady and hasLoadedOnce on lock/reset to prevent stale data flash on re-unlock (src/providers/wallet.tsx) - Use stable component types with animate prop instead of swapping between WalletStaggerContainer/Passthrough to avoid React remounts (src/components/WalletLoadIn.tsx, src/screens/Wallet/Index.tsx) - Gate keyboard/scanner overlay animations on prefers-reduced-motion (src/screens/Wallet/Receive/Amount.tsx, src/screens/Wallet/Send/Form.tsx)
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/screens/Wallet/Receive/Amount.tsx (1)
191-198: Consider extracting the inline style object to a constant.The overlay style object is identical to the one in
Send/Form.tsx(line 415–422), where it's already extracted asoverlayStyle. For consistency and to avoid duplication, extract this to a shared constant or reuse the same pattern.♻️ Suggested diff
+const overlayStyle = { + position: 'absolute' as const, + inset: 0, + zIndex: 10, + display: 'flex', + flexDirection: 'column' as const, + background: 'var(--ion-background-color)', +} + export default function ReceiveAmount() {Then replace the inline style:
- style={{ - position: 'absolute', - inset: 0, - zIndex: 10, - display: 'flex', - flexDirection: 'column', - background: 'var(--ion-background-color)', - }} + style={overlayStyle}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Receive/Amount.tsx` around lines 191 - 198, Extract the inline style object used in the overlay div inside the Amount component into a shared constant (e.g., overlayStyle) and reuse the same constant pattern already used in Send/Form.tsx (where overlayStyle is defined) to avoid duplication; locate the overlay div in Receive/Amount.tsx (the element with style={{ position: 'absolute', inset: 0, zIndex: 10, display: 'flex', flexDirection: 'column', background: 'var(--ion-background-color)' }}) and replace the inline object with a reference to the shared overlayStyle constant, exporting/importing the constant if needed so both Send/Form.tsx and Receive/Amount.tsx use the same symbol.src/screens/Wallet/Send/Form.tsx (1)
490-508:keyboardOverlayvariant name is misleading for the scanner overlay.The scanner overlay reuses
keyboardOverlayvariants (line 494). Since both overlays share the same slide-up animation, consider renaming the animation token to something generic likebottomSheetOverlayorslideUpOverlayinsrc/lib/animations.tsto better reflect its reuse across different overlay types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Send/Form.tsx` around lines 490 - 508, The animation variant named keyboardOverlay is misleadingly used for the Scanner overlay in the Send Form (motion.div that wraps Scanner); rename the variant to a generic name (e.g., slideUpOverlay or bottomSheetOverlay) in src/lib/animations.ts and update all references to that symbol (replace keyboardOverlay with the new name wherever imported/used, including the motion.div in Send/Form.tsx and any other components that import it) so the name reflects reuse across overlays and keep the variant behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/screens/Wallet/Receive/Amount.tsx`:
- Around line 191-198: Extract the inline style object used in the overlay div
inside the Amount component into a shared constant (e.g., overlayStyle) and
reuse the same constant pattern already used in Send/Form.tsx (where
overlayStyle is defined) to avoid duplication; locate the overlay div in
Receive/Amount.tsx (the element with style={{ position: 'absolute', inset: 0,
zIndex: 10, display: 'flex', flexDirection: 'column', background:
'var(--ion-background-color)' }}) and replace the inline object with a reference
to the shared overlayStyle constant, exporting/importing the constant if needed
so both Send/Form.tsx and Receive/Amount.tsx use the same symbol.
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 490-508: The animation variant named keyboardOverlay is
misleadingly used for the Scanner overlay in the Send Form (motion.div that
wraps Scanner); rename the variant to a generic name (e.g., slideUpOverlay or
bottomSheetOverlay) in src/lib/animations.ts and update all references to that
symbol (replace keyboardOverlay with the new name wherever imported/used,
including the motion.div in Send/Form.tsx and any other components that import
it) so the name reflects reuse across overlays and keep the variant behavior
unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/WalletLoadIn.tsxsrc/providers/wallet.tsxsrc/screens/Wallet/Index.tsxsrc/screens/Wallet/Receive/Amount.tsxsrc/screens/Wallet/Send/Form.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/WalletLoadIn.tsx
- src/providers/wallet.tsx
…overlaySlideUp Address CodeRabbit nitpicks: extract duplicated inline overlay style to a shared constant in animations.ts, and rename keyboardOverlay to overlaySlideUp since it's used for both keyboard and scanner overlays.
Member
|
Brave browser iOS Cursor is not aligned and 0 is defined value
|
Member
|
Moreover the keyboard should not be shown here: we do have the Keypad UI to control the user experience This is how in master
|
Contributor
Author
totally, my bad. will take a look |
Collaborator
|
@sahilc0 I fixed merged conflicts. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/TransactionsList.tsx (1)
145-151: Avoid creating a new fallback wrapper component on every render.Line 145 creates a new component function each render. Prefer conditional JSX to keep identity stable and simpler.
♻️ Suggested simplification
- const Wrap = ItemWrapper ?? (({ children }: { children: ReactNode }) => <>{children}</>) - return ( <div style={{ width: 'calc(100% + 2rem)', margin: '0 -1rem' }}> - <Wrap> - <TextLabel>Transaction history</TextLabel> - </Wrap> + {ItemWrapper ? ( + <ItemWrapper> + <TextLabel>Transaction history</TextLabel> + </ItemWrapper> + ) : ( + <TextLabel>Transaction history</TextLabel> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TransactionsList.tsx` around lines 145 - 151, The current code creates a new fallback component assigned to Wrap on every render (const Wrap = ItemWrapper ?? (({ children }: { children: ReactNode }) => <>{children}</>)), which changes identity each render; instead, remove the inline component creation and use conditional JSX where TransactionsList renders: if ItemWrapper is provided, render <ItemWrapper>...children...</ItemWrapper>, otherwise render fragments or a plain wrapper (<>...children...</>), ensuring you update the places that reference Wrap so they use ItemWrapper directly or plain JSX to keep component identity stable.src/screens/Wallet/Send/Form.tsx (1)
468-500: Consider mutually exclusive state to prevent both overlays rendering simultaneously.
keysandscanare two independent boolean states, each independently setting their overlay active. While the normal interaction flow makes simultaneoustruevalues unlikely, there is no explicit guard preventing both from beingtrueat once (e.g.,setStateresetsscanbut notkeys). If both aretrue, both overlays render atzIndex: 10; the scanner wins by DOM order. Closing either independently could leave the other unexpectedly visible.🛡️ Suggested guard: reset one when the other opens
const handleFocus = () => { - if (isMobileBrowser) setKeys(true) + if (isMobileBrowser) { + setScan(false) + setKeys(true) + } }And in
openScan:- openScan={() => setScan(true)} + openScan={() => { setKeys(false); setScan(true) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Send/Form.tsx` around lines 468 - 500, Both overlays can render simultaneously because `keys` and `scan` are independent booleans; make them mutually exclusive by updating the handlers that open each overlay to also close the other — e.g., whenever you call `setKeys(true)` ensure you call `setScan(false)`, and whenever you call `setScan(true)` ensure you call `setKeys(false)` (also update any toggle/open helpers that set `keys` or `scan` to true). Apply this to the functions that launch the Keyboard and Scanner (references: `setKeys`, `setScan`, `Keyboard`, `Scanner`, and any open/toggle handlers) so only one overlay can be true/rendered at a time.src/App.tsx (1)
168-169:directionvseffectiveDirection— confirm intentional divergence.
AnimatePresenceat lines 185/196/207 receivescustom={effectiveDirection}(reduced-motion-aware), while thePageTransitionchildren at lines 187/198/209 receivedirection={direction}(raw). This works becausePageTransitionindependently callsuseReducedMotion()and derives its own effective direction. However, this meansuseReducedMotion()is subscribed to twice per render — once here and once insidePageTransition. It's correct, but the split responsibility is non-obvious. A brief comment at line 169 explaining whyAnimatePresenceneeds its own reduced-motion-awarecustom(for exit-animation resolution on outgoing pages) would aid future maintainers.Also applies to: 185-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 168 - 169, The code intentionally computes both prefersReduced/useReducedMotion() here and inside PageTransition, but the divergence between direction and effectiveDirection is non-obvious; add a short inline comment near the assignment of effectiveDirection (next to "const effectiveDirection = prefersReduced ? 'none' : direction") explaining that AnimatePresence needs a reduced-motion-aware custom value (effectiveDirection) so exit animations on outgoing pages resolve correctly, while PageTransition still calls useReducedMotion() internally to derive its own direction for entry animations—this clarifies why both subscriptions exist and prevents future accidental consolidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/TransactionsList.tsx`:
- Around line 154-169: The derived key/id variable k (from key(tx)) can collide
and is used both as React key and as the DOM id for Focusable; change the map so
k is guaranteed unique by appending a stable fallback (e.g. include the loop
index or a unique tx property like tx.id/tx.hash) and use that unique value for
both the React key and the Focusable id; update usages of key(tx) -> uniqueKey
(used for key prop and id prop on Focusable) while keeping TransactionLine,
handleClick, ariaLabel, ItemWrapper logic unchanged.
In `@vite.config.ts`:
- Line 19: The committed wildcard allowedHosts entry (allowedHosts:
['.trycloudflare.com']) in vite.config.ts is too permissive; update the config
to load allowedHosts from a local, gitignored override or env (e.g., read
process.env.VITE_ALLOWED_HOSTS or a vite.config.local.ts) and remove the
hardcoded '.trycloudflare.com' from the main file, or at minimum add a clear
comment and README note; ensure the code path that reads allowedHosts (the
allowedHosts variable in vite.config.ts) falls back to a safe default when the
local override is absent.
---
Nitpick comments:
In `@src/App.tsx`:
- Around line 168-169: The code intentionally computes both
prefersReduced/useReducedMotion() here and inside PageTransition, but the
divergence between direction and effectiveDirection is non-obvious; add a short
inline comment near the assignment of effectiveDirection (next to "const
effectiveDirection = prefersReduced ? 'none' : direction") explaining that
AnimatePresence needs a reduced-motion-aware custom value (effectiveDirection)
so exit animations on outgoing pages resolve correctly, while PageTransition
still calls useReducedMotion() internally to derive its own direction for entry
animations—this clarifies why both subscriptions exist and prevents future
accidental consolidation.
In `@src/components/TransactionsList.tsx`:
- Around line 145-151: The current code creates a new fallback component
assigned to Wrap on every render (const Wrap = ItemWrapper ?? (({ children }: {
children: ReactNode }) => <>{children}</>)), which changes identity each render;
instead, remove the inline component creation and use conditional JSX where
TransactionsList renders: if ItemWrapper is provided, render
<ItemWrapper>...children...</ItemWrapper>, otherwise render fragments or a plain
wrapper (<>...children...</>), ensuring you update the places that reference
Wrap so they use ItemWrapper directly or plain JSX to keep component identity
stable.
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 468-500: Both overlays can render simultaneously because `keys`
and `scan` are independent booleans; make them mutually exclusive by updating
the handlers that open each overlay to also close the other — e.g., whenever you
call `setKeys(true)` ensure you call `setScan(false)`, and whenever you call
`setScan(true)` ensure you call `setKeys(false)` (also update any toggle/open
helpers that set `keys` or `scan` to true). Apply this to the functions that
launch the Keyboard and Scanner (references: `setKeys`, `setScan`, `Keyboard`,
`Scanner`, and any open/toggle handlers) so only one overlay can be
true/rendered at a time.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.jsonsrc/App.tsxsrc/components/Header.tsxsrc/components/TransactionsList.tsxsrc/index.csssrc/lib/animations.tssrc/screens/Wallet/Receive/Amount.tsxsrc/screens/Wallet/Send/Form.tsxvite.config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Header.tsx
- src/lib/animations.ts
- package.json
| ].filter(Boolean), | ||
| server: { | ||
| port: 3002, | ||
| allowedHosts: ['.trycloudflare.com'], |
Contributor
There was a problem hiding this comment.
Wildcard tunnel host committed permanently — consider scoping to local override.
'.trycloudflare.com' accepts any active Cloudflare tunnel URL to reach the local dev server. While useful for mobile testing, committing it permanently means every developer's machine silently accepts connections from any *.trycloudflare.com origin. Consider moving this to a .env.local / vite.config.local.ts that is gitignored, or at minimum document it clearly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.ts` at line 19, The committed wildcard allowedHosts entry
(allowedHosts: ['.trycloudflare.com']) in vite.config.ts is too permissive;
update the config to load allowedHosts from a local, gitignored override or env
(e.g., read process.env.VITE_ALLOWED_HOSTS or a vite.config.local.ts) and remove
the hardcoded '.trycloudflare.com' from the main file, or at minimum add a clear
comment and README note; ensure the code path that reads allowedHosts (the
allowedHosts variable in vite.config.ts) falls back to a safe default when the
local override is absent.
- SVG sizing: replace maxHeight 50% with 40vh + maxWidth 70% to prevent overflow and scrolling on mobile viewports - Stagger animations: remove nested OnboardStaggerContainer — the outer motion.div's animate prop was blocking variant propagation. Now the motion.div IS the stagger container with direct OnboardStaggerChild children. StepBars moved inside AnimatePresence to join the sequence. - Add onboard stagger animations to Init, Password, Restore, Success - Fix TransactionsList key stability (use txid instead of concatenation) - Wrap pre-tab pages in PageTransition for consistent nav animations - Header back handler: simplify conditional logic - Mobile inputs: set readOnly on amount fields for keyboard overlay UX
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/components/TransactionsList.tsx (1)
120-120:⚠️ Potential issue | 🟠 MajorUse truthy fallback for tx identifiers;
??preserves empty strings.Line 120 can return
''when txid fields are empty, which then propagates to DOMidand Reactkey(Lines 125, 160-161). That can break first-row focus and cause unstable row reconciliation.🐛 Proposed fix
- const key = (tx: Tx, index: number) => tx.roundTxid ?? tx.redeemTxid ?? tx.boardingTxid ?? `tx-${index}` + const key = (tx: Tx, index: number) => { + const stable = tx.roundTxid || tx.redeemTxid || tx.boardingTxid + return stable ? `tx-${stable}` : `tx-${index}` + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TransactionsList.tsx` at line 120, The key generator currently uses the nullish coalescing operator so empty string tx ids (tx.roundTxid, tx.redeemTxid, tx.boardingTxid) can pass through and produce invalid DOM ids/unstable React keys; update the key function (symbol: key) to use a truthy fallback (e.g., the logical OR behavior) or explicitly test for non-empty strings before falling back to `tx-${index}` so empty strings are treated as missing values and the stable fallback is used.
🧹 Nitpick comments (8)
.github/workflows/playwright.yml (1)
21-21: Consider pinning the Nigiri action to a commit SHA.
vulpemventures/nigiri-github-action@v1is a mutable tag; if it's ever force-pushed, the workflow silently runs different code. Pinning to a full commit SHA (e.g.,@<sha>) eliminates this risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/playwright.yml at line 21, Replace the mutable tag vulpemventures/nigiri-github-action@v1 with a pinned commit SHA for the Nigiri action; update the uses entry (the line containing "vulpemventures/nigiri-github-action@v1") to reference the full commit SHA (e.g., @<commit-sha>) so the workflow consistently runs the exact action version and avoids drifting when tags are changed.src/screens/Init/Password.tsx (1)
73-77: Single-childOnboardStaggerContainerfor the password path — stagger has no effect.Wrapping
NewPasswordin a singleOnboardStaggerChildonly provides a container fade-in + child fade-up, not a true stagger. This is fine as an entrance animation, but if the sole intent is a fade-in, it follows the same over-engineering pattern noted inSuccess.tsx. No functional issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Init/Password.tsx` around lines 73 - 77, This wraps NewPassword in OnboardStaggerContainer + OnboardStaggerChild which yields no real stagger for a single child; simplify by removing the stagger wrapper and using a single-entry animation container (or direct fade-in) around NewPassword — update the JSX that currently uses OnboardStaggerContainer and OnboardStaggerChild in Password.tsx to render NewPassword with a simple fade wrapper or no wrapper, matching the simplified approach used for Success.tsx.src/components/OnboardLoadIn.tsx (1)
32-41: EachOnboardStaggerChildinstance independently subscribes touseReducedMotion().With many stagger children on screen, each registers its own
MediaQueryListevent listener viauseReducedMotion. Since the preference is app-wide, a single subscription from the parent (or a shared context value) would be cleaner and more efficient. This is fine at the current scale but worth noting if the stagger pattern is used extensively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OnboardLoadIn.tsx` around lines 32 - 41, OnboardStaggerChild calls useReducedMotion() per instance which creates many MediaQueryList listeners; change the pattern to call useReducedMotion once at a higher level and pass the boolean down (e.g., add a prefersReduced prop to OnboardStaggerChild) or implement a small context/provider that reads useReducedMotion once and exposes the value to all children; update OnboardStaggerChild to accept and use the prefersReduced prop (and keep its current logic using onboardStaggerChild when prefersReduced is false) so individual children no longer call useReducedMotion themselves.src/lib/animations.ts (1)
80-88:walletLoadInChildinitialx: -2offset is unexplained.The 2 px horizontal nudge on every wallet stagger item is easy to miss as an intentional design choice. A brief comment would clarify whether this is deliberate (
// subtle diagonal entrance) or a stray value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/animations.ts` around lines 80 - 88, The walletLoadInChild animation has an unexplained horizontal offset (initial x: -2) causing a subtle diagonal entrance; either document the intent or remove/change it—update the walletLoadInChild Variants by adding a one-line comment above it (e.g., "// subtle diagonal entrance to give items a slight left-to-right motion") if the offset is intentional, or set x: 0 (or another deliberate value) in the initial state of walletLoadInChild to eliminate the nudge if it was accidental.src/screens/Init/Init.tsx (1)
57-59:OnboardStaggerChildalways renders amotion.diveven whenErrorMessagereturnsnull.When
errorisfalse,ErrorMessagereturnsnull, butOnboardStaggerChildstill mounts amotion.divaround it — an empty animated slot. This wastes one stagger step and creates an invisible animated element in the DOM. Conditional rendering at the wrapper level avoids the stale slot.♻️ Proposed fix
- <OnboardStaggerChild> - <ErrorMessage error={error} text='Ark server unreachable' /> - </OnboardStaggerChild> + {error && ( + <OnboardStaggerChild> + <ErrorMessage error={error} text='Ark server unreachable' /> + </OnboardStaggerChild> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Init/Init.tsx` around lines 57 - 59, The wrapper OnboardStaggerChild is always mounting its motion.div even when ErrorMessage returns null; change the rendering so OnboardStaggerChild is only rendered when there is an actual error (i.e., base the condition on the same `error` value passed to ErrorMessage). Locate the usage in Init.tsx where OnboardStaggerChild wraps <ErrorMessage error={error} ...> and replace it with a conditional rendering that checks `error` before rendering OnboardStaggerChild so no empty motion.div/stagger slot is created when ErrorMessage returns null.src/screens/Init/Success.tsx (1)
29-33: Single-childOnboardStaggerContainerprovides no stagger effect; considercenteredprop for layout consistency.With one child,
staggerChildrenhas no effect and only the container fade-in plays. If the intent is a simple fade-in entrance, theOnboardStaggerContainer/OnboardStaggerChildpair is more overhead than needed. Additionally,Successrenders aCenterScreeninternally — passingcenteredtoOnboardStaggerContainerwould give the motion wrapper the same flex-center layout, keeping alignment consistent if the wrapper ever gains siblings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Init/Success.tsx` around lines 29 - 33, Replace the unnecessary two-level motion wrapper with a single motion container or add the centered prop to the existing wrapper: either remove OnboardStaggerChild and wrap Success directly with a single OnboardStaggerContainer that performs the fade-in, or keep OnboardStaggerContainer but pass centered to it so the motion wrapper uses the same flex-center layout as Success (which renders CenterScreen) — update the JSX around OnboardStaggerContainer/OnboardStaggerChild and ensure OnboardStaggerContainer accepts/uses the centered prop.src/App.tsx (1)
168-169:useReducedMotionis called twice — consider passingeffectiveDirectiontoPageTransition.
effectiveDirectionis computed inAppand correctly passed toAnimatePresence(needed so exit animations respect reduced-motion). However,PageTransitionignores it and callsuseReducedMotion()itself on every render of every page. The hook is cheap, but the pattern creates an inconsistency: thedirectionprop onPageTransitionis always the raw value, whilecustomonAnimatePresenceis already clamped. PassingeffectiveDirectionthrough toPageTransitionas well (and removing its internaluseReducedMotioncall) would make the data flow explicit and eliminate the duplicate subscription.♻️ Sketch of unified data flow
- <PageTransition key={String(page)} direction={direction} pageKey={String(page)}> + <PageTransition key={String(page)} direction={effectiveDirection} pageKey={String(page)}>Then
PageTransitioncan drop its ownuseReducedMotion()call and usedirectiondirectly.Also applies to: 181-185, 191-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 168 - 169, App computes effectiveDirection using useReducedMotion and passes it to AnimatePresence; update the PageTransition usage to receive that same effectiveDirection (pass effectiveDirection as the direction prop wherever PageTransition is rendered) and remove the internal useReducedMotion() call inside the PageTransition component so it uses the incoming direction prop directly; adjust all PageTransition render sites (the other occurrences mentioned) to pass effectiveDirection and delete the duplicate hook and any local clamping logic so the clamped value is derived only once in App and propagated down.src/screens/Wallet/Onboard.tsx (1)
45-52: ReplaceanywithCSSPropertiesforimageStyle.This improves type safety by catching invalid style keys/values at compile time.
♻️ Proposed change
-import { ReactNode, useContext, useState } from 'react' +import { CSSProperties, ReactNode, useContext, useState } from 'react' ... - const imageStyle: any = { + const imageStyle: CSSProperties = { alignItems: 'center', display: 'flex', justifyContent: 'center', margin: '0 auto', maxHeight: '40vh', maxWidth: '70%', }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Onboard.tsx` around lines 45 - 52, Replace the loose any typing on the imageStyle constant with a proper CSSProperties type: change the declaration of imageStyle to use CSSProperties (e.g., const imageStyle: CSSProperties = { ... } or React.CSSProperties) and ensure you import CSSProperties from 'react' (or reference React.CSSProperties) so the compiler validates style keys/values for the imageStyle object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/OnboardLoadIn.tsx`:
- Around line 11-29: The effect in OnboardStaggerContainer should explicitly
reset the started state when prefersReduced flips to true; update the useEffect
for started so it sets started = true when prefersReduced is false and sets
started = false when prefersReduced is true (e.g., setStarted(!prefersReduced)
or conditional setStarted(prefersReduced ? false : true)), keeping
prefersReduced in the dependency list so the state is updated whenever the
preference changes.
In `@src/index.css`:
- Line 207: There is an unresolved Git conflict marker "<<<<<<< HEAD" in
src/index.css; open the file, locate the conflict block beginning with "<<<<<<<
HEAD" (and the matching "=======" / ">>>>>>>"), decide which of the conflicting
CSS rules to keep or manually merge them into a single valid rule set, remove
all conflict markers ("<<<<<<< HEAD", "=======", ">>>>>>> ..."), ensure the
resulting CSS is syntactically valid (no leftover markers or duplicate
selectors) and re-run your linter/biome to confirm the parse error is resolved.
In `@src/lib/animations.ts`:
- Around line 93-98: The parent variant onboardStaggerContainer currently
defines opacity transitions which multiply with child opacities and block
per-child fades; remove the opacity keys from onboardStaggerContainer's initial
and animate states (leave transition: { staggerChildren: ONBOARD_STAGGER_DELAY,
duration: STAGGER_DURATION, ease: EASE_OUT_QUINT } intact) so each
onboardStaggerChild controls its own opacity animation via its own variants;
update the Variants object onboardStaggerContainer accordingly (remove or omit
opacity properties) and verify onboardStaggerChild still defines initial/animate
opacity values.
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 468-500: Currently both overlays use independent booleans (keys,
scan) so they can mount together; ensure mutual exclusion by clearing the other
flag whenever one is opened: call setKeys(false) when opening the scanner (wire
this into the InputAddress prop used to open the scanner, e.g. openScan) and
call setScan(false) when opening the keyboard/amount input (e.g. in the handler
that sets keys true or the Keyboard.back flow), so Keyboard and Scanner cannot
both be mounted and the onSats events won’t be captured by a hidden overlay.
---
Duplicate comments:
In `@src/components/TransactionsList.tsx`:
- Line 120: The key generator currently uses the nullish coalescing operator so
empty string tx ids (tx.roundTxid, tx.redeemTxid, tx.boardingTxid) can pass
through and produce invalid DOM ids/unstable React keys; update the key function
(symbol: key) to use a truthy fallback (e.g., the logical OR behavior) or
explicitly test for non-empty strings before falling back to `tx-${index}` so
empty strings are treated as missing values and the stable fallback is used.
---
Nitpick comments:
In @.github/workflows/playwright.yml:
- Line 21: Replace the mutable tag vulpemventures/nigiri-github-action@v1 with a
pinned commit SHA for the Nigiri action; update the uses entry (the line
containing "vulpemventures/nigiri-github-action@v1") to reference the full
commit SHA (e.g., @<commit-sha>) so the workflow consistently runs the exact
action version and avoids drifting when tags are changed.
In `@src/App.tsx`:
- Around line 168-169: App computes effectiveDirection using useReducedMotion
and passes it to AnimatePresence; update the PageTransition usage to receive
that same effectiveDirection (pass effectiveDirection as the direction prop
wherever PageTransition is rendered) and remove the internal useReducedMotion()
call inside the PageTransition component so it uses the incoming direction prop
directly; adjust all PageTransition render sites (the other occurrences
mentioned) to pass effectiveDirection and delete the duplicate hook and any
local clamping logic so the clamped value is derived only once in App and
propagated down.
In `@src/components/OnboardLoadIn.tsx`:
- Around line 32-41: OnboardStaggerChild calls useReducedMotion() per instance
which creates many MediaQueryList listeners; change the pattern to call
useReducedMotion once at a higher level and pass the boolean down (e.g., add a
prefersReduced prop to OnboardStaggerChild) or implement a small
context/provider that reads useReducedMotion once and exposes the value to all
children; update OnboardStaggerChild to accept and use the prefersReduced prop
(and keep its current logic using onboardStaggerChild when prefersReduced is
false) so individual children no longer call useReducedMotion themselves.
In `@src/lib/animations.ts`:
- Around line 80-88: The walletLoadInChild animation has an unexplained
horizontal offset (initial x: -2) causing a subtle diagonal entrance; either
document the intent or remove/change it—update the walletLoadInChild Variants by
adding a one-line comment above it (e.g., "// subtle diagonal entrance to give
items a slight left-to-right motion") if the offset is intentional, or set x: 0
(or another deliberate value) in the initial state of walletLoadInChild to
eliminate the nudge if it was accidental.
In `@src/screens/Init/Init.tsx`:
- Around line 57-59: The wrapper OnboardStaggerChild is always mounting its
motion.div even when ErrorMessage returns null; change the rendering so
OnboardStaggerChild is only rendered when there is an actual error (i.e., base
the condition on the same `error` value passed to ErrorMessage). Locate the
usage in Init.tsx where OnboardStaggerChild wraps <ErrorMessage error={error}
...> and replace it with a conditional rendering that checks `error` before
rendering OnboardStaggerChild so no empty motion.div/stagger slot is created
when ErrorMessage returns null.
In `@src/screens/Init/Password.tsx`:
- Around line 73-77: This wraps NewPassword in OnboardStaggerContainer +
OnboardStaggerChild which yields no real stagger for a single child; simplify by
removing the stagger wrapper and using a single-entry animation container (or
direct fade-in) around NewPassword — update the JSX that currently uses
OnboardStaggerContainer and OnboardStaggerChild in Password.tsx to render
NewPassword with a simple fade wrapper or no wrapper, matching the simplified
approach used for Success.tsx.
In `@src/screens/Init/Success.tsx`:
- Around line 29-33: Replace the unnecessary two-level motion wrapper with a
single motion container or add the centered prop to the existing wrapper: either
remove OnboardStaggerChild and wrap Success directly with a single
OnboardStaggerContainer that performs the fade-in, or keep
OnboardStaggerContainer but pass centered to it so the motion wrapper uses the
same flex-center layout as Success (which renders CenterScreen) — update the JSX
around OnboardStaggerContainer/OnboardStaggerChild and ensure
OnboardStaggerContainer accepts/uses the centered prop.
In `@src/screens/Wallet/Onboard.tsx`:
- Around line 45-52: Replace the loose any typing on the imageStyle constant
with a proper CSSProperties type: change the declaration of imageStyle to use
CSSProperties (e.g., const imageStyle: CSSProperties = { ... } or
React.CSSProperties) and ensure you import CSSProperties from 'react' (or
reference React.CSSProperties) so the compiler validates style keys/values for
the imageStyle object.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/playwright.ymlsrc/App.tsxsrc/components/Header.tsxsrc/components/OnboardLoadIn.tsxsrc/components/TransactionsList.tsxsrc/index.csssrc/lib/animations.tssrc/screens/Init/Init.tsxsrc/screens/Init/Password.tsxsrc/screens/Init/Restore.tsxsrc/screens/Init/Success.tsxsrc/screens/Wallet/Index.tsxsrc/screens/Wallet/Onboard.tsxsrc/screens/Wallet/Receive/Amount.tsxsrc/screens/Wallet/Send/Form.tsxvite.config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- vite.config.ts
- src/screens/Init/Restore.tsx
- src/components/Header.tsx
Comment on lines
+93
to
+98
| export const onboardStaggerContainer: Variants = { | ||
| initial: { opacity: 0 }, | ||
| animate: { | ||
| opacity: 1, | ||
| transition: { staggerChildren: ONBOARD_STAGGER_DELAY, duration: STAGGER_DURATION, ease: EASE_OUT_QUINT }, | ||
| }, |
Contributor
There was a problem hiding this comment.
onboardStaggerContainer carries its own opacity: 0 → 1 transition, which stacks multiplicatively with children's opacity.
Because CSS opacity multiplies through the tree, children whose onboardStaggerChild variant already starts at opacity: 0 will remain invisible until the parent finishes fading in, producing a compound effect that doesn't match the intended per-child stagger. The parent opacity is redundant — the children drive the stagger themselves.
🐛 Proposed fix
export const onboardStaggerContainer: Variants = {
- initial: { opacity: 0 },
- animate: {
- opacity: 1,
- transition: { staggerChildren: ONBOARD_STAGGER_DELAY, duration: STAGGER_DURATION, ease: EASE_OUT_QUINT },
- },
+ initial: {},
+ animate: {
+ transition: { staggerChildren: ONBOARD_STAGGER_DELAY },
+ },
}This lets each onboardStaggerChild drive its own opacity fade without interference from the parent layer.
📝 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
| export const onboardStaggerContainer: Variants = { | |
| initial: { opacity: 0 }, | |
| animate: { | |
| opacity: 1, | |
| transition: { staggerChildren: ONBOARD_STAGGER_DELAY, duration: STAGGER_DURATION, ease: EASE_OUT_QUINT }, | |
| }, | |
| export const onboardStaggerContainer: Variants = { | |
| initial: {}, | |
| animate: { | |
| transition: { staggerChildren: ONBOARD_STAGGER_DELAY }, | |
| }, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/animations.ts` around lines 93 - 98, The parent variant
onboardStaggerContainer currently defines opacity transitions which multiply
with child opacities and block per-child fades; remove the opacity keys from
onboardStaggerContainer's initial and animate states (leave transition: {
staggerChildren: ONBOARD_STAGGER_DELAY, duration: STAGGER_DURATION, ease:
EASE_OUT_QUINT } intact) so each onboardStaggerChild controls its own opacity
animation via its own variants; update the Variants object
onboardStaggerContainer accordingly (remove or omit opacity properties) and
verify onboardStaggerChild still defines initial/animate opacity values.
- Remove merge conflict marker in index.css - Reset stagger `started` state when reduced motion toggles on - Remove stacked opacity from onboardStaggerContainer (children already animate opacity individually, stacking caused compound fade) - Dismiss keyboard overlay when opening scanner in Send form
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/screens/Wallet/Send/Form.tsx (1)
429-432: Previous overlay mutual-exclusion concern has been addressed.
openScannow dismisses the keyboard overlay before opening the scanner, preventing both overlays from being mounted simultaneously.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Send/Form.tsx` around lines 429 - 432, Ensure the scanner overlay is opened only after the keyboard overlay is dismissed by updating the openScan handler to first call setKeys(false) and then setScan(true); this guarantees mutual exclusion between the keyboard overlay (controlled by setKeys) and the scanner overlay (controlled by setScan) inside the openScan function.
🧹 Nitpick comments (2)
src/screens/Wallet/Send/Form.tsx (1)
471-503: Overlay animation blocks look correct; consider a small DRY extraction.Both the keyboard and scanner overlays use an identical
motion.divwrapper pattern with the same reduced-motion conditional logic. This works well but is duplicated. An optional cleanup would be to extract a smallAnimatedOverlaywrapper component.Example helper (could live in a shared component or this file)
+const AnimatedOverlay = ({ show, id, children }: { show: boolean; id: string; children: React.ReactNode }) => ( + <AnimatePresence> + {show ? ( + <motion.div + key={id} + variants={prefersReduced ? undefined : overlaySlideUp} + initial={prefersReduced ? false : 'initial'} + animate={prefersReduced ? undefined : 'animate'} + exit={prefersReduced ? undefined : 'exit'} + style={overlayStyle} + > + {children} + </motion.div> + ) : null} + </AnimatePresence> +)Then usage becomes:
<AnimatedOverlay show={keys && !amountIsReadOnly} id="keyboard"> <Keyboard back={() => setKeys(false)} onSats={handleAmountChange} value={amount} /> </AnimatedOverlay> <AnimatedOverlay show={scan} id="scanner"> <Scanner close={() => setScan(false)} label="Recipient address" onData={setRecipient} onError={smartSetError} /> </AnimatedOverlay>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Send/Form.tsx` around lines 471 - 503, Extract the duplicated motion.div wrapper into a small reusable component (e.g., AnimatedOverlay) that accepts props show, id/key, and children and internally uses the same prefersReduced conditional logic, variants={prefersReduced ? undefined : overlaySlideUp}, initial/animate/exit conditionals, and style={overlayStyle}; then replace the two instances that wrap Keyboard and Scanner (currently gated by keys && !amountIsReadOnly and scan) with <AnimatedOverlay show={keys && !amountIsReadOnly} id="keyboard">...Keyboard...</AnimatedOverlay> and <AnimatedOverlay show={scan} id="scanner">...Scanner...</AnimatedOverlay>, keeping the same props passed to Keyboard (back, onSats, value) and Scanner (close, label, onData, onError).src/lib/animations.ts (1)
4-4: Nit: consider typingEASE_OUT_QUINTas a tuple for stricter type safety.Framer Motion's
EasingFunctionfor cubic bezier expects[number, number, number, number]. The currentnumber[]inference works at runtime but loses compile-time arity checking.Proposed fix
-export const EASE_OUT_QUINT = [0.23, 1, 0.32, 1] +export const EASE_OUT_QUINT: [number, number, number, number] = [0.23, 1, 0.32, 1]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/animations.ts` at line 4, The exported constant EASE_OUT_QUINT should be typed as a fixed 4-number tuple so TypeScript enforces the cubic-bezier arity; change its declaration to annotate the type as [number, number, number, number] (e.g., export const EASE_OUT_QUINT: [number, number, number, number] = [0.23, 1, 0.32, 1]) so it matches Framer Motion's expected EasingFunction shape and provides compile-time safety while keeping the same value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 429-432: Ensure the scanner overlay is opened only after the
keyboard overlay is dismissed by updating the openScan handler to first call
setKeys(false) and then setScan(true); this guarantees mutual exclusion between
the keyboard overlay (controlled by setKeys) and the scanner overlay (controlled
by setScan) inside the openScan function.
---
Nitpick comments:
In `@src/lib/animations.ts`:
- Line 4: The exported constant EASE_OUT_QUINT should be typed as a fixed
4-number tuple so TypeScript enforces the cubic-bezier arity; change its
declaration to annotate the type as [number, number, number, number] (e.g.,
export const EASE_OUT_QUINT: [number, number, number, number] = [0.23, 1, 0.32,
1]) so it matches Framer Motion's expected EasingFunction shape and provides
compile-time safety while keeping the same value.
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 471-503: Extract the duplicated motion.div wrapper into a small
reusable component (e.g., AnimatedOverlay) that accepts props show, id/key, and
children and internally uses the same prefersReduced conditional logic,
variants={prefersReduced ? undefined : overlaySlideUp}, initial/animate/exit
conditionals, and style={overlayStyle}; then replace the two instances that wrap
Keyboard and Scanner (currently gated by keys && !amountIsReadOnly and scan)
with <AnimatedOverlay show={keys && !amountIsReadOnly}
id="keyboard">...Keyboard...</AnimatedOverlay> and <AnimatedOverlay show={scan}
id="scanner">...Scanner...</AnimatedOverlay>, keeping the same props passed to
Keyboard (back, onSats, value) and Scanner (close, label, onData, onError).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/OnboardLoadIn.tsxsrc/index.csssrc/lib/animations.tssrc/screens/Wallet/Send/Form.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/index.css
- src/components/OnboardLoadIn.tsx
Contributor
Author
|
@tiero should be good to go now! |
6 tasks
1 of 3 tasks
1 of 3 tasks
This was referenced Mar 13, 2026
This was referenced Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
customprop. Forward slides right-to-left, back slides left-to-right, tab switches are instant. AnimatePresence mode changed frompopLayouttosyncfor smoother concurrent enter/exit.backprop acceptstrue(usesgoBack()) instead of explicit navigate callbacks. 10+ screens simplified.prefers-reduced-motionviauseReducedMotionhook.Files changed (30 files)
Core animation system
src/lib/animations.ts— UnifiedpageTransitionVariantswith dynamic direction,keyboardOverlay, wallet stagger variantssrc/components/PageTransition.tsx— New page transition wrappersrc/components/SettingsPageTransition.tsx— Settings tab equivalentsrc/components/WalletLoadIn.tsx— Stagger container/child withinitial={false}bypasssrc/hooks/useReducedMotion.ts— Reduced motion detection hooksrc/index.css— Transition container, overflow containmentNavigation + data loading
src/providers/navigation.tsx— Direction state,goBack(),isInitialLoadflagsrc/providers/wallet.tsx—dataReadyflag, delays navigation until data loadedsrc/providers/options.tsx— Settings navigation directionScreens updated
src/App.tsx— AnimatePresence wrapping each tabsrc/screens/Wallet/Index.tsx— Stagger all elements, per-row tx staggersrc/screens/Wallet/Unlock.tsx— Wait for dataReady before navigatingsrc/screens/Wallet/Send/Form.tsx— Keyboard + scanner overlay animationssrc/screens/Wallet/Receive/Amount.tsx— Keyboard overlay animationsrc/components/Header.tsx— Booleanbackprop supportback={true}Test plan
prefers-reduced-motion: reduce→ all animations disabledSummary by CodeRabbit
New Features
Behavior Changes
Chores