feat(tokenselector): 7 - implement SelectTokenModal with enhanced token selection #6527
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 WalkthroughThis PR refactors the SelectTokenModal component from a monolithic structure into a modular, composition-based architecture. It extracts helpers, styled components, and type definitions into separate modules; introduces new components (TokenColumnContent, TitleBarActions); and reorganizes the modal's internal layout using a shell pattern with dedicated content subcomponents. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx
Outdated
Show resolved
Hide resolved
| ) | ||
| } | ||
|
|
||
| function renderLegacyChainSelector( |
There was a problem hiding this comment.
also will be better to move it from this file
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.ts
Show resolved
Hide resolved
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.ts (1)
13-13: Leaving the TODO is fine, but link it to a tracking issue/PR if one exists.
(There’s been prior feedback about prop drilling; a concrete reference helps prevent TODO rot.)
🧹 Nitpick comments (3)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx (1)
31-43: Prefer a simplerchainsMockconstruction (avoidreducefor readability).
Current code works, butfilter(Boolean).map(...)(orflatMap) is easier to scan and avoids the manualaccmutation.apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsx (1)
13-27: Consider reusing an existing exported “category state” type instead of re-defining it here.
Helps keep the prop surface consistent (especially since the modal already centralizes types intypes.ts).apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.ts (1)
14-37: Tighten chain ID types (avoid plainnumberwhere possible).
If these are alwaysSupportedChainId/ChainId, typing them as such will prevent invalid IDs from leaking into UI logic (defaultChainId,selectedTargetChainId).Also applies to: 39-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsx(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/helpers.tsx(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx(4 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx(2 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/styled.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 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/pure/SelectTokenModal/helpers.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.tsapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/styled.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/pure/SelectTokenModal/helpers.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.tsapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/styled.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/pure/SelectTokenModal/helpers.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.tsapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx
📚 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/pure/SelectTokenModal/types.tsapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.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/pure/SelectTokenModal/types.tsapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx
📚 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/tokensList/pure/SelectTokenModal/types.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/pure/SelectTokenModal/types.tsapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx
📚 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/pure/SelectTokenModal/index.tsxapps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx
🧬 Code graph analysis (4)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/helpers.tsx (2)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.ts (1)
SelectTokenModalProps(54-56)apps/cowswap-frontend/src/modules/tokensList/types.ts (1)
SelectTokenContext(13-24)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/types.ts (5)
libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)libs/tokens/src/types.ts (2)
UnsupportedTokensState(35-35)TokenListTags(56-56)apps/cowswap-frontend/src/modules/permit/types.ts (1)
PermitCompatibleTokens(51-51)apps/cowswap-frontend/src/modules/tokensList/types.ts (2)
ChainsToSelectState(26-30)TokenSelectionHandler(11-11)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx (1)
SelectTokenModalProps(15-15)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsx (4)
apps/cowswap-frontend/src/modules/tokensList/types.ts (2)
TokenSelectionHandler(11-11)ChainsToSelectState(26-30)apps/cowswap-frontend/src/modules/tokensList/containers/LpTokenListsWidget/index.tsx (1)
LpTokenListsWidget(57-110)apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/SelectTokenModalContent.tsx (1)
SelectTokenModalContent(12-24)apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx (1)
ChainsSelector(32-112)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/styled.ts (2)
libs/ui/src/consts.ts (1)
Media(23-43)apps/cowswap-frontend/src/modules/tokensList/pure/commonElements.ts (1)
blankButtonMixin(5-16)
🔇 Additional comments (8)
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx (1)
18-18: Fixture updates (wider modal +modalTitle) look consistent with the new shell.Also applies to: 71-71
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsx (1)
65-85: Legacy chain selector gating looks good (renders only when chains exist).apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/helpers.tsx (1)
13-47:useSelectTokenContextmemoization and dependency list look solid.apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx (2)
129-141: Enter key handling: verify it doesn’t trigger unintended parent handlers.
Depending on how the modal is mounted, you may needevent.preventDefault()/event.stopPropagation()when Enter is pressed (to avoid “submit”/close shortcuts).
52-94: Shell composition +legacyChainsStategating is clean and matches the rollout goals.Also applies to: 109-149
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/styled.ts (3)
81-100: SearchInput styling is tightly coupled toSearchInput’s internal DOM (> div).
If@cowprotocol/uichanges markup, this breaks silently—worth confirming the DOM contract is stable or adding a wrapper element you control around the input.
24-156: New layout primitives (title/search/body/legacy chain wrapper) look consistent and reusable.Also applies to: 172-187
7-22: The$hasChainPanelprop appears to be unused in the codebase.The
hasChainPanelprop is defined in the component signature but defaults tofalseand is never passed astruefrom the container component (SelectTokenWidgetContent) or in any fixtures. The CSS cascade concern aboutMedia.upToMedium()not overridingborder-*-right-radiusis technically valid, but this code path never executes in practice. Consider either removing the dead code (including the associated border-radius logic) or clarifying the intended use of this prop. IfhasChainPanelis meant for future use, the media query should explicitly override the right-corner radius properties.Likely an incorrect or invalid review comment.
| export function TitleBarActions({ | ||
| showManageButton, | ||
| onDismiss, | ||
| onOpenManageWidget, | ||
| title, | ||
| }: TitleBarActionsProps): ReactNode { | ||
| return ( | ||
| <styledEl.TitleBar> | ||
| <styledEl.TitleGroup> | ||
| <BackButton onClick={onDismiss} /> | ||
| <styledEl.ModalTitle>{title}</styledEl.ModalTitle> | ||
| </styledEl.TitleGroup> | ||
| {showManageButton && ( | ||
| <styledEl.TitleActions> | ||
| <styledEl.TitleActionButton | ||
| id="token-selector-title-manage-button" | ||
| onClick={onOpenManageWidget} | ||
| aria-label="Manage token lists" | ||
| title="Manage token lists" | ||
| > | ||
| <SettingsIcon /> | ||
| </styledEl.TitleActionButton> | ||
| </styledEl.TitleActions> | ||
| )} | ||
| </styledEl.TitleBar> | ||
| ) |
There was a problem hiding this comment.
Localize the “Manage token lists” labels (aria-label/title).
These are user-facing strings and currently bypass i18n, which can break the “switch locales” acceptance criteria.
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/helpers.tsx
around lines 62 to 87, the aria-label and title for the manage button are
hardcoded ("Manage token lists"); replace them with localized strings by using
the project's i18n hook (e.g., useTranslation/useI18n) at the top of the
TitleBarActions component, call the translation function to get localized values
for both aria-label and title, and swap the hardcoded literals for those
translated strings; also add corresponding translation keys (e.g.,
tokens.manageLists) to the locale JSON files so all locales have this string.
| chainsToSelect: { | ||
| chains: chainsMock, | ||
| isLoading: false, | ||
| defaultChainId: SupportedChainId.MAINNET, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx | head -80Repository: cowprotocol/cowswap
Length of output: 2790
🏁 Script executed:
cd apps/cowswap-frontend && grep -n "SEPOLIA\|MAINNET" src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsxRepository: cowprotocol/cowswap
Length of output: 238
🏁 Script executed:
cd apps/cowswap-frontend && head -50 src/modules/tokensList/pure/SelectTokenModal/index.tsxRepository: cowprotocol/cowswap
Length of output: 1499
🏁 Script executed:
cd apps/cowswap-frontend && grep -B5 -A5 "balancesState:" src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsxRepository: cowprotocol/cowswap
Length of output: 325
🏁 Script executed:
cd apps/cowswap-frontend && sed -n '31,43p' src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsxRepository: cowprotocol/cowswap
Length of output: 326
Fix chain ID mismatch in fixture: defaultChainId (MAINNET) vs balancesState.chainId (SEPOLIA).
The balancesState is set to SEPOLIA, which is not even in the available chains list (chainsMock contains only MAINNET, BASE, ARBITRUM_ONE). This causes inconsistency: the default selectable chain is MAINNET but balances display for an unavailable chain. Either add SEPOLIA to chainsMock and update defaultChainId accordingly, or change balancesState.chainId to MAINNET to match the default.
Also applies to: 66-67
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.cosmos.tsx
around lines 54-58 (and also apply same fix to lines 66-67), the fixture has a
chain ID mismatch: defaultChainId is SupportedChainId.MAINNET while
balancesState.chainId is SEPOLIA which is not present in chainsMock. Fix by
making fixtures consistent — either add SEPOLIA to chainsMock and set
defaultChainId to SEPOLIA (and ensure chainsMock ordering includes it), or
change balancesState.chainId to SupportedChainId.MAINNET so it matches the
existing chainsMock and defaultChainId; update any related mock data
accordingly.
| const [inputValue, setInputValue, trimmedInputValue] = useTokenSearchInput(defaultInputValue) | ||
| const selectTokenContext = useSelectTokenContext(props) | ||
| const resolvedModalTitle = modalTitle ?? t`Select token` | ||
| const legacyChainsState = | ||
| !hasChainPanel && chainsToSelect && (chainsToSelect.chains?.length ?? 0) > 0 ? chainsToSelect : undefined | ||
|
|
There was a problem hiding this comment.
Search value is inconsistently trimmed across content paths — consider standardizing.
TokensContent receives trimmedInputValue, but TokenColumnContent (and thus LP widget search) receives raw inputValue. If trimming is desired everywhere, pass the trimmed value consistently (or trim inside LP widget filter).
Also applies to: 64-75, 83-84
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/index.tsx
around lines 46-51 (and similarly at lines 64-75 and 83-84), the search string
is passed inconsistently: TokensContent gets trimmedInputValue while
TokenColumnContent and the LP widget get raw inputValue; standardize by passing
trimmedInputValue everywhere the search/filter prop is used (or, alternatively,
apply .trim() inside the LP widget/filter code). Update all places that
currently forward inputValue to instead forward trimmedInputValue so all token
lists and the LP widget use the same trimmed search value.
| export const TitleActionButton = styled.button` | ||
| ${blankButtonMixin}; | ||
| display: flex; | ||
| width: 100%; | ||
| align-items: center; | ||
| flex-direction: row; | ||
| justify-content: center; | ||
| gap: 10px; | ||
| padding: 2px; | ||
| border-radius: 8px; | ||
| cursor: pointer; | ||
| padding: 20px 0; | ||
| margin: 0; | ||
| font-size: 16px; | ||
| font-weight: 500; | ||
| color: inherit; | ||
| opacity: 0.6; | ||
| transition: opacity var(${UI.ANIMATION_DURATION}) ease-in-out; | ||
| transition: background var(${UI.ANIMATION_DURATION}) ease-in-out; | ||
| &:hover { | ||
| opacity: 1; | ||
| background: var(${UI.COLOR_PAPER_DARKER}); | ||
| } | ||
| ` |
There was a problem hiding this comment.
Add :focus-visible styles for clickable controls.
TitleActionButton, ListTitleActionButton, and ActionButton have hover states but no keyboard focus affordance, which is a common accessibility regression in refactors.
Also applies to: 158-170, 188-209
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/styled.ts
around lines 58-73 (also apply to 158-170 and 188-209), the styled button
components lack keyboard focus affordance; add a :focus-visible rule to each
clickable control that mirrors the hover visual but provides a clear keyboard
focus indicator (for example, set a visible outline or box-shadow and, if
appropriate, the same background change used on :hover), ensure the focus style
respects color/contrast and does not rely on :focus alone (use :focus-visible)
so keyboard users see the state while mouse users are unaffected.
| }: TokenColumnContentProps): ReactNode { | ||
| if (displayLpTokenLists) { | ||
| return ( | ||
| <LpTokenListsWidget | ||
| account={account} | ||
| search={inputValue} | ||
| onSelectToken={onSelectToken} | ||
| openPoolPage={openPoolPage} | ||
| disableErc20={disableErc20} | ||
| tokenListCategoryState={tokenListCategoryState} | ||
| > | ||
| {children} | ||
| </LpTokenListsWidget> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <LegacyChainSelector chainsToSelect={chainsToSelect} onSelectChain={onSelectChain} /> | ||
| <SelectTokenModalContent isRouteAvailable={isRouteAvailable}>{children}</SelectTokenModalContent> | ||
| </> | ||
| ) | ||
| } |
There was a problem hiding this comment.
isRouteAvailable is ignored when displayLpTokenLists is true.
Right now the “route not supported” message (SelectTokenModalContent) is only applied in the non-LP branch; if isRouteAvailable === false during LP view, the warning won’t render. Consider wrapping children with SelectTokenModalContent in both branches (or documenting why LP mode is exempt).
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/tokensList/pure/SelectTokenModal/TokenColumnContent.tsx
around lines 41 to 63, the isRouteAvailable prop is ignored when
displayLpTokenLists is true so the "route not supported" UI never shows for LP
view; update the LP branch to wrap the children passed into LpTokenListsWidget
with <SelectTokenModalContent
isRouteAvailable={isRouteAvailable}>...</SelectTokenModalContent> (or move
SelectTokenModalContent to wrap the entire returned JSX in both branches) so the
route-availability warning renders consistently; ensure props and children are
preserved and that LegacyChainSelector remains in the non-LP branch as currently
intended.
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Same comment as the previous PR, still broken (should I comment this until I reach the one where it's fixed?)
Some other minor comments that can be addressed in a follow up PR
| export function useTokenSearchInput(defaultInputValue = ''): [string, (value: string) => void, string] { | ||
| const [inputValue, setInputValue] = useState<string>(defaultInputValue) | ||
|
|
||
| return [inputValue, setInputValue, inputValue.trim()] |
There was a problem hiding this comment.
Memoize, maybe?
While the values are strings, it's returning a new array which is not? I might be wrong, not sure I'm mixing things up.
There was a problem hiding this comment.
Got this assessment on this, let me know if we should add it on this branch or not:
trim() returns a primitive, so deps like [searchInput,…] only rerun when the text actually changes; the tuple is destructured, so array identity doesn’t leak. Memoizing would be optional polish with negligible impact—only cost now is a trivial .trim() per render.
| aria-label="Manage token lists" | ||
| title="Manage token lists" |
There was a problem hiding this comment.
Should they be marked for translation?
There was a problem hiding this comment.
Yes, but addressed in branch 10 (05327ab9e)
Summary
Rebuilt the SelectTokenModal shell per Step 7 of the rollout plan so the header/search/chain sections match the upcoming sidebar-ready layout while preserving existing functionality.
Test
Summary by CodeRabbit
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.