feat(bridge): add collapsibl 8000 e banner for proxy account information #5895
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThis update refactors the banner components in the UI library, replacing the old Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QuoteSwapContent
participant ProxyAccountBanner
participant CollapsibleInlineBanner
User->>QuoteSwapContent: Opens quote swap UI
QuoteSwapContent->>ProxyAccountBanner: Renders with recipient, chainId
ProxyAccountBanner->>CollapsibleInlineBanner: Renders banner (collapsed/expanded)
User->>CollapsibleInlineBanner: Toggles banner for more/less info
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ 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 (
|
9bbc822 to
a35e9ba
Compare
| if (!isCollapsible) return | ||
|
|
||
| const newExpanded = !isExpanded | ||
| setIsExpanded(newExpanded) |
There was a problem hiding this comment.
P.s. it can be shorter:
setIsExpanded(state => !state)
|
|
||
| import { CollapsibleInlineBannerProps } from '../shared/types' | ||
|
|
||
| export function CollapsibleInlineBanner({ |
There was a problem hiding this comment.
- The component doesn't have external deps, so it's pure
- It looks generic enough and can be placed into
uilib
There was a problem hiding this comment.
Addressed, move it to a separate pure component (separate from InlineBanner folder too)
| import { InlineBannerProps } from '../shared/types' | ||
| import { renderIcon } from '../shared/utils' | ||
|
|
||
| export function InlineBanner({ |
There was a problem hiding this comment.
It's also pure, so it should be in pure directory instead of containers
| import { Icon, IconType } from '../../../pure/Icon' | ||
| import { StatusColorEnums, StatusColorVariant } from '../../../theme/statusColors' | ||
|
|
||
| export function renderIcon( |
There was a problem hiding this comment.
Render functions are not preferrable due to performance reasons, it's better to make it a component
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/ui/src/containers/InlineBanner/InlineBanner/index.tsx (1)
42-45: Consider simplifying the content wrapping logic.The conditional wrapping appears redundant since the content is already inside a span wrapper:
<span> {renderIcon(...)} {noWrapContent ? children : <span>{children}</span>} </span>Consider simplifying to:
<span> {renderIcon(...)} {children} </span>Unless there's a specific styling reason for the double span wrapper that isn't apparent from this context.
libs/ui/src/containers/InlineBanner/shared/utils.ts (1)
8-41: Consider safer type handling for icon property.The function logic is sound and handles all icon rendering scenarios appropriately. However, the type assertion on line 28 could be unsafe if
colorEnums.icondoesn't match theIconTypeenum.Consider adding runtime validation or using a type guard:
if (colorEnums.icon) { + // Type guard to ensure icon is valid IconType + const isValidIconType = Object.values(IconType).includes(colorEnums.icon as IconType) + if (!isValidIconType) { + console.warn(`Invalid icon type: ${colorEnums.icon}`) + return null + } + return React.createElement(Icon, { image: colorEnums.icon as IconType, size: iconSize, color: colorEnums.color, description: bannerType, padding: iconPadding, }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/cowswap-frontend/src/modules/bridge/pure/QuoteDetails/index.tsx(2 hunks)apps/cowswap-frontend/src/modules/cowShed/pure/CoWShedFAQ.tsx(2 hunks)apps/cowswap-frontend/src/modules/cowShed/pure/TokensInProxyBanner/index.tsx(2 hunks)apps/cowswap-frontend/src/modules/swap/containers/SwapConfirmModal/index.tsx(0 hunks)libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/index.tsx(1 hunks)libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/styled.ts(1 hunks)libs/ui/src/containers/InlineBanner/InlineBanner/index.tsx(1 hunks)libs/ui/src/containers/InlineBanner/InlineBanner/styled.ts(1 hunks)libs/ui/src/containers/InlineBanner/index.ts(1 hunks)libs/ui/src/containers/InlineBanner/index.tsx(0 hunks)libs/ui/src/containers/InlineBanner/shared/types.ts(1 hunks)libs/ui/src/containers/InlineBanner/shared/utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/src/modules/swap/containers/SwapConfirmModal/index.tsx
- libs/ui/src/containers/InlineBanner/index.tsx
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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: 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: shoom3301
PR: cowprotocol/cowswap#5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient address doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5762
File: apps/cowswap-frontend/src/legacy/state/orders/utils.ts:499-503
Timestamp: 2025-05-27T12:20:54.659Z
Learning: In the CowSwap frontend, when displaying volume fees in the UI (like in ReceiptModal), zero fees (0 bps) should be treated as "free" and hidden from display. Only non-zero fees should show the "Total fee" line. This provides a cleaner UX by not cluttering the interface with "0.00%" fee displays.
Learnt from: shoom3301
PR: cowprotocol/cowswap#5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
apps/cowswap-frontend/src/modules/cowShed/pure/TokensInProxyBanner/index.tsx (2)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient address doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
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.
libs/ui/src/containers/InlineBanner/InlineBanner/index.tsx (1)
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.
apps/cowswap-frontend/src/modules/bridge/pure/QuoteDetails/index.tsx (7)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient address doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
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: shoom3301
PR: cowprotocol/cowswap#5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
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#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: 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.
apps/cowswap-frontend/src/modules/cowShed/pure/CoWShedFAQ.tsx (2)
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: shoom3301
PR: cowprotocol/cowswap#5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient addr
67ED
ess doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
libs/ui/src/containers/InlineBanner/InlineBanner/styled.ts (1)
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.
libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/index.tsx (2)
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.
libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/styled.ts (1)
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.
🧬 Code Graph Analysis (5)
libs/ui/src/containers/InlineBanner/shared/utils.ts (1)
libs/ui/src/theme/statusColors.ts (1)
StatusColorEnums(13-20)
apps/cowswap-frontend/src/modules/bridge/pure/QuoteDetails/index.tsx (1)
libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/index.tsx (1)
CollapsibleInlineBanner(11-58)
apps/cowswap-frontend/src/modules/cowShed/pure/CoWShedFAQ.tsx (1)
libs/ui/src/containers/ExternalLink/index.tsx (1)
ExternalLink(50-89)
libs/ui/src/containers/InlineBanner/InlineBanner/styled.ts (1)
libs/ui/src/theme/statusColors.ts (1)
StatusColorEnums(13-20)
libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/index.tsx (2)
libs/ui/src/containers/InlineBanner/shared/types.ts (1)
CollapsibleInlineBannerProps(29-35)libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/styled.ts (4)
StyledCollapsibleBanner(60-66)ClickableWrapper(52-58)ToggleIconContainer(6-21)StyledToggleArrow(23-50)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cypress
🔇 Additional comments (19)
libs/ui/src/containers/InlineBanner/shared/types.ts (3)
5-8: Well-designed enum with clear naming convention.The
BannerOrientationenum uses consistent string values that clearly indicate the layout direction.
10-27: Comprehensive interface with good flexibility.The
InlineBannerPropsinterface provides extensive customization options while keeping all properties optional, allowing for flexible usage patterns.
29-35: Smart design pattern for collapsible content.The
CollapsibleInlineBannerPropsinterface cleverly omitschildrenfrom the base interface and instead requirescollapsedContentandexpandedContent, which is the appropriate pattern for a component that displays different content based on its state.libs/ui/src/containers/InlineBanner/index.ts (1)
1-3: Standard and well-organized re-export pattern.This consolidation creates a clean public API for the banner components, making imports easier for consumers.
apps/cowswap-frontend/src/modules/cowShed/pure/TokensInProxyBanner/index.tsx (2)
39-39: Improved grammar with proper article usage.Adding "the" before "following tokens" makes the sentence grammatically correct.
51-51: Enhanced clarity and sentence structure.The revised text improves readability by using conditional phrasing ("If something goes wrong") and clearer instructions ("using the Recover funds tab").
apps/cowswap-frontend/src/modules/bridge/pure/QuoteDetails/index.tsx (2)
4-6: Appropriate imports for the new banner functionality.The imports for
CollapsibleInlineBannerandAddressLinkare correctly added to support the new informational banner.
60-80: Well-implemented informational banner with good UX.The CollapsibleInlineBanner implementation provides excellent user experience:
- Uses appropriate styling (Info variant, horizontal orientation, 13px font size)
- Collapsed state is concise while expanded state provides helpful detail
- Consistent use of AddressLink for the recipient address in both states
- Clear messaging about proxy account functionality
- Logically positioned between swap and bridge sections
libs/ui/src/containers/InlineBanner/InlineBanner/index.tsx (3)
9-27: Well-structured component with comprehensive props.The component signature provides extensive customization options with sensible defaults, supporting the flexible design requirements.
47-47: Proper conditional rendering of close functionality.The close icon is correctly rendered only when an
onClosecallback is provided, maintaining clean UI when dismissal isn't needed.
52-52: Convenient re-export of shared types.Re-exporting the shared types from this component file provides easy access for consumers.
apps/cowswap-frontend/src/modules/cowShed/pure/CoWShedFAQ.tsx (1)
17-34: Text improvements enhance clarity and professionalism.The revised wording makes the FAQ more user-friendly and professional:
- "also known as" reads better than "aka"
- "improves" is more formal than "enhances"
- The contract deployment explanation is clearer and more structured
- The fund recovery explanation is more precise about potential edge cases
libs/ui/src/containers/InlineBanner/InlineBanner/styled.ts (2)
8-92: Well-structured styling with comprehensive customization options.The styled components provide excellent flexibility with proper theme integration and responsive design. The complex nested selectors appropriately target specific content elements within the banner structure.
94-109: Clean close icon implementation with proper positioning and interactions.The close icon styling is well-implemented with smooth transitions, proper positioning, and clear hover states using CSS variables and theme constants.
libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/index.tsx (2)
11-58: Excellent implementation with proper accessibility and state management.The component demonstrates solid React practices:
- Clean state management with appropriate default values
- Proper keyboard accessibility (Enter/Space key support)
- ARIA attributes for screen readers
- Guard clauses for conditional behavior
- Consistent event handling patterns
The conditional rendering logic correctly handles both collapsible and non-collapsible modes.
29-34: Good keyboard accessibility implementation.The keyboard event handler properly supports both Enter and Space keys with
preventDefault()to avoid unintended page scrolling, following accessibility best practices.libs/ui/src/containers/InlineBanner/CollapsibleInlineBanner/styled.ts (3)
6-21: Well-designed toggle icon container with smooth interactions.The styling provides excellent user feedback with smooth opacity and background transitions on hover, using consistent theme variables for timing and colors.
23-50: Smooth arrow rotation animation with proper sizing.The toggle arrow implementation is excellent:
- Clean rotation logic based on
isOpenstate- Consistent sizing with CSS custom properties
- Proper color inheritance and hover states
- Smooth transitions for both transform and fill properties
60-66: Appropriate extension of base InlineBanner with conditional padding.The collapsible banner correctly extends the base component while adjusting padding based on expansion state, maintaining visual hierarchy between collapsed and expanded states.
…ce CollapsibleInlineBanner
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
libs/ui/src/pure/InlineBanner/shared/utils.ts (1)
8-41: Consider using JSX for better readability.The function logic is correct and handles all icon rendering scenarios properly. However, the
React.createElementcalls could be simplified using JSX for better readability:if (customIcon) { return typeof customIcon === 'string' ? ( - React.createElement(SVG, { src: customIcon, width: iconSize, height: iconSize }) + <SVG src={customIcon} width={iconSize} height={iconSize} /> ) : ( customIcon ) } if (colorEnums.icon) { - return React.createElement(Icon, { - image: colorEnums.icon as IconType, - size: iconSize, - color: colorEnums.color, - description: bannerType, - padding: iconPadding, - }) + return ( + <Icon + image={colorEnums.icon as IconType} + size={iconSize} + color={colorEnums.color} + description={bannerType} + padding={iconPadding} + /> + ) } if (colorEnums.iconText) { - return React.createElement('i', null, colorEnums.iconText) + return <i>{colorEnums.iconText}</i> }libs/ui/src/pure/InlineBanner/InlineBanner/styled.ts (1)
8-35: Consider reducing prop complexity for better maintainability.The Wrapper component accepts many props (9 optional props), which provides flexibility but may indicate high complexity. While this supports the refactoring goals mentioned in the PR objectives, consider if some props could be grouped or if default styling could be extracted into design tokens.
The implementation correctly uses CSS variables and provides sensible defaults. The conditional padding logic for dismissable banners is well-handled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
libs/ui/src/index.ts(1 hunks)libs/ui/src/pure/CollapsibleInlineBanner/index.tsx(1 hunks)libs/ui/src/pure/CollapsibleInlineBanner/styled.ts(1 hunks)libs/ui/src/pure/DismissableInlineBanner/index.tsx(1 hunks)libs/ui/src/pure/InlineBanner/InlineBanner/index.tsx(1 hunks)libs/ui/src/pure/InlineBanner/InlineBanner/styled.ts(1 hunks)libs/ui/src/pure/InlineBanner/index.ts(1 hunks)libs/ui/src/pure/InlineBanner/shared/BannerIcon.tsx(1 hunks)libs/ui/src/pure/InlineBanner/shared/types.ts(1 hunks)libs/ui/src/pure/InlineBanner/shared/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- libs/ui/src/pure/InlineBanner/index.ts
- libs/ui/src/index.ts
- libs/ui/src/pure/CollapsibleInlineBanner/styled.ts
- libs/ui/src/pure/InlineBanner/shared/types.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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: shoom3301
PR: cowprotocol/cowswap#5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
libs/ui/src/pure/CollapsibleInlineBanner/index.tsx (1)
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.
libs/ui/src/pure/InlineBanner/shared/BannerIcon.tsx (1)
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.
libs/ui/src/pure/InlineBanner/InlineBanner/styled.ts (1)
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.
🧬 Code Graph Analysis (5)
libs/ui/src/pure/DismissableInlineBanner/index.tsx (1)
libs/ui/src/pure/InlineBanner/shared/types.ts (1)
InlineBannerProps(10-27)
libs/ui/src/pure/CollapsibleInlineBanner/index.tsx (2)
libs/ui/src/pure/InlineBanner/shared/types.ts (1)
CollapsibleInlineBannerProps(29-35)libs/ui/src/pure/CollapsibleInlineBanner/styled.ts (4)
StyledCollapsibleBanner(60-66)ClickableWrapper(52-58)ToggleIconContainer(6-21)StyledToggleArrow(23-50)
libs/ui/src/pure/InlineBanner/shared/BannerIcon.tsx (1)
libs/ui/src/theme/statusColors.ts (1)
StatusColorEnums(13-20)
libs/ui/src/pure/InlineBanner/shared/utils.ts (1)
libs/ui/src/theme/statusColors.ts (1)
StatusColorEnums(13-20)
libs/ui/src/pure/InlineBanner/InlineBanner/styled.ts (1)
libs/ui/src/theme/statusColors.ts (1)
StatusColorEnums(13-20)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (7)
libs/ui/src/pure/DismissableInlineBanner/index.tsx (1)
1-1: LGTM! Clean refactoring to support the new pure component structure.The import updates and explicit return type annotation properly align with the component migration from containers to pure modules.
Also applies to: 4-4, 10-10
libs/ui/src/pure/InlineBanner/InlineBanner/index.tsx (2)
9-57: Excellent implementation of the new InlineBanner component.The component demonstrates good practices:
- Clean prop destructuring with sensible defaults
- Proper separation of concerns with the BannerIcon component
- Conditional rendering logic is clear and efficient
- Type safety maintained throughout
59-59: Good practice re-exporting shared types.This maintains a clean API boundary for consumers of the component.
libs/ui/src/pure/CollapsibleInlineBanner/index.tsx (2)
11-58: Excellent implementation with proper accessibility and React best practices.The component demonstrates several strengths:
- Accessibility: Proper ARIA labels, keyboard navigation support (Enter/Space), and semantic button role
- State management: Clean useState implementation with optional callback support
- Performance: Early return for non-collapsible case avoids unnecessary rendering
- UX: Proper event handling with preventDefault for keyboard interactions
The conditional rendering logic is clear and the component API is well-designed.
60-60: Good practice re-exporting shared types.This maintains a consistent API for consumers who need access to the type definitions.
libs/ui/src/pure/InlineBanner/InlineBanner/styled.ts (2)
1-6: LGTM: Clean imports and proper dependencies.The imports are well-organized and appropriate for the styled components implementation.
94-109: LGTM: Well-implemented close icon with good UX.The CloseIcon implementation follows good practices:
- Uses CSS variables for consistent sizing
- Appropriate positioning and hover effects
- Good use of the UI.ANIMATION_DURATION constant for consistent transitions
- Proper cursor indication for interactivity
| // Icon + Text content wrapper | ||
| > span { | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| flex-flow: ${({ orientation = BannerOrientation.Vertical }) => | ||
| orientation === BannerOrientation.Horizontal ? 'row' : 'column wrap'}; | ||
| gap: 10px; | ||
| width: auto; | ||
| } | ||
|
|
||
| > span > svg > path { | ||
| fill: ${({ colorEnums }) => `var(${colorEnums.iconColor})`}; | ||
| } | ||
|
|
||
| // Text content | ||
| > span > span { | ||
| width: 100%; | ||
| display: flex; | ||
| flex-flow: row wrap; | ||
| align-items: center; | ||
| gap: 10px; | ||
| justify-content: ${({ orientation = BannerOrientation.Vertical }) => | ||
| orientation === BannerOrientation.Horizontal ? 'flex-start' : 'center'}; | ||
|
|
||
| a { | ||
| color: inherit; | ||
| text-decoration: underline; | ||
| } | ||
|
|
||
| > strong { | ||
| display: flex; | ||
| align-items: center; | ||
| text-align: center; | ||
| gap: 6px; | ||
| } | ||
|
|
||
| > p { | ||
| line-height: 1.4; | ||
| margin: auto; | ||
| padding: 0; | ||
| width: 100%; | ||
| text-align: ${({ orientation = BannerOrientation.Vertical }) => | ||
| orientation === BannerOrientation.Horizontal ? 'left' : 'center'}; | ||
| } | ||
|
|
||
| > i { | ||
| font-style: normal; | ||
| font-size: 32px; | ||
| line-height: 1; | ||
| } | ||
|
|
||
| > ol { | ||
| padding-left: 20px; | ||
| } | ||
| } | ||
| ` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify deeply nested selectors for better maintainability.
The nested selectors (e.g., > span > svg > path, > span > span) create tight coupling to the DOM structure and can be fragile during refactoring.
Consider using CSS classes or data attributes for more maintainable styling:
- > span > svg > path {
- fill: ${({ colorEnums }) => `var(${colorEnums.iconColor})`};
- }
+ [data-banner-icon] path {
+ fill: ${({ colorEnums }) => `var(${colorEnums.iconColor})`};
+ }
- > span > span {
+ [data-banner-content] {
width: 100%;
display: flex;
// ... rest of stylesThis approach would require adding corresponding data attributes to the JSX elements but provides more resilient styling.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In libs/ui/src/pure/InlineBanner/InlineBanner/styled.ts between lines 36 and 92,
the current deeply nested selectors like '> span > svg > path' and '> span >
span' tightly couple styles to the DOM structure, making maintenance and
refactoring difficult. To fix this, replace these nested selectors with CSS
classes or data attributes applied directly to the JSX elements, then update the
styled components to target those classes or attributes instead. This will
decouple styling from the DOM hierarchy and improve maintainability.
…roxy account information
|
@elena-zh thank you for your review!
|
|
@fairlighteth , for me it looks like there is too much info close one to another (and partially repeated) Btw, I see that there are some issues with the build: |
…proxy account details
|
@elena-zh Came up with a different solution now, where I removed the tooltip and conslidated things into the expandable inline banner:
|
… fix/cowshed-displaying-style
Summary
Fixes UI clutter in bridge flow by adding collapsible banners and improving proxy account information display.
Collapsed (default)
Expanded
To Test
Background
Summary by CodeRabbit
New Features
Improvements
Removals
Style