feat: add defi, activity and net worth delta to mobile wallet (wallet popover P2) #10409
Conversation
📝 WalkthroughWalkthroughAdds a compact/mobile rendering mode across staking and earn components, introduces a WalletBalanceChange component and 24-hour change label, expands the wallet popover with DeFi and Activity tabs (TransactionActivity), and adjusts related header/dashboard tab styling and theming. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Popover as PopoverWallet (UI)
participant Tabs
participant DeFi as DeFiEarn / PositionTable
participant Staking as StakingPositionsByProvider
participant Actions as DefiAction
User->>Popover: open wallet popover
Popover->>Tabs: render tabs (My Crypto, Watchlist, DeFi, Activity)
User->>Tabs: select "DeFi"
Tabs->>DeFi: mount DeFiEarn (forceCompactView if mobile)
DeFi->>PositionTable: render with forceCompactView
PositionTable->>Staking: render PositionDetails / StakingPositionsByProvider (compact)
User->>Staking: click row (compact mode)
Staking->>Actions: trigger DefiAction.Overview (open overview)
sequenceDiagram
participant UI
participant WalletComp as WalletBalanceChange
participant Store as Redux selectors / hooks
participant Tooltip
UI->>WalletComp: mount
WalletComp->>Store: select portfolio balance, loading, chart data
Store-->>WalletComp: totals, loading state, history
WalletComp->>WalletComp: calculate percentChange, fiatChange
alt change available
WalletComp->>Tooltip: render "24-hour change" with value (green/red)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Poem
✨ 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. Comment |
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
from
7694361 to
6c1f6d7
Compare
premiumjibles
force-pushed
the
10285_mobile_wallet
branch
from
e2406bc to
6d41444
Compare
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
from
6c1f6d7 to
da84d45
Compare
premiumjibles
changed the title
src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx
Show resolved
Hide resolved
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
6 times, most recently
from
0230304 to
be48783
Compare
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
from
be48783 to
96ab7f8
Compare
premiumjibles
force-pushed
the
10285_mobile_wallet
branch
from
6d41444 to
4f28147
Compare
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
2 times, most recently
from
44c49ca to
9ac4bd1
Compare
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
Looks like working very well, the defi tab is fine as it is with your best efforts adaptation, quite happy with this one minus some nitpick comments/preferences
src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx
Show resolved
Hide resolved
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
from
9ac4bd1 to
1cbbb42
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/StakingVaults/PositionTable.tsx (2)
159-169: IconButton is non-functional (no onClick) and not keyboard-activatableThe expander IconButton has no handler. Add onClick (and aria-expanded) to make it operable and accessible.
<Flex justifyContent='flex-end' width='full'> <IconButton variant='ghost' size='md' aria-label={translate('common.table.expandRow')} isActive={row.isExpanded} + onClick={() => row.toggleRowExpanded()} + aria-expanded={row.isExpanded} icon={row.isExpanded ? <ArrowUpIcon /> : <ArrowDownIcon />} /> </Flex>
183-186: Case-insensitive address search compares against unnormalized inputYou build search = filterValue.trim().toLowerCase() but compare to filterValue, breaking uppercase address searches. Also guard potential undefined assetReference.
- if (isEthAddress(filterValue)) { - return rows.filter( - row => fromAssetId(row.assetId).assetReference.toLowerCase() === filterValue, - ) - } + if (isEthAddress(filterValue)) { + return rows.filter( + row => fromAssetId(row.assetId).assetReference?.toLowerCase() === search, + ) + }
♻️ Duplicate comments (4)
src/assets/translations/en/main.json (1)
643-644: Keep common.activity as a backward-compatible alias to avoid ripple effects.Prior reviews noted existing translations for
common.activity. Keeping it (in addition tonavBar.activity) reduces i18n churn and avoids regressions where old keys may still be referenced.Apply:
"common": { + "activity": "Activity",Also verify no remaining usages of the old key:
#!/bin/bash # Find any remaining references to common.activity rg -n "common\\.activity" -Ssrc/pages/History/History.tsx (1)
93-94: Activity tab label update aligns with i18n changes.Consistent with
navBar.activityshift across the app.src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
76-85: Use Number.isFinite and consider hiding neutral (0%) changes
- Prefer Number.isFinite to avoid coercion.
- Design question: do we want to hide 0% deltas (consistent with PriceChangeTag behavior)? If yes, gate on non-zero.
Apply:
- {isFinite(percentChange) && ( + {Number.isFinite(percentChange) && percentChange !== 0 && ( <Skeleton mt={2} isLoaded={isChangeLoaded}> <TooltipWithTouch label={translate('defi.walletBalanceChange24Hr')}> <Flex gap={1} fontSize={changeFontSize} color={color} fontWeight='medium'> <Amount.Fiat value={fiatChange} /> ( - {formattedPercentChange}) + <Amount.Percent value={percentChange} />) </Flex> </TooltipWithTouch> </Skeleton> )}src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx (1)
74-80: Keeping JSX constants outside component is fineThe ESLint disable is justified to avoid recreating nodes; the pattern matches prior decisions to keep perf simple without over-memoizing.
Also applies to: 257-266
🧹 Nitpick comments (11)
src/components/TransactionHistoryRows/TransactionRow.tsx (1)
93-97: Make toggleOpen stable and avoid stale state with a functional update.Use a functional state update and drop
isOpenfrom deps to prevent stale captures and unnecessary re-creations.Apply:
- const toggleOpen = useCallback( - () => (disableCollapse ? undefined : setIsOpen(!isOpen)), - [disableCollapse, isOpen], - ) + const toggleOpen = useCallback(() => { + if (disableCollapse) return + setIsOpen(prev => !prev) + }, [disableCollapse])src/components/WalletBalanceChange/WalletBalanceChange.tsx (3)
55-60: Delegate percent formatting to Amount.PercentAvoid local string formatting and keep visuals consistent with the rest of the app.
Apply:
- const formattedPercentChange = useMemo(() => { - return `${percentChange.toFixed(1)}%` - }, [percentChange]) + // Use <Amount.Percent /> in render; no local string formatting needed
55-56: Neutral color for exactly 0%Green for 0 can be misleading. Consider neutral on zero.
Apply:
- const color = percentChange >= 0 ? positiveColor : negativeColor + const color = percentChange > 0 ? positiveColor : percentChange < 0 ? negativeColor : 'text.subtle'
61-67: Error boundary coveragePer project guidelines, ensure this component is rendered within an error boundary (e.g., where used in PopoverWallet/Dashboard). If not already covered, wrap at the parent level with a user-friendly fallback and error toast.
src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx (1)
238-240: Placement is correct, but confirm 0% policyIf we decide to hide 0% deltas in WalletBalanceChange (see component review), mobile header will follow suit. Please confirm the intended UX.
src/components/Layout/Header/NavBar/PopoverWallet.tsx (2)
102-111: Avoid passing an empty translation key to WalletBalanceChange (label='').Passing an empty string can trigger i18n warnings and makes intent unclear. Prefer an explicit API to hide the label.
Apply this diff here:
- <WalletBalanceChange - timeframe={HistoryTimeframe.DAY} - balanceFontSize='4xl' - label='' - showErroredAccounts={false} - alignItems='center' - justifyContent='center' - /> + <WalletBalanceChange + timeframe={HistoryTimeframe.DAY} + balanceFontSize='4xl' + showLabel={false} + showErroredAccounts={false} + alignItems='center' + justifyContent='center' + />And extend WalletBalanceChange to support
showLabel:export const WalletBalanceChange = memo( ({ timeframe, - label = 'defi.walletBalance', + label = 'defi.walletBalance', + showLabel = true, balanceFontSize = '4xl', changeFontSize = 'md', showErroredAccounts = true, ...flexProps }: WalletBalanceChangeProps) => { @@ - <Heading as='div' color='text.subtle'> - <Skeleton isLoaded={isLoaded}> - <Text translation={label} /> - </Skeleton> - </Heading> + {showLabel && ( + <Heading as='div' color='text.subtle'> + <Skeleton isLoaded={isLoaded}> + <Text translation={label} /> + </Skeleton> + </Heading> + )}I can wire up the prop type update if helpful.
128-159: Wrap tab panels in an ErrorBoundary to prevent popover-wide crashes.These panels render complex async/selector-driven trees (DeFiEarn, TransactionActivity). Per guidelines, wrap in an error boundary with a user-friendly fallback.
Apply this diff:
+import { ErrorBoundary } from '@/components/ErrorBoundary/ErrorBoundary' +import { ErrorFallback } from '@/components/ErrorBoundary/ErrorFallback' @@ - <TabPanels flex='1' overflow='auto' maxHeight={'35vh'} className='scroll-container'> - <TabPanel p={0} pt={2}> - <AccountTable forceCompactView onRowClick={onClose} /> - </TabPanel> - <TabPanel p={0} pt={2}> - <WatchlistTable forceCompactView onRowClick={onClose} hideExploreMore /> - </TabPanel> - <TabPanel p={0} pt={2}> - <DeFiEarn forceCompactView /> - </TabPanel> - <TabPanel p={0} pt={2}> - <TransactionActivity forceCompactView /> - </TabPanel> - </TabPanels> + <ErrorBoundary FallbackComponent={ErrorFallback}> + <TabPanels flex='1' overflow='auto' maxHeight={'35vh'} className='scroll-container'> + <TabPanel p={0} pt={2}> + <AccountTable forceCompactView onRowClick={onClose} /> + </TabPanel> + <TabPanel p={0} pt={2}> + <WatchlistTable forceCompactView onRowClick={onClose} hideExploreMore /> + </TabPanel> + <TabPanel p={0} pt={2}> + <DeFiEarn forceCompactView /> + </TabPanel> + <TabPanel p={0} pt={2}> + <TransactionActivity forceCompactView /> + </TabPanel> + </TabPanels> + </ErrorBoundary>If the project uses a different boundary component, swap the import accordingly.
src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx (1)
25-26: Defensive: guard against missing staking ids.At runtime, opportunities.staking could be absent/empty; passing undefined may break child prop types. Cheap guard improves resilience.
Apply this diff:
- <StakingPositionsByProvider ids={opportunities.staking} forceCompactView={forceCompactView} /> + <StakingPositionsByProvider + ids={opportunities.staking ?? []} + forceCompactView={forceCompactView} + />src/components/StakingVaults/DeFiEarn.tsx (1)
75-79: Forward positionTableProps to PositionTable.Consumers can customize the table (e.g., initialState). It’s currently unused.
Apply this diff:
- <PositionTable - chainId={selectedChainId} - searchQuery={searchQuery} - forceCompactView={forceCompactView} - /> + <PositionTable + {...positionTableProps} + chainId={selectedChainId} + searchQuery={searchQuery} + forceCompactView={forceCompactView} + />src/components/StakingVaults/PositionTable.tsx (2)
101-109: Avoid O(n) lookup andanyhere; use row.indexUse row.index rather than flatRows.indexOf(row), and drop the any.
-Cell: ({ row, flatRows }: { row: RowProps; flatRows: any }) => ( - <RawText>{flatRows.indexOf(row) + 1}</RawText> -), +Cell: ({ row }: { row: RowProps }) => <RawText>{row.index + 1}</RawText>,
113-117: O(n) per-cell asset lookupassets.find(...) runs per row render. Precompute a map and read in O(1).
-const asset = assets.find(a => a.assetId === row.original.assetId) -return <AssetCell assetId={row.original.assetId} subText={asset?.symbol} /> +const asset = assetById.get(row.original.assetId) +return <AssetCell assetId={row.original.assetId} subText={asset?.symbol} />Add once above (outside this hunk):
const assetById = useMemo(() => new Map(assets.map(a => [a.assetId, a])), [assets])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
src/assets/translations/en/main.json(3 hunks)src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx(1 hunks)src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx(6 hunks)src/components/Layout/Header/NavBar/PopoverWallet.tsx(4 hunks)src/components/StakingVaults/DeFiEarn.tsx(2 hunks)src/components/StakingVaults/PositionTable.tsx(8 hunks)src/components/Tabs/Tabs.theme.ts(1 hunks)src/components/TransactionHistory/TransactionActivity.tsx(1 hunks)src/components/TransactionHistoryRows/TransactionRow.tsx(1 hunks)src/components/WalletBalanceChange/WalletBalanceChange.tsx(1 hunks)src/pages/Dashboard/Dashboard.tsx(2 hunks)src/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsx(1 hunks)src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx(5 hunks)src/pages/History/History.tsx(1 hunks)src/pages/RFOX/components/History/History.tsx(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/TransactionHistoryRows/TransactionRow.tsxsrc/pages/RFOX/components/History/History.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsxsrc/pages/History/History.tsxsrc/components/TransactionHistory/TransactionActivity.tsxsrc/pages/Dashboard/Dashboard.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Tabs/Tabs.theme.tssrc/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/EarnDashboard/components/PositionDetails/PositionDetails.tsxsrc/components/StakingVaults/DeFiEarn.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/TransactionHistoryRows/TransactionRow.tsxsrc/pages/RFOX/components/History/History.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsxsrc/pages/History/History.tsxsrc/components/TransactionHistory/TransactionActivity.tsxsrc/pages/Dashboard/Dashboard.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/EarnDashboard/components/PositionDetails/PositionDetails.tsxsrc/components/StakingVaults/DeFiEarn.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/TransactionHistoryRows/TransactionRow.tsxsrc/pages/RFOX/components/History/History.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsxsrc/pages/History/History.tsxsrc/components/TransactionHistory/TransactionActivity.tsxsrc/pages/Dashboard/Dashboard.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Tabs/Tabs.theme.tssrc/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/EarnDashboard/components/PositionDetails/PositionDetails.tsxsrc/assets/translations/en/main.jsonsrc/components/StakingVaults/DeFiEarn.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/TransactionHistoryRows/TransactionRow.tsxsrc/pages/RFOX/components/History/History.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsxsrc/pages/History/History.tsxsrc/components/TransactionHistory/TransactionActivity.tsxsrc/pages/Dashboard/Dashboard.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/EarnDashboard/components/PositionDetails/PositionDetails.tsxsrc/components/StakingVaults/DeFiEarn.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/TransactionHistoryRows/TransactionRow.tsxsrc/pages/RFOX/components/History/History.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsxsrc/pages/History/History.tsxsrc/components/TransactionHistory/TransactionActivity.tsxsrc/pages/Dashboard/Dashboard.tsxsrc/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Tabs/Tabs.theme.tssrc/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/EarnDashboard/components/PositionDetails/PositionDetails.tsxsrc/components/StakingVaults/DeFiEarn.tsx
🧠 Learnings (8)
📓 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-22T12:59:01.210Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/components/TransactionHistory/TransactionActivity.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsx
📚 Learning: 2025-09-04T17:26:19.399Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/PriceChangeTag/PriceChangeTag.tsx:25-36
Timestamp: 2025-09-04T17:26:19.399Z
Learning: In the shapeshift/web project, NeOMakinG prefers to hide 0% price changes in the PriceChangeTag component rather than displaying "0.00%", so the falsy check `if (!changePercent24Hr) return null` is intentional design behavior.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.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/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsxsrc/components/StakingVaults/PositionTable.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Applied to files:
src/components/StakingVaults/PositionTable.tsx
📚 Learning: 2025-09-04T12:16:47.728Z
Learnt from: gomesalexandre
PR: shapeshift/web#10430
File: src/components/Layout/Header/NavBar/PopoverWallet.tsx:72-94
Timestamp: 2025-09-04T12:16:47.728Z
Learning: gomesalexandre declined to add error boundaries to the PopoverWallet component in src/components/Layout/Header/NavBar/PopoverWallet.tsx, stating he didn't touch this component and preferring not to expand the scope of the PR with error boundary additions.
Applied to files:
src/components/Layout/Header/NavBar/PopoverWallet.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} : EXTRACT complex logic into custom hooks
Applied to files:
src/components/Layout/Header/NavBar/PopoverWallet.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} : BREAK DOWN large components into smaller, reusable pieces
Applied to files:
src/components/StakingVaults/DeFiEarn.tsx
🧬 Code graph analysis (8)
src/components/TransactionHistory/TransactionActivity.tsx (3)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/txHistorySlice/selectors.ts (1)
selectTxIdsBasedOnSearchTermAndFilters(235-281)src/components/TransactionHistory/TransactionHistoryList.tsx (1)
TransactionHistoryList(19-68)
src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx (1)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
WalletBalanceChange(28-89)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (5)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/portfolioSlice/selectors.ts (2)
selectPortfolioTotalUserCurrencyBalance(190-199)selectIsPortfolioLoading(332-338)src/hooks/useBalanceChartData/useBalanceChartData.ts (1)
useBalanceChartData(324-439)src/components/Text/Text.tsx (1)
Text(19-83)src/components/TooltipWithTouch.tsx (1)
TooltipWithTouch(7-73)
src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx (3)
src/theme/theme.tsx (1)
breakpoints(37-44)src/components/StakingVaults/PositionTable.tsx (1)
RowProps(34-34)src/components/ReactTable/ReactTable.tsx (1)
ReactTable(132-297)
src/components/StakingVaults/PositionTable.tsx (6)
src/state/slices/opportunitiesSlice/types.ts (1)
AggregatedOpportunitiesByAssetIdReturn(240-248)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/assetsSlice/selectors.ts (1)
selectAssetsSortedByMarketCap(57-64)src/state/slices/opportunitiesSlice/selectors/combined.ts (1)
selectIsAnyOpportunitiesApiQueryPending(238-239)src/theme/theme.tsx (1)
breakpoints(37-44)src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx (1)
PositionDetails(12-28)
src/components/Layout/Header/NavBar/PopoverWallet.tsx (3)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
WalletBalanceChange(28-89)src/components/StakingVaults/DeFiEarn.tsx (1)
DeFiEarn(31-83)src/components/TransactionHistory/TransactionActivity.tsx (1)
TransactionActivity(13-37)
src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx (3)
src/state/slices/opportunitiesSlice/types.ts (1)
AggregatedOpportunitiesByAssetIdReturn(240-248)src/theme/theme.tsx (1)
breakpoints(37-44)src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx (1)
StakingPositionsByProvider(89-364)
src/components/StakingVaults/DeFiEarn.tsx (2)
src/components/ChainDropdown/ChainDropdown.tsx (1)
ChainDropdown(40-107)src/components/StakingVaults/PositionTable.tsx (1)
PositionTable(47-255)
⏰ 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/RFOX/components/History/History.tsx (1)
67-67: Translation key switch looks good.
navBar.activityaligns with the new i18n keys introduced in this PR.src/pages/Dashboard/Dashboard.tsx (1)
95-97: LGTM on relying on themed soft-rounded tabs.Removing custom tab styles in favor of
Tabs.theme.tskeeps styling consistent across the app.src/components/Tabs/Tabs.theme.ts (1)
89-89: Mode-aware selected/hover states look correct.The dynamic bg/fg for
_selectedand hover are consistent with the app’s dark/light theming.Also applies to: 95-99
src/components/TransactionHistory/TransactionActivity.tsx (1)
14-21: Remove unnecessarytypesdefault;nullis already coerced to[]
The selectorselectTransactionTypesParamFromFilterusesfilter?.types ?? [], so passingtypes: nullis safe and won’t cause.lengtherrors. No change needed.Likely an incorrect or invalid review comment.
src/assets/translations/en/main.json (2)
548-549: New key for 24-hour change looks good.
common.walletBalanceChange24Hradded and is consistent with the feature.
2278-2279: Added transaction details key looks correct.
transactionHistory.transactionDetailsmatches UI additions.src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx (1)
61-72: Mobile daily-change block: solid integrationUsing WalletBalanceChange with HistoryTimeframe.DAY, hidden label, and no errored accounts fits the mobile header well.
src/components/Layout/Header/NavBar/PopoverWallet.tsx (2)
14-24: Imports and memoization look consistent with new usage.New deps (HistoryTimeframe, DeFiEarn, TransactionActivity, WalletBalanceChange) are correctly referenced and memo usage is appropriate.
152-157: Compact DeFi and Activity panels fit the mobile popover scope.Forwarding forceCompactView through both panels is coherent with the compact mode strategy.
src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx (2)
12-21: Compact-mode calculation is sound and SSR-safe.useMediaQuery with { ssr: false } avoids hydration mismatches; combining with forceCompactView is correct.
1-10: Type and breakpoint wiring LGTM.Local prop type extension and theme breakpoint import align with related table/provider components.
src/components/StakingVaults/DeFiEarn.tsx (2)
20-24: Prop addition LGTM.forceCompactView is optional and well-scoped to drive header/table compaction.
49-73: Header-only render gating is correct for compact mode.Hiding ChainDropdown and GlobalFilter in compact is aligned with popover constraints.
src/components/StakingVaults/PositionTable.tsx (2)
216-220: Verify PositionDetails prop shapeYou spread {...original}. Ensure PositionDetailsProps accepts extra props; otherwise pass only required props.
Minimal safe alternative:
-<PositionDetails key={original.assetId} {...original} forceCompactView={forceCompactView} /> +<PositionDetails + key={original.assetId} + opportunities={original.opportunities} + forceCompactView={forceCompactView} />
252-253: Header toggle in compact mode — LGTM (after isCompactCols fix)displayHeaders={!isCompactCols} aligns with the compact column behavior.
src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx
Show resolved
Hide resolved
src/pages/Dashboard/components/DashboardHeader/DashboardHeader.tsx
Outdated
Show resolved
Hide resolved
|
@NeOMakinG actioned feedback here 831c146 |
premiumjibles
force-pushed
the
10285_mobile_wallet_p2
branch
from
831c146 to
da14e38
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (2)
60-68: Use Number.isFinite and drop BigNumber importAvoid importing bn just to check finiteness; use the built-in.
-import { bn } from '@/lib/bignumber/bignumber' @@ - {bn(percentChange).isFinite() && ( + {Number.isFinite(percentChange) && (Also applies to: 10-10
19-22: Tooltip copy should not be hard-coded to 24HrEither lock timeframe to DAY or make the label configurable. Suggest making it configurable via prop to keep the component generic.
type WalletBalanceChangeProps = { timeframe: HistoryTimeframe showErroredAccounts?: boolean + tooltipLabel?: string } & FlexProps @@ -export const WalletBalanceChange = memo( - ({ timeframe, showErroredAccounts = true, ...flexProps }: WalletBalanceChangeProps) => { +export const WalletBalanceChange = memo( + ({ timeframe, showErroredAccounts = true, tooltipLabel, ...flexProps }: WalletBalanceChangeProps) => { @@ - <TooltipWithTouch label={translate('defi.walletBalanceChange24Hr')}> + <TooltipWithTouch label={translate(tooltipLabel ?? 'defi.walletBalanceChange24Hr')}>Follow-up: callers can pass a timeframe-appropriate i18n key; happy to wire a switch if preferred.
Also applies to: 25-25, 62-62
🧹 Nitpick comments (6)
src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx (4)
61-67: Remove snarky comment and keep the eslint-disable rationale conciseThe "react-memo you're drunk" comment is unprofessional. Keep the disable but explain why succinctly.
- // react-memo you're drunk, this is outside of component scope - // eslint-disable-next-line react-memo/require-usememo + // eslint-disable-next-line react-memo/require-usememo -- false positive: static JSX at module scope
69-75: Same here: clean up the comment toneMirror the concise rationale for this block too.
- // react-memo you're drunk, this is outside of component scope - // eslint-disable-next-line react-memo/require-usememo + // eslint-disable-next-line react-memo/require-usememo -- false positive: static JSX at module scope
221-236: Optional: prefer small memoized components over static element constantsStatic element objects at module scope can make future prop/theming changes harder. Consider memoized leaf components instead (MobileNetWorth/DesktopNetWorth) and render them as JSX.
-const mobileNetWorth = ( +const MobileNetWorth = memo(() => ( <Flex alignItems='center' flexDir={containerInnerFlexDir} gap={4} gridColumn={profileGridColumn}> <WalletBalanceChange timeframe={HistoryTimeframe.DAY} showErroredAccounts={false} /> </Flex> -) +)) -const desktopNetWorth = ( +const DesktopNetWorth = memo(() => ( <Flex alignItems='center' flexDir={containerInnerFlexDir} gap={4} gridColumn={profileGridColumn}> <WalletBalance balanceFontSize={balanceFontSize} /> </Flex> -) +)) @@ - {mobileNetWorth} + <MobileNetWorth /> @@ - {desktopNetWorth} + <DesktopNetWorth />Note: Fine to defer to a follow-up PR per team preference. [learned preference acknowledged]
Also applies to: 252-262
105-106: Consider wrapping with an ErrorBoundary at the page/drawer levelPer guidelines, components should be wrapped in error boundaries with a user-friendly fallback and logging. If already handled higher up, ignore.
src/components/WalletBalanceChange/WalletBalanceChange.tsx (2)
46-48: Avoid showing “-0.0%” due to floating-point roundingNormalize negative zero before formatting.
- const formattedPercentChange = useMemo(() => { - return `${percentChange.toFixed(1)}%` - }, [percentChange]) + const formattedPercentChange = useMemo(() => { + const normalized = Object.is(percentChange, -0) ? 0 : percentChange + return `${normalized.toFixed(1)}%` + }, [percentChange])
24-31: Optional: add an ErrorBoundary at usage sitesThis component reads from multiple selectors/hooks; wrap it with an error boundary (and useErrorToast on failure) where it’s used (e.g., Dashboard header, wallet popover) to align with guidelines.
Also applies to: 50-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/assets/translations/en/main.json(2 hunks)src/components/Layout/Header/NavBar/PopoverWallet.tsx(4 hunks)src/components/WalletBalanceChange/WalletBalanceChange.tsx(1 hunks)src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Layout/Header/NavBar/PopoverWallet.tsx
- src/assets/translations/en/main.json
🧰 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/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.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/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.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/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.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/components/DashboardHeader/DashboardHeaderTop.tsxsrc/components/WalletBalanceChange/WalletBalanceChange.tsx
🧠 Learnings (3)
📓 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-22T12:59:01.210Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsx
📚 Learning: 2025-09-04T17:26:19.447Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/PriceChangeTag/PriceChangeTag.tsx:25-36
Timestamp: 2025-09-04T17:26:19.447Z
Learning: In the shapeshift/web project, NeOMakinG prefers to hide 0% price changes in the PriceChangeTag component rather than displaying "0.00%", so the falsy check `if (!changePercent24Hr) return null` is intentional design behavior.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsx
🧬 Code graph analysis (2)
src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx (1)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
WalletBalanceChange(24-72)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (3)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/portfolioSlice/selectors.ts (2)
selectPortfolioTotalUserCurrencyBalance(190-199)selectIsPortfolioLoading(332-338)src/hooks/useBalanceChartData/useBalanceChartData.ts (1)
useBalanceChartData(324-439)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/StakingVaults/PositionTable.tsx (1)
47-51: Defaulting forceCompactView fixes the undefined boolean pitfallThanks for adding forceCompactView = false; this prevents undefined leaking into isCompactCols.
🧹 Nitpick comments (2)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
59-67: Guard with Number.isFinite and compute display-only values lazilyMinor polish: avoid constructing color/formatting when the change isn’t renderable. Also prefer Number.isFinite over bn() for a plain number check.
- {bn(percentChange).isFinite() && ( - <Skeleton mt={2} isLoaded={isChangeLoaded}> - <TooltipWithTouch label={translate('defi.walletBalanceChange24Hr')}> - <Flex gap={1} fontSize='md' color={color} fontWeight='medium'> - <Amount.Fiat value={fiatChange} /> ({formattedPercentChange}) - </Flex> - </TooltipWithTouch> - </Skeleton> - )} + {Number.isFinite(percentChange) && ( + <Skeleton mt={2} isLoaded={isChangeLoaded}> + <TooltipWithTouch label={translate('defi.walletBalanceChange24Hr')}> + {(() => { + const color = percentChange >= 0 ? positiveColor : negativeColor + const formattedPercentChange = `${percentChange.toFixed(1)}%` + return ( + <Flex gap={1} fontSize='md' color={color} fontWeight='medium'> + <Amount.Fiat value={fiatChange} /> ({formattedPercentChange}) + </Flex> + ) + })()} + </TooltipWithTouch> + </Skeleton> + )}src/components/StakingVaults/PositionTable.tsx (1)
113-117: Avoid O(n) asset lookup per row by pre-indexingassets.find() in a cell runs per-row render. Precompute a Map once for O(1) lookups.
import { AssetCell } from '@/components/StakingVaults/Cells' ... - const columns: Column<AggregatedOpportunitiesByAssetIdReturn>[] = useMemo( + const assetsById = useMemo(() => { + const map = new Map<string, { symbol?: string }>() + for (const a of assets) map.set(a.assetId, a) + return map + }, [assets]) + + const columns: Column<AggregatedOpportunitiesByAssetIdReturn>[] = useMemo( () => [ ... { Header: translate('defi.asset'), accessor: 'assetId', - Cell: ({ row }: { row: RowProps }) => { - const asset = assets.find(a => a.assetId === row.original.assetId) - return <AssetCell assetId={row.original.assetId} subText={asset?.symbol} /> - }, + Cell: ({ row }: { row: RowProps }) => ( + <AssetCell + assetId={row.original.assetId} + subText={assetsById.get(row.original.assetId)?.symbol} + /> + ), disableSortBy: true, }, ... - [translate, isCompactCols, assets], + [translate, isCompactCols, assetsById], )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/assets/translations/en/main.json(2 hunks)src/components/Layout/Header/NavBar/PopoverWallet.tsx(4 hunks)src/components/StakingVaults/PositionTable.tsx(8 hunks)src/components/WalletBalanceChange/WalletBalanceChange.tsx(1 hunks)src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/assets/translations/en/main.json
- src/pages/Dashboard/components/DashboardHeader/DashboardHeaderTop.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/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/StakingVaults/PositionTable.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/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/StakingVaults/PositionTable.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/StakingVaults/PositionTable.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/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/StakingVaults/PositionTable.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/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/Layout/Header/NavBar/PopoverWallet.tsxsrc/components/StakingVaults/PositionTable.tsx
🧠 Learnings (12)
📓 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.
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsx
📚 Learning: 2025-09-04T17:26:19.447Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/PriceChangeTag/PriceChangeTag.tsx:25-36
Timestamp: 2025-09-04T17:26:19.447Z
Learning: In the shapeshift/web project, NeOMakinG prefers to hide 0% price changes in the PriceChangeTag component rather than displaying "0.00%", so the falsy check `if (!changePercent24Hr) return null` is intentional design behavior.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsx
📚 Learning: 2025-09-08T22:00:48.005Z
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/StakingVaults/PositionTable.tsx
📚 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/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/StakingVaults/PositionTable.tsx
📚 Learning: 2025-08-27T09:47:06.275Z
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.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.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/WalletBalanceChange/WalletBalanceChange.tsxsrc/components/StakingVaults/PositionTable.tsx
📚 Learning: 2025-08-08T15:00:22.321Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14
Timestamp: 2025-08-08T15:00:22.321Z
Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.
Applied to files:
src/components/WalletBalanceChange/WalletBalanceChange.tsx
📚 Learning: 2025-09-04T12:16:47.748Z
Learnt from: gomesalexandre
PR: shapeshift/web#10430
File: src/components/Layout/Header/NavBar/PopoverWallet.tsx:72-94
Timestamp: 2025-09-04T12:16:47.748Z
Learning: gomesalexandre declined to add error boundaries to the PopoverWallet component in src/components/Layout/Header/NavBar/PopoverWallet.tsx, stating he didn't touch this component and preferring not to expand the scope of the PR with error boundary additions.
Applied to files:
src/components/Layout/Header/NavBar/PopoverWallet.tsx
📚 Learning: 2025-07-24T21:05:13.642Z
Learnt from: 0xApotheosis
PR: shapeshift/web#10073
File: src/components/Layout/Header/ActionCenter/components/Details/ClaimDetails.tsx:10-11
Timestamp: 2025-07-24T21:05:13.642Z
Learning: In the ShapeShift web repository, translation workflow follows a two-step process: 1) First PR adds only English translations to src/assets/translations/en/main.json, 2) Globalization team handles follow-up PRs to add keys to remaining language files (de, es, fr, id, ja, ko, pt, ru, tr, uk, zh). Don't suggest verifying all locale files simultaneously during initial feature PRs.
Applied to files:
src/components/Layout/Header/NavBar/PopoverWallet.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} : EXTRACT complex logic into custom hooks
Applied to files:
src/components/Layout/Header/NavBar/PopoverWallet.tsx
🧬 Code graph analysis (3)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (4)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/portfolioSlice/selectors.ts (2)
selectPortfolioTotalUserCurrencyBalance(190-199)selectIsPortfolioLoading(332-338)src/hooks/useBalanceChartData/useBalanceChartData.ts (1)
useBalanceChartData(324-439)src/components/TooltipWithTouch.tsx (1)
TooltipWithTouch(7-73)
src/components/Layout/Header/NavBar/PopoverWallet.tsx (3)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
WalletBalanceChange(23-71)src/components/StakingVaults/DeFiEarn.tsx (1)
DeFiEarn(31-83)src/components/TransactionHistory/TransactionActivity.tsx (1)
TransactionActivity(13-37)
src/components/StakingVaults/PositionTable.tsx (5)
src/state/slices/opportunitiesSlice/types.ts (1)
AggregatedOpportunitiesByAssetIdReturn(240-248)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/assetsSlice/selectors.ts (1)
selectAssetsSortedByMarketCap(57-64)src/theme/theme.tsx (1)
breakpoints(37-44)src/components/EarnDashboard/components/PositionDetails/PositionDetails.tsx (1)
PositionDetails(12-28)
⏰ 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 (7)
src/components/WalletBalanceChange/WalletBalanceChange.tsx (1)
61-61: Translation key path is correct
The keydefi.walletBalanceChange24Hrexists underdefiinsrc/assets/translations/en/main.json; no changes required.src/components/Layout/Header/NavBar/PopoverWallet.tsx (4)
15-15: Memo import usage LGTMUsing memo on leaf components here is fine and low-risk.
101-103: Good reuse: centralizing balance UIReplacing the bespoke net-worth block with WalletBalanceChange (errored accounts hidden) keeps consistency across surfaces.
144-149: Tabs content wiring LGTMforceCompactView usage for DeFiEarn and TransactionActivity aligns with the popover constraints.
132-136: i18n keys verified Confirmed thatdashboard.portfolio.myCrypto,watchlist.title,navBar.defi, andcommon.activityexist insrc/assets/translations/en/main.json.src/components/StakingVaults/PositionTable.tsx (2)
57-60: Responsive compact logic LGTMuseMediaQuery + forceCompactView cleanly drives header/column visibility.
216-220: Propagating compact view to details LGTMPassing forceCompactView through keeps subcomponents consistent.
Description
This is part two of the mobile wallet popover PR. Here we add the new defi and activity tabs, as well as a wallet balance 24hr delta as per the designs.
Note that defi tab was not really designed for this small view, so i've done a best effort approach here for what seems reasonable that we can iterate on
Issue (if applicable)
closes #10285
Risk
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/39ea3737-3c93-4fbe-971a-2057d8910e5f
Summary by CodeRabbit
New Features
UI/Style
Translations