feat: new mobile wallet popover P1 #10395
Conversation
📝 WalkthroughWalkthroughAdds a VITE_FEATURE_NEW_WALLET_MANAGER flag and wiring, implements a popover-based Wallet Manager (WalletButton, WalletManagerPopover, PopoverWallet, PopoverWalletHeader), refactors UserMenu to use WalletButton, adds responsive compact props to several tables, and updates env/config/preferences and mocks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Header
participant FeatureFlags
participant WalletManagerPopover
participant UserMenu
User->>Header: Load app
Header->>FeatureFlags: get(NewWalletManager)
alt NewWalletManager enabled
Header->>WalletManagerPopover: Render
else
Header->>UserMenu: Render
end
sequenceDiagram
autonumber
actor User
participant WalletButton
participant WalletManagerPopover
participant PopoverWallet
participant WalletStore
participant Modals
User->>WalletButton: Click
WalletButton-->>WalletManagerPopover: trigger
alt Connected && not Locked
WalletManagerPopover->>PopoverWallet: open/populate
User->>PopoverWallet: Send / Receive
PopoverWallet->>Modals: open send/receive
User->>PopoverWallet: Switch provider / Disconnect
PopoverWallet->>WalletStore: dispatch actions
else Not connected
WalletButton->>WalletStore: dispatch SET_WALLET_MODAL(true)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (1)
47-50: Avoid mutating selector output when sortingrowData.sort(...) mutates the array returned by the selector. Clone before sorting to prevent side effects.
-const sortedRows = useMemo(() => { - return rowData.sort((a, b) => Number(b.fiatAmount) - Number(a.fiatAmount)) -}, [rowData]) +const sortedRows = useMemo(() => { + return [...rowData].sort((a, b) => Number(b.fiatAmount) - Number(a.fiatAmount)) +}, [rowData])
🧹 Nitpick comments (20)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (2)
38-43: Export the props typeExport AccountTableProps for reuse and consistency with our typing guidelines.
-type AccountTableProps = { +export type AccountTableProps = { forceCompactView?: boolean } -export const AccountTable = memo(({ forceCompactView = false }: AccountTableProps) => { +export const AccountTable = memo(({ forceCompactView = false }: AccountTableProps) => {
55-63: Memoize derived responsive flags per project guidelinesPer our guideline to always use useMemo for conditional values, memoize these derived values.
-const [isLargerThanLg] = useMediaQuery(`(min-width: ${breakpoints['lg']})`, { ssr: false }) - -const isCompactCols = !isLargerThanLg || forceCompactView - -const showHeaders = isLargerThanMd && !forceCompactView -const stackTextAlign = isCompactCols ? 'right' : 'left' -const buttonWidth = isCompactCols ? 'full' : 'auto' +const [isLargerThanLg] = useMediaQuery(`(min-width: ${breakpoints['lg']})`, { ssr: false }) + +const isCompactCols = useMemo( + () => !isLargerThanLg || forceCompactView, + [isLargerThanLg, forceCompactView], +) +const showHeaders = useMemo( + () => isLargerThanMd && !forceCompactView, + [isLargerThanMd, forceCompactView], +) +const stackTextAlign = useMemo(() => (isCompactCols ? 'right' : 'left'), [isCompactCols]) +const buttonWidth = useMemo(() => (isCompactCols ? 'full' : 'auto'), [isCompactCols])src/components/MarketsTable.tsx (4)
32-36: Prop addition LGTM; consider a defaultAdding forceCompactView is good. Consider a default to improve clarity.
-export const MarketsTable: React.FC<MarketsTableProps> = memo( - ({ rows, onRowClick, forceCompactView }) => { +export const MarketsTable: React.FC<MarketsTableProps> = memo( + ({ rows, onRowClick, forceCompactView = false }) => {
42-46: Memoize conditional values (breakpoints/compact flags/padding)Align with our guideline to memoize conditional values and simple transforms.
-const [isLargerThanMd] = useMediaQuery(`(min-width: ${breakpoints['md']})`, { ssr: false }) -const [isLargerThanLg] = useMediaQuery(`(min-width: ${breakpoints['lg']})`, { ssr: false }) -const showHeaders = isLargerThanMd && !forceCompactView -const isCompactCols = !isLargerThanLg || forceCompactView +const [isLargerThanMd] = useMediaQuery(`(min-width: ${breakpoints['md']})`, { ssr: false }) +const [isLargerThanLg] = useMediaQuery(`(min-width: ${breakpoints['lg']})`, { ssr: false }) +const showHeaders = useMemo( + () => isLargerThanMd && !forceCompactView, + [isLargerThanMd, forceCompactView], +) +const isCompactCols = useMemo( + () => !isLargerThanLg || forceCompactView, + [isLargerThanLg, forceCompactView], +) -const paddingX = showHeaders ? 4 : 0 +const paddingX = useMemo(() => (showHeaders ? 4 : 0), [showHeaders])Also applies to: 50-50
82-94: Disable sort on non-data columnsPrevent accidental sorting on the sparkline column.
{ id: 'sparkline', + disableSortBy: true, Cell: ({ row }: { row: RowProps }) => {
119-131: Column visibility and actions in compact mode are appropriate; add a11y label to TradeHide-on-compact is clean. Add aria-label to Trade for accessibility.
-<Button data-asset-id={row.original.assetId} onClick={handleTradeClick}> +<Button + data-asset-id={row.original.assetId} + onClick={handleTradeClick} + aria-label={translate('assets.assetCards.assetActions.trade')} +>Also applies to: 136-141, 143-150
.env.production (1)
6-6: Fix dotenv key ordering to satisfy dotenv-linterMove VITE_FEATURE_NEW_WALLET_MANAGER before VITE_FEATURE_QUICK_BUY to appease UnorderedKey warning.
-VITE_FEATURE_QUICK_BUY=false -VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_QUICK_BUY=false.env.development (1)
10-10: Fix dotenv key ordering to satisfy dotenv-linterReorder per linter suggestion.
-VITE_FEATURE_QUICK_BUY=true -VITE_FEATURE_NEW_WALLET_MANAGER=true +VITE_FEATURE_NEW_WALLET_MANAGER=true +VITE_FEATURE_QUICK_BUY=true.env (1)
54-54: Optional: reorder the new flag to satisfy dotenv-linterdotenv-linter warns about key ordering. Move
VITE_FEATURE_NEW_WALLET_MANAGERnext to the otherNEW_*flags (afterVITE_FEATURE_NEW_LIMIT_FLOW) to quiet the warning and keep grouping consistent.VITE_FEATURE_NEW_WALLET_FLOW=true VITE_FEATURE_NEW_LIMIT_FLOW=true +VITE_FEATURE_NEW_WALLET_MANAGER=false VITE_FEATURE_THORCHAIN_SWAPPER_ACK=false VITE_FEATURE_ACTION_CENTER=true VITE_FEATURE_QUICK_BUY=false -VITE_FEATURE_NEW_WALLET_MANAGER=false VITE_FEATURE_CHATWOOT=truesrc/components/Layout/Header/Header.tsx (1)
14-14: Flag gating looks good; consider lazy-loading WalletManagerPopover to avoid adding to the default header bundleCurrent static import pulls the popover even when the flag is off. Lazy-loading aligns with how
WalletConnectToDappsHeaderButtonis handled and avoids unnecessary bytes.-import { WalletManagerPopover } from './NavBar/WalletManagerPopover' +// Lazy-load to avoid adding popover code to the default header bundle +const WalletManagerPopover = lazy(() => + import('./NavBar/WalletManagerPopover').then(m => ({ default: m.WalletManagerPopover })), +) @@ - const isNewWalletManagerEnabled = useFeatureFlag('NewWalletManager') + const isNewWalletManagerEnabled = useFeatureFlag('NewWalletManager') @@ - {isNewWalletManagerEnabled ? <WalletManagerPopover /> : <UserMenu />} + {isNewWalletManagerEnabled ? ( + <Suspense fallback={null}> + <WalletManagerPopover /> + </Suspense> + ) : ( + <UserMenu /> + )}Also, confirm this area is covered by an ErrorBoundary at a higher level; if not, wrap this popover slot with one to follow our TSX error-boundary guideline.
Also applies to: 57-57, 124-124
src/pages/Home/WatchlistTable.tsx (1)
19-22: Add explicit component typing and memoize the prop-driven componentPer our TS/TSX guidelines, add an explicit component type and wrap with
memosince it now receives props.-import { useCallback, useMemo } from 'react' +import { memo, useCallback, useMemo } from 'react' @@ -export const WatchlistTable = ({ forceCompactView = false }: WatchlistTableProps) => { +export const WatchlistTable: React.FC<WatchlistTableProps> = memo( + ({ forceCompactView = false }: WatchlistTableProps) => { @@ -} +})src/components/Layout/Header/NavBar/UserMenu.tsx (3)
20-20: Avoid duplicate/ambiguous exports for initialEntries.entries is exported here and also from WalletButton.tsx. Make this a local constant to prevent confusion and accidental cross-imports.
-export const entries = [WalletConnectedRoutes.Connected] +const INITIAL_ENTRIES = [WalletConnectedRoutes.Connected] ... - <MemoryRouter initialEntries={entries}> + <MemoryRouter initialEntries={INITIAL_ENTRIES}>Also applies to: 49-50
22-24: Constant naming consistency (nit).Follow the project’s constant style; prefer UPPER_SNAKE_CASE for static, module-level constants.
-const maxWidthProp = { base: 'full', md: 'xs' } -const minWidthProp = { base: 0, md: 'xs' } +const MAX_WIDTH_PROP = { base: 'full', md: 'xs' } +const MIN_WIDTH_PROP = { base: 0, md: 'xs' } ... - maxWidth={maxWidthProp} - minWidth={minWidthProp} + maxWidth={MAX_WIDTH_PROP} + minWidth={MIN_WIDTH_PROP}Also applies to: 90-91
49-58: Wrap WalletConnectedMenu in a ComponentErrorBoundary
To prevent a menu rendering error from unmounting the entire header, enclose it in your existingComponentErrorBoundary:<MemoryRouter initialEntries={entries}> - <WalletConnectedMenu + <ComponentErrorBoundary> + <WalletConnectedMenu isConnected={props.isConnected} walletInfo={props.walletInfo} onDisconnect={props.onDisconnect} onSwitchProvider={props.onSwitchProvider} connectedType={props.connectedType} onClose={props.onClose} - /> + /> + </ComponentErrorBoundary> </MemoryRouter>src/components/Layout/Header/NavBar/WalletManagerPopover.tsx (2)
23-27: Use functional state update for reliability.Avoid stale captures when toggling open state.
- const handleOpen = useCallback(() => { - if (!isConnected) return - setIsOpen(!isOpen) - }, [isConnected, isOpen]) + const handleOpen = useCallback(() => { + if (!isConnected) return + setIsOpen(prev => !prev) + }, [isConnected])
43-47: Duplicate fixed width on PopoverBody.PopoverContent already constrains width; duplicating on PopoverBody risks layout quirks. Let the body inherit.
- <PopoverContent width='430px' bg={'background.surface.base'} overflow='hidden' p={0}> - <PopoverBody p={4} width='430px' maxHeight='70vh' overflow='auto'> + <PopoverContent width='430px' bg='background.surface.base' overflow='hidden' p={0}> + <PopoverBody p={4} maxHeight='70vh' overflow='auto'>src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx (2)
52-55: Fix useMemo deps for derived label.Depend on maybeMipdProvider (object) rather than a nested property to avoid missed updates.
- const label = useMemo( - () => maybeMipdProvider?.info?.name || walletInfo?.meta?.label || walletInfo?.name, - [walletInfo, maybeMipdProvider?.info?.name], - ) + const label = useMemo( + () => maybeMipdProvider?.info?.name || walletInfo?.meta?.label || walletInfo?.name, + [walletInfo, maybeMipdProvider], + )
69-71: Use RDNS-mapped provider icon for consistency.Elsewhere we prefer maybeMipdProvider?.info over raw walletInfo for icon/name. Align header visuals with trigger button.
- const walletImageIcon = useMemo(() => <WalletImage walletInfo={walletInfo} />, [walletInfo]) + const walletImageIcon = useMemo( + () => <WalletImage walletInfo={maybeMipdProvider?.info || walletInfo} />, + [maybeMipdProvider, walletInfo], + ) ... - <Text>{walletInfo?.name}</Text> + <Text>{label}</Text>Also applies to: 97-101
src/components/Layout/Header/NavBar/WalletButton.tsx (2)
9-12: Remove unused export and import.entries is unused here and duplicates a similar export in UserMenu. Drop both the import and export to avoid API sprawl.
-import { WalletConnectedRoutes } from '@/components/Layout/Header/NavBar/hooks/useMenuRoutes' ... -export const entries = [WalletConnectedRoutes.Connected]Also applies to: 19-21
21-22: Constant naming (nit).Consider WIDTH_RESPONSIVE for style consistency, then update the usage.
-const widthProp = { base: '100%', lg: 'auto' } +const WIDTH_RESPONSIVE = { base: '100%', lg: 'auto' } ... - width={widthProp} + width={WIDTH_RESPONSIVE}Also applies to: 94-95
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
.env(1 hunks).env.development(1 hunks).env.production(1 hunks)src/components/Layout/Header/Header.tsx(3 hunks)src/components/Layout/Header/NavBar/PopoverWallet.tsx(1 hunks)src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx(1 hunks)src/components/Layout/Header/NavBar/UserMenu.tsx(2 hunks)src/components/Layout/Header/NavBar/WalletButton.tsx(1 hunks)src/components/Layout/Header/NavBar/WalletManagerPopover.tsx(1 hunks)src/components/MarketsTable.tsx(1 hunks)src/config.ts(1 hunks)src/pages/Dashboard/components/AccountList/AccountTable.tsx(8 hunks)src/pages/Home/WatchlistTable.tsx(2 hunks)src/state/slices/preferencesSlice/preferencesSlice.ts(2 hunks)src/test/mocks/store.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/components/Layout/Header/Header.tsxsrc/test/mocks/store.tssrc/components/Layout/Header/NavBar/PopoverWalletHeader.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/Layout/Header/NavBar/WalletManagerPopover.tsxsrc/config.tssrc/state/slices/preferencesSlice/preferencesSlice.tssrc/components/MarketsTable.tsxsrc/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/Layout/Header/NavBar/UserMenu.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/components/Layout/Header/Header.tsxsrc/components/Layout/Header/NavBar/PopoverWalletHeader.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/Layout/Header/NavBar/WalletManagerPopover.tsxsrc/components/MarketsTable.tsxsrc/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/Layout/Header/NavBar/UserMenu.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/components/Layout/Header/Header.tsxsrc/test/mocks/store.tssrc/components/Layout/Header/NavBar/PopoverWalletHeader.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/Layout/Header/NavBar/WalletManagerPopover.tsxsrc/config.tssrc/state/slices/preferencesSlice/preferencesSlice.tssrc/components/MarketsTable.tsxsrc/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/Layout/Header/NavBar/UserMenu.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/components/Layout/Header/Header.tsxsrc/components/Layout/Header/NavBar/PopoverWalletHeader.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/Layout/Header/NavBar/WalletManagerPopover.tsxsrc/components/MarketsTable.tsxsrc/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/Layout/Header/NavBar/UserMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/components/Layout/Header/Header.tsxsrc/test/mocks/store.tssrc/components/Layout/Header/NavBar/PopoverWalletHeader.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/Layout/Header/NavBar/WalletManagerPopover.tsxsrc/config.tssrc/state/slices/preferencesSlice/preferencesSlice.tssrc/components/MarketsTable.tsxsrc/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/Layout/Header/NavBar/UserMenu.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
📚 Learning: 2025-08-22T13:02:38.078Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/Explore/Explore.tsx:91-92
Timestamp: 2025-08-22T13:02:38.078Z
Learning: In the ShapeShift web codebase, feature flags are consistently used as string literals with the useFeatureFlag hook (e.g., useFeatureFlag('RfoxFoxEcosystemPage')), not as enums. The hook accepts keyof FeatureFlags which provides type safety through the TypeScript type system rather than requiring an enum. This is the established pattern used throughout the entire codebase.
Applied to files:
src/components/Layout/Header/Header.tsxsrc/state/slices/preferencesSlice/preferencesSlice.ts
📚 Learning: 2025-07-30T20:57:48.176Z
Learnt from: NeOMakinG
PR: shapeshift/web#10148
File: src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx:88-91
Timestamp: 2025-07-30T20:57:48.176Z
Learning: In src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx, NeOMakinG prefers keeping the hardcoded overscan value (40) over dynamic calculation based on viewport height, prioritizing code simplicity over precision when the current approach works effectively.
Applied to files:
src/components/MarketsTable.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for conditional values and simple transformations
Applied to files:
src/pages/Dashboard/components/AccountList/AccountTable.tsx
🧬 Code graph analysis (9)
src/components/Layout/Header/NavBar/WalletButton.tsx (7)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/localWalletSlice/selectors.ts (1)
selectWalletRdns(21-24)src/lib/mipd.ts (1)
useMipdProviders(7-8)src/components/Layout/Header/NavBar/WalletImage.tsx (1)
WalletImage(10-16)src/lib/vibrate.ts (1)
vibrate(6-16)src/components/MiddleEllipsis/MiddleEllipsis.tsx (1)
MiddleEllipsis(11-17)src/components/Text/Text.tsx (2)
RawText(15-17)Text(19-83)
src/components/Layout/Header/Header.tsx (2)
src/components/Layout/Header/NavBar/WalletManagerPopover.tsx (1)
WalletManagerPopover(11-50)src/components/Layout/Header/NavBar/UserMenu.tsx (1)
UserMenu(62-113)
src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx (8)
src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/localWalletSlice/selectors.ts (1)
selectWalletRdns(21-24)src/lib/mipd.ts (1)
useMipdProviders(7-8)src/context/WalletProvider/config.ts (1)
SUPPORTED_WALLETS(339-444)src/components/Layout/Header/NavBar/WalletImage.tsx (1)
WalletImage(10-16)src/pages/Dashboard/components/ProfileAvatar/ProfileAvatar.tsx (1)
ProfileAvatar(11-28)src/components/Text/Text.tsx (1)
Text(19-83)
src/components/Layout/Header/NavBar/PopoverWallet.tsx (7)
src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/context/AppProvider/hooks/useDiscoverAccounts.tsx (1)
useDiscoverAccounts(17-150)src/state/slices/portfolioSlice/selectors.ts (2)
selectPortfolioTotalUserCurrencyBalance(187-196)selectIsAnyPortfolioGetAccountLoading(1018-1023)src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx (1)
PopoverWalletHeader(40-125)src/components/TooltipWithTouch.tsx (1)
TooltipWithTouch(7-73)src/pages/Dashboard/components/AccountList/AccountTable.tsx (1)
AccountTable(42-217)src/pages/Home/WatchlistTable.tsx (1)
WatchlistTable(23-66)
src/components/Layout/Header/NavBar/WalletManagerPopover.tsx (2)
src/components/Layout/Header/NavBar/WalletButton.tsx (1)
WalletButton(30-122)src/components/Layout/Header/NavBar/PopoverWallet.tsx (1)
PopoverWallet(66-178)
src/state/slices/preferencesSlice/preferencesSlice.ts (1)
src/config.ts (1)
getConfig(202-204)
src/components/MarketsTable.tsx (6)
src/theme/theme.tsx (1)
breakpoints(37-44)src/state/slices/marketDataSlice/selectors.ts (2)
selectMarketDataUserCurrency(19-55)selectIsAnyMarketDataApiQueryPending(155-156)src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/apis/fiatRamps/hooks.ts (1)
useFetchFiatAssetMarketData(8-20)src/hooks/useInfiniteScroll/useInfiniteScroll.tsx (1)
useInfiniteScroll(11-49)src/components/ReactTable/ReactTableNoPager.tsx (1)
ReactTableNoPager(48-198)
src/pages/Home/WatchlistTable.tsx (1)
src/components/MarketsTable.tsx (1)
MarketsTable(38-180)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (3)
src/theme/theme.tsx (1)
breakpoints(37-44)src/components/Amount/Amount.tsx (1)
Amount(21-42)src/components/Text/Text.tsx (1)
Text(19-83)
🪛 dotenv-linter (3.3.0)
.env
[warning] 54-54: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_OPTIMISM key
(UnorderedKey)
.env.development
[warning] 10-10: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_QUICK_BUY key
(UnorderedKey)
.env.production
[warning] 6-6: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_QUICK_BUY key
(UnorderedKey)
⏰ 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: Install and Cache
🔇 Additional comments (15)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (4)
82-83: Responsive column visibility/alignment looks goodThe conditional display/justification cleanly adapts to compact mode.
Also applies to: 109-110, 117-118, 138-139
146-147: Columns memo deps are sufficientIncluding isCompactCols, stackTextAlign, and textColor covers the dynamic bits.
181-182: Empty-state callback deps are correctButton width and handler are properly captured.
204-212: InfiniteTable correctly supports displayHeadersThe
displayHeadersprop is defined in its interface, defaulted totrue, and used to conditionally render headers—no further changes needed.src/components/MarketsTable.tsx (5)
52-55: Efficient market data prefetchingUsing useMemo for assetIds and prefetching fiat market data per asset is solid.
59-67: Trade navigation handler is correctStops propagation and guards missing assetId. Nice.
96-117: Price + mobile change tag implementation looks goodGood UX: price primary, percent change tucked into Mobile-only tag.
154-160: Long-press action wiring LGTMOpens the asset actions drawer with proper context.
161-178: Loading/headers/Load more integration is coherentHeaders respect showHeaders, isLoading tied to market API, and paging via Load more is consistent.
src/test/mocks/store.ts (1)
164-165: Mock flag added correctlyNewWalletManager added to featureFlags with default false—matches production gating.
src/state/slices/preferencesSlice/preferencesSlice.ts (2)
92-93: Type extension LGTM
FeatureFlagscorrectly addsNewWalletManager: boolean. No issues.
215-216: Init wiring LGTM
initialState.featureFlags.NewWalletManagercorrectly sourced from config.src/config.ts (1)
189-190: Validator addition LGTM
VITE_FEATURE_NEW_WALLET_MANAGERadded with a boolean validator and defaultfalse. Matches usage downstream.src/pages/Home/WatchlistTable.tsx (1)
60-60: Prop threading LGTM
forceCompactViewis correctly forwarded toMarketsTableto drive compact layout.src/components/Layout/Header/NavBar/UserMenu.tsx (1)
81-88: Good extraction: WalletButton now correctly drives Menu trigger in menu context.Passing isMenuContext ensures Chakra’s MenuButton behavior while reusing the shared WalletButton UI. Looks solid.
premiumjibles
force-pushed
the
10285_mobile_wallet
branch
2 times, most recently
from
9a07f7b to
2d4f454
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (1)
43-55: Do not mutate Redux-derived arrays when sortingrowData.sort(...) mutates the selector result in place, which may corrupt memoized state and cause subtle bugs. Copy before sorting; also prefer bnOrZero for numeric correctness.
- const sortedRows = useMemo(() => { - return rowData.sort((a, b) => Number(b.fiatAmount) - Number(a.fiatAmount)) - }, [rowData]) + const sortedRows = useMemo(() => { + return [...rowData].sort((a, b) => + bnOrZero(b.fiatAmount).comparedTo(bnOrZero(a.fiatAmount)), + ) + }, [rowData])
🧹 Nitpick comments (8)
src/pages/Home/WatchlistTable.tsx (4)
19-24: Export the props type; consider positive boolean naming
- Export the props type if external consumers need it.
- Optional: prefer positive boolean names with is/has/can/should. hideExploreMore could become showExploreMore (default true) or isExploreMoreHidden.
-type WatchlistTableProps = { +export type WatchlistTableProps = { forceCompactView?: boolean onRowClick?: () => void hideExploreMore?: boolean }
25-29: Add an explicit return type; confirm error boundary coverage
- Add a JSX.Element return type per TS guidelines.
- Per project rules, ensure this component is wrapped by an ErrorBoundary upstream (e.g., the popover container) and that errors surface via useErrorToast.
-export const WatchlistTable = ({ +export const WatchlistTable = ({ forceCompactView = false, hideExploreMore = false, onRowClick, -}: WatchlistTableProps) => { +}: WatchlistTableProps): JSX.Element => {
42-50: Row-click side effects order — keep consistent across tablesHere you call onRowClick before vibrate; in AccountTable it’s the opposite. Pick one order project-wide (suggest: onRowClick first to close UI promptly), for consistent UX. No change needed here.
68-72: CTA rendering is fine; optional consistency tweakOptional: align styling/props with the empty-state CTA for visual consistency (e.g., size='lg', colorScheme='blue', width='full'), unless design intentionally differs.
src/pages/Dashboard/components/AccountList/AccountTable.tsx (4)
38-44: Export the props type; add explicit return type and confirm error boundary
- Export AccountTableProps if you plan to reuse it.
- Add an explicit JSX.Element return type.
- Ensure an ErrorBoundary wraps this component upstream per guidelines; surface errors via useErrorToast.
-type AccountTableProps = { +export type AccountTableProps = { forceCompactView?: boolean onRowClick?: () => void } -export const AccountTable = memo(({ forceCompactView = false, onRowClick }: AccountTableProps) => { +export const AccountTable = memo( + ({ forceCompactView = false, onRowClick }: AccountTableProps): JSX.Element => {
57-64: Responsive compact-mode logic looks good; minor naming nitLogic for isCompactCols/showHeaders/stackTextAlign/buttonWidth is sound. Optional: rename isCompactCols to isCompactColumns to avoid abbreviations per naming rules.
115-145: Sorting comparator should return 0 for equal valuesBoth sortType implementations return only 1/-1. Return 0 when equal to avoid unstable ordering.
- sortType: (a: RowProps, b: RowProps): number => - bnOrZero(a.original.priceChange).gt(bnOrZero(b.original.priceChange)) ? 1 : -1, + sortType: (a: RowProps, b: RowProps): number => { + const left = bnOrZero(a.original.priceChange) + const right = bnOrZero(b.original.priceChange) + return left.eq(right) ? 0 : left.gt(right) ? 1 : -1 + }, ... - sortType: (a: RowProps, b: RowProps): number => - bnOrZero(a.original.allocation).gt(bnOrZero(b.original.allocation)) ? 1 : -1, + sortType: (a: RowProps, b: RowProps): number => { + const left = bnOrZero(a.original.allocation) + const right = bnOrZero(b.original.allocation) + return left.eq(right) ? 0 : left.gt(right) ? 1 : -1 + },
184-193: Fire onRowClick before haptics for consistency and quicker closeThis matches WatchlistTable and closes the popover immediately; haptics can still run after.
- vibrate('heavy') - onRowClick?.() + onRowClick?.() + vibrate('heavy')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/components/Layout/Header/NavBar/PopoverWallet.tsx(1 hunks)src/components/Layout/Header/NavBar/WalletButton.tsx(1 hunks)src/components/Layout/Header/NavBar/WalletManagerPopover.tsx(1 hunks)src/pages/Dashboard/components/AccountList/AccountTable.tsx(8 hunks)src/pages/Home/WatchlistTable.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Layout/Header/NavBar/PopoverWallet.tsx
- src/components/Layout/Header/NavBar/WalletManagerPopover.tsx
- src/components/Layout/Header/NavBar/WalletButton.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/pages/Home/WatchlistTable.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for conditional values and simple transformations
Applied to files:
src/pages/Dashboard/components/AccountList/AccountTable.tsx
🧬 Code graph analysis (2)
src/pages/Home/WatchlistTable.tsx (2)
src/lib/vibrate.ts (1)
vibrate(6-16)src/components/MarketsTable.tsx (1)
MarketsTable(38-180)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (3)
src/theme/theme.tsx (1)
breakpoints(37-44)src/state/slices/portfolioSlice/selectors.ts (1)
AccountRowData(726-736)src/lib/vibrate.ts (1)
vibrate(6-16)
⏰ 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: Call / Static
🔇 Additional comments (5)
src/pages/Home/WatchlistTable.tsx (1)
67-67: Prop forwarding to MarketsTable looks correctforceCompactView and onRowClick are correctly wired through.
src/pages/Dashboard/components/AccountList/AccountTable.tsx (4)
1-1: Typed-only import is goodImporting FlexProps as a type avoids runtime footprint.
80-114: Balance column alignment/display changes — LGTMConditional justifyContent and column visibility behave as expected in compact mode.
164-183: Empty-state button width dependency — LGTMIncluding buttonWidth in renderEmptyComponent dependencies is correct; ensures width updates on layout changes.
206-218: Headers visibility wiring — LGTMPassing displayHeaders={showHeaders} aligns with the new compact-mode logic.
premiumjibles
force-pushed
the
10285_mobile_wallet
branch
from
9c56fe6 to
e2406bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Dashboard/Dashboard.tsx (1)
150-155: Fix route shadowing: firstpath="*"swallows the NotFound route.On Desktop, the catch-all at Line 150 matches everything, so the NotFound at Line 154 is unreachable. Make the default route an index route instead.
- <Route path='*' element={walletDashboard} /> + <Route index element={walletDashboard} /> <Route path='earn' element={earnDashboard} /> <Route path='accounts/*' element={accounts} /> <Route path='activity' element={transactionHistory} /> <Route path='*' element={notFound} />
🧹 Nitpick comments (2)
src/pages/Dashboard/Dashboard.tsx (2)
47-47: Name constant in UPPER_SNAKE_CASE and reuse consistently.Follow project naming guidelines for constants; rename and update usages.
-const unselectedTabStyle = { color: 'text.base' } +const UNSELECTED_TAB_STYLE = { color: 'text.base' }- <Tab _selected={selectedTabStyle} sx={unselectedTabStyle}> + <Tab _selected={selectedTabStyle} sx={UNSELECTED_TAB_STYLE}> {translate('dashboard.portfolio.myCrypto')} </Tab> - <Tab _selected={selectedTabStyle} sx={unselectedTabStyle}> + <Tab _selected={selectedTabStyle} sx={UNSELECTED_TAB_STYLE}> {translate('watchlist.title')} </Tab>Also applies to: 103-108
53-58: Prefer semantic tokens over raw 'black'/'white'.To stay consistent with the design system and future theme tweaks, consider using theme/semantic tokens (e.g., brand or surface/text tokens) instead of hardcoded colors. If tokens aren’t available yet, feel free to defer to a follow-up per team preference.
(We have a learning that UX polish is often deferred; confirm if you want this change now or in a follow-up.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/Layout/Header/NavBar/PopoverWallet.tsx(1 hunks)src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx(1 hunks)src/pages/Dashboard/Dashboard.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx
- src/components/Layout/Header/NavBar/PopoverWallet.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/pages/Dashboard/Dashboard.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/pages/Dashboard/Dashboard.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/pages/Dashboard/Dashboard.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/pages/Dashboard/Dashboard.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/pages/Dashboard/Dashboard.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
⏰ 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: Call / Static
🔇 Additional comments (1)
src/pages/Dashboard/Dashboard.tsx (1)
2-2: LGTM: color-mode-aware import is appropriate.Importing useColorModeValue aligns with the new tab styling needs.
premiumjibles
force-pushed
the
10285_mobile_wallet
branch
from
e2406bc to
6d41444
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (1)
48-51: Do not mutate selector-derived arrays in sort()rowData.sort(...) mutates the source array and may cause subtle UI/state bugs. Clone before sorting.
Apply:
- const sortedRows = useMemo(() => { - return rowData.sort((a, b) => Number(b.fiatAmount) - Number(a.fiatAmount)) - }, [rowData]) + const sortedRows = useMemo(() => { + return [...rowData].sort((a, b) => Number(b.fiatAmount) - Number(a.fiatAmount)) + }, [rowData])
♻️ Duplicate comments (2)
src/components/Layout/Header/NavBar/WalletButton.tsx (2)
122-124: Forward ButtonProps to the connect button too.Keeps API consistent and enables data-, aria-, and test hooks in both states.
- <Button onClick={onConnect} leftIcon={connectIcon}> + <Button onClick={onConnect} leftIcon={connectIcon} {...otherProps}>
44-46: Guard getAddress with isAddress to prevent runtime throws for non-EVM wallets.getAddress() throws on invalid/non-EVM addresses. This can crash ENS resolution when a non-ETH wallet is active.
-import { getAddress } from 'viem' +import { getAddress, isAddress } from 'viem' @@ - const { data: ensName } = useEnsName({ - address: walletInfo?.meta?.address ? getAddress(walletInfo.meta.address) : undefined, - }) + const rawAddress = walletInfo?.meta?.address + const checksummed = + rawAddress && isAddress(rawAddress) ? getAddress(rawAddress) : undefined + const { data: ensName } = useEnsName({ address: checksummed })
🧹 Nitpick comments (7)
.env.development (1)
10-10: Sort flag key to satisfy dotenv-linter.Place VITE_FEATURE_NEW_WALLET_MANAGER before VITE_FEATURE_QUICK_BUY to keep alphabetical order and silence the UnorderedKey warning.
VITE_FEATURE_THORCHAIN_TCY_ACTIVITY=true -VITE_FEATURE_QUICK_BUY=true -VITE_FEATURE_NEW_WALLET_MANAGER=true +VITE_FEATURE_NEW_WALLET_MANAGER=true +VITE_FEATURE_QUICK_BUY=true.env (1)
54-54: Keep feature flags sorted; move before QUICK_BUY.For consistency and to avoid dotenv-linter noise, position this “NEW_” flag before “QUICK_BUY”.
-VITE_FEATURE_QUICK_BUY=false -VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_QUICK_BUY=falseIf you prefer, run dotenv-linter with --fix to auto-sort.
.env.production (1)
6-6: Sort flag key before QUICK_BUY.Match ordering across envs to keep diffs clean and satisfy dotenv-linter.
-VITE_FEATURE_QUICK_BUY=false -VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_QUICK_BUY=falsesrc/pages/Dashboard/components/AccountList/AccountTable.tsx (2)
184-193: Minor: simplify navigate callassetId is non-optional on AccountRowData; the ternary adds noise. Safe to inline.
- const { assetId } = row.original - const url = assetId ? `/assets/${assetId}` : '' - navigate(url) + const { assetId } = row.original + navigate(`/assets/${assetId}`)
43-44: Consider wrapping in an error boundary (can be in parent)Per guidelines, components should be wrapped in an error boundary with a user-friendly fallback. Ok to defer to a follow-up PR.
src/components/MarketsTable.tsx (2)
162-178: Scalability: consider virtualization if rows can exceed ~100Infinite scroll helps, but dense rows with sparklines and tags can still get heavy. If typical counts exceed 100, consider the existing Virtualized table variant.
What’s the expected max row count for Markets/Watchlist in the popover? If >100, I can propose a minimal swap to the virtualized table.
38-41: Add an error boundary at usage sitePer guidelines, ensure MarketsTable is rendered within an error boundary that logs and shows a translated fallback. Fine to defer to a follow-up PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
.env(1 hunks).env.development(1 hunks).env.production(1 hunks)src/components/Layout/Header/Header.tsx(3 hunks)src/components/Layout/Header/NavBar/PopoverWallet.tsx(1 hunks)src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx(1 hunks)src/components/Layout/Header/NavBar/UserMenu.tsx(2 hunks)src/components/Layout/Header/NavBar/WalletButton.tsx(1 hunks)src/components/Layout/Header/NavBar/WalletManagerPopover.tsx(1 hunks)src/components/MarketsTable.tsx(1 hunks)src/config.ts(1 hunks)src/pages/Dashboard/Dashboard.tsx(3 hunks)src/pages/Dashboard/components/AccountList/AccountTable.tsx(8 hunks)src/pages/Home/WatchlistTable.tsx(3 hunks)src/state/slices/preferencesSlice/preferencesSlice.ts(2 hunks)src/test/mocks/store.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/Layout/Header/NavBar/WalletManagerPopover.tsx
- src/test/mocks/store.ts
- src/pages/Dashboard/Dashboard.tsx
- src/components/Layout/Header/Header.tsx
- src/components/Layout/Header/NavBar/PopoverWallet.tsx
- src/pages/Home/WatchlistTable.tsx
- src/components/Layout/Header/NavBar/UserMenu.tsx
- src/config.ts
- src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/state/slices/preferencesSlice/preferencesSlice.tssrc/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/MarketsTable.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/state/slices/preferencesSlice/preferencesSlice.tssrc/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/MarketsTable.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/state/slices/preferencesSlice/preferencesSlice.tssrc/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/MarketsTable.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/MarketsTable.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsxsrc/components/MarketsTable.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
📚 Learning: 2025-08-22T13:02:38.078Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/Explore/Explore.tsx:91-92
Timestamp: 2025-08-22T13:02:38.078Z
Learning: In the ShapeShift web codebase, feature flags are consistently used as string literals with the useFeatureFlag hook (e.g., useFeatureFlag('RfoxFoxEcosystemPage')), not as enums. The hook accepts keyof FeatureFlags which provides type safety through the TypeScript type system rather than requiring an enum. This is the established pattern used throughout the entire codebase.
Applied to files:
src/state/slices/preferencesSlice/preferencesSlice.ts
📚 Learning: 2025-08-25T23:32:13.876Z
Learnt from: premiumjibles
PR: shapeshift/web#10361
File: src/pages/Markets/components/CardWithSparkline.tsx:83-92
Timestamp: 2025-08-25T23:32:13.876Z
Learning: In shapeshift/web PR #10361, premiumjibles considered the nested button accessibility issue (ChartErrorFallback retry Button inside Card rendered as Button in CardWithSparkline.tsx) out of scope for the error boundaries feature PR, consistent with deferring minor a11y improvements to follow-up PRs.
Applied to files:
src/components/Layout/Header/NavBar/WalletButton.tsx
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/pages/Explore/ExploreCategory.tsx:231-238
Timestamp: 2025-08-08T14:59:40.422Z
Learning: In src/pages/Explore/ExploreCategory.tsx, for the PageHeader filter trigger, NeOMakinG considers changing a clickable Chakra Icon to IconButton too nitpicky for this PR and prefers to keep the current Icon-based trigger; such minor a11y/UI nitpicks should be deferred to a follow-up if needed.
Applied to files:
src/components/Layout/Header/NavBar/WalletButton.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for conditional values and simple transformations
Applied to files:
src/pages/Dashboard/components/AccountList/AccountTable.tsx
📚 Learning: 2025-07-30T20:57:48.176Z
Learnt from: NeOMakinG
PR: shapeshift/web#10148
File: src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx:88-91
Timestamp: 2025-07-30T20:57:48.176Z
Learning: In src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx, NeOMakinG prefers keeping the hardcoded overscan value (40) over dynamic calculation based on viewport height, prioritizing code simplicity over precision when the current approach works effectively.
Applied to files:
src/components/MarketsTable.tsx
🧬 Code graph analysis (4)
src/state/slices/preferencesSlice/preferencesSlice.ts (1)
src/config.ts (1)
getConfig(202-204)
src/components/Layout/Header/NavBar/WalletButton.tsx (7)
src/components/Layout/Header/NavBar/UserMenu.tsx (1)
entries(20-20)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/localWalletSlice/selectors.ts (1)
selectWalletRdns(21-24)src/lib/mipd.ts (1)
useMipdProviders(7-8)src/components/Layout/Header/NavBar/WalletImage.tsx (1)
WalletImage(10-16)src/lib/vibrate.ts (1)
vibrate(6-16)src/components/Text/Text.tsx (2)
RawText(15-17)Text(19-83)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (3)
src/theme/theme.tsx (1)
breakpoints(37-44)src/state/slices/portfolioSlice/selectors.ts (1)
AccountRowData(726-736)src/lib/vibrate.ts (1)
vibrate(6-16)
src/components/MarketsTable.tsx (6)
src/theme/theme.tsx (1)
breakpoints(37-44)src/state/slices/marketDataSlice/selectors.ts (2)
selectMarketDataUserCurrency(19-55)selectIsAnyMarketDataApiQueryPending(155-156)src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/apis/fiatRamps/hooks.ts (1)
useFetchFiatAssetMarketData(8-20)src/hooks/useInfiniteScroll/useInfiniteScroll.tsx (1)
useInfiniteScroll(11-49)src/components/ReactTable/ReactTableNoPager.tsx (1)
ReactTableNoPager(48-198)
🪛 dotenv-linter (3.3.0)
.env
[warning] 54-54: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_OPTIMISM key
(UnorderedKey)
.env.development
[warning] 10-10: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_QUICK_BUY key
(UnorderedKey)
.env.production
[warning] 6-6: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_QUICK_BUY key
(UnorderedKey)
⏰ 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: Install and Cache
🔇 Additional comments (9)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (4)
57-64: Viewport-aware compact/header logic looks goodThe md/lg split and the forceCompactView override are clean and predictable across breakpoints.
83-83: Column visibility/alignment toggles are consistentjustifyContent and display flags correctly adapt for compact mode without layout thrash.
Also applies to: 110-111, 118-119, 139-140
171-179: CTA width adapts correctly to compact modeUsing buttonWidth from the same isCompactCols source avoids divergent UI states.
Also applies to: 182-182
206-217: No changes needed: InfiniteTable supportsdisplayHeadersThe prop is defined inInfiniteTableProps, defaults totrue, and is used to conditionally render the header row.src/components/MarketsTable.tsx (5)
35-46: forceCompactView and breakpoint logic are solidThe md/lg split and opt-in compact override align with the new wallet popover needs and mirror AccountTable.
50-50: Padding ties to header visibility appropriatelypx driven by showHeaders avoids awkward gutters in compact mode.
52-55: Fetching market data only for visible assetIdsMemoized assetIds keeps the effect stable and prevents redundant dispatches. Nice.
60-67: Good UX: prevent row navigation when clicking TradestopPropagation avoids accidental navigations and respects onRowClick handlers.
68-153: Columns compose well; ensure memo deps are minimalDependencies look scoped. If marketDataUserCurrencyById is a new object each tick, columns will re-create—acceptable, but watch for perf in very large lists.
If you see re-renders, consider moving dynamic lookups into Cell using row.original and keeping columns stable.
premiumjibles
force-pushed
the
10285_mobile_wallet
branch
from
6d41444 to
4f28147
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (1)
48-50: Avoid mutating selector output; use non-mutating sort with robust numeric compare.sort() mutates the array returned by the selector. Clone before sorting and use bnOrZero to avoid NaN from string numbers.
Apply:
- const sortedRows = useMemo(() => { - return rowData.sort((a, b) => Number(b.fiatAmount) - Number(a.fiatAmount)) - }, [rowData]) + const sortedRows = useMemo(() => { + return [...rowData].sort((a, b) => + bnOrZero(b.fiatAmount).comparedTo(bnOrZero(a.fiatAmount)) + ) + }, [rowData])
♻️ Duplicate comments (2)
src/components/Layout/Header/NavBar/WalletButton.tsx (2)
122-124: Forward ButtonProps to the connect state too.Ensure
data-*,aria-*, sizing, and test IDs also apply when disconnected.- <Button onClick={onConnect} leftIcon={connectIcon}> + <Button onClick={onConnect} leftIcon={connectIcon} {...otherProps}>
44-46: Guard getAddress() with isAddress() to prevent runtime throws.
getAddress()throws on invalid input; guard it so malformed/chain-unknown addresses don’t crash the render.-import { getAddress } from 'viem' +import { getAddress, isAddress } from 'viem' @@ - const { data: ensName } = useEnsName({ - address: walletInfo?.meta?.address ? getAddress(walletInfo.meta.address) : undefined, - }) + const { data: ensName } = useEnsName({ + address: + walletInfo?.meta?.address && isAddress(walletInfo.meta.address) + ? getAddress(walletInfo.meta.address) + : undefined, + })Also applies to: 7-7
🧹 Nitpick comments (8)
.env.production (1)
6-6: Fix dotenv-linter ordering for feature flags.Keep keys sorted to satisfy dotenv-linter and maintain consistency.
-VITE_FEATURE_QUICK_BUY=false -VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_NEW_WALLET_MANAGER=false +VITE_FEATURE_QUICK_BUY=false.env.development (1)
10-10: Fix dotenv-linter ordering for feature flags.Place NEW_WALLET_MANAGER before QUICK_BUY (N < Q).
-VITE_FEATURE_QUICK_BUY=true -VITE_FEATURE_NEW_WALLET_MANAGER=true +VITE_FEATURE_NEW_WALLET_MANAGER=true +VITE_FEATURE_QUICK_BUY=true.env (1)
54-54: Keep feature flags alphabetized within the block.Move NEW_WALLET_MANAGER into the existing “NEW_*” cluster to pass dotenv-linter and keep consistency.
- VITE_FEATURE_QUICK_BUY=false -VITE_FEATURE_NEW_WALLET_MANAGER=false VITE_FEATURE_CHATWOOT=true + + # keep alphabetized + VITE_FEATURE_NEW_WALLET_MANAGER=false + VITE_FEATURE_CHATWOOT=trueAlternatively, insert right after
VITE_FEATURE_NEW_LIMIT_FLOW=trueand beforeVITE_FEATURE_THORCHAIN_SWAPPER_ACK=false.src/components/Layout/Header/NavBar/WalletButton.tsx (2)
56-75: Derive label with useMemo instead of useEffect + state.This avoids an extra render and keeps derived state local to inputs (
walletInfo,ensName).- useEffect(() => { - setWalletLabel('') - setShouldShorten(true) - if (!walletInfo || !walletInfo.meta) return setWalletLabel('') - if (!walletInfo?.meta?.address && walletInfo.meta.label) { - setShouldShorten(false) - return setWalletLabel(walletInfo.meta.label) - } - if (ensName) { - setShouldShorten(false) - return setWalletLabel(ensName) - } - return setWalletLabel(walletInfo?.meta?.address ?? '') - }, [ensName, walletInfo]) + const { label: derivedLabel, shorten } = useMemo(() => { + if (!walletInfo?.meta) return { label: '', shorten: true } + if (!walletInfo.meta.address && walletInfo.meta.label) + return { label: walletInfo.meta.label, shorten: false } + if (ensName) return { label: ensName, shorten: false } + return { label: walletInfo.meta.address ?? '', shorten: true } + }, [ensName, walletInfo]) + + useEffect(() => { + setWalletLabel(derivedLabel) + setShouldShorten(shorten) + }, [derivedLabel, shorten])
20-21: Type entries as readonly.Prevents accidental mutation.
-export const entries = [WalletConnectedRoutes.Connected] +export const entries = [WalletConnectedRoutes.Connected] as constsrc/pages/Dashboard/components/AccountList/AccountTable.tsx (3)
56-58: Use one useMediaQuery call to reduce listeners.- const [isLargerThanMd] = useMediaQuery(`(min-width: ${breakpoints['md']})`, { ssr: false }) - const [isLargerThanLg] = useMediaQuery(`(min-width: ${breakpoints['lg']})`, { ssr: false }) + const [isLargerThanMd, isLargerThanLg] = useMediaQuery( + [`(min-width: ${breakpoints['md']})`, `(min-width: ${breakpoints['lg']})`], + { ssr: false }, + )
119-121: Return 0 for equal values in custom sorters.Prevents inconsistent ordering when values are equal.
- sortType: (a: RowProps, b: RowProps): number => - bnOrZero(a.original.priceChange).gt(bnOrZero(b.original.priceChange)) ? 1 : -1, + sortType: (a: RowProps, b: RowProps): number => { + const av = bnOrZero(a.original.priceChange) + const bv = bnOrZero(b.original.priceChange) + return av.eq(bv) ? 0 : av.gt(bv) ? 1 : -1 + }, ... - sortType: (a: RowProps, b: RowProps): number => - bnOrZero(a.original.allocation).gt(bnOrZero(b.original.allocation)) ? 1 : -1, + sortType: (a: RowProps, b: RowProps): number => { + const av = bnOrZero(a.original.allocation) + const bv = bnOrZero(b.original.allocation) + return av.eq(bv) ? 0 : av.gt(bv) ? 1 : -1 + },Also applies to: 143-145
121-133: Neutral styling for 0% change (no arrow, subtle color).- color={value > 0 ? 'green.500' : 'red.500'} + color={value === 0 ? 'text.subtle' : value > 0 ? 'green.500' : 'red.500'} > - <StatArrow ml={1} type={value > 0 ? 'increase' : 'decrease'} /> + {value !== 0 && ( + <StatArrow ml={1} type={value > 0 ? 'increase' : 'decrease'} /> + )} <Amount.Percent value={value * 0.01} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
.env(1 hunks).env.development(1 hunks).env.production(1 hunks)src/components/Layout/Header/Header.tsx(3 hunks)src/components/Layout/Header/NavBar/PopoverWallet.tsx(1 hunks)src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx(1 hunks)src/components/Layout/Header/NavBar/UserMenu.tsx(2 hunks)src/components/Layout/Header/NavBar/WalletButton.tsx(1 hunks)src/components/Layout/Header/NavBar/WalletManagerPopover.tsx(1 hunks)src/components/MarketsTable.tsx(1 hunks)src/config.ts(1 hunks)src/pages/Dashboard/Dashboard.tsx(3 hunks)src/pages/Dashboard/components/AccountList/AccountTable.tsx(8 hunks)src/pages/Home/WatchlistTable.tsx(3 hunks)src/state/slices/preferencesSlice/preferencesSlice.ts(2 hunks)src/test/mocks/store.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- src/components/Layout/Header/Header.tsx
- src/components/Layout/Header/NavBar/WalletManagerPopover.tsx
- src/test/mocks/store.ts
- src/config.ts
- src/state/slices/preferencesSlice/preferencesSlice.ts
- src/pages/Dashboard/Dashboard.tsx
- src/components/Layout/Header/NavBar/PopoverWalletHeader.tsx
- src/components/MarketsTable.tsx
- src/pages/Home/WatchlistTable.tsx
- src/components/Layout/Header/NavBar/UserMenu.tsx
- src/components/Layout/Header/NavBar/PopoverWallet.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/Layout/Header/NavBar/WalletButton.tsxsrc/pages/Dashboard/components/AccountList/AccountTable.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
📚 Learning: 2025-08-25T23:32:13.876Z
Learnt from: premiumjibles
PR: shapeshift/web#10361
File: src/pages/Markets/components/CardWithSparkline.tsx:83-92
Timestamp: 2025-08-25T23:32:13.876Z
Learning: In shapeshift/web PR #10361, premiumjibles considered the nested button accessibility issue (ChartErrorFallback retry Button inside Card rendered as Button in CardWithSparkline.tsx) out of scope for the error boundaries feature PR, consistent with deferring minor a11y improvements to follow-up PRs.
Applied to files:
src/components/Layout/Header/NavBar/WalletButton.tsx
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/pages/Explore/ExploreCategory.tsx:231-238
Timestamp: 2025-08-08T14:59:40.422Z
Learning: In src/pages/Explore/ExploreCategory.tsx, for the PageHeader filter trigger, NeOMakinG considers changing a clickable Chakra Icon to IconButton too nitpicky for this PR and prefers to keep the current Icon-based trigger; such minor a11y/UI nitpicks should be deferred to a follow-up if needed.
Applied to files:
src/components/Layout/Header/NavBar/WalletButton.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for conditional values and simple transformations
Applied to files:
src/pages/Dashboard/components/AccountList/AccountTable.tsx
🧬 Code graph analysis (2)
src/components/Layout/Header/NavBar/WalletButton.tsx (7)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/localWalletSlice/selectors.ts (1)
selectWalletRdns(21-24)src/lib/mipd.ts (1)
useMipdProviders(7-8)src/components/Layout/Header/NavBar/WalletImage.tsx (1)
WalletImage(10-16)src/lib/vibrate.ts (1)
vibrate(6-16)src/components/MiddleEllipsis/MiddleEllipsis.tsx (1)
MiddleEllipsis(11-17)src/components/Text/Text.tsx (2)
RawText(15-17)Text(19-83)
src/pages/Dashboard/components/AccountList/AccountTable.tsx (3)
src/theme/theme.tsx (1)
breakpoints(37-44)src/state/slices/portfolioSlice/selectors.ts (1)
AccountRowData(726-736)src/lib/vibrate.ts (1)
vibrate(6-16)
🪛 dotenv-linter (3.3.0)
.env
[warning] 54-54: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_OPTIMISM key
(UnorderedKey)
.env.development
[warning] 10-10: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_QUICK_BUY key
(UnorderedKey)
.env.production
[warning] 6-6: [UnorderedKey] The VITE_FEATURE_NEW_WALLET_MANAGER key should go before the VITE_FEATURE_QUICK_BUY key
(UnorderedKey)
⏰ 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: Install and Cache
🔇 Additional comments (9)
src/components/Layout/Header/NavBar/WalletButton.tsx (1)
92-101: LGTM: MenuButton/Button split is correct.Using
MenuButton as={Button}for menu context and plainButtonotherwise matches Chakra patterns.src/pages/Dashboard/components/AccountList/AccountTable.tsx (8)
38-41: Props look good and minimal.forceCompactView and onRowClick are clear and scoped to the popover use case.
59-64: Compact/responsive derivations are clear and consistent.
110-111: Column visibility toggling via display is a clean approach for compact mode.Also applies to: 118-119, 139-140
147-148: Correct deps for columns memoization.Includes isCompactCols, stackTextAlign, textColor as needed.
182-182: Good: renderEmptyComponent depends on buttonWidth and handler.
187-193: Nice UX: close popover before navigation.onRowClick?.() before navigate avoids dangling state.
211-211: Headers toggle matches compact/forced-compact behavior.
43-43: Ensure an ErrorBoundary wraps this component in production paths.Per guidelines, components should be wrapped with an error boundary and use translated error toasts upstream.
Would you like me to add a scoped ErrorBoundary around AccountTable in the popover entry points?
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
Looks straighforward, not blocking for the sake of progression but I noticed it's a popover
As long as I remember from our meetings, it was supposed ot be a drawer, to be confirmed with product team but I think it is! Can we note that for a follow up?
|
@NeOMakinG oft 🤦♂️. Not sure why I got in my head it was a popover. I'll try and do a follow up asap |
Description
This implements part of our new wallet popover. The primary goal of this PR is to get the popover to feature parity (ish) with the mobile wallet page. We make sure it renders nicely, features work, feature flag is set up, prework is ready to add new features
Make sure to hide whitespace changes for this review
Developer notes
I spent quite a bit of time trying to get this component shared with the actual mobile wallet page but found that overall it deviated too much to be worth it. The mobile wallet dialog has a few different elements (like search, QR code scan, trade button) but most importantly it's state management is totally different.
In the mobile version we're managing state through URL's and history to better facilitate going back. For the popover that doesn't really make sense, we just want internal state. If we pulled out all the stuff that actually makes sense to share, all we end up with is a few shared action buttons. In the end it just seemed like a better idea to keep these decoupled.
Issue (if applicable)
closes #10285
Risk
Low risk, this is behind a feature flag. Any changes that aren't behind a feature flag have the goal of not changing anything and are fairly surgical and minimal.
Testing
Feature flag is
NewWalletManagerEngineering
Operations
Screenshots (if applicable)
https://jam.dev/c/4bf621d4-5f2e-457b-bcf5-d3e5b9b23c47
Summary by CodeRabbit
New Features
Refactor
Chores