Conversation
|
Caution Review failedThe pull request is closed. WalkthroughSingle file modification expanding protocol overlay logic in asset logo display. Introduces overlay support for specific parent assets (ETH-BASE, ETH-ARB20) alongside child assets, changing protocolTicker source from parent asset id to resolvedId.subClass.iconTicker. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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.
Pull Request Overview
This PR updates the asset logo display logic to fix an issue with BASE network assets. The change modifies how protocol overlay icons are determined for both child assets (tokens) and specific parent EVM chain assets.
Key Changes
- Added logic to display protocol overlays for specific parent assets (ETH-BASE and ETH-ARB20) in addition to child assets
- Changed from using
parentId?.idtosubClass.iconTickerfor determining the protocol icon to display - Updated comments to clarify the purpose of the protocol icon overlay
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final bool isParentWithOverlay = | ||
| resolvedId != null && | ||
| !isChildAsset && | ||
| (resolvedId.id == 'ETH-BASE' || resolvedId.id == 'ETH-ARB20'); |
There was a problem hiding this comment.
The hardcoded asset IDs 'ETH-BASE' and 'ETH-ARB20' create a maintainability issue. If more parent EVM chain assets (like ETH-MATIC, ETH-AVX20, etc.) need similar overlay treatment, they would need to be added individually to this condition.
Consider a more scalable approach such as:
- Using a property/flag on the AssetId or CoinSubClass to indicate if a parent asset should display a protocol overlay
- Checking if the asset is a parent EVM chain asset (using
evmCoinSubClasses.contains(resolvedId.subClass)) - Creating a set of subclasses that should display overlays when they are parent assets
Example:
final isParentWithOverlay = resolvedId != null &&
!isChildAsset &&
evmCoinSubClasses.contains(resolvedId.subClass) &&
resolvedId.id.startsWith('ETH-');This would automatically handle any ETH-prefixed parent asset on EVM-compatible chains without requiring individual hardcoding.
| (resolvedId.id == 'ETH-BASE' || resolvedId.id == 'ETH-ARB20'); | |
| evmCoinSubClasses.contains(resolvedId.subClass) && | |
| resolvedId.id.startsWith('ETH-'); |
|
Visit the preview URL for this PR (updated for commit e277b31): https://kdf-sdk--pr293-dev-0j7tzh9y.web.app (expires Thu, 20 Nov 2025 11:34:16 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9c1b6e6c010cf0b965c455ba7a69c4aedafa8a1d |
|
Visit the preview URL for this PR (updated for commit e277b31): https://komodo-playground--pr293-dev-8grb3acr.web.app (expires Thu, 20 Nov 2025 11:34:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47 |
fix BASE
Summary by CodeRabbit