feat(tokenselector): 8 - enhance SelectTokenModal with chain selection #6529
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 ✨ 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 |
fairlighteth
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| ))} | ||
| </styledEl.Wrapper> | ||
| ) | ||
| const CHAIN_ACCENT_VAR_MAP: Record<SupportedChainId, ChainAccentVars> = { |
There was a problem hiding this comment.
nitpick: I think it's better to move it to another const file to make the component file more clear
There was a problem hiding this comment.
This has been addressed.
| const legacyChainsState = | ||
| !showChainPanel && chainsToSelect && (chainsToSelect.chains?.length ?? 0) > 0 ? chainsToSelect : undefined | ||
| const resolvedChainPanelTitle = chainsPanelTitle ?? t`Cross chain swap` | ||
| const chainPanel = |
There was a problem hiding this comment.
nitpick: I think it makes sense to wrap it by useMemo
const chainPanel = useMemo(
() =>
showChainPanel && chainsToSelect ? (
) : null,
[showChainPanel, chainsToSelect, onSelectChain, resolvedChainPanelTitle],
)
There was a problem hiding this comment.
This has been addressed.
Resolved conflicts by accepting removal of SelectTokenModalContent wrapper (inlined in feat/token-selector-4).
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Same as previous:
Approving with that caveat
| @@ -1,4 +1,4 @@ | |||
| /* eslint-disable no-restricted-imports */ // TODO: Don't use 'modules' import | |||
| // TODO: Don't use 'modules' import | |||
There was a problem hiding this comment.
Is this comment still relevant?
| <styledEl.ChainText>{chain.label}</styledEl.ChainText> | ||
| </styledEl.ChainInfo> | ||
| {isActive && ( | ||
| <styledEl.ActiveIcon aria-hidden="true" accent$={accent} color$={chain.color}> |
There was a problem hiding this comment.
Tiny nipick:
| <styledEl.ActiveIcon aria-hidden="true" accent$={accent} color$={chain.color}> | |
| <styledEl.ActiveIcon aria-hidden accent$={accent} color$={chain.color}> |
Summary
Delivered the Step 8 desktop chain sidebar: added a reusable ChainPanel (search, loading, empty states) plus a vertical/accented ChainsSelector, and wired both into SelectTokenModal so bridging flows render the sidebar while non-bridging contexts fall back to a single-column layout. SelectTokenWidget now gates the panel via useIsBridgingEnabled and provides localized titles.
To Test
Background
Part of the token-selector split rollout (Step 8): migrate the desktop chain sidebar from the monolithic branch while keeping the mobile experience unchanged ahead of Step 9.