feat: add pull down to refresh portfolio and tx history data #11314
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a mobile-specific pull-to-refresh feature allowing users to drag down from the top to reload account balances, portfolio data, and transaction history. Two new reusable components (PullToRefreshWrapper and PullToRefreshList) are created and integrated into multiple pages. A new npm dependency (use-pull-to-refresh) is added. Portfolio, AssetAccounts, and TransactionHistoryContent components are updated with mobile-aware rendering and refresh handlers. Changes
Sequence DiagramsequenceDiagram
actor User
participant Mobile as Mobile UI<br/>(PullToRefreshList)
participant GestureDetector as Gesture Detector<br/>(use-pull-to-refresh)
participant StateManager as State Manager<br/>(hooks)
participant API as API Layer<br/>(Redux queries)
participant Renderer as Visual Feedback<br/>(Icon/Spinner)
User->>GestureDetector: Pull down gesture
GestureDetector->>StateManager: Update pullProgress (y position)
StateManager->>Renderer: Render pull indicator<br/>with opacity based on progress
alt Dragged Beyond Threshold (>200px)
GestureDetector->>StateManager: Trigger onRefresh
StateManager->>StateManager: Set isRefreshing = true
StateManager->>Renderer: Show spinner,<br/>display "refreshing" text
par Parallel API Calls
StateManager->>API: Reset portfolio state
StateManager->>API: Reset tx history state
StateManager->>API: Refetch getAccount (all accounts)
StateManager->>API: Conditionally refetch<br/>getAllTxHistory<br/>(if LazyTxHistory disabled)
and
API-->>StateManager: Fetch responses received
end
StateManager->>StateManager: Set isRefreshing = false
StateManager->>Renderer: Show refresh icon,<br/>display "pull to refresh" text
Renderer->>User: Content updated with fresh data
else Drag Distance < Threshold
GestureDetector->>StateManager: Reset animation (y → 0)
StateManager->>Renderer: Collapse indicator
Renderer->>User: Return to default state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/PullToRefresh/PullToRefreshWrapper.tsx (2)
156-203: The async function pattern doesn't properly track refresh completion.
handleTouchEndis markedasyncand usesawait onRefresh(), butonRefreshdoesn't return a Promise - it just dispatches Redux actions synchronously. Thetry/finallyblock withsetTimeoutwill execute immediately after the dispatches, not after data is actually fetched.However, looking at lines 63-68, the visual state is correctly reset when
isFetchingtransitions fromtruetofalse, so the UX should still work correctly. ThesetTimeoutin the finally block acts as a fallback.This is acceptable given the actual reset happens via the
useEffectwatchingisFetching, but removing the misleadingasync/awaitpattern would improve clarity:-const handleTouchEnd = async () => { +const handleTouchEnd = () => { // ... validation logic ... if (shouldRefresh) { setShowIndicator(true) setPullProgress(1) - - try { - await onRefresh() - } finally { - setTimeout(() => { - setPullProgress(0) - setTimeout(() => setShowIndicator(false), 300) - }, 300) - } + onRefresh() + // Visual state reset is handled by useEffect watching isFetching } else { // ... non-refresh cleanup ... } }
206-214: Event listener cleanup should match registration options.Event listeners are registered with
{ passive: true }but removed without this option. While browsers typically handle this correctly, it's best practice to use matching options:return () => { - document.removeEventListener('touchstart', handleTouchStart) - document.removeEventListener('touchmove', handleTouchMove) - document.removeEventListener('touchend', handleTouchEnd) + document.removeEventListener('touchstart', handleTouchStart, { passive: true } as EventListenerOptions) + document.removeEventListener('touchmove', handleTouchMove, { passive: true } as EventListenerOptions) + document.removeEventListener('touchend', handleTouchEnd, { passive: true } as EventListenerOptions) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/assets/translations/en/main.json(1 hunks)src/components/PullToRefresh/PullToRefreshWrapper.tsx(1 hunks)src/context/AppProvider/AppContext.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always runyarn lint --fixandyarn type-checkafter making changes
Avoidletvariable assignments - preferconstwith inline IIFE switch statements or extract to functions for conditional logic
Files:
src/context/AppProvider/AppContext.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern:userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables inuseMemoand callbacks inuseCallbackwhere possible
For static JSX icon elements (e.g.,<TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode usinguseColorModeValuehook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook:useTranslate()fromreact-polyglot
UseuseFeatureFlag('FlagName')hook to access feature flag values in components
Prefertypeoverinterfacefor type definitions
Use strict typing - avoidany
UseNominaltypes for domain identifiers (e.g.,WalletId,AccountId)
Import types from@shapeshiftoss/caipfor chain/account/asset IDs
UseuseAppSelectorfor Redux state
UseuseAppDispatchfor Redux actions
Memoize expensive computations withuseMemo
Memoize callbacks withuseCallback
**/*.{ts,tsx}: UseResult<T, E>pattern for error handling in swappers and APIs; ALWAYS useOk()andErr()from@sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from@shapeshiftoss/errorswith meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...
Files:
src/context/AppProvider/AppContext.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{tsx,jsx}: ALWAYS wrap React components in error boundaries and provide user-friendly fallback components with error logging
ALWAYS useuseErrorToasthook for displaying errors with translated error messages and handle different error types appropriatelyUse PascalCase for React component names and match the component name to the file name
Files:
src/context/AppProvider/AppContext.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Usehandleprefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names withis,has,can,shouldprefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names likedata,item,obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names likefn,func, orcallback
Files:
src/context/AppProvider/AppContext.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{jsx,tsx}: 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, and 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
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly in async operations
ALWAYS provide user-friendly error messages in error handling
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components using React.lazy for code splitting
ALWAYS use Suspense wrapper for lazy loaded components
USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places
Wrap components receiving props withmemofor performance optimization
Files:
src/context/AppProvider/AppContext.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Ensure TypeScript types are explicit and proper; avoid use of
anytype
Files:
src/context/AppProvider/AppContext.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
src/assets/translations/en/main.json
📄 CodeRabbit inference engine (CLAUDE.md)
Add English copy to
src/assets/translations/en/main.json(find appropriate section)
Files:
src/assets/translations/en/main.json
🧠 Learnings (10)
📓 Common learnings
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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
Repo: shapeshift/web PR: 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.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.
Applied to files:
src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : EXTRACT complex logic into custom hooks
Applied to files:
src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Use the translation hook: `useTranslate()` from `react-polyglot`
Applied to files:
src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : ALWAYS use Suspense wrapper for lazy loaded components
Applied to files:
src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to src/assets/translations/en/main.json : Add English copy to `src/assets/translations/en/main.json` (find appropriate section)
Applied to files:
src/assets/translations/en/main.json
📚 Learning: 2025-10-21T23:21:22.304Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10759
File: src/components/Modals/Send/hooks/useFormSend/useFormSend.tsx:41-50
Timestamp: 2025-10-21T23:21:22.304Z
Learning: In the shapeshift/web repository, the translation workflow follows an "English-first" approach: English translations in src/assets/translations/en/main.json are updated first in PRs, and translations for the other supported languages (de, es, fr, id, ja, ko, pt, ru, tr, uk, zh) are updated "after the fact" in follow-up work. Temporary mismatches between English and other language translation keys/formats during active development are expected and acceptable.
<!--
Applied to files:
src/assets/translations/en/main.json
📚 Learning: 2025-09-04T12:16:47.748Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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/PullToRefresh/PullToRefreshWrapper.tsx
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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/PullToRefresh/PullToRefreshWrapper.tsx
🧬 Code graph analysis (2)
src/context/AppProvider/AppContext.tsx (2)
src/test/mocks/accounts.ts (1)
assetIds(15-22)src/components/PullToRefresh/PullToRefreshWrapper.tsx (1)
PullToRefreshWrapper(28-286)
src/components/PullToRefresh/PullToRefreshWrapper.tsx (5)
src/state/store.ts (2)
useAppSelector(144-144)useAppDispatch(152-152)src/state/slices/common-selectors.ts (1)
selectEnabledWalletAccountIds(52-60)src/state/slices/portfolioSlice/selectors.ts (1)
selectIsAnyPortfolioGetAccountLoading(1240-1245)src/state/slices/portfolioSlice/portfolioSlice.ts (1)
portfolioApi(228-298)src/state/slices/txHistorySlice/txHistorySlice.ts (1)
txHistoryApi(165-273)
⏰ 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 (6)
src/context/AppProvider/AppContext.tsx (1)
14-14: LGTM!The integration is clean and minimal. Wrapping children with
PullToRefreshWrapperonly when assets are loaded (Boolean(assetIds.length)) correctly ensures the pull-to-refresh feature is available after the app has initialized.Also applies to: 252-252
src/assets/translations/en/main.json (1)
2643-2647: LGTM!Translation keys are well-structured and the copy is clear. The three states (pulling, release threshold reached, refreshing) cover the necessary UI feedback.
src/components/PullToRefresh/PullToRefreshWrapper.tsx (4)
88-103: Resetting all API state may cause unintended side effects.
resetApiState()clears the entire cache for bothportfolioApiandtxHistoryApi, including any unrelated queries or data that might be in-flight. This could cause:
- UI flickering as all cached data is invalidated
- Loss of data for queries unrelated to the current wallet accounts
- Potential race conditions if other components are fetching data simultaneously
Consider using more targeted invalidation. For example, you could invalidate only the specific account queries being refreshed:
const onRefresh = useCallback(() => { - dispatch(portfolioApi.util.resetApiState()) - dispatch(txHistoryApi.util.resetApiState()) - const { getAllTxHistory } = txHistoryApi.endpoints enabledWalletAccountIds.forEach(accountId => { - dispatch(portfolioApi.endpoints.getAccount.initiate({ accountId, upsertOnFetch: true })) + dispatch( + portfolioApi.endpoints.getAccount.initiate( + { accountId, upsertOnFetch: true }, + { forceRefetch: true }, + ), + ) }) if (isLazyTxHistoryEnabled) return enabledWalletAccountIds.forEach(requestedAccountId => { - dispatch(getAllTxHistory.initiate(requestedAccountId)) + dispatch(getAllTxHistory.initiate(requestedAccountId, { forceRefetch: true })) }) }, [dispatch, enabledWalletAccountIds, isLazyTxHistoryEnabled])Could you verify if the full cache reset is intentional for this feature, or if targeted invalidation would better preserve user experience?
70-86: LGTM - Well-structured memoized computations.The
displayText,iconRotation, andindicatorTransformvalues are properly memoized with correct dependencies. The logic for transitioning between states is clear and the visual feedback thresholds (0.3 for showing text, 1.0 for release prompt) provide good UX.
221-285: LGTM - Clean render implementation with proper mobile considerations.The component correctly:
- Uses fixed positioning with safe-area-inset for notched devices
- Applies appropriate z-index for overlay
- Uses
pointerEvents='none'to avoid blocking interactions- Provides smooth transitions between states
- Handles both light and dark color modes
The conditional rendering
(showIndicator || isFetching)ensures the indicator is visible during the entire refresh cycle.
105-154: Touch event handling is well-designed with appropriate safety checks.The implementation correctly:
- Tracks initial scroll position to detect pull-from-top
- Uses refs for touch coordinates to avoid re-renders during gesture
- Requires minimum pull distance (30px) to register as intentional pull
- Uses passive event listeners to not block scrolling
- Calculates progress based on 200px threshold
The velocity check (< 2 px/ms) and minimum duration (250ms) at lines 176-181 are good safeguards against accidental triggers.
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Tested locally on mobile devtools (not on mobile app, since this is also exposed for regular mobile viewports):
https://jam.dev/c/016d65f3-8a76-4e53-84ec-9659744ac3eb
OK:
- refresh does work ✅
NOK (?):
- not contextual, refresh refetches all portfolio accounts regardless of where I am in the app which seems like an overfetching nightmare, should be able to extract accountId from route and refetch whatever needs be (or do nothing in case of not being in a route with portfolio bits?)
Maybe OK but weird:
- Seeing the ugly component on app load, feels like a bug
- Not sure about the UX of having a component at the top as a simple overlay, shouldn't the whole app effectively scroll? At least that's the usual UX pattern for this
| } | ||
|
|
||
| // Track if user actually pulled down (not just touched) | ||
| if (distance > 30) { |
There was a problem hiding this comment.
No magic more happy, constant? (ditto across this file)
| } finally { | ||
| setTimeout(() => { | ||
| setPullProgress(0) | ||
| setTimeout(() => setShowIndicator(false), 300) |
There was a problem hiding this comment.
is this nested guy on purpose? sounds v. bad
| setTimeout(() => setShowIndicator(false), 300) | ||
| }, 300) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
q: can we avoid the spaghettiness
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/AssetAccounts/AssetAccounts.tsx (1)
73-118: Memoize the content JSX to avoid unnecessary re-renders.The
contentvariable contains an expensive JSX tree that should be wrapped inuseMemoto prevent recreation on every render when its dependencies haven't changed.Apply this diff:
- const content = ( + const content = useMemo(() => ( <Card> <CardBody pt={0}> <Stack spacing={2} mt={2} mx={-4}> {/* ... existing JSX ... */} </Stack> </CardBody> </Card> - ) + ), [accountIds, assetId, translate])
🧹 Nitpick comments (6)
src/pages/Dashboard/Portfolio.tsx (2)
43-58: Theasynckeyword is unnecessary here.The
handleRefreshfunction is markedasyncbut contains noawaitexpressions. All dispatches are fire-and-forget. Removeasyncto avoid misleading readers, or add a comment explaining the intentional fire-and-forget pattern.- const handleRefresh = useCallback(async () => { + const handleRefresh = useCallback(() => { dispatch(portfolioApi.util.resetApiState()) dispatch(txHistoryApi.util.resetApiState()) const { getAllTxHistory } = txHistoryApi.endpoints enabledWalletAccountIds.forEach(accountId => { dispatch(portfolioApi.endpoints.getAccount.initiate({ accountId, upsertOnFetch: true })) }) if (isLazyTxHistoryEnabled) return enabledWalletAccountIds.forEach(requestedAccountId => { dispatch(getAllTxHistory.initiate(requestedAccountId)) }) }, [dispatch, enabledWalletAccountIds, isLazyTxHistoryEnabled])
60-60: Consider memoizing thecontentvariable.Per coding guidelines, derived values should typically be memoized. This is a minor optimization since both child components are already memoized.
- const content = shouldRenderAccountTable ? <AccountTable /> : <AccountTableSkeleton /> + const content = useMemo( + () => (shouldRenderAccountTable ? <AccountTable /> : <AccountTableSkeleton />), + [shouldRenderAccountTable], + )src/components/PullToRefresh/PullToRefreshList.tsx (2)
30-34: Remove unusedisPullingvariable.The
isPullingvalue is destructured fromusePullToRefreshbut never used in the component. This creates dead code.- const { isRefreshing: isPulling, pullPosition } = usePullToRefresh({ + const { pullPosition } = usePullToRefresh({ onRefresh, maximumPullLength: 150, refreshThreshold: 100, })
21-25: Wrap the component withmemofor performance.Per coding guidelines, components receiving props should be wrapped with
memoto prevent unnecessary re-renders.+import { memo, useMemo } from 'react' -import { useMemo } from 'react' -export const PullToRefreshList: React.FC<PullToRefreshListProps> = ({ +export const PullToRefreshList: React.FC<PullToRefreshListProps> = memo(({ children, onRefresh, isRefreshing = false, -}) => { +}) => { // ... component body -} +})src/components/TransactionHistory/TransactionHistoryContent.tsx (2)
68-83: Theasynckeyword is unnecessary here.Same as in
Portfolio.tsx,handleRefreshis markedasyncbut contains noawaitexpressions.- const handleRefresh = useCallback(async () => { + const handleRefresh = useCallback(() => { dispatch(portfolioApi.util.resetApiState()) dispatch(txHistoryApi.util.resetApiState()) const { getAllTxHistory } = txHistoryApi.endpoints visibleAccountIds.forEach(accountId => { dispatch(portfolioApi.endpoints.getAccount.initiate({ accountId, upsertOnFetch: true })) }) if (isLazyTxHistoryEnabled) return visibleAccountIds.forEach(requestedAccountId => { dispatch(getAllTxHistory.initiate(requestedAccountId)) }) }, [dispatch, visibleAccountIds, isLazyTxHistoryEnabled])
96-116: Consider memoizing thecontentvariable.The
contentJSX has multiple dependencies. Memoizing it could prevent unnecessary re-renders when unrelated state changes.- const content = ( + const content = useMemo(() => ( <Stack mx={stackMargin}> {/* ... */} </Stack> - ) + ), [stackMargin, headingPadding, inputRef, isCompact, handleInputChange, handleReset, setFilters, filters, txIds])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
package.json(1 hunks)src/components/AssetAccounts/AssetAccounts.tsx(3 hunks)src/components/PullToRefresh/PullToRefreshList.tsx(1 hunks)src/components/PullToRefresh/PullToRefreshWrapper.tsx(1 hunks)src/components/TransactionHistory/TransactionHistoryContent.tsx(5 hunks)src/pages/Dashboard/Portfolio.tsx(1 hunks)src/pages/Dashboard/components/AccountTableSkeleton.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always runyarn lint --fixandyarn type-checkafter making changes
Avoidletvariable assignments - preferconstwith inline IIFE switch statements or extract to functions for conditional logic
Files:
src/components/PullToRefresh/PullToRefreshList.tsxsrc/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsxsrc/pages/Dashboard/components/AccountTableSkeleton.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern:userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables inuseMemoand callbacks inuseCallbackwhere possible
For static JSX icon elements (e.g.,<TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode usinguseColorModeValuehook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook:useTranslate()fromreact-polyglot
UseuseFeatureFlag('FlagName')hook to access feature flag values in components
Prefertypeoverinterfacefor type definitions
Use strict typing - avoidany
UseNominaltypes for domain identifiers (e.g.,WalletId,AccountId)
Import types from@shapeshiftoss/caipfor chain/account/asset IDs
UseuseAppSelectorfor Redux state
UseuseAppDispatchfor Redux actions
Memoize expensive computations withuseMemo
Memoize callbacks withuseCallback
**/*.{ts,tsx}: UseResult<T, E>pattern for error handling in swappers and APIs; ALWAYS useOk()andErr()from@sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from@shapeshiftoss/errorswith meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...
Files:
src/components/PullToRefresh/PullToRefreshList.tsxsrc/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsxsrc/pages/Dashboard/components/AccountTableSkeleton.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{tsx,jsx}: ALWAYS wrap React components in error boundaries and provide user-friendly fallback components with error logging
ALWAYS useuseErrorToasthook for displaying errors with translated error messages and handle different error types appropriatelyUse PascalCase for React component names and match the component name to the file name
Files:
src/components/PullToRefresh/PullToRefreshList.tsxsrc/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsxsrc/pages/Dashboard/components/AccountTableSkeleton.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Usehandleprefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names withis,has,can,shouldprefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names likedata,item,obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names likefn,func, orcallback
Files:
src/components/PullToRefresh/PullToRefreshList.tsxsrc/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsxsrc/pages/Dashboard/components/AccountTableSkeleton.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{jsx,tsx}: 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, and 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
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly in async operations
ALWAYS provide user-friendly error messages in error handling
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components using React.lazy for code splitting
ALWAYS use Suspense wrapper for lazy loaded components
USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places
Wrap components receiving props withmemofor performance optimization
Files:
src/components/PullToRefresh/PullToRefreshList.tsxsrc/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsxsrc/pages/Dashboard/components/AccountTableSkeleton.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Ensure TypeScript types are explicit and proper; avoid use of
anytype
Files:
src/components/PullToRefresh/PullToRefreshList.tsxsrc/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsxsrc/pages/Dashboard/components/AccountTableSkeleton.tsx
🧠 Learnings (32)
📓 Common learnings
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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/PullToRefresh/PullToRefreshList.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Account for responsive mobile designs in all UI components
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-22T12:58:26.590Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx:108-111
Timestamp: 2025-08-22T12:58:26.590Z
Learning: In the RFOX GenericTransactionDisplayType flow in src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx, the txHash is always guaranteed to be present according to NeOMakinG, so defensive null checks for txLink are not needed in this context.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Use Chakra UI components and conventions
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsxsrc/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : EXTRACT complex logic into custom hooks
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-08-06T09:47:29.865Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:174-178
Timestamp: 2025-08-06T09:47:29.865Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG prefers keeping complex height calculations inline rather than extracting them to useMemo, prioritizing perceived performance/execution speed over code readability, even when the calculation would only run when the dependency (isSearching) changes.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx:97-108
Timestamp: 2025-08-14T17:54:32.563Z
Learning: In ReusableLpStatus component (src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx), the txAssets dependency is stable from first render because poolAsset, baseAsset, actionSide, and action are all defined first render, making the current txAssetsStatuses initialization pattern safe without needing useEffect synchronization.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsxsrc/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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/TransactionHistoryContent.tsxsrc/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsxsrc/components/PullToRefresh/PullToRefreshWrapper.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : ALWAYS use `useMemo` for expensive computations, object/array creations, and filtered data
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-11-05T23:37:30.632Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10912
File: src/context/WalletProvider/NewWalletViews/NewWalletViewsSwitch.tsx:290-290
Timestamp: 2025-11-05T23:37:30.632Z
Learning: In the ShapeShift web codebase, `isMobile` imported from '@/lib/globals' is a module-level constant (defined as `export const isMobile = Boolean(window?.isShapeShiftMobile)`) that is evaluated once at module load time. It is a stable reference that does not need to be included in useCallback/useMemo/useEffect dependency arrays.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Use `useAppSelector` for Redux state
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-08-22T13:02:58.824Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/hooks/useRfoxRewardDistributionActionSubscriber.tsx:33-41
Timestamp: 2025-08-22T13:02:58.824Z
Learning: In src/pages/RFOX/hooks/useRfoxRewardDistributionActionSubscriber.tsx, NeOMakinG declined optimizing useMemo dependencies to depend on lifetimeRewardDistributionsQuery.data instead of the entire query object, indicating indifference toward this type of performance optimization.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Portfolio state is filtered by active `walletId`
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-10-17T07:53:40.088Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10783
File: src/components/Modals/Snaps/Snaps.tsx:36-38
Timestamp: 2025-10-17T07:53:40.088Z
Learning: Chakra UI's ModalContent component supports the containerProps prop, which accepts an object with sx and other styling properties. When using the modal stack manager's useModalRegistration hook, spreading {...modalContentProps} into ModalContent is the correct approach, as modalContentProps contains { containerProps: { sx: { zIndex, pointerEvents } } }. Do not suggest extracting sx from modalContentProps.containerProps.
Applied to files:
src/components/TransactionHistory/TransactionHistoryContent.tsx
📚 Learning: 2025-08-21T22:14:39.915Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 10326
File: src/hooks/useActionCenterSubscribers/useFetchBasePortfolio.ts:18-29
Timestamp: 2025-08-21T22:14:39.915Z
Learning: In the ShapeShift web codebase, temporary Base portfolio management hacks (marked with "TEMP HACK FOR BASE" comments) may use burst patterns like calling portfolioApi.getAccount with forceRefetch in tight loops, which are acceptable given they're temporary solutions and already used elsewhere in the app.
Applied to files:
src/pages/Dashboard/Portfolio.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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/pages/Dashboard/Portfolio.tsxsrc/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-06T11:02:09.501Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:54-72
Timestamp: 2025-08-06T11:02:09.501Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG indicated that memoizing the ExploreCard component would be unnecessary, questioning "why would we memoize that", suggesting that like CategoryCard, this component doesn't benefit from memoization despite receiving props, likely due to its specific usage patterns in the carousel with static content.
Applied to files:
src/pages/Dashboard/Portfolio.tsx
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-05T22:41:35.473Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Import types from `shapeshiftoss/caip` for chain/account/asset IDs
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property (signature: (state, { assetId })), not a raw AssetId string. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter is expected to have an assetId property. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-08-22T12:59:01.702Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:37-53
Timestamp: 2025-08-22T12:59:01.702Z
Learning: In RewardDistributionActionCard component (src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx), NeOMakinG confirmed that runeAsset is expected to always be defined when the component renders, so defensive guards against undefined runeAsset are not needed.
Applied to files:
src/components/AssetAccounts/AssetAccounts.tsx
📚 Learning: 2025-11-19T22:20:25.661Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10767
File: package.json:324-324
Timestamp: 2025-11-19T22:20:25.661Z
Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).
Applied to files:
package.json
📚 Learning: 2025-09-04T12:16:47.748Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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/PullToRefresh/PullToRefreshWrapper.tsx
📚 Learning: 2025-07-30T20:57:48.176Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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/pages/Dashboard/components/AccountTableSkeleton.tsx
🧬 Code graph analysis (2)
src/components/PullToRefresh/PullToRefreshList.tsx (1)
src/components/Text/Text.tsx (1)
Text(19-83)
src/components/AssetAccounts/AssetAccounts.tsx (6)
src/theme/theme.tsx (1)
breakpoints(37-44)src/state/store.ts (2)
useAppDispatch(152-152)useAppSelector(144-144)src/state/slices/portfolioSlice/selectors.ts (1)
selectIsAnyPortfolioGetAccountLoading(1240-1245)src/state/slices/portfolioSlice/portfolioSlice.ts (1)
portfolioApi(228-298)src/state/slices/txHistorySlice/txHistorySlice.ts (1)
txHistoryApi(165-273)src/components/PullToRefresh/PullToRefreshList.tsx (1)
PullToRefreshList(21-124)
⏰ 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 (12)
src/pages/Dashboard/components/AccountTableSkeleton.tsx (1)
4-32: LGTM!The skeleton component is well-structured and properly memoized. The reduction from 5 to 4 rows is a reasonable UI adjustment.
src/pages/Dashboard/Portfolio.tsx (1)
17-80: Good mobile-responsive implementation.The component correctly:
- Detects mobile viewport using
useMediaQuery- Aggregates multiple loading states for
isRefreshing- Conditionally wraps content in
PullToRefreshListonly on mobile- Uses early return pattern for cleaner code flow
src/components/PullToRefresh/PullToRefreshList.tsx (1)
57-123: Well-structured pull-to-refresh UI implementation.The component:
- Properly uses
useColorModeValuefor light/dark mode theming- Correctly memoizes derived values (
pullProgress,indicatorOpacity,displayText,iconRotation)- Uses translation keys for all user-facing text
- Implements smooth visual feedback with progress bar and icon rotation
src/components/TransactionHistory/TransactionHistoryContent.tsx (2)
59-66: Good optimization withvisibleAccountIds.Deriving account IDs from the filtered
txIdsensures only relevant accounts are refreshed, rather than refreshing all enabled wallet accounts. This is a thoughtful optimization for the transaction history context.
26-127: Consistent mobile pull-to-refresh implementation.The component correctly mirrors the pattern established in
Portfolio.tsx:
- Mobile detection via
useMediaQuery- Aggregated
isRefreshingstate- Conditional
PullToRefreshListwrapping- Early return for non-mobile
package.json (1)
213-213: Theuse-pull-to-refreshpackage at version 2.4.1 is actively maintained (latest update November 2024) and explicitly supports React 19 via its peerDependencies ("react": "18.x || 19.x"). No known security vulnerabilities were found for this package. This dependency is safe to add.src/components/AssetAccounts/AssetAccounts.tsx (2)
1-37: LGTM!The imports and initial setup are well-structured. The use of breakpoints constant for the media query maintains consistency with the theme configuration.
54-69: LGTM!The
handleRefreshcallback is properly memoized withuseCallbackand includes all necessary dependencies. The logic correctly resets API state and re-fetches data conditionally based on the LazyTxHistory feature flag.src/components/PullToRefresh/PullToRefreshWrapper.tsx (4)
1-29: LGTM!The imports and type definitions follow project conventions correctly. The
MotionBoxconstant is appropriately defined outside the component.
56-72: LGTM!The Framer Motion setup and effects are correctly implemented with proper dependency arrays and cleanup handling.
74-104: LGTM!The memoized computed values (
displayText,iconRotation) and theonRefreshcallback are correctly implemented with proper dependencies.
122-198: LGTM!The JSX rendering logic is well-structured with proper layering, safe area handling, and responsive color mode values. The fixed positioning and z-index coordination between the drag handle and indicator work correctly.
| const isRefreshing = | ||
| getAccountFetching > 0 || | ||
| portalsAccountFetching > 0 || | ||
| portalsPlatformsFetching > 0 || | ||
| isPortfolioLoading |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Memoize the derived isRefreshing value.
The isRefreshing value is computed from multiple state sources and recalculates on every render. Per coding guidelines, derived values should be wrapped in useMemo.
Apply this diff:
- const isRefreshing =
- getAccountFetching > 0 ||
- portalsAccountFetching > 0 ||
- portalsPlatformsFetching > 0 ||
- isPortfolioLoading
+ const isRefreshing = useMemo(
+ () =>
+ getAccountFetching > 0 ||
+ portalsAccountFetching > 0 ||
+ portalsPlatformsFetching > 0 ||
+ isPortfolioLoading,
+ [getAccountFetching, portalsAccountFetching, portalsPlatformsFetching, isPortfolioLoading],
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isRefreshing = | |
| getAccountFetching > 0 || | |
| portalsAccountFetching > 0 || | |
| portalsPlatformsFetching > 0 || | |
| isPortfolioLoading | |
| const isRefreshing = useMemo( | |
| () => | |
| getAccountFetching > 0 || | |
| portalsAccountFetching > 0 || | |
| portalsPlatformsFetching > 0 || | |
| isPortfolioLoading, | |
| [getAccountFetching, portalsAccountFetching, portalsPlatformsFetching, isPortfolioLoading], | |
| ) |
🤖 Prompt for AI Agents
In src/components/AssetAccounts/AssetAccounts.tsx around lines 44 to 48, the
derived isRefreshing boolean is computed on every render; wrap that computation
in React.useMemo to memoize it. Import useMemo from React if not already
imported, replace the direct assignment with useMemo(() => getAccountFetching >
0 || portalsAccountFetching > 0 || portalsPlatformsFetching > 0 ||
isPortfolioLoading, [getAccountFetching, portalsAccountFetching,
portalsPlatformsFetching, isPortfolioLoading]) so the value only recalculates
when those four dependencies change.
|
|
||
| export const PullToRefreshWrapper: React.FC<PullToRefreshWrapperProps> = ({ children }) => { | ||
| const translate = useTranslate() | ||
| const [isMobile] = useMediaQuery('(max-width: 768px)') |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace hardcoded media query with breakpoints constant.
The hardcoded '(max-width: 768px)' should use the theme's breakpoints constant for consistency. The AssetAccounts.tsx file in this same PR uses breakpoints['md'] for the same breakpoint.
Apply this diff:
- const [isMobile] = useMediaQuery('(max-width: 768px)')
+ const [isMobile] = useMediaQuery(`(max-width: ${breakpoints['md']})`, { ssr: false })Add the breakpoints import at the top:
import { useAppDispatch, useAppSelector } from '@/state/store'
+import { breakpoints } from '@/theme/theme'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [isMobile] = useMediaQuery('(max-width: 768px)') | |
| import { useAppDispatch, useAppSelector } from '@/state/store' | |
| import { breakpoints } from '@/theme/theme' | |
| // ... other code ... | |
| const [isMobile] = useMediaQuery(`(max-width: ${breakpoints['md']})`, { ssr: false }) |
🤖 Prompt for AI Agents
In src/components/PullToRefresh/PullToRefreshWrapper.tsx around line 33, the
media query is hardcoded as '(max-width: 768px)'; import the theme breakpoints
constant (where other files use breakpoints['md']) at the top of the file and
replace the hardcoded string with the breakpoints-based value so the
useMediaQuery call uses the project's breakpoint constant (e.g.
breakpoints['md']) for consistency with AssetAccounts.tsx.
| const handleDragEnd = useCallback((_event: any, info: any) => { | ||
| const shouldRefresh = info.offset.y > 200 | ||
|
|
||
| if (shouldRefresh) { | ||
| onRefresh() | ||
| } else { | ||
| animate(y, 0, { type: 'spring', stiffness: 300, damping: 30 }) | ||
| } | ||
| }, [onRefresh, y]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use proper types for handleDragEnd parameters.
The callback parameters use any types and the unused _event parameter is prefixed with an underscore. Per coding guidelines, avoid any types and don't use underscore prefixes for unused parameters.
Apply this diff to use proper types from framer-motion:
- const handleDragEnd = useCallback((_event: any, info: any) => {
+ const handleDragEnd = useCallback((_event: MouseEvent | TouchEvent | PointerEvent, info: { offset: { x: number; y: number } }) => {
const shouldRefresh = info.offset.y > 200Or, if the event is truly unused, consider restructuring to omit it (though framer-motion's API requires both parameters).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleDragEnd = useCallback((_event: any, info: any) => { | |
| const shouldRefresh = info.offset.y > 200 | |
| if (shouldRefresh) { | |
| onRefresh() | |
| } else { | |
| animate(y, 0, { type: 'spring', stiffness: 300, damping: 30 }) | |
| } | |
| }, [onRefresh, y]) | |
| const handleDragEnd = useCallback((event: MouseEvent | TouchEvent | PointerEvent, info: { offset: { x: number; y: number } }) => { | |
| const shouldRefresh = info.offset.y > 200 | |
| if (shouldRefresh) { | |
| onRefresh() | |
| } else { | |
| animate(y, 0, { type: 'spring', stiffness: 300, damping: 30 }) | |
| } | |
| }, [onRefresh, y]) |
🤖 Prompt for AI Agents
In src/components/PullToRefresh/PullToRefreshWrapper.tsx around lines 106–114,
the handleDragEnd callback uses unsafe any types and an underscore-prefixed
unused param; replace these with framer-motion types by importing PanInfo from
'framer-motion' and typing the parameters as (event: PointerEvent | MouseEvent |
TouchEvent, info: PanInfo) => void (or the narrower Event type your project
prefers), remove the underscore prefix from the event name, and keep both
parameters (framer-motion expects two) while ensuring the function body only
uses the typed info parameter.
| return <>{children}</> | ||
| } | ||
|
|
||
| const showRefreshIndicator = pullProgressValue > 0 || (isFetching && isManualRefresh.current) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Memoize the derived showRefreshIndicator value.
The showRefreshIndicator value is computed from state and should be wrapped in useMemo per coding guidelines for derived values.
Apply this diff:
- const showRefreshIndicator = pullProgressValue > 0 || (isFetching && isManualRefresh.current)
+ const showRefreshIndicator = useMemo(
+ () => pullProgressValue > 0 || (isFetching && isManualRefresh.current),
+ [pullProgressValue, isFetching],
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const showRefreshIndicator = pullProgressValue > 0 || (isFetching && isManualRefresh.current) | |
| const showRefreshIndicator = useMemo( | |
| () => pullProgressValue > 0 || (isFetching && isManualRefresh.current), | |
| [pullProgressValue, isFetching], | |
| ) |
🤖 Prompt for AI Agents
In src/components/PullToRefresh/PullToRefreshWrapper.tsx around line 120, the
derived boolean showRefreshIndicator is computed inline from pullProgressValue
and isFetching/isManualRefresh and should be memoized; replace the direct
expression with a useMemo that returns pullProgressValue > 0 || (isFetching &&
isManualRefresh.current), adding useMemo to imports if missing, and ensure its
dependency array includes pullProgressValue, isFetching, and
isManualRefresh.current (or isManualRefresh if using ref current properly) so
the memo updates correctly.
|
Holding on that for now as it's not 100% a priority plus it needs a bigger lift |
Description
Adds a component wrapping all other the app, that allow users to pull down the app to refresh the portfolio and tx history data
Issue (if applicable)
closes #11107
Risk
Low
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/0a0da730-c8a8-45a0-ae7a-82426e853f0e
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.