fix(coin-details): display fiat balance for addresses#3049
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 WalkthroughThe changes update formatting and display logic in two widget files. In Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CoinAddressesWidget
participant SDK
User->>CoinAddressesWidget: View coin address details
CoinAddressesWidget->>SDK: Fetch last known USD price for coin
SDK-->>CoinAddressesWidget: Return USD price
CoinAddressesWidget->>User: Display balance with ticker and USD equivalent
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
takenagain
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request replaces a local balance widget with the reusable CoinBalance component to display both asset balance and fiat equivalent for addresses. The changes improve code reuse and provide consistent balance display across the application.
- Removes the local
_Balancewidget and replaces it with the existingCoinBalancecomponent - Refactors the
CoinBalancewidget to improve code organization and formatting - Adds import for
formatters.dartto support balance formatting functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart | Replaces local _Balance widget with CoinBalance component and adds necessary import |
| lib/shared/widgets/coin_balance.dart | Refactors code organization, removes unused parameters, and adds formatters import |
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
Outdated
Show resolved
Hide resolved
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
Outdated
Show resolved
Hide resolved
|
Visit the preview URL for this PR (updated for commit 95a9c56): https://walletrc--pull-3049-merge-kt98qyrq.web.app (expires Tue, 12 Aug 2025 11:41:14 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart (1)
743-743: Address deprecation warning for QR code styling.The
foregroundColorproperty is deprecated. Update the QrImageView to use the newer color properties.- foregroundColor: theme.custom.dexPageTheme.emptyPlace, + eyeStyle: QrEyeStyle( + eyeShape: QrEyeShape.square, + color: theme.custom.dexPageTheme.emptyPlace, + ), + dataModuleStyle: QrDataModuleStyle( + dataModuleShape: QrDataModuleShape.square, + color: theme.custom.dexPageTheme.emptyPlace, + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/shared/widgets/coin_balance.dart(2 hunks)lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2570
File: lib/views/fiat/fiat_inputs.dart:198-212
Timestamp: 2025-03-25T18:39:03.280Z
Learning: The `showBalanceIndicator` parameter for the `SourceAddressField` widget will be added in a future SDK update, as noted by the developer.
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.738Z
Learning: In the CoinsManagerBloc, `state.selectedCoins` is used separately from `state.coins` to indicate whether a coin is enabled or not in the UI. The order of Set deduplication when merging coin lists doesn't affect UI behavior because selection state is tracked independently from the coin list used for filtering and sorting.
📚 Learning: in the komodo wallet project, part files share imports with their parent files. the import for `app_...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
lib/shared/widgets/coin_balance.dartlib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
📚 Learning: the `excludedassetlist` from `app_config.dart` is used in the `_filterexcludedassets` method in `coi...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Applied to files:
lib/shared/widgets/coin_balance.dartlib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
📚 Learning: it's acceptable to use unconditional `dart:io` imports in the komodo wallet codebase when the usage ...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2608
File: lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart:2-3
Timestamp: 2025-05-01T21:00:36.970Z
Learning: It's acceptable to use unconditional `dart:io` imports in the Komodo wallet codebase when the usage is guarded by `!kIsWeb` conditional checks that prevent the platform-specific code from being executed in web environments.
Applied to files:
lib/shared/widgets/coin_balance.dartlib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
📚 Learning: the `showbalanceindicator` parameter for the `sourceaddressfield` widget will be added in a future s...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2570
File: lib/views/fiat/fiat_inputs.dart:198-212
Timestamp: 2025-03-25T18:39:03.280Z
Learning: The `showBalanceIndicator` parameter for the `SourceAddressField` widget will be added in a future SDK update, as noted by the developer.
Applied to files:
lib/shared/widgets/coin_balance.dartlib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
📚 Learning: in the coinsmanagerbloc, `state.selectedcoins` is used separately from `state.coins` to indicate whe...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.738Z
Learning: In the CoinsManagerBloc, `state.selectedCoins` is used separately from `state.coins` to indicate whether a coin is enabled or not in the UI. The order of Set deduplication when merging coin lists doesn't affect UI behavior because selection state is tracked independently from the coin list used for filtering and sorting.
Applied to files:
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
📚 Learning: in the komodo wallet project, test files are structured to define test functions that are called fro...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.
Applied to files:
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
📚 Learning: in the komodo wallet project, test functions are defined in individual files under `test_units/tests...
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2611
File: test_units/tests/system_health/http_head_time_provider_test.dart:8-24
Timestamp: 2025-05-08T19:05:13.083Z
Learning: In the Komodo Wallet project, test functions are defined in individual files under `test_units/tests/` but are executed through the central entry point in `test_units/main.dart`, so individual test files don't need their own `main()` function.
Applied to files:
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
🪛 GitHub Actions: takenagain is validating code guidelines 🚀
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
[info] 743-743: 'foregroundColor' is deprecated. Use colors in eyeStyle and dataModuleStyle instead.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (windows)
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Mobile (iOS)
🔇 Additional comments (3)
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart (2)
17-18: LGTM! New imports are properly utilized.The new imports are correctly added to support the fiat balance display feature:
formatters.dartprovidesformatUsdValuefor USD formattinglegacy_coin_migration_extensions.dartprovidesabbr2Tickerfor ticker conversion
290-299: Excellent implementation of fiat balance display!The updated balance display logic correctly:
- Calculates USD value using the coin's last known price
- Handles null price scenarios gracefully with
formatUsdValue- Shows clear format: "balance TICKER ($fiat)"
- Uses the appropriate
address.balance.totalfor individual address balancesThis perfectly addresses the PR objective of displaying fiat values instead of redundant balance information.
lib/shared/widgets/coin_balance.dart (1)
10-10: LGTM! Clean formatting improvements.The consolidation of multi-line expressions into single lines improves code readability while maintaining identical functionality. These formatting changes align well with the overall code style.
Also applies to: 18-18, 35-35, 39-39, 43-43
| final balance = address.balance.total.toDouble(); | ||
| final price = coin.lastKnownUsdPrice(context.sdk); | ||
| final usdValue = price == null ? null : price * balance; | ||
| final fiat = formatUsdValue(usdValue); |
There was a problem hiding this comment.
Consider using a stream-based approach so that we're resilient to edge cases where the user is on the page before the prices are loaded or if they have a momentary dip in their connection.
I generally avoid using stream builders because they clutter up the UI and suggest that the state is modelled correctly incorrectly, but all fiat-display UI code is going to be overhauled in the near future when we add the UX enhancement for multiple fiat currency options.
EDIT: strike out and replace a word which changes the meaning of the entire comment (@smk762 @takenagain)
|
Confirmed fiat display in the wallet page There is some time prior to data being available, during while the display is suboptimal. As mentioned previously, when there is nothing inside the brackets, use a spinner. The current UI in this state is bad UX. There is also a state where there is something between the brackets, but it's a default ( This should be applied in all places where a fiat value is shown. I've received some negative user feedback where their experience was diminished by the implication that their favorite low cap coin was worthless because we lacked price info, so it value was shown as I'll move this commentary into an issue, as the scope of this PR was already fulfilled and its merge should not be delayed. |
TODO: bump SDK version once merged
* refactor: drop balance override from CoinBalance * chore: remove unused import Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(address-card): revert change to coinbalance and use extensions * feat(coin-addresses): migrate to stream-based pubkey watch function TODO: bump SDK version once merged --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Update AddressCard to show the fiat value for the address balance instead of showing the balance twice.
Before
After
Summary by CodeRabbit
New Features
Style