-
Notifications
You must be signed in to change notification settings - Fork 122
feat: refactor and add more n 8000 etworks to the token details page swap w… #5900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
""" WalkthroughThe SwapWidget component was refactored for improved type safety, modularity, and UI consistency. Network and token mappings were centralized, reusable logic was extracted, and support for multiple networks was expanded. The network constants file now includes chain IDs and default tokens for each network, enhancing configuration and maintainability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SwapWidget
participant NetworkMap
User->>SwapWidget: Selects network or tab, enters amount
SwapWidget->>NetworkMap: Fetches network ID and default tokens
SwapWidget->>SwapWidget: Determines buy/sell tokens via getBuyAndSellTokens
SwapWidget->>SwapWidget: Generates swap URL via getSwapUrl
SwapWidget->>User: Renders updated UI with correct token and network info
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings
apps/cow-fi/components/SwapWidget.tsx (9)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (11)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cow-fi/components/SwapWidget.tsx (1)
239-294
: Consider extracting image dimensions to reduce duplication.The function works correctly, but the hardcoded width and height values are repeated multiple times.
Consider extracting the dimensions:
const getDropdownBody = (platforms: Platforms, handleSelect: (network: string) => void): JSX.Element => { const { ethereum, xdai, base, 'arbitrum-one': arbitrum, avalanche, 'polygon-pos': polygon } = platforms - const width = 20 - const height = 20 + const IMAGE_SIZE = { width: 20, height: 20 } return ( <DropdownBody> {ethereum?.contractAddress && ( <DropdownOption => handleSelect('ethereum')}> - <Image src={NETWORK_IMAGE_MAP.ethereum} alt={NETWORK_MAP.ethereum} width={width} height={height} /> + <Image src={NETWORK_IMAGE_MAP.ethereum} alt={NETWORK_MAP.ethereum} {...IMAGE_SIZE} /> {NETWORK_MAP.ethereum} </DropdownOption> )} {/* Apply to other Image components */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cow-fi/components/SwapWidget.tsx
(3 hunks)apps/cow-fi/const/networkMap.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
Learnt from: cowdan
PR: cowprotocol/cowswap#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.
apps/cow-fi/const/networkMap.ts (1)
Learnt from: cowdan
PR: cowprotocol/cowswap#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.
apps/cow-fi/components/SwapWidget.tsx (9)
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5830
File: apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx:1-2
Timestamp: 2025-06-16T15:58:00.268Z
Learning: JSX can be imported as a named export from React in modern React versions (React 17+). The import `import { JSX } from 'react'` is valid and does not cause compilation errors.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5831
File: apps/cowswap-frontend/src/modules/application/containers/AppMenu/index.tsx:7-9
Timestamp: 2025-06-16T16:01:46.729Z
Learning: React Router v7 restructured packages - NavLink and other core routing components should be imported from 'react-router' (not 'react-router-dom'). In v7, 'react-router-dom' mainly re-exports for backward compatibility, while 'react-router' is the new preferred import path for most components.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5831
File: apps/cowswap-frontend/src/modules/application/containers/AppMenu/index.tsx:7-9
Timestamp: 2025-06-16T16:01:46.729Z
Learning: In React Router v6+, NavLink can be imported directly from 'react-router' package, not just 'react-router-dom'. The package structure has evolved and the import from 'react-router' is valid for the latest versions.
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5782
File: apps/cow-fi/app/(learn)/learn/topics/page.tsx:1-1
Timestamp: 2025-06-05T08:31:01.284Z
Learning: Next.js App Router official documentation states that Client Components marked with 'use client' should NOT be declared as async functions, as this can lead to infinite loops or application hangs. The recommended pattern for async operations in Client Components is to use useEffect hooks. Server Components (without 'use client') can be async, which might be a source of confusion.
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5780
File: CONTRIBUTING.md:121-121
Timestamp: 2025-06-02T12:52:34.606Z
Learning: React 19.0.0 was released on December 5, 2024, introducing new features like Actions, Server Components, and hooks such as useActionState and useOptimistic.
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5782
File: apps/cow-fi/app/(learn)/learn/topics/page.tsx:1-1
Timestamp: 2025-06-05T08:31:01.284Z
Learning: Next.js App Router supports async client components with the 'use client' directive. Unlike traditional React, where client components must be synchronous, Next.js App Router allows client components to be async functions. The async operations execute on the client side after hydration, enabling components that use client-side hooks to also perform data fetching. This pattern is commonly used when components need both interactive client-side features and async data operations.
Learnt from: cowdan
PR: cowprotocol/cowswap#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.
🔇 Additional comments (9)
apps/cow-fi/const/networkMap.ts (2)
10-17
: LGTM: Chain IDs are correct.The network chain ID mappings are accurate and properly typed.
37-53
: LGTM: Default token mappings are appropriate.The default sell and buy token mappings use wrapped native currencies for each network, which is the standard approach. The use of WPOL for Polygon correctly reflects the recent MATIC to POL rebrand.
apps/cow-fi/components/SwapWidget.tsx (7)
1-1
: LGTM: Import improvements enhance maintainability.The addition of JSX import (valid in React 17+), Next.js Image for optimization, and centralized network constants improves code organization and performance.
Also applies to: 5-5, 13-20
205-207
: LGTM: Improved type safety with union types.The explicit typing of Tab as a union type and default constants improve type safety and code clarity.
226-237
: LGTM: Clean modularization of tab rendering.The Tabs component is well-structured with proper typing and clear separation of concerns.
320-331
: LGTM: Proper input validation for amount handling.The input validation correctly prevents negative values and handles numeric conversion appropriately.
333-344
: LGTM: Proper swap URL generation.The URL generation correctly uses the centralized network mappings and includes all necessary parameters for the swap interface.
355-356
: LGTM: Performance improvement with Next.js Image components.The switch from
<img>
tags to Next.js<Image>
components provides better performance through automatic optimization, and using centralized mappings improves maintainability.Also applies to: 366-366
209-224
: ```shell
#!/bin/bashLocate getBuyAndSellTokens usage and inspect URL construction in SwapWidget.tsx
Find where the utility is invoked
rg -n "getBuyAndSellTokens" apps/cow-fi/components/SwapWidget.tsx
Dump lines around the invocation to see how sellToken/buyToken are embedded in the URL
sed -n '180,260p' apps/cow-fi/components/SwapWidget.tsx
Check for literal swap URL patterns in this file
rg -n "https://swap.cow.fi" apps/cow-fi/components/SwapWidget.tsx
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
const [network, setNetwork] = useState<string | null>(null) | ||
return ( | ||
<DropdownBody> | ||
{ethereum?.contractAddress && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function DropdownNetworkOption({ network }: { network: Network }) {
return (
<DropdownOption => handleSelect(network)}>
<Image src={NETWORK_IMAGE_MAP[network]} alt={NETWORK_MAP[network]} width={width} height={height} />
{NETWORK_MAP[network]}
</DropdownOption>
)
}
{ ethereum?.contractAddress && <DropdownNetworkOption network="ethereum" /> }
{ base?.contractAddress && <DropdownNetworkOption network="base" /> }
...
else if (arbitrum?.contractAddress) setNetwork('arbitrum-one') | ||
else if (polygon?.contractAddress) setNetwork('polygon-pos') | ||
else if (avalanche?.contractAddress) setNetwork('avalanche') | ||
else if (xdai?.contractAddress) setNetwork('xdai') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function getNetworkFromPlatforms(platforms): Network {
const { ethereum, xdai, base, 'arbitrum-one': arbitrum, avalanche, 'polygon-pos': polygon } = platforms
if (ethereum?.contractAddress) return 'ethereum'
if (base?.contractAddress) return 'base'
if (arbitrum?.contractAddress) return 'arbitrum-one'
...
}
useEffect(() => {
setNetwork(getNetworkFromPlatforms(platforms))
}, [platforms])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just needs some refacting to get rid of code duplcations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @cowdan , works great, thank you.
FYI: I've link the original task that is fixed by this PR
setNetwork(network) | ||
setIsOpen(false) | ||
} | ||
|
||
function getNetworkFromPlatforms(platforms: Platforms): Network { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function should be outside of the component body
…idget
Summary
Fixes #5892
To Test
[/tokens](https://cowfi-mqb5k0j8h-cowswap.vercel.app/tokens)
Aave
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements