feat(tokenselector): 9 - implement SelectTokenWidget with enhanced chain … #6533
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe SelectTokenWidget component is refactored from a monolithic implementation into a controller-driven architecture. Six new controller modules introduce hooks for managing widget state, dependencies, modal props, and view state. The SelectTokenWidgetContent component is removed and its conditional rendering logic is distributed across these controllers. The main component is simplified to delegate to the controller and view components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
fairlighteth
force-pushed
the
feat/token-selector-9
branch
from
554cd6a to
259c318
Compare
|
@coderabbitai review |
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.ts
Show resolved
Hide resolved
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Now that you mentioned LIMIT/TWAP, I forgot to check it in earlier PRs.
In this one, the selector is broken.
The page height gets very long, but no tokens can be seen at all
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts (2)
148-150: useState called inline within useWidgetMetadata may cause issues.The
useStatecall at line 148-150 is invoked directly withinuseWidgetMetadata. If the parameters (field,oppositeToken,lpTokensWithBalancesCount) change between renders, the initial value passed touseStatewill be ignored after the first render, potentially causing stale category state.Consider lifting this state to the consuming component or using
useEffectto reset the state when dependencies change.
302-304: Consider making the utility more explicit.
hasAvailableChainsreturnsBoolean(chainsToSelect)which checks for truthiness. IfChainsToSelectStatecan be an empty object or have empty arrays, this might not accurately reflect "available chains."-export function hasAvailableChains(chainsToSelect: ChainsToSelectState | undefined): boolean { - return Boolean(chainsToSelect) +export function hasAvailableChains(chainsToSelect: ChainsToSelectState | undefined): boolean { + return Boolean(chainsToSelect && (chainsToSelect.isLoading || (chainsToSelect.chains?.length ?? 0) > 0)) }apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.ts (1)
146-214: Consider extracting the inline type annotation.The
createSelectTokenModalPropsfunction has a large inline type annotation (lines 168-190). Consider extracting this to a named interface for reusability and readability.interface CreateSelectTokenModalPropsArgs { account: string | undefined chainsPanelTitle: string // ... rest of properties } function createSelectTokenModalProps(args: CreateSelectTokenModalPropsArgs): SelectTokenModalProps { // ... }apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.ts (2)
17-40: Avoid exportingReturnType<typeof useX>-shaped API types (brittle surface).Since
WidgetViewDependenciesResultis exported, tying its field types to hook ReturnTypes will force downstream churn on any refactor of those hooks. Prefer exporting explicit named function types (or the shared interfaces likeRecentTokenSection,PoolPageHandlers, etc.) instead.
31-40: PreferactiveChainId?: numberoveractiveChainId: number | undefinedfor args.This reads more idiomatically for optional inputs and reduces call-site noise.
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.ts (1)
126-183: Consider makinghasChainPanel: trueimplyonSelectChainis required (type-level).Right now
onSelectChainis optional and silently no-ops; a discriminated union onhasChainPanelwould prevent “panel shown but selection doesn’t work” states.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controller.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerViewState.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/helpers.tsx(0 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/styled.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/types.ts(0 hunks)apps/cowswap-frontend/src/modules/tokensList/hooks/recentTokensStorage.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/helpers.tsx
- apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/types.ts
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/styled.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.tsapps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsxapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerViewState.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controller.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsxapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.tsapps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsxapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerViewState.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controller.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsxapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-08-12T05:57:08.021Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6138
File: libs/hook-dapp-lib/src/hookDappsRegistry.ts:1-1
Timestamp: 2025-08-12T05:57:08.021Z
Learning: The matchHooksToDapps function in libs/hook-dapp-lib/src/utils.ts provides backward compatibility for permit hooks through function selector detection (EIP_2612_PERMIT_SELECTOR and DAI_PERMIT_SELECTOR) rather than dappId matching, making it robust against dappId changes.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.ts
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsxapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx
📚 Learning: 2025-07-18T08:07:55.497Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/tokens/src/const/tokensList.json:135-167
Timestamp: 2025-07-18T08:07:55.497Z
Learning: Token lists for CoW Swap are maintained in a separate repository at https://github.com/cowprotocol/token-lists, not in the main cowswap repository. Issues related to missing token lists should be tracked in the token-lists repository.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-05-26T12:39:29.009Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 5715
File: libs/common-const/src/tokens.ts:539-555
Timestamp: 2025-05-26T12:39:29.009Z
Learning: The team accepts using NATIVE_CURRENCY_ADDRESS as a temporary placeholder for COW token contract addresses on new networks (Polygon, Avalanche) until actual COW contracts are deployed.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-08-05T14:27:05.023Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/wallet/src/web3-react/utils/switchChain.ts:36-38
Timestamp: 2025-08-05T14:27:05.023Z
Learning: In libs/wallet/src/web3-react/utils/switchChain.ts, the team prefers using Record<SupportedChainId, string | null> over Partial<Record<SupportedChainId, string>> for WALLET_RPC_SUGGESTION to enforce that all supported chain IDs have explicit values set, even if some might be null. This ensures compile-time completeness checking.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerViewState.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerViewState.tsapps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/hooks/recentTokensStorage.ts
📚 Learning: 2025-08-12T06:33:19.348Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6137
File: libs/tokens/src/state/tokens/allTokensAtom.ts:34-65
Timestamp: 2025-08-12T06:33:19.348Z
Learning: In libs/tokens/src/utils/parseTokenInfo.ts, the parseTokenInfo() function returns a new instance of TokenInfo using object spread syntax ({ ...token, ... }), making it safe to mutate properties like lpTokenProvider on the returned object without side effects.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/hooks/recentTokensStorage.ts
🧬 Code graph analysis (6)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/styled.ts (1)
libs/common-hooks/src/useTheme.ts (1)
styled(26-26)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.ts (1)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts (8)
useManageWidgetVisibility(92-99)usePoolPageHandlers(182-195)useRecentTokenSection(241-260)useTokenSelectionHandler(262-300)useImportFlowCallbacks(197-239)useTokenDataSources(116-138)useTokenAdminActions(101-114)useDismissHandler(172-180)
apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx (1)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx (1)
SelectTokenWidget(17-31)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.ts (5)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controller.ts (1)
SelectTokenWidgetViewProps(85-85)libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)libs/tokens/src/types.ts (1)
ListState(39-42)apps/cowswap-frontend/src/modules/tokensList/types.ts (2)
ChainsToSelectState(26-30)TokenSelectionHandler(11-11)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts (1)
TokenDataSources(54-66)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.ts (4)
libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerDependencies.ts (1)
WidgetViewDependenciesResult(17-29)apps/cowswap-frontend/src/modules/tokensList/hooks/useOnSelectChain.ts (1)
useOnSelectChain(14-32)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts (6)
useRecentTokenSection(241-260)useTokenDataSources(116-138)useWidgetMetadata(140-156)useManageWidgetVisibility(92-99)usePoolPageHandlers(182-195)useTokenSelectionHandler(262-300)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controller.ts (5)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.ts (1)
SelectTokenWidgetViewProps(14-34)apps/cowswap-frontend/src/modules/tokensList/hooks/useOnSelectChain.ts (1)
useOnSelectChain(14-32)libs/common-hooks/src/useIsBridgingEnabled.ts (1)
useIsBridgingEnabled(5-7)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts (4)
useManageWidgetVisibility(92-99)useTokenDataSources(116-138)useTokenAdminActions(101-114)useWidgetMetadata(140-156)libs/wallet/src/api/hooks.ts (1)
useWalletInfo(24-26)
🔇 Additional comments (20)
apps/cowswap-frontend/src/modules/tokensList/hooks/recentTokensStorage.ts (1)
6-7: Storage schema/migration comment matches implementation; keep it in sync with versioning.This accurately reflects
readStoredTokens()handling of legacy array payloads vs map payloads and theStoredRecentTokensByChaintype.apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerState.ts (7)
1-36: Well-organized imports and type definitions.The module organization is clean, with clear separation between external dependencies, internal modules, and local imports. The type definitions (interfaces) provide good documentation of the expected shapes.
92-114: LGTM - Clean hook implementations.
useManageWidgetVisibilityanduseTokenAdminActionsare well-structured with properuseCallbackmemoization and clear return types.
116-138: LGTM - Token data aggregation hook.
useTokenDataSourcescleanly aggregates multiple data sources into a single return object with clear typing.
172-195: LGTM - Handler hooks are well-memoized.Both
useDismissHandlerandusePoolPageHandlerscorrectly useuseCallbackwith appropriate dependency arrays.
197-239: LGTM - Import flow callbacks with proper error handling.The
useImportFlowCallbackshook correctly handles the import flow with try/catch for error cases and proper cleanup viaupdateSelectTokenWidget.
241-260: LGTM - Recent token section wrapper.Clean wrapper around
useRecentTokenswith proper memoization of the click handler.
280-286: The code is correct. Theinoperator works reliably with TypeScript numeric enums because the compiled output includes reverse mappings where numeric values become string keys. This pattern is used throughout the codebase and is the established convention here.apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx (1)
26-26: LGTM - Clean slot-based integration.The change correctly delegates rendering control to
SelectTokenWidgetitself (viashouldRenderin the controller), while maintaining slot override capability for custom implementations.apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/styled.ts (1)
1-22: LGTM - Well-structured styled components.The layout components are clean and follow styled-components conventions. The transient prop
$hasSidebaris correctly prefixed to avoid DOM attribute forwarding, and the responsive flex-direction logic is appropriate.apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerModalProps.ts (3)
20-20: Good practice - stable empty array reference.Using a module-level constant
EMPTY_FAV_TOKENSprevents unnecessary re-renders whenstandaloneis true, as the array reference remains stable across renders.
37-78: LGTM - Well-structured modal props hook.The
useWidgetModalPropshook correctly assembles all necessary props and delegates memoization touseSelectTokenModalPropsMemo. The conditional favoriteTokens handling for standalone mode is appropriate.
100-144: LGTM - Clean props transformation.
getSelectTokenWidgetViewPropsArgsis a pure function that maps internal hook results to the view props interface. The property renaming (e.g.,importFlows.resetTokenImport→onBackFromImport) improves API clarity.apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controller.ts (5)
24-33: LGTM - Clear interface definitions.The
SelectTokenWidgetPropsandSelectTokenWidgetControllerinterfaces provide clear contracts for the component's input and output, supporting the controller/view architecture pattern.
35-58: LGTM - Well-organized hook initialization.The controller correctly aggregates all required state and data sources. The
resolvedFieldfallback toField.INPUTprovides a safe default.
60-76: LGTM - Clean delegation to view state hook.The
useSelectTokenWidgetViewStatecall centralizes view state computation with all necessary dependencies passed through.
85-85: LGTM - Type re-export for public API.Re-exporting
SelectTokenWidgetViewPropsallows consumers to type their props when implementing custom views.
78-83: Verify shouldRender condition matches previous behavior.The condition
widgetState.onSelectToken && (widgetState.open || widgetState.forceOpen)determines when the widget renders. Ensure this matches the previous implementation - particularly thatonSelectTokenbeing falsy should prevent rendering.#!/bin/bash # Search for previous shouldRender or open/forceOpen logic patterns rg -n "shouldRender|forceOpen|\.open" --type ts apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/ -C 3apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerViewState.ts (1)
122-136:resolveActiveChainId()ordering looks sensible; helper is appropriately defensive.apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx (1)
67-125: Blocking view selection is clean and readable; ordering matches expected precedence.
| export function useSelectTokenModalPropsMemo(props: SelectTokenModalProps): SelectTokenModalProps { | ||
| return useMemo( | ||
| () => ({ | ||
| standalone: props.standalone, | ||
| displayLpTokenLists: props.displayLpTokenLists, | ||
| unsupportedTokens: props.unsupportedTokens, | ||
| selectedToken: props.selectedToken, | ||
| allTokens: props.allTokens, | ||
| favoriteTokens: props.favoriteTokens, | ||
| recentTokens: props.recentTokens, | ||
| balancesState: props.balancesState, | ||
| permitCompatibleTokens: props.permitCompatibleTokens, | ||
| onSelectToken: props.onSelectToken, | ||
| onTokenListItemClick: props.onTokenListItemClick, | ||
| onInputPressEnter: props.onInputPressEnter, | ||
| onDismiss: props.onDismiss, | ||
| onOpenManageWidget: props.onOpenManageWidget, | ||
| openPoolPage: props.openPoolPage, | ||
| tokenListCategoryState: props.tokenListCategoryState, | ||
| disableErc20: props.disableErc20, | ||
| account: props.account, | ||
| areTokensLoading: props.areTokensLoading, | ||
| tokenListTags: props.tokenListTags, | ||
| areTokensFromBridge: props.areTokensFromBridge, | ||
| isRouteAvailable: props.isRouteAvailable, | ||
| modalTitle: props.modalTitle, | ||
| hasChainPanel: props.hasChainPanel, | ||
| hideFavoriteTokensTooltip: props.hideFavoriteTokensTooltip, | ||
| chainsPanelTitle: props.chainsPanelTitle, | ||
| selectedTargetChainId: props.selectedTargetChainId, | ||
| onSelectChain: props.onSelectChain, | ||
| onClearRecentTokens: props.onClearRecentTokens, | ||
| }), | ||
| [ | ||
| props.standalone, | ||
| props.displayLpTokenLists, | ||
| props.unsupportedTokens, | ||
| props.selectedToken, | ||
| props.allTokens, | ||
| props.favoriteTokens, | ||
| props.recentTokens, | ||
| props.balancesState, | ||
| props.permitCompatibleTokens, | ||
| props.onSelectToken, | ||
| props.onTokenListItemClick, | ||
| props.onInputPressEnter, | ||
| props.onDismiss, | ||
| props.onOpenManageWidget, | ||
| props.openPoolPage, | ||
| props.tokenListCategoryState, | ||
| props.disableErc20, | ||
| props.account, | ||
| props.areTokensLoading, | ||
| props.tokenListTags, | ||
| props.areTokensFromBridge, | ||
| props.isRouteAvailable, | ||
| props.modalTitle, | ||
| props.hasChainPanel, | ||
| props.chainsPanelTitle, | ||
| props.hideFavoriteTokensTooltip, | ||
| props.selectedTargetChainId, | ||
| props.onSelectChain, | ||
| props.onClearRecentTokens, | ||
| ], | ||
| ) |
There was a problem hiding this comment.
BUG: useSelectTokenModalPropsMemo() drops chainsToSelect (and doesn’t depend on it).
buildSelectTokenModalPropsInput() provides chainsToSelect, but the memo hook doesn’t include it in the returned object or deps, so the modal can lose chain state (or become stale).
export function useSelectTokenModalPropsMemo(props: SelectTokenModalProps): SelectTokenModalProps {
return useMemo(
() => ({
@@
modalTitle: props.modalTitle,
hasChainPanel: props.hasChainPanel,
+ chainsToSelect: props.chainsToSelect,
hideFavoriteTokensTooltip: props.hideFavoriteTokensTooltip,
chainsPanelTitle: props.chainsPanelTitle,
selectedTargetChainId: props.selectedTargetChainId,
onSelectChain: props.onSelectChain,
onClearRecentTokens: props.onClearRecentTokens,
}),
[
@@
props.modalTitle,
props.hasChainPanel,
+ props.chainsToSelect,
props.chainsPanelTitle,
props.hideFavoriteTokensTooltip,
props.selectedTargetChainId,
props.onSelectChain,
props.onClearRecentTokens,
],
)
}📝 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.
| export function useSelectTokenModalPropsMemo(props: SelectTokenModalProps): SelectTokenModalProps { | |
| return useMemo( | |
| () => ({ | |
| standalone: props.standalone, | |
| displayLpTokenLists: props.displayLpTokenLists, | |
| unsupportedTokens: props.unsupportedTokens, | |
| selectedToken: props.selectedToken, | |
| allTokens: props.allTokens, | |
| favoriteTokens: props.favoriteTokens, | |
| recentTokens: props.recentTokens, | |
| balancesState: props.balancesState, | |
| permitCompatibleTokens: props.permitCompatibleTokens, | |
| onSelectToken: props.onSelectToken, | |
| onTokenListItemClick: props.onTokenListItemClick, | |
| onInputPressEnter: props.onInputPressEnter, | |
| onDismiss: props.onDismiss, | |
| onOpenManageWidget: props.onOpenManageWidget, | |
| openPoolPage: props.openPoolPage, | |
| tokenListCategoryState: props.tokenListCategoryState, | |
| disableErc20: props.disableErc20, | |
| account: props.account, | |
| areTokensLoading: props.areTokensLoading, | |
| tokenListTags: props.tokenListTags, | |
| areTokensFromBridge: props.areTokensFromBridge, | |
| isRouteAvailable: props.isRouteAvailable, | |
| modalTitle: props.modalTitle, | |
| hasChainPanel: props.hasChainPanel, | |
| hideFavoriteTokensTooltip: props.hideFavoriteTokensTooltip, | |
| chainsPanelTitle: props.chainsPanelTitle, | |
| selectedTargetChainId: props.selectedTargetChainId, | |
| onSelectChain: props.onSelectChain, | |
| onClearRecentTokens: props.onClearRecentTokens, | |
| }), | |
| [ | |
| props.standalone, | |
| props.displayLpTokenLists, | |
| props.unsupportedTokens, | |
| props.selectedToken, | |
| props.allTokens, | |
| props.favoriteTokens, | |
| props.recentTokens, | |
| props.balancesState, | |
| props.permitCompatibleTokens, | |
| props.onSelectToken, | |
| props.onTokenListItemClick, | |
| props.onInputPressEnter, | |
| props.onDismiss, | |
| props.onOpenManageWidget, | |
| props.openPoolPage, | |
| props.tokenListCategoryState, | |
| props.disableErc20, | |
| props.account, | |
| props.areTokensLoading, | |
| props.tokenListTags, | |
| props.areTokensFromBridge, | |
| props.isRouteAvailable, | |
| props.modalTitle, | |
| props.hasChainPanel, | |
| props.chainsPanelTitle, | |
| props.hideFavoriteTokensTooltip, | |
| props.selectedTargetChainId, | |
| props.onSelectChain, | |
| props.onClearRecentTokens, | |
| ], | |
| ) | |
| export function useSelectTokenModalPropsMemo(props: SelectTokenModalProps): SelectTokenModalProps { | |
| return useMemo( | |
| () => ({ | |
| standalone: props.standalone, | |
| displayLpTokenLists: props.displayLpTokenLists, | |
| unsupportedTokens: props.unsupportedTokens, | |
| selectedToken: props.selectedToken, | |
| allTokens: props.allTokens, | |
| favoriteTokens: props.favoriteTokens, | |
| recentTokens: props.recentTokens, | |
| balancesState: props.balancesState, | |
| permitCompatibleTokens: props.permitCompatibleTokens, | |
| onSelectToken: props.onSelectToken, | |
| onTokenListItemClick: props.onTokenListItemClick, | |
| onInputPressEnter: props.onInputPressEnter, | |
| onDismiss: props.onDismiss, | |
| onOpenManageWidget: props.onOpenManageWidget, | |
| openPoolPage: props.openPoolPage, | |
| tokenListCategoryState: props.tokenListCategoryState, | |
| disableErc20: props.disableErc20, | |
| account: props.account, | |
| areTokensLoading: props.areTokensLoading, | |
| tokenListTags: props.tokenListTags, | |
| areTokensFromBridge: props.areTokensFromBridge, | |
| isRouteAvailable: props.isRouteAvailable, | |
| modalTitle: props.modalTitle, | |
| hasChainPanel: props.hasChainPanel, | |
| chainsToSelect: props.chainsToSelect, | |
| hideFavoriteTokensTooltip: props.hideFavoriteTokensTooltip, | |
| chainsPanelTitle: props.chainsPanelTitle, | |
| selectedTargetChainId: props.selectedTargetChainId, | |
| onSelectChain: props.onSelectChain, | |
| onClearRecentTokens: props.onClearRecentTokens, | |
| }), | |
| [ | |
| props.standalone, | |
| props.displayLpTokenLists, | |
| props.unsupportedTokens, | |
| props.selectedToken, | |
| props.allTokens, | |
| props.favoriteTokens, | |
| props.recentTokens, | |
| props.balancesState, | |
| props.permitCompatibleTokens, | |
| props.onSelectToken, | |
| props.onTokenListItemClick, | |
| props.onInputPressEnter, | |
| props.onDismiss, | |
| props.onOpenManageWidget, | |
| props.openPoolPage, | |
| props.tokenListCategoryState, | |
| props.disableErc20, | |
| props.account, | |
| props.areTokensLoading, | |
| props.tokenListTags, | |
| props.areTokensFromBridge, | |
| props.isRouteAvailable, | |
| props.modalTitle, | |
| props.hasChainPanel, | |
| props.chainsToSelect, | |
| props.chainsPanelTitle, | |
| props.hideFavoriteTokensTooltip, | |
| props.selectedTargetChainId, | |
| props.onSelectChain, | |
| props.onClearRecentTokens, | |
| ], | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/controllerProps.ts
around lines 185 to 249, the memoized props object omits chainsToSelect and the
useMemo dependency array does not include it, causing chain selection state to
be lost or stale; update the returned object to include chainsToSelect:
props.chainsToSelect and add props.chainsToSelect to the dependency array so the
memo updates when chainsToSelect changes.
| // TODO: Re-enable once Yield should support cross-network selection in the modal. | ||
| const ENABLE_YIELD_CHAIN_PANEL = false | ||
|
|
||
| export function useSelectTokenWidgetViewState(args: SelectTokenWidgetViewStateArgs): ViewStateResult { | ||
| const { | ||
| displayLpTokenLists, | ||
| standalone, | ||
| widgetState, | ||
| chainsToSelect, | ||
| onSelectChain, | ||
| manageWidget, | ||
| updateSelectTokenWidget, | ||
| account, | ||
| closeTokenSelectWidget, | ||
| tokenData, | ||
| onTokenListAddingError, | ||
| tokenAdminActions, | ||
| widgetMetadata, | ||
| walletChainId, | ||
| isBridgeFeatureEnabled, | ||
| } = args | ||
|
|
||
| const activeChainId = resolveActiveChainId(widgetState, walletChainId) | ||
| const widgetDeps = useWidgetViewDependencies({ | ||
| manageWidget, | ||
| closeTokenSelectWidget, | ||
| updateSelectTokenWidget, | ||
| tokenData, | ||
| tokenAdminActions, | ||
| onTokenListAddingError, | ||
| widgetState, | ||
| activeChainId, | ||
| }) | ||
| const shouldDisableChainPanelForYield = widgetState.tradeType === TradeType.YIELD && !ENABLE_YIELD_CHAIN_PANEL | ||
| const isChainPanelEnabled = | ||
| isBridgeFeatureEnabled && hasAvailableChains(chainsToSelect) && !shouldDisableChainPanelForYield | ||
| const selectTokenModalProps = useWidgetModalProps({ | ||
| account, | ||
| chainsToSelect, | ||
| displayLpTokenLists, | ||
| widgetDeps, | ||
| hasChainPanel: isChainPanelEnabled, | ||
| onSelectChain, | ||
| recentTokens: widgetDeps.recentTokens, | ||
| standalone, | ||
| tokenData, | ||
| widgetMetadata, | ||
| widgetState, | ||
| isInjectedWidgetMode: isInjectedWidget(), | ||
| }) |
There was a problem hiding this comment.
Keep a single source of truth for “chain panel enabled” vs “visible”.
This hook clearly defines isChainPanelEnabled and passes it into modal-props construction. Ensure the view layer does not later override hasChainPanel based on layout/visibility (see index.tsx), otherwise Yield/feature-flag gating can be bypassed or chain selection can disappear on mobile.
| const showDesktopChainPanel = isChainPanelVisible && isChainPanelEnabled && chainsToSelect | ||
|
|
||
| const { | ||
| tokens: allTokens, | ||
| isLoading: areTokensLoading, | ||
| favoriteTokens, | ||
| areTokensFromBridge, | ||
| isRouteAvailable, | ||
| } = useTokensToSelect() | ||
| const { recentTokens, addRecentToken, clearRecentTokens } = useRecentTokens({ | ||
| allTokens, | ||
| favoriteTokens, | ||
| activeChainId: selectedTargetChainId ?? walletChainId, | ||
| }) | ||
| const handleTokenListItemClick = useCallback( | ||
| (token: TokenWithLogo) => { | ||
| addRecentToken(token) | ||
| }, | ||
| [addRecentToken], | ||
| return ( | ||
| <> | ||
| <ModalContainer> | ||
| <SelectTokenModal {...selectTokenModalProps} hasChainPanel={isChainPanelVisible} /> | ||
| </ModalContainer> | ||
| {showDesktopChainPanel && ( | ||
| <ChainPanel title={chainsPanelTitle} chainsState={chainsToSelect} onSelectChain={onSelectChain} /> | ||
| )} | ||
| </> |
There was a problem hiding this comment.
BUG: SelectTokenModal overrides hasChainPanel after spread (breaks enablement gating / mobile access).
selectTokenModalProps already contains the correct hasChainPanel derived from controller state; overriding it with isChainPanelVisible conflates “layout sidebar visible” with “feature enabled”.
- <SelectTokenModal {...selectTokenModalProps} hasChainPanel={isChainPanelVisible} />
+ <SelectTokenModal {...selectTokenModalProps} />If the modal truly needs a layout signal, pass a separate prop (or rename at the pure component level) rather than reusing hasChainPanel.
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx
around lines 53 to 63, the component spread `selectTokenModalProps` already
contains the correct hasChainPanel from controller state but you are overriding
it with `isChainPanelVisible` which conflates layout visibility with feature
enablement; fix by removing the override so the modal receives the hasChainPanel
from selectTokenModalProps, or if you need to convey layout-only visibility pass
a separately named prop (e.g., layoutHasChainPanel or isSidebarVisible) instead
of reusing hasChainPanel.
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Approving to get this moving forward, however:
- There's the issue I pointed out in the previous entry
- The
controllerpattern is only used on https://github.com/cowprotocol/cowswap/blob/develop/apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts, and at least from how it's implemented here doesn't look great.
There seems to be a lot of repetition on the types and props, and the mapping between them.
I like how it makes the fns smaller though.
We can discuss this in a more in depth architectural review.
Anyway, maybe those concerns have been addressed in a future PR as it's mentioned in a reply to Denis.
…/token-selector-9 # Conflicts: # apps/cowswap-frontend/src/modules/tokensList/hooks/recentTokensStorage.ts
|
@alfetopito thanks for reviewing:
|
Summary
This PR splits the selector into a proper controller/view structure without changing the UI:
No visual changes yet—this is purely to unblock the subsequent UX PR.
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.