fix(asset-list): hide and use placeholders until fiat prices are available#3134
fix(asset-list): hide and use placeholders until fiat prices are available#3134
Conversation
rather than displaying the default value of 0.00 USD
|
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 WalkthroughAdjusts fiat balance display to hide or defer values when data is unavailable, introduces nullable totals across wallet overview and summary, adds placeholders, and removes legacy header widgets/actions in favor of updated top bar logic. Also tweaks coin balance parentheses handling and updates StatisticCard to support null values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant W as WalletOverview
participant B as CoinsBloc
participant PG as PortfolioGrowthBloc
participant UI as StatisticCard/BalanceSummary
U->>W: Open Wallet
W->>B: Get coins + USD balances
B-->>W: List<Coin> (with usdBalance?)
alt Any USD balance available
W->>W: _getTotalBalance() → double
W->>PG: Fetch 24h change (if ready)
W->>UI: Render total balance, trend, stats
else No USD balance available
W->>W: _getTotalBalance() → null
W->>UI: Render placeholders, hide trend
end
sequenceDiagram
autonumber
participant L as CoinListItem
participant C as Coin
participant F as CoinFiatBalance
L->>C: lastKnownUsdBalance()
alt usdBalance != null
L->>F: Build text " ($value)"
F-->>L: Render fiat with parentheses
else usdBalance == null
L-->>L: Hide fiat segment (no brackets)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Visit the preview URL for this PR (updated for commit ea60dc7): https://walletrc--pull-3134-merge-mrh4796e.web.app (expires Fri, 19 Sep 2025 14:02:43 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (1)
180-201: Guard trend when 24h % is unavailableIf get24hChangeForAsset returns null, TrendPercentageText may receive a null percentage. Prefer hiding the row in that case to avoid crashes or misleading “0%”.
Apply:
- final change24hPercent = usdBalance == 0.0 - ? 0.0 - : state.get24hChangeForAsset(widget.coin.id); + final change24hPercent = usdBalance == 0.0 + ? 0.0 + : state.get24hChangeForAsset(widget.coin.id); + if (change24hPercent == null) { + return const SizedBox.shrink(); + } @@ - final change24hValue = - change24hPercent != null && usdBalance > 0 - ? (change24hPercent * usdBalance / 100) - : 0.0; + final change24hValue = usdBalance > 0 + ? (change24hPercent * usdBalance / 100) + : 0.0;
🧹 Nitpick comments (6)
lib/views/main_layout/widgets/main_layout_top_bar.dart (3)
68-71: Numeric type hygiene in foldUse a double literal for the seed and for the null-coalesce to avoid inference pitfalls.
- double total = coins.fold( - 0, - (prev, coin) => prev + (coin.usdBalance(context.sdk) ?? 0), - ); + final total = coins.fold<double>( + 0.0, + (prev, coin) => prev + (coin.usdBalance(context.sdk) ?? 0.0), + );
31-41: Avoid reserving 200px when balance is hiddenReturning an empty widget keeps
leadingnon-null, soleadingWidth: 200still reserves space. Makeleadingnull whentotalBalanceis null.One approach: lift the BlocBuilder up to build the AppBar so you can pass
leadingandleadingWidthconditionally.- leading: BlocBuilder<CoinsBloc, CoinsState>( - builder: (context, state) { - final totalBalance = _getTotalBalance( - state.walletCoins.values, - context, - ); - if (totalBalance == null) { - return const SizedBox.shrink(); - } - return Padding( + leading: BlocBuilder<CoinsBloc, CoinsState>( + builder: (context, state) { + final totalBalance = _getTotalBalance(state.walletCoins.values, context); + if (totalBalance == null) return null; + return Padding( padding: const EdgeInsets.symmetric(horizontal: 32), child: ActionTextButton( text: LocaleKeys.balance.tr(), secondaryText: '\$${formatAmt(totalBalance)}', onTap: null, ), ); }, ), - leadingWidth: 200, + leadingWidth: 200, // Consider setting to 0 only when leading == nullNote: You’ll need to change the AppBar creation site to allow
leading: nullfrom the builder (e.g., wrap the whole AppBar in the BlocBuilder).Also applies to: 52-52
73-78: Clamping to $0.01 may misleadShowing 0.01 for totals below 0.01 overstates value. Prefer '< $0.01' or show a placeholder until ≥ $0.01.
- if (total > 0.01) { - return total; - } - return total != 0 ? 0.01 : 0; + if (total >= 0.01) return total; + // Return null to trigger placeholder, or use a sentinel handled by UI as '< $0.01' + return null;And format in UI as: secondaryText: '< $0.01' when desired.
packages/komodo_ui_kit/lib/src/display/statistic_card.dart (1)
113-132: Remove unused stored BuildContext; avoid shadowingThe field
final BuildContext context;is unused and shadows the build parameter. Simplify the widget.-class _ValuePlaceholder extends StatelessWidget { - const _ValuePlaceholder({required this.context}); - - final BuildContext context; +class _ValuePlaceholder extends StatelessWidget { + const _ValuePlaceholder({super.key}); @override Widget build(BuildContext context) { return Container( height: 24, width: 120, decoration: BoxDecoration( - color: Theme.of( - context, - ).colorScheme.surfaceContainerHighest.withValues(alpha: 0.3), + color: Theme.of(context) + .colorScheme + .surfaceContainerHighest + .withOpacity(0.3), // see note below borderRadius: BorderRadius.circular(4), ), - child: const SizedBox.shrink(), ); } }lib/views/wallet/wallet_page/wallet_main/wallet_overview.dart (2)
122-168: De-duplicate theme lookups in trend widgetMinor: cache themeCustom once to cut repeated Theme.of(...).extension calls.
- return TrendPercentageText( - percentage: totalChange, - upColor: - Theme.of(context).brightness == Brightness.dark - ? Theme.of( - context, - ).extension<ThemeCustomDark>()!.increaseColor - : Theme.of( - context, - ).extension<ThemeCustomLight>()!.increaseColor, - downColor: - Theme.of(context).brightness == Brightness.dark - ? Theme.of( - context, - ).extension<ThemeCustomDark>()!.decreaseColor - : Theme.of( - context, - ).extension<ThemeCustomLight>()!.decreaseColor, + final themeCustom = Theme.of(context).brightness == Brightness.dark + ? Theme.of(context).extension<ThemeCustomDark>()! + : Theme.of(context).extension<ThemeCustomLight>()!; + return TrendPercentageText( + percentage: totalChange, + upColor: themeCustom.increaseColor, + downColor: themeCustom.decreaseColor,
260-281: _getTotalBalance: single-pass + prefer lastKnownUsdBalance for consistency
- Avoid the two passes — aggregate in one loop and use coin.lastKnownUsdBalance(context.sdk) (matches most of the codebase) instead of coin.usdBalance(...). Change in: lib/views/wallet/wallet_page/wallet_main/wallet_overview.dart (and also lib/views/main_layout/widgets/main_layout_top_bar.dart which still uses usdBalance).
- I couldn't find TrendPercentageText in the repo; verify its constructor nullability separately before changing call sites that depend on nullable vs non-null values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
lib/shared/widgets/coin_balance.dart(1 hunks)lib/shared/widgets/coin_fiat_balance.dart(1 hunks)lib/views/common/header/actions/header_actions.dart(0 hunks)lib/views/common/header/app_header.dart(0 hunks)lib/views/main_layout/widgets/main_layout_top_bar.dart(5 hunks)lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart(5 hunks)lib/views/wallet/wallet_page/wallet_main/balance_summary_widget.dart(2 hunks)lib/views/wallet/wallet_page/wallet_main/wallet_overview.dart(6 hunks)packages/komodo_ui_kit/lib/src/display/statistic_card.dart(6 hunks)
💤 Files with no reviewable changes (2)
- lib/views/common/header/app_header.dart
- lib/views/common/header/actions/header_actions.dart
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-01T15:51:37.060Z
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/views/wallet/wallet_page/common/expandable_coin_list_item.dartlib/views/main_layout/widgets/main_layout_top_bar.dart
📚 Learning: 2025-04-01T15:51:37.060Z
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/views/wallet/wallet_page/common/expandable_coin_list_item.dart
⏰ 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). (10)
- GitHub Check: build_and_preview
- GitHub Check: Build Desktop (macos)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (windows)
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Test web-app-macos
- GitHub Check: unit_tests
- GitHub Check: Build Mobile (iOS)
- GitHub Check: validate_code_guidelines
- GitHub Check: Build Mobile (Android)
🔇 Additional comments (10)
lib/shared/widgets/coin_balance.dart (1)
40-40: Good: parentheses now owned by CoinFiatBalancePrevents double/bracket duplication and simplifies layout.
packages/komodo_ui_kit/lib/src/display/statistic_card.dart (2)
10-11: Null-aware value with placeholder: LGTMThe value nullable-ization and skeleton fallback are consistent with the PR goals.
Also applies to: 26-35, 76-87
124-127: Check Flutter min SDK for Color.withValues
withValuesis only available on newer Flutter versions; if your min SDK is older, usewithOpacity(0.3).Would you like me to scan pubspecs to confirm the Flutter SDK constraint and replace with
withOpacityif needed?lib/views/wallet/wallet_page/wallet_main/balance_summary_widget.dart (1)
90-93: Verify ColorScheme API availability
surfaceContainerHighest.withValues(alpha: 0.3)requires recent Flutter. UsewithOpacity(0.3)if you target older versions.I can update both placeholders to
withOpacity(0.3)if the current SDK doesn’t supportwithValues.Also applies to: 108-111
lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (3)
3-4: Theme extension imports: LGTMEnsures access to increase/decrease colors for TrendPercentageText.
171-176: Hide fiat until available: LGTMGating the USD text on lastKnownUsdBalance prevents the transient “$0.00” flicker.
234-237: Desktop: hide trend without fiatGood guard on null usdBalance to hide the trend row.
lib/views/wallet/wallet_page/wallet_main/wallet_overview.dart (3)
69-73: Nullable totalBalance gate: LGTMReturning null when no USD data exists cleanly triggers placeholders instead of “$0.00”.
74-82: Analytics value source may not match displayed valueYou gate logging on totalBalance != null but log stateWithData.totalValue.value. Consider logging totalBalance for consistency with what users see, or confirm product intent.
Would you like me to update the event payload to use totalBalance?
106-113: Copy-to-clipboard gating: LGTMAvoids copying meaningless “$0.00” when fiat is unavailable.
takenagain
force-pushed
the
bugfix/default-fiat-values
branch
from
16ef4bd to
ea60dc7
Compare
takenagain
added
QA
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues with asset balance display by hiding USD balances and 24h change data when fiat prices are unavailable, and implementing placeholder skeletons during loading states. This addresses the problem of showing misleading zeros when market data isn't available.
- Added conditional rendering to hide USD balance/change data when null
- Implemented skeleton placeholder components for loading states
- Refactored balance calculation methods to return null when no USD data is available
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/komodo_ui_kit/lib/src/display/statistic_card.dart | Added nullable value support and skeleton placeholder widget |
| lib/views/wallet/wallet_page/wallet_main/wallet_overview.dart | Updated total balance calculation to return null when no USD data available |
| lib/views/wallet/wallet_page/wallet_main/balance_summary_widget.dart | Added placeholder widgets and nullable balance support |
| lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart | Added null checks to hide USD balance when unavailable |
| lib/views/main_layout/widgets/main_layout_top_bar.dart | Updated balance display logic with null handling |
| lib/views/common/header/app_header.dart | Removed entire file (legacy header component) |
| lib/views/common/header/actions/header_actions.dart | Removed entire file (legacy header actions) |
| lib/shared/widgets/coin_fiat_balance.dart | Added null check for USD balance display |
| lib/shared/widgets/coin_balance.dart | Simplified balance display structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| final formattedBalance = formatUsdValue(usdBalance); | ||
| final balanceStr = ' ($formattedBalance)'; |
There was a problem hiding this comment.
The variable balanceStr is defined but never used. The parentheses formatting that was previously applied to the balance display has been moved into this unused variable, but the actual widget construction on line 48 uses formattedBalance directly.
Fixes #3053 and partial solution to #3073
Desktop
Mobile
Summary by CodeRabbit