feat(suite): update network icons #16784
Conversation
WalkthroughThe changes update the Assessment against linked issues
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 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 (
|
![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)
packages/product-components/src/components/CoinLogo/networks.ts (1)
3-28: Consider enhancing type safety for network icons.The current implementation uses a simple Record type. Consider using a more specific type that ensures all
NetworkSymbolvalues are covered.-export const NETWORK_ICONS: Record<NetworkSymbol, string> = { +type NetworkIconMap = { [K in NetworkSymbol]: string }; +export const NETWORK_ICONS: NetworkIconMap = {packages/product-components/src/components/CoinLogo/CoinLogo.tsx (1)
37-47: Consider enhancing network icon accessibility.The network icon styling is well-implemented, but consider adding
aria-labelfor better accessibility..network-symbol { + /* Add aria-label in the ReactSVG component */ position: absolute; bottom: 0; right: 0;packages/suite/src/components/wallet/WalletLayout/AccountsMenu/CoinsFilter.tsx (1)
100-116: Consider adding tooltip for network icons.The network icon addition is well-implemented, but consider enhancing the tooltip to include network-specific information.
<StyledCoinLogo data-testid={`@account-menu/filter/${networkSymbol}`} symbol={networkSymbol} showNetworkIcon size={16} + title={`${getNetwork(networkSymbol).name} Network`} data-test-activated={coinFilter === networkSymbol} $isSelected={isSelected}packages/suite/src/components/suite/layouts/SuiteLayout/PageHeader/PageNames/AccountName/AccountDetails.tsx (1)
110-161: Consider memoizing complex components.The component structure is well-organized, but consider memoizing the MetadataLabeling component to optimize performance.
+const MemoizedMetadataLabeling = memo(MetadataLabeling); + return ( <DetailsContainer $isBalanceShown={isBalanceShown} $shouldAnimate={shouldAnimate}> <CoinLogo size={LOGO_SIZE} symbol={symbol} /> <div> <AccountHeading $isBalanceShown={isBalanceShown}> - <MetadataLabeling + <MemoizedMetadataLabeling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
packages/product-components/src/images/networks/ada.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/arb.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/base.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/bnb.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/btc.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/eth.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/op.svgis excluded by!**/*.svgpackages/product-components/src/images/networks/sol.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
packages/product-components/src/components/CoinLogo/CoinLogo.tsx(1 hunks)packages/product-components/src/components/CoinLogo/networks.ts(1 hunks)packages/suite/src/components/suite/CoinList/Coin.tsx(1 hunks)packages/suite/src/components/suite/layouts/SuiteLayout/PageHeader/PageNames/AccountName/AccountDetails.tsx(3 hunks)packages/suite/src/components/wallet/WalletLayout/AccountTopPanel/AccountTopPanel.tsx(0 hunks)packages/suite/src/components/wallet/WalletLayout/AccountsMenu/CoinsFilter.tsx(2 hunks)packages/suite/src/views/wallet/send/Outputs/TokenSelect/TokenSelect.tsx(1 hunks)packages/suite/src/views/wallet/transactions/TradeBox/TradeBox.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/suite/src/components/wallet/WalletLayout/AccountTopPanel/AccountTopPanel.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/suite/src/views/wallet/send/Outputs/TokenSelect/TokenSelect.tsx
- packages/suite/src/views/wallet/transactions/TradeBox/TradeBox.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: build-web
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (5)
packages/product-components/src/components/CoinLogo/CoinLogo.tsx (1)
14-15: LGTM! Well-structured size calculation.The
getSizefunction effectively encapsulates size calculation logic with support for borders and divisors.packages/suite/src/components/wallet/WalletLayout/AccountsMenu/CoinsFilter.tsx (1)
14-27: LGTM! Consistent styling updates.The border-radius update aligns with the design system's
borders.radii.xxsvalue.packages/suite/src/components/suite/layouts/SuiteLayout/PageHeader/PageNames/AccountName/AccountDetails.tsx (2)
20-20: LGTM! Good use of constants.The
LOGO_SIZEconstant improves maintainability and consistency.
50-58: LGTM! Modern layout approach.The use of
gapfor spacing is a modern and maintainable approach.packages/suite/src/components/suite/CoinList/Coin.tsx (1)
204-204: LGTM! The network icon integration looks good.The addition of the
showNetworkIconprop aligns well with the PR objectives to implement network icons for Layer 2 networks.
| // TODO: add missing network icons | ||
| bch: require('../../images/coins/bch.svg'), | ||
| bsc: require('../../images/coins/bsc.svg'), | ||
| doge: require('../../images/coins/doge.svg'), | ||
| dsol: require('../../images/coins/dsol.svg'), | ||
| etc: require('../../images/coins/etc.svg'), | ||
| ltc: require('../../images/coins/ltc.svg'), | ||
| pol: require('../../images/coins/pol.svg'), | ||
| regtest: require('../../images/coins/btc_test.svg'), | ||
| tada: require('../../images/coins/tada.svg'), | ||
| test: require('../../images/coins/btc_test.svg'), | ||
| thol: require('../../images/coins/thol.svg'), | ||
| tsep: require('../../images/coins/tsep.svg'), | ||
| txrp: require('../../images/coins/txrp.svg'), | ||
| xrp: require('../../images/coins/xrp.svg'), | ||
| zec: require('../../images/coins/zec.svg'), | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Missing dedicated network icons for several networks
The fallback keys in networks.ts are all using coin icons because no dedicated network icons exist in packages/product-components/src/images/networks for these entries. Specifically, the following network keys are missing dedicated icons:
- bch
- bsc
- doge
- dsol
- etc
- ltc
- pol
- regtest
- tada
- test
- thol
- tsep
- txrp
- xrp
- zec
These should be addressed to ensure visual consistency.
🔗 Analysis chain
Address missing network icons.
Several networks are using coin icons as fallbacks, which may not be the intended design. This could affect the visual consistency of the network icons.
Let's verify which networks are missing dedicated icons:
Would you like me to create an issue to track the implementation of missing network icons?
This script re-checks which network keys in networks.ts that use coin icons lack a dedicated counterpart in the networks folder. Once you run it and review its output, we can determine whether the review comment “Address missing network icons.” is valid. Please run and verify the results.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for missing network icons
echo "Networks using coin icons as fallbacks:"
rg -l "networks/" packages/product-components/src/images | grep -v "ada\|arb\|base\|btc\|eth\|op\|sol"
Length of output: 191
Script:
#!/bin/bash
# Extract network icon names referenced in networks.ts
echo "Extracting network icon references from networks.ts..."
grep -E "^[[:space:]]*[a-z0-9]+:" packages/product-components/src/components/CoinLogo/networks.ts \
| sed -E "s/.*require\('\.\.\/\.\.\/images\/coins\/([^']+)\.svg'\).*/\1/" > /tmp/referenced_icons.txt
echo "Network icons referenced in networks.ts:"
cat /tmp/referenced_icons.txt
# List actual SVG files available in the coins directory
echo "Listing SVG files available in packages/product-components/src/images/coins..."
find packages/product-components/src/images/coins -type f -name "*.svg" \
| sed 's/.*\/\(.*\)\.svg/\1/' > /tmp/available_icons.txt
echo "SVG icons available in coins folder:"
cat /tmp/available_icons.txt
# Sort both lists and compare to find missing icons.
sort /tmp/referenced_icons.txt > /tmp/referenced_sorted.txt
sort /tmp/available_icons.txt > /tmp/available_sorted.txt
echo "Icons referenced in networks.ts but missing in coins folder:"
comm -23 /tmp/referenced_sorted.txt /tmp/available_sorted.txt
Length of output: 1852
Script:
#!/bin/bash
# We’re now verifying which networks in the networks.ts file are relying on coin icons
# (i.e. using a fallback) instead of a dedicated network icon.
#
# Step 1: Extract the network keys that are mapped to coin icons
echo "Extracting fallback network keys from networks.ts (using coin icons)..."
grep "coins/" packages/product-components/src/components/CoinLogo/networks.ts \
| sed -E "s/^[[:space:]]*([^:]+):.*coins\/([^']+)\.svg'.*/\1/" | sort > /tmp/fallback_network_keys.txt
echo "Fallback network keys:"
cat /tmp/fallback_network_keys.txt
# Step 2: List the dedicated network icons that exist in the networks folder
echo "Listing dedicated network icons from packages/product-components/src/images/networks..."
find packages/product-components/src/images/networks -type f -name "*.svg" \
| sed 's/.*\/\(.*\)\.svg/\1/' | sort > /tmp/dedicated_network_icons.txt
echo "Dedicated network icons available:"
cat /tmp/dedicated_network_icons.txt
# Step 3: Identify fallback network keys that do not have a corresponding dedicated icon.
echo "Fallback network keys missing a dedicated network icon:"
while read -r key; do
if ! grep -Fxq "$key" /tmp/dedicated_network_icons.txt; then
echo "$key"
fi
done < /tmp/fallback_network_keys.txt
Length of output: 2378
| export const CoinLogo = ({ | ||
| symbol, | ||
| showNetworkIcon = false, | ||
| className, | ||
| size = DEFAULT_SIZE, | ||
| ...rest | ||
| }: CoinLogoProps) => { | ||
| const networkSymbol = getNetworkDisplaySymbol( | ||
| symbol as NetworkSymbol, | ||
| ).toLowerCase() as NetworkSymbol; | ||
|
|
||
| const symbolSrc = showNetworkIcon | ||
| ? NETWORK_ICONS[symbol as NetworkSymbol] | ||
| : COINS[networkSymbol != symbol ? networkSymbol : symbol]; | ||
|
|
||
| const badge = | ||
| !showNetworkIcon && networkSymbol != symbol ? NETWORK_ICONS[symbol as NetworkSymbol] : null; | ||
|
|
||
| return ( | ||
| <SvgWrapper className={className} size={size} {...rest}> | ||
| <ReactSVG | ||
| src={symbolSrc ?? COINS[symbol]} | ||
| beforeInjection={svg => { | ||
| svg.setAttribute('width', getSize(size)); | ||
| svg.setAttribute('height', getSize(size)); | ||
| }} | ||
| loading={() => <span className="loading" />} | ||
| className="coin-symbol" | ||
| /> | ||
| {badge && symbolSrc != null && ( | ||
| <ReactSVG | ||
| src={badge} | ||
| beforeInjection={svg => { | ||
| svg.setAttribute('width', getSize(size, 0, 3)); | ||
| svg.setAttribute('height', getSize(size, 0, 3)); | ||
| }} | ||
| loading={() => <span className="loading" />} | ||
| className="network-symbol" | ||
| /> | ||
| )} | ||
| </SvgWrapper> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Action: Add explicit error boundaries for missing icon errors
Our verification confirms that while the component falls back to the coins asset using the ?? operator when a network icon lookup fails, it does not incorporate an explicit error boundary to catch errors (for example, potential failures in loading an SVG via ReactSVG). This might lead to unhandled runtime issues even though the component currently “handles” missing icons by falling back.
- In
packages/product-components/src/components/CoinLogo/CoinLogo.tsx, the missing icon lookup uses fallbacks (e.g.,src={symbolSrc ?? COINS[symbol]}) without any explicit error handling mechanisms.
🔗 Analysis chain
Verify error handling for missing icons.
The component handles missing icons gracefully but could benefit from explicit error boundaries.
Let's verify the error handling:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential missing icon references
echo "Checking for icon references in the codebase:"
rg "NETWORK_ICONS\[.*?\]" --type ts
Length of output: 418
Description
Use new network icons and add a badge for L2 (Base, Arbitrum, Optimism).
WIP: Still missing some icons.
Related Issue
Resolve #16163
Screenshots: