Add clickable token address #5462
Conversation
WalkthroughThis pull request introduces a new React component, ClickableAddress, along with its styled counterparts to present blockchain addresses in a user-friendly manner. Utility functions handle URL generation, title creation, and token detection. The TokenInfo component now integrates this clickable address, and a new styled wrapper for the token symbol is added. Additionally, modifications in TokenListItem update the click handling logic and styling, ensuring proper event propagation when selecting tokens. A minor punctuation fix is applied in the README, and the CopyHelper component is refactored to use a new interface for clearer prop definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClickableAddress
participant HoverTooltip
participant CopyHelper
User->>ClickableAddress: Hover/Click on address
ClickableAddress->>HoverTooltip: Display title tooltip
ClickableAddress->>CopyHelper: Enable address copying
User->>Browser: Opens target URL in new tab
sequenceDiagram
participant User
participant TokenListItem
participant onSelectToken
User->>TokenListItem: Click on token item
TokenListItem->>TokenListItem: Execute handleClick()
alt Token is selected
TokenListItem->>User: Stop event propagation
else Token is not selected
TokenListItem->>onSelectToken: Invoke onSelectToken(token)
end
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/cowswap-frontend/src/common/pure/CopyToClipboard/index.tsx (2)
36-42: Improve accessibility by adding keyboard support.The component currently only responds to click events, but keyboard users should also be able to copy the text.
<styledEl.CopyWrapper> {copied ? ( <styledEl.CopyMessage>{copiedMessage}</styledEl.CopyMessage> ) : ( <styledEl.CopyIcon className={iconClassName} onClick={copyToClipboard} + onKeyDown={(e) => e.key === 'Enter' && copyToClipboard(e as unknown as MouseEvent<HTMLDivElement>)} + tabIndex={0} + role="button" + aria-label="Copy to clipboard" /> )} </styledEl.CopyWrapper>
11-44: Consider adding browser compatibility check for Clipboard API.The Clipboard API is not supported in all browsers. Consider adding a fallback mechanism for older browsers.
export const CopyToClipboard = ({ text, copiedMessage = 'Copied!', iconClassName }: CopyToClipboardProps) => { const [copied, setCopied] = useState(false) + const [clipboardSupported] = useState(() => !!navigator.clipboard); const copyToClipboard = async (event: MouseEvent<HTMLDivElement>) => { event.preventDefault() event.stopPropagation() try { + if (clipboardSupported) { await navigator.clipboard.writeText(text) + } else { + // Fallback for browsers without Clipboard API + const textArea = document.createElement('textarea'); + textArea.value = text; + textArea.style.position = 'fixed'; + document.body.appendChild(textArea); + textArea.focus(); + textArea.select(); + document.execCommand('copy'); + document.body.removeChild(textArea); + } setCopied(true) } catch (error) { console.error('Failed to copy:', error) } }apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx (2)
14-18: Prepare for multi-chain support with a more robust explorer mapping.The TODO comment indicates plans for more chains, but the current implementation has a few limitations:
- No error handling if a chainId doesn't match any known explorer
- Limited to just two networks currently
// TODO: once we have more chains, we should use the chainId to get the correct explorer -const mapChainIdToExplorer: { [key: string]: { name: string; url: string } } = { +type ExplorerInfo = { name: string; url: string } + +const mapChainIdToExplorer: Record<string, ExplorerInfo> = { } + +// Default to Ethereum mainnet if chain is not supported +const getExplorerForChain = (chainId?: number): ExplorerInfo => { + const chainKey = chainId?.toString() || '1' + return mapChainIdToExplorer[chainKey] || mapChainIdToExplorer['1'] +}
20-28: Refactor explorer utilities to use the safer lookup function.There's duplicate code for getting the explorer information and no error handling if the chainId doesn't match.
const getTarget = (address: string, chainId?: number) => { - const explorer = chainId ? mapChainIdToExplorer[chainId.toString()] : mapChainIdToExplorer['1'] + const explorer = getExplorerForChain(chainId) return `${explorer.url}/${address}` } const getTitle = (address: string, chainId?: number) => { - const explorer = chainId ? mapChainIdToExplorer[chainId.toString()] : mapChainIdToExplorer['1'] + const explorer = getExplorerForChain(chainId) return `View ${address} on ${explorer.name}` }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/cowswap-frontend/public/images/click-to-copy.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx(1 hunks)apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.ts(1 hunks)apps/cowswap-frontend/src/common/pure/CopyToClipboard/index.tsx(1 hunks)apps/cowswap-frontend/src/common/pure/CopyToClipboard/styled.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/TokenInfo/index.tsx(2 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/TokenInfo/styled.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/styled.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/cowswap-frontend/src/common/pure/CopyToClipboard/styled.ts
- apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (9)
apps/cowswap-frontend/src/modules/tokensList/pure/TokenInfo/styled.ts (1)
30-35: Good addition of a flex container for the token symbol and address.The
TokenSymbolWrappercomponent creates a clean horizontal layout with proper spacing (4px gap) for the token symbol and clickable address components.apps/cowswap-frontend/src/modules/tokensList/pure/TokenInfo/index.tsx (2)
5-5: Good import of the ClickableAddress component.This import brings in the new functionality for displaying and interacting with token addresses.
21-24: Well implemented token info layout with clickable address.The token symbol and clickable address are properly wrapped in the new
TokenSymbolWrappercomponent, maintaining a clean layout. The ClickableAddress component receives the necessary props for displaying and linking to the token's address on the blockchain explorer.apps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/index.tsx (2)
30-37: Good implementation of the click handler logic.This approach elegantly solves the issue mentioned in the PR by preventing event propagation only when the token is already selected. For unselected tokens, it properly calls
onSelectTokento select the token.
46-47: Good update to use the new click handler and CSS class.Replacing the
disabledattribute with a CSS class and the new click handler ensures that click events can propagate correctly to child components likeCopyToClipboardwhile maintaining the visual distinction for selected tokens.apps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/styled.ts (3)
25-25: Setting default opacity to 1 ensures visibility by default.This change supports the removal of the
disabledattribute by ensuring tokens are fully visible by default.
30-32: Good use of CSS class for selected tokens.Using a CSS class instead of the
disabledattribute for styling selected tokens allows click events to propagate while maintaining visual differentiation.
47-49: Prevents background change on hover for selected tokens.This ensures consistent visual feedback by preventing hover effects on already selected tokens.
apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx (1)
53-53:❓ Verification inconclusive
Verify if the copy icon meets the requirement to appear only on hover.
According to the PR objectives, the copy icon should appear only when hovering over the address. Ensure the styled components are set up correctly to handle this behavior.
🏁 Script executed:
#!/bin/bash # Look for hover-related styling in the styled components fd "styled.ts" --exec grep -l "hover" {} \; fd "styled.ts" --exec grep -l "clickable-address-copy-to-clipboard" {} \;Length of output: 6532
Hover Behavior Verification for Copy Icon
- The styled component file at
apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.tscontains both hover-related styling and the identifierclickable-address-copy-to-clipboard, which suggests that the hover behavior was addressed.- However, the grep results alone don’t confirm that the copy icon is hidden by default and only becomes visible on hover. Please manually inspect the hover styles in the styled file to ensure that the icon is correctly set to appear exclusively when the user hovers over the address.
apps/cowswap-frontend/src/common/pure/CopyToClipboard/index.tsx
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx
Outdated
Show resolved
Hide resolved
|
recheck |
|
I have read the CLA Document and I hereby sign the CLA recheck |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
292-293: Refine Typographical Punctuation and Wording.The updated line contains two consecutive periods ("..") at the end and a minor typo ("bellow" should be "below"), along with a slightly awkward phrasing ("only a PR is labeled" could be clearer as "only when a PR is labeled"). Consider addressing these issues for consistency and clarity.
- Some apps (see the list bellow) are not required to be built on each PR so we run them only a PR is labeled with a specific label.. + Some apps (see the list below) are not required to be built on each PR, so we run them only when a PR is labeled with a specific label.🧰 Tools
🪛 LanguageTool
[typographical] ~292-~292: Two consecutive dots
Context: ...ly a PR is labeled with a specific label.. This label is defined in the project se...(DOUBLE_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~292-~292: Two consecutive dots
Context: ...ly a PR is labeled with a specific label.. This label is defined in the project se...
(DOUBLE_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
| margin-left: 4px; | ||
| max-width: 20px; | ||
| background-color: transparent; | ||
| background-image: url('images/click-to-copy.svg'); |
There was a problem hiding this comment.
Didn't we have already a copy icon https://github.com/cowprotocol/cowswap/blob/main/apps/cowswap-frontend/src/modules/account/containers/CopyHelper/index.tsx#L10
There was a problem hiding this comment.
Yes, we use import { Copy } from 'react-feather' on this cowswap-frontend app specifically.
There was a problem hiding this comment.
Fixed, I've used the CopyHelper
| } | ||
|
|
||
| // TODO: once we have more chains, we should use the chainId to get the correct explorer | ||
| const mapChainIdToExplorer: { [key: string]: { name: string; url: string } } = { |
There was a problem hiding this comment.
Might be good to already move this to a shared const in our libs folder
There was a problem hiding this comment.
We already have CHAIN_INFO in libs/common-consts
There was a problem hiding this comment.
p.s. for such type of maps we use Record<SupportedChainId, ValueType> type in order to be safe with adding new networks. With this type If we add a new network we will see compilation errors for all necessary maps.
There was a problem hiding this comment.
Fixed, used getChainInfo method
| align-items: center; | ||
|
|
||
| &:hover { | ||
| .clickable-address-copy-to-clipboard { |
There was a problem hiding this comment.
hard coded class name required?
I'd suggest to use that and/or take this as an opportunity to refactor (move away from 'legacy' and/or make it somehow a shared lib if appropriate).
Develop: 
|
|
|
||
| const shortAddress = shortenAddress(address) | ||
|
|
||
| const target = useMemo(() => getTarget(address, chainId), [address, chainId]) |
There was a problem hiding this comment.
useMemo is not necessary here
There was a problem hiding this comment.
Hey @cowdan , great job. thank you!!
However, some issues from my side:
- When I open a token list in another chain, the app is crashed
To reproduce:
- No need to connect to a wallet
- Change a chain using network selector
- Open a token list in the sell/buy field
- WDYT about changing a chain name to a more generic word like 'block explorer'? If we decide to show addresses in other chains, there will no need to change this word for every chain
- In the dark mode 'copy' icon is barely visible:
- It is impossible to copy an address in a mobile view: when I press on the coly button (on a real device), a token is picked. @fairlighteth , should we still have a copy button in a mobile view?
Thanks!
Fixed
It was the anchor element
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
292-292: Fixed punctuation and possible grammar issue.The period at the end of the sentence is a good addition for correct punctuation. However, there appears to be a grammatical issue in "we run them only a PR is labeled" - consider revising to "we run them only when a PR is labeled" for clarity.
-Some apps (see the list bellow) are not required to be built on each PR so we run them only a PR is labeled with a specific label. +Some apps (see the list bellow) are not required to be built on each PR so we run them only when a PR is labeled with a specific label.🧰 Tools
🪛 LanguageTool
[uncategorized] ~292-~292: Possible missing comma found.
Context: ...w) are not required to be built on each PR so we run them only a PR is labeled wit...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx(1 hunks)apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.ts(1 hunks)apps/cowswap-frontend/src/legacy/components/Copy/CopyMod.tsx(1 hunks)apps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/styled.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx
- apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.ts
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~292-~292: Possible missing comma found.
Context: ...w) are not required to be built on each PR so we run them only a PR is labeled wit...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (6)
apps/cowswap-frontend/src/legacy/components/Copy/CopyMod.tsx (2)
36-40: Good refactoring of props into a dedicated interface.Extracting the props into a named interface improves code readability and maintainability. This will make it easier to reuse and extend these props in the future.
42-42: Props type updated to use the new interface.The function signature now uses the newly defined interface, which is more maintainable than the previous inline type definition.
apps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/styled.ts (4)
25-25: Simplified opacity handling improves event propagation.Setting a default opacity of 1 and removing the conditional opacity based on the
disabledprop helps address the issue where click events weren't propagating to theCopyToClipboardcomponent.
30-32: New class for selected token items with reduced opacity.The
.token-item-selectedclass with opacity 0.5 provides visual feedback for selected tokens without using thedisabledattribute, which ensures click events can properly propagate through the component.
48-50: Selected tokens don't change background on hover.This ensures that selected tokens maintain their visual state without additional highlighting on hover, providing a clear distinction between selected and non-selected states.
45-46:❓ Verification inconclusive
Verify hover background color consistency.
The hover background color has been set to
var(${UI.COLOR_PAPER_DARKER}). Based on the PR feedback, please ensure this matches the previous implementation in the develop branch and works consistently in both light and dark modes.
🏁 Script executed:
#!/bin/bash # Check if the color variable is defined consistently rg "COLOR_PAPER_DARKER" apps/cowswap-frontend/src --type ts --type tsxLength of output: 98
I'm seeing that the original command failed with an error because the file type "tsx" isn't recognized. To address this, please re-run the search using a corrected command that adds a custom file type for tsx files. For example:
This should list all instances of
COLOR_PAPER_DARKERin both .ts and .tsx files, helping verify that the hover background color (set asvar(${UI.COLOR_PAPER_DARKER})) is implemented consistently across light and dark modes as per the develop branch. Please run this updated script and manually verify that the color variable is defined and used consistently.
🏁 Script executed:
#!/bin/bash # Correctly search for COLOR_PAPER_DARKER usage by defining the tsx file type rg "COLOR_PAPER_DARKER" apps/cowswap-frontend/src --type-add 'tsx:*.tsx' --type ts --type tsxLength of output: 17430
Hover background color consistency appears in order.
Our search confirms that
var(${UI.COLOR_PAPER_DARKER})is used consistently across the codebase—including inapps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/styled.ts(lines 45–46) and multiple other components. This consistency suggests that the hover background color should behave as intended relative to the develop branch implementation.Please verify manually that the hover state renders correctly in both light and dark modes.
There was a problem hiding this comment.
Hey @cowdan , great job!!!
I have only 2 tiny nitpicks to report:
- WDYT about adding 'Sepolia' to the Etherscan in the tooltip when a user is connected to this network?
- Sometime I face difficulties to press on the copy button even in a desktop view: instead of copying, the token is picked into the field. See the video . Might it be possible to fix this somehow? E.g. increase the 'copy' tap area or so?
Thank you!
|
|
Hey @cowdan , the 1st case LGTM, thank you! As for the 2nd issue, I still can face it: each time I click on the copy button, a token is picked into the field. See the video: https://www.loom.com/share/60b6f9e029d741e9b73de07c03e84b32 |
Done
This is fixed now! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx (3)
11-14: Effective URL generation, but consider handling when chainInfo is undefinedThe target URL generation works well using the chain info explorer URL. However, there's no specific error handling or fallback for cases when chainInfo might be undefined.
Consider adding a more graceful fallback or logging when chainInfo is undefined:
const getTarget = (address: string, chainId: number) => { const chainInfo = getChainInfo(chainId) - return chainInfo ? `${chainInfo.explorer}/address/${address}` : undefined + if (!chainInfo) { + console.warn(`Chain info not found for chainId: ${chainId}`) + return undefined + } + return `${chainInfo.explorer}/address/${address}` }
31-34: Consider using typed chainId for better type safetyBased on previous feedback, the team prefers using a specific type for chainId rather than just number.
- export type ClickableAddressProps = { - address: string - chainId: number - } + import { SupportedChainId } from '@cowprotocol/common-const' + + export type ClickableAddressProps = { + address: string + chainId: SupportedChainId + }This would enforce type safety and align with the team's practice mentioned in previous comments.
53-57: Tooltip implementation with excessive props spreadingThe tooltip is implemented correctly, but there's unnecessary props spreading which could pass irrelevant props to the HoverTooltip component.
- <HoverTooltip {...props} placement="bottom" content={title} wrapInContainer> + <HoverTooltip placement="bottom" content={title} wrapInContainer>Since you're not using any other props from
propsin the HoverTooltip, it's cleaner to avoid spreading all props.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx(1 hunks)apps/cowswap-frontend/src/legacy/components/Copy/CopyMod.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cowswap-frontend/src/legacy/components/Copy/CopyMod.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Cypress
🔇 Additional comments (6)
apps/cowswap-frontend/src/common/pure/ClickableAddress/index.tsx (6)
16-24: Well-structured title generation with good UX considerationsThe title generation logic nicely handles the special case for non-mainnet Etherscan explorers. This provides users with clearer context about which network they're viewing.
26-29: Native token detection is correctly implementedThis aligns with the feedback about not showing addresses for native tokens like ETH since they might lead to unrelated addresses.
44-44: Good UX decision to hide native token addressesThe condition to hide the native token address is well-implemented and aligns with the feedback to omit token addresses for native tokens like ETH.
46-48: Click handler correctly prevents event propagationThis satisfies the PR objective to maintain the current token list view without redirecting to the main swap screen when clicking the address link.
58-60: Correctly using existing CopyHelper componentFollowing the feedback, you've used the existing CopyHelper component from the legacy codebase rather than creating a duplicate. Setting
clickableLink={false}is appropriate to avoid conflicts with the address link.
52-61:✅ Verification successful
Verify hover state styling in styled components
Make sure the styled components implement the hover state appropriately according to the PR objectives. The address should be visually highlighted (darkened and underlined) on hover, and the copy icon should appear only when hovering.
🏁 Script executed:
#!/bin/bash # Check if the styled components implement the hover effects properly # Find the styled.ts file cat apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.ts # Look for hover styles in other files referencing ClickableAddress rg -A 5 -B 5 "ClickableAddress" --type ts --type tsx | grep -i "hover\|transform\|transition\|css"Length of output: 747
Hover styling verified for ClickableAddress components
The styled components in
apps/cowswap-frontend/src/common/pure/ClickableAddress/styled.tsimplement the required hover behavior:
- The
AddressWrapperdarkens (opacity changes to 0.9) and applies an underline on hover.- The
Wrapperensures that child buttons (e.g., the copy icon) are hidden by default and become visible (opacity 0.9) on hover.Based on the verification, the hover state styling aligns with the PR objectives.
|
Congratulation on your first PR getting merged |
Summary
Add to a token contract address a link (at the moment only Etherscan is supported) and a
copy to clipboardiconTo Test
copyicon should not be displayedcopyicon should be displayed.copyicon click, it should display, after very short delay, the text "Copied!" and then going back to show thecopyicon. The token address should be available on the clipboard memory.Background
The
disabledattribute onTokenListItemstopped propagating a click event generated by theCopyToClipboardcomponent, so I had to remove it and replicate the behaviour (styling and click propagation)Summary by CodeRabbit
New Features
Enhancements
Style
Documentation