feat: show destination network in swap activity description when source and destination symbols are the same#568
Conversation
WalkthroughThe changes in this pull request focus on enhancing the Changes
Possibly related PRs
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
Documentation and Community
|
|
💼 Build Files |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/extension/src/ui/action/views/network-activity/components/network-activity-transaction.vue (1)
169-184: Enhance message construction and maintainabilityWhile the logic is correct, the message construction could be improved:
- String concatenation could be replaced with template literals
- Common strings could be extracted as constants
Consider this refactoring:
+const MESSAGE_TEMPLATES = { + BASE: 'Swap from %from% to %to%', + WITH_NETWORK: 'Swap from %from% to %to% (%network%)', + LOADING: 'Swap from %from% to %to% (Loading...)', + UNKNOWN: 'Swap from %from% to %to% (Unknown)', +} as const; const getSwapActivityDescriptionSync = ( data: SwapActivityDescriptionData, ): string => { const { fromNetworkName, toNetworkName, fromTokenSymbol, toTokenSymbol } = data; if (fromNetworkName === toNetworkName || fromTokenSymbol !== toTokenSymbol) { - return `Swap from` + ` ${fromTokenSymbol}` + ` to ${toTokenSymbol}`; + return MESSAGE_TEMPLATES.BASE + .replace('%from%', fromTokenSymbol) + .replace('%to%', toTokenSymbol); } - return ( - `Swap from` + - ` ${fromTokenSymbol}` + - ` to ${toTokenSymbol}` + - ` (Loading...)` - ); + return MESSAGE_TEMPLATES.LOADING + .replace('%from%', fromTokenSymbol) + .replace('%to%', toTokenSymbol); };Similar changes can be applied to
getSwapActivityDescriptionAsync.Also applies to: 186-211
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/extension/src/ui/action/views/network-activity/components/network-activity-transaction.vue(4 hunks)
🔇 Additional comments (3)
packages/extension/src/ui/action/views/network-activity/components/network-activity-transaction.vue (3)
84-84: LGTM: Template binding looks good
The replacement of static text with the dynamic swapMessage binding is implemented correctly.
213-238: LGTM: Watch implementation is robust
The watch implementation is well-designed with:
- Proper type casting
- Caching to prevent unnecessary updates
- Clean handling of async state updates
186-211: Verify network name display format
The implementation correctly shows the destination network using name_long from the network object. Let's verify the network name format across the codebase.
✅ Verification successful
Network name format is consistent across the codebase
The implementation correctly uses name_long property from the network object, which is consistent with the codebase usage. The search results show:
- Network objects consistently use
name_longfor displaying full network names (e.g., in UI components, headers, and transaction views) - The implementation aligns with this pattern by using
toNetwork.name_longin the swap activity description - No inconsistencies found in network name display format across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check network name usage consistency
# Check for network name property usage
rg "name_long|name_short" --type ts --type vue
# Check for other network name formats in swap-related code
rg "networkInfo.*name" --type ts --type vue
Length of output: 150
Script:
#!/bin/bash
# Description: Check network name usage consistency with correct file types
# Check for network name property usage
rg "name_long|name_short" -t typescript -t vue-template -t vue
# Check for other network name formats in swap-related code
rg "networkInfo.*name" -t typescript -t vue-template -t vue
# Check network object structure
fd -e ts -e vue | xargs rg "network.*name"
Length of output: 37975
Previously when swapping WETH on Polygon to WETH on Arbitrum, the activity description was
Swap from WETH to WETH. This changes the activity description toSwap from WETH to WETH (Arbitrum).Summary by CodeRabbit
New Features
Bug Fixes