-
Notifications
You must be signed in to change notification settings - Fork 159
refactor: centralize CoW Swap CTA cta constants #6725
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
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA new centralized CTA constant Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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: 0
🧹 Nitpick comments (1)
apps/cow-fi/components/Layout/const.ts (1)
114-119: Consider usingCOW_SWAP_CTA.actionfor analytics consistency.Line 117 uses
'Click Trade on CoW Swap'as the analytics action, while the centralized constant definesCOW_SWAP_CTA.actionas'click-trade-on-cow-swap'. This creates an inconsistency in analytics tracking compared to other locations (e.g., apps/cow-fi/app/(main)/cow-swap/page.tsx lines 95, 429) that useCOW_SWAP_CTA.action.🔎 Apply this diff to align with the centralized action:
onClick: () => analytics.sendEvent({ category: CowFiCategory.NAVIGATION, - action: 'Click Trade on CoW Swap', + action: COW_SWAP_CTA.action, label: 'menubar-nav-button', }),
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/cow-fi/app/(main)/cow-swap/page.tsx(5 hunks)apps/cow-fi/components/Layout/const.ts(2 hunks)apps/cow-fi/const/cta.ts(1 hunks)apps/cow-fi/data/home/const.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
📚 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/cow-fi/components/Layout/const.tsapps/cow-fi/const/cta.tsapps/cow-fi/data/home/const.tsxapps/cow-fi/app/(main)/cow-swap/page.tsx
📚 Learning: 2025-05-28T16:50:12.273Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 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.
Applied to files:
apps/cow-fi/components/Layout/const.tsapps/cow-fi/data/home/const.tsxapps/cow-fi/app/(main)/cow-swap/page.tsx
📚 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/cow-fi/components/Layout/const.tsapps/cow-fi/const/cta.tsapps/cow-fi/app/(main)/cow-swap/page.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/cow-fi/components/Layout/const.tsapps/cow-fi/const/cta.tsapps/cow-fi/data/home/const.tsxapps/cow-fi/app/(main)/cow-swap/page.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/cow-fi/const/cta.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cow-fi/data/home/const.tsxapps/cow-fi/app/(main)/cow-swap/page.tsx
📚 Learning: 2025-06-05T08:31:01.284Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 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.
Applied to files:
apps/cow-fi/app/(main)/cow-swap/page.tsx
🧬 Code graph analysis (3)
apps/cow-fi/components/Layout/const.ts (1)
apps/cow-fi/const/cta.ts (1)
COW_SWAP_CTA(1-7)
apps/cow-fi/data/home/const.tsx (1)
apps/cow-fi/const/cta.ts (1)
COW_SWAP_CTA(1-7)
apps/cow-fi/app/(main)/cow-swap/page.tsx (1)
apps/cow-fi/const/cta.ts (1)
COW_SWAP_CTA(1-7)
⏰ 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). (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (6)
apps/cow-fi/const/cta.ts (1)
1-7: LGTM! Excellent centralization of CTA configuration.The constant is well-structured with all necessary fields (text, action, utmContent, href, deeplinkHref) and properly typed with
as constfor immutability and type inference. This establishes a single source of truth for CoW Swap CTA configuration.apps/cow-fi/data/home/const.tsx (1)
12-12: LGTM! Proper integration of centralized CTA.The import organization is clean, and the CoW Swap product card correctly uses
COW_SWAP_CTA.deeplinkHref,COW_SWAP_CTA.text, andCOW_SWAP_CTA.action. The page-specificlinkUtmContenton line 53 remains distinct for tracking purposes, which is appropriate.Also applies to: 15-15, 49-51
apps/cow-fi/components/Layout/const.ts (1)
6-7: LGTM! Proper use of centralized CTA configuration.The navigation button correctly uses
COW_SWAP_CTA.textandCOW_SWAP_CTA.deeplinkHref, maintaining consistency with the centralized configuration.Also applies to: 111-112
apps/cow-fi/app/(main)/cow-swap/page.tsx (3)
3-3: LGTM! Good TypeScript practice.Adding the
ReactNodereturn type improves type safety and code clarity.Also applies to: 46-46
20-20: LGTM! Excellent integration of centralized CTA in hero section.The hero CTA correctly uses
COW_SWAP_CTA.href,COW_SWAP_CTA.utmContent,COW_SWAP_CTA.action, andCOW_SWAP_CTA.text, properly centralizing all CTA configuration. The use ofhref(base URL) instead ofdeeplinkHrefis appropriate for the landing page context.Also applies to: 88-99
422-434: LGTM! Consistent CTA implementation in footer section.The footer CTA mirrors the hero section's proper use of
COW_SWAP_CTAproperties, maintaining consistency across the page.
elena-zh
left a comment
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.
Thank you, looks good
Summary
COW_SWAP_CTA(text/action/utm/href/deeplink) and wired all current usages to it./cow-swapto use shared href/labels/analytics metadata./and/productsplus nav button (visible on all pages) to use the shared CTA config.To Test
Open
/cow-swaphttps://swap.cow.fi/, and firesclick-trade-on-cow-swapwith utmContentcow-swap-trade-on-cow-swap-button.Open
/(home)https://swap.cow.fi/#/1/swap/USDC/COW, and uses the shared action/utm metadata.Open
/productsAny page with the main menubar
https://swap.cow.fi/#/1/swap/USDC/COW.Background
CTA copy and tracking were duplicated across pages; the shared object keeps text/action/utm/urls consistent and easier to evolve.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.