-
Notifications
You must be signed in to change notification settings - Fork 231
fix(ui): use new AssetLogo
widget in place of CoinIcon
and CoinLogo
#2848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aligns asset logo usage with the updated SDK by pointing dependencies to the new branch and replacing legacy AssetIcon
/CoinLogo
calls with AssetLogo.fromTicker
and AssetLogo.fromId
, and removes the old coin_logo.dart
.
- Bump SDK git refs in
pubspec.yaml
and UI kit pubspec tocodex/update-assetlogo-with-fromticker-and-fromid-methods
- Replace
AssetIcon
andCoinLogo
withAssetLogo.fromTicker
/fromId
across views and widgets - Remove legacy
coin_logo.dart
file
Reviewed Changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
pubspec.yaml | Updated SDK dependency refs to the new branch |
packages/komodo_ui_kit/pubspec.yaml | Updated SDK dependency refs to the new branch |
lib/shared/widgets/coin_item/coin_logo.dart | Removed legacy coin logo implementation |
lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart | Swapped AssetIcon.ofTicker for AssetLogo.fromTicker |
lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart | Swapped AssetIcon for AssetLogo.fromId |
lib/views/wallet/coin_details/coin_details_info/charts/portfolio_profit_loss_chart.dart | Swapped AssetIcon for AssetLogo.fromId |
lib/views/market_maker_bot/coin_selection_and_amount_input.dart | Updated import and swapped CoinLogo for AssetLogo.fromId |
lib/views/dex/simple/form/taker/coin_item/coin_group.dart | Updated import and swapped CoinLogo for AssetLogo.fromId |
lib/views/custom_token_import/custom_token_import_dialog.dart | Swapped AssetIcon.ofTicker for AssetLogo.fromTicker |
lib/shared/widgets/coin_select_item_widget.dart | Swapped AssetIcon.ofTicker for AssetLogo.fromTicker |
lib/shared/widgets/coin_item/coin_item.dart | Swapped CoinLogo for AssetLogo.fromId |
lib/shared/widgets/asset_item/asset_item.dart | Swapped AssetIcon for AssetLogo.fromId |
Comments suppressed due to low confidence (6)
lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart:261
- Missing import for AssetLogo; please add
import 'package:komodo_ui/komodo_ui.dart';
at the top of this file to resolve the symbol.
AssetLogo.fromTicker(
lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart:610
- Missing import for AssetLogo; please add
import 'package:komodo_ui/komodo_ui.dart';
to ensureAssetLogo
is available.
AssetLogo.fromId(asset.id),
lib/views/wallet/coin_details/coin_details_info/charts/portfolio_profit_loss_chart.dart:101
- Missing import for AssetLogo; please add
import 'package:komodo_ui/komodo_ui.dart';
so the widget compiles.
: AssetLogo.fromId(
lib/views/custom_token_import/custom_token_import_dialog.dart:303
- Missing import for AssetLogo; please add
import 'package:komodo_ui/komodo_ui.dart';
to this file.
AssetLogo.fromTicker(
lib/shared/widgets/coin_select_item_widget.dart:76
- Missing import for AssetLogo; please add
import 'package:komodo_ui/komodo_ui.dart';
to resolve the widget.
child: AssetLogo.fromTicker(coinId),
lib/shared/widgets/asset_item/asset_item.dart:30
- Missing import for AssetLogo; please add
import 'package:komodo_ui/komodo_ui.dart';
to ensure compilation.
AssetLogo.fromId(
AssetLogo
widget in place of CoinIcon
and CoinLogo
Visit the preview URL for this PR (updated for commit 8412727): https://walletrc--pull-2848-merge-363x3g0r.web.app (expires Sun, 13 Jul 2025 09:47:15 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
taker and maker forms previously relied on the CoinLogo placeholder widget when the coin was null in the form
- remove coin_icon - update references to AssetIcon - update imports - update precaching in app_bloc_root
fabe731
to
06c43d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take care of SDK #121 for refactoring before the following release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for this update!
Summary
Closes #2626 , Closes #2627
CoinLogo
andCoinIcon
withAssetLogo
andAssetIcon
from the SDK.AssetLogo
, which includes the network overlay iconsExpected behaviour
From #2626
BNB /ERC tokens too)
From #2627