feat(add-assets): show wallet enabled coins in the "Add Assets" page#2976
feat(add-assets): show wallet enabled coins in the "Add Assets" page#2976
Conversation
* feat(wallet): show enabled coins in add asset * fix(wallet): show enabled assets in add coins
|
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 WalkthroughThis change set introduces new dependencies and event handlers for the Coins Manager feature, refactors sorting and coin removal logic into the bloc, and updates state management accordingly. It also modifies coin activation logic to allow notification suppression, removes deprecated fields and widgets, and adjusts UI logic to rely on bloc-driven state and events. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant CoinsManagerBloc
participant SettingsRepository
participant TradingEntitiesBloc
participant CoinsRepo
UI->>CoinsManagerBloc: CoinsManagerSortChanged / CoinsManagerCoinRemoveRequested
CoinsManagerBloc->>SettingsRepository: (for filtering/sorting settings)
CoinsManagerBloc->>TradingEntitiesBloc: (check for open orders/swaps)
CoinsManagerBloc->>CoinsRepo: activateCoinsSync/deactivateCoinsSync (with notify flag)
CoinsRepo-->>CoinsManagerBloc: Activation/deactivation result
CoinsManagerBloc-->>UI: Updated state (with sort/removal info)
UI->>CoinsManagerBloc: CoinsManagerCoinRemoveConfirmed/Cancelled (via dialog)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Possibly related PRs
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. 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 (
|
|
Visit the preview URL for this PR (updated for commit f6edb10): https://walletrc--pull-2976-merge-4dyp53me.web.app (expires Fri, 01 Aug 2025 00:39:55 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
takenagain
added
the
UX
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements wallet coin visibility in the "Add Assets" page by showing enabled (active) coins with their checkboxes pre-selected. The key changes include significant refactoring of the coins manager to improve separation of concerns, state management, and coin removal flow handling.
Key Changes:
- Added wallet coins to the "Add Assets" list with pre-selected checkboxes for active coins
- Refactored coin removal logic into a more structured state-driven approach with proper trading activity validation
- Moved sorting logic and coin filtering from UI components to the bloc for better state management
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/bloc/coins_manager/coins_manager_bloc.dart | Major refactoring with new coin removal state handling, improved sorting logic, and wallet coin integration |
| lib/bloc/coins_manager/coins_manager_state.dart | Added removal state tracking and sort data to state management |
| lib/bloc/coins_manager/coins_manager_event.dart | Added new events for removal flow and sort changes |
| lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart | Simplified UI logic by moving sorting and filtering to bloc |
| lib/model/coin.dart | Made Coin extend Equatable and removed deprecated accounts field |
| lib/bloc/coins_bloc/coins_repo.dart | Added notify parameter to activation methods for better control |
Comments suppressed due to low confidence (1)
lib/bloc/coins_manager/coins_manager_bloc.dart:292
- Using
assets.singlewhen iterating over multiple assets will cause a runtime error. Should use the currentassetfrom the loop instead.
? coins
![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
🔭 Outside diff range comments (2)
lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart (1)
297-329: Replace deprecated withOpacity method.The pipeline flags that
withOpacityis deprecated and should be replaced withwithValues()to avoid precision loss.- color: Theme.of(context).colorScheme.onSurface.withOpacity(0.7), + color: Theme.of(context).colorScheme.onSurface.withValues(alpha: 0.7),Apply similar changes to other instances of
withOpacityin this section.lib/views/wallet/coins_manager/coins_manager_page.dart (1)
6-7: Remove unused imports.The pipeline flags that the imports for
CoinsBlocandCoinsManagerBlocare no longer used after the refactoring.-import 'package:web_dex/bloc/coins_bloc/coins_bloc.dart'; -import 'package:web_dex/bloc/coins_manager/coins_manager_bloc.dart';
🧹 Nitpick comments (3)
lib/views/wallet/coins_manager/coins_manager_list_header.dart (1)
8-9: Consider using super parameter for the key.The pipeline suggests converting the
keyparameter to a super parameter for better performance and cleaner code.- const CoinsManagerListHeader({ - Key? key, + const CoinsManagerListHeader({ + super.key, required this.sortData, required this.isAddAssets, required this.onSortChange, - }) : super(key: key); + });lib/views/wallet/coins_manager/coins_manager_page.dart (1)
14-19: Consider using super parameter for the key.The pipeline suggests converting the
keyparameter to a super parameter for consistency with modern Dart practices.- const CoinsManagerPage({ - Key? key, + const CoinsManagerPage({ + super.key, required this.action, required this.closePage, - }) : super(key: key); + });lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart (1)
83-136: Well-structured coin removal flow!The method properly handles all edge cases for coin removal with appropriate user feedback. The sequential checks for active swaps, open orders, and parent-child relationships ensure safe coin removal.
Consider extracting the dialog confirmation logic into separate methods for better testability:
+ Future<void> _handleOpenOrdersConfirmation( + BuildContext context, + CoinRemovalState removalState, + CoinsManagerBloc bloc, + ) async { + final confirmed = await confirmCoinDisableWithOrders( + context, + coin: removalState.coin.abbr, + ordersCount: removalState.openOrdersCount, + ); + bloc.add(confirmed + ? const CoinsManagerCoinRemoveConfirmed() + : const CoinsManagerCoinRemovalCancelled()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
lib/bloc/app_bloc_root.dart(1 hunks)lib/bloc/coins_bloc/coins_bloc.dart(2 hunks)lib/bloc/coins_bloc/coins_repo.dart(6 hunks)lib/bloc/coins_manager/coins_manager_bloc.dart(5 hunks)lib/bloc/coins_manager/coins_manager_event.dart(2 hunks)lib/bloc/coins_manager/coins_manager_sort.dart(1 hunks)lib/bloc/coins_manager/coins_manager_state.dart(4 hunks)lib/model/coin.dart(3 hunks)lib/views/wallet/coin_details/coin_details_info/address_select.dart(0 hunks)lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart(4 hunks)lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fill_form_trezor_sender_address.dart(0 hunks)lib/views/wallet/coins_manager/coins_manager_list_header.dart(1 hunks)lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart(2 hunks)lib/views/wallet/coins_manager/coins_manager_page.dart(1 hunks)test_units/tests/utils/test_util.dart(0 hunks)
🧠 Learnings (13)
📓 Common learnings
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.
lib/views/wallet/coins_manager/coins_manager_list_header.dart (4)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
lib/bloc/app_bloc_root.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_bloc/coins_bloc.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/views/wallet/coins_manager/coins_manager_page.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_manager/coins_manager_sort.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_manager/coins_manager_event.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_bloc/coins_repo.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart (6)
Learnt from: takenagain
PR: #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: #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.
Learnt from: takenagain
PR: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
lib/bloc/coins_manager/coins_manager_state.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/model/coin.dart (6)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart (6)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
lib/bloc/coins_manager/coins_manager_bloc.dart (6)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
🪛 GitHub Actions: takenagain is validating code guidelines 🚀
lib/views/wallet/coins_manager/coins_manager_list_header.dart
[info] 8-8: Parameter 'key' could be a super parameter.
lib/views/wallet/coins_manager/coins_manager_page.dart
[warning] 6-7: Unused imports: 'package:web_dex/bloc/coins_bloc/coins_bloc.dart' and 'package:web_dex/bloc/coins_manager/coins_manager_bloc.dart'.
[info] 15-15: Parameter 'key' could be a super parameter.
lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart
[info] 297-329: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
lib/model/coin.dart
[warning] 11-11: This class (or a class that this class inherits from) is marked as '@immutable', but one or more of its instance fields aren't final: Coin.usdPrice, Coin.isCustomCoin, Coin.address, Coin.fallbackSwapContract, Coin.parentCoin, Coin.state, Coin.sendableBalance.
💤 Files with no reviewable changes (3)
- test_units/tests/utils/test_util.dart
- lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fill_form_trezor_sender_address.dart
- lib/views/wallet/coin_details/coin_details_info/address_select.dart
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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.
lib/views/wallet/coins_manager/coins_manager_list_header.dart (4)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
lib/bloc/app_bloc_root.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_bloc/coins_bloc.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/views/wallet/coins_manager/coins_manager_page.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_manager/coins_manager_sort.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_manager/coins_manager_event.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/bloc/coins_bloc/coins_repo.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart (6)
Learnt from: takenagain
PR: #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: #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.
Learnt from: takenagain
PR: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
lib/bloc/coins_manager/coins_manager_state.dart (2)
Learnt from: takenagain
PR: #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: #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.
lib/model/coin.dart (6)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart (6)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
lib/bloc/coins_manager/coins_manager_bloc.dart (6)
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
🪛 GitHub Actions: takenagain is validating code guidelines 🚀
lib/views/wallet/coins_manager/coins_manager_list_header.dart
[info] 8-8: Parameter 'key' could be a super parameter.
lib/views/wallet/coins_manager/coins_manager_page.dart
[warning] 6-7: Unused imports: 'package:web_dex/bloc/coins_bloc/coins_bloc.dart' and 'package:web_dex/bloc/coins_manager/coins_manager_bloc.dart'.
[info] 15-15: Parameter 'key' could be a super parameter.
lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart
[info] 297-329: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
lib/model/coin.dart
[warning] 11-11: This class (or a class that this class inherits from) is marked as '@immutable', but one or more of its instance fields aren't final: Coin.usdPrice, Coin.isCustomCoin, Coin.address, Coin.fallbackSwapContract, Coin.parentCoin, Coin.state, Coin.sendableBalance.
🔇 Additional comments (30)
lib/views/wallet/coins_manager/coins_manager_list_header.dart (1)
4-5: LGTM! Import reorganization aligns with refactoring.The import changes correctly reflect the move of sorting functionality to the bloc layer and removal of the list wrapper dependency.
lib/bloc/app_bloc_root.dart (1)
288-296: LGTM! Dependency injection enhances CoinsManagerBloc functionality.The addition of
SettingsRepositoryandTradingEntitiesBlocdependencies enables the enhanced coins manager features including sorting, coin removal workflows, and preventing removal of coins with active swaps or orders.lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart (4)
5-5: LGTM! SDK integration improves balance accuracy.The addition of KomodoDefiSdk import enables more accurate balance retrieval directly from the SDK.
357-375: LGTM! Improved send button logic with SDK balance.The send button now uses the SDK's last known balance and consistent
isActivestatus check, which provides more accurate state management.
420-420: LGTM! Consistent coin status check.The swap button now uses
!coin.isActivewhich aligns with the unified coin status checking approach throughout the codebase.
427-429: LGTM! Consistent status check in Bitrefill tooltip.The tooltip logic now uses
!coin.isActivefor consistency with other button status checks.lib/model/coin.dart (4)
1-1: LGTM! Equatable import supports proper equality comparison.The Equatable import enables value-based equality comparison for the Coin class.
11-11: LGTM! Coin class extends Equatable for proper equality.Extending Equatable enables proper value comparison based on the
idfield, which is more reliable than default reference equality.
209-212: LGTM! Equality based on AssetId is appropriate.Using only the
id(AssetId) for equality comparison is the correct approach, as it uniquely identifies a coin regardless of other mutable properties.
11-213: Consider addressing mutable fields in Equatable class.The pipeline warns that this Equatable class has mutable fields (
usdPrice,isCustomCoin,address, etc.). While this appears to be existing technical debt, consider whether these fields should be immutable or if the class design needs refactoring for better consistency.This is likely existing technical debt rather than a new issue introduced by this PR, but it's worth noting for future cleanup.
lib/views/wallet/coins_manager/coins_manager_page.dart (1)
43-55: LGTM! UI simplification improves separation of concerns.The refactoring from
BlocConsumer<CoinsBloc, CoinsState>toBlocBuilder<AuthBloc, AuthBlocState>simplifies the UI by removing direct coin state management, which has been properly moved to the bloc layer.lib/bloc/coins_manager/coins_manager_sort.dart (1)
1-23: LGTM! Clean sorting abstraction implementation.The sorting types and data structure are well-defined and follow good Dart practices. The enum values are appropriate for coin management (protocol, balance, name, none) and the implementation correctly follows the generic
SortDatainterface pattern.lib/bloc/coins_bloc/coins_bloc.dart (2)
116-142: Approve the simplified balance refresh logic.The refactored
_onCoinsRefreshedmethod improves the code by:
- Removing unnecessary wallet type branching
- Using
emit.forEachfor cleaner stream handling- Maintaining proper error handling for coin synchronization
The logic is now more streamlined while preserving functionality.
424-424: Approve the notification suppression during activation.Adding
notify: falseto theactivateCoinsSynccall is appropriate for preventing duplicate state broadcasts during coin activation, which aligns with the broader notification control improvements in the coins repository.lib/bloc/coins_bloc/coins_repo.dart (5)
190-190: Approve the more explicit asset selection.Changing from
assets.firsttoassets.singleis more appropriate here as it clearly expresses the expectation of exactly one asset and will throw if multiple assets are found, making debugging easier.
270-273: Approve the notification control parameter.Adding the optional
notifyparameter with a default value oftruemaintains backward compatibility while enabling fine-grained control over state broadcasts during activation.
289-309: Approve the conditional notification broadcasts.The conditional
_broadcastAssetcalls based on thenotifyparameter provide proper control over state notifications during activation, preventing duplicate broadcasts when needed.
329-332: Approve the consistent method signature update.The
activateCoinsSyncmethod signature update maintains consistency withactivateAssetsSyncby adding the same notification control parameter.
361-385: Approve the notification control implementation.The conditional notification logic is consistently applied throughout the coin activation flow, providing proper control over state broadcasts while maintaining existing functionality.
lib/bloc/coins_manager/coins_manager_event.dart (2)
23-25: Approve the proper deprecation with clear reasoning.The deprecation annotation includes a helpful message explaining why the event is no longer needed, which will guide developers during refactoring.
46-63: Approve the new event classes for enhanced functionality.The new events properly support the expanded coin management features:
CoinsManagerSortChangedintegrates with the new sorting system- The removal workflow events (
CoinsManagerCoinRemoveRequested,CoinsManagerCoinRemoveConfirmed,CoinsManagerCoinRemovalCancelled) provide a clear state machine for coin removal with user confirmationAll events follow consistent naming patterns and include appropriate data payloads.
lib/bloc/coins_manager/coins_manager_state.dart (4)
10-21: Approve the state extensions for new functionality.The addition of
sortDataandremovalStatefields properly extends the state to support sorting and coin removal confirmation workflows. MakingremovalStateoptional is appropriate since it's only needed during removal operations.
33-38: Approve the sensible default sorting configuration.Setting the initial sort direction and type to
noneis appropriate, allowing users to explicitly choose their preferred sorting without imposing a default order.
48-61: Approve the proper state mutation handling.The
copyWithmethod correctly handles the new fields, withremovalStateproperly supporting null assignment for clearing the removal state.
82-107: Approve the comprehensive removal state management.The
CoinRemovalBlockReasonenum andCoinRemovalStateclass provide a well-structured approach to handling coin removal blocking scenarios:
- Clear enum values for different blocking reasons
- Comprehensive state tracking including child coins and order counts
- Convenient getter methods for checking blocking conditions
- Proper equality implementation
This design enables robust removal confirmation workflows with clear user feedback.
lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart (2)
36-76: Clean refactoring to BlocConsumer!The simplification from nested BlocListener/BlocBuilder to a single BlocConsumer improves readability. The widget now properly delegates all state management to the bloc.
138-151: Clean separation of concerns!The method correctly delegates coin selection logic to the bloc based on the action mode, allowing the bloc to handle all trading checks and state management.
lib/bloc/coins_manager/coins_manager_bloc.dart (3)
177-248: Well-implemented asynchronous coin activation!The implementation properly addresses the PR's concern about preventing activation spamming by:
- Immediately updating the UI state (line 204)
- Processing activation/deactivation asynchronously in the background
- Including error handling with logging
The analytics tracking for enable/disable events is also a nice addition.
361-462: Excellent implementation of safe coin removal workflow!The implementation properly addresses the PR's TODO about testing behavior with failing coins by:
- Checking for active swaps (including child coins)
- Checking for open orders (including child coins)
- Canceling orders when confirmed
- Handling both add mode (deselection) and remove mode
The separation of concerns between checking conditions and handling confirmation is well-designed.
220-221: Clarify notification suppression usage in CoinManagerBlocThe
activateCoinsSyncanddeactivateCoinsSyncAPIs use a namednotifyflag (defaulting totrue)—there is nosuppressNotificationsparameter. In your CoinManagerBloc you’re calling them without overridingnotify, so notifications will still be emitted. If the intent is to suppress notifications in these flows, you’ll need to passnotify: false.Locations to review:
- lib/bloc/coins_manager/coins_manager_bloc.dart
• Line ~220:await _coinsRepo.deactivateCoinsSync([event.coin]);
• Line ~236:await _coinsRepo.activateCoinsSync([event.coin]);Suggested diff:
- await _coinsRepo.deactivateCoinsSync([event.coin]); + await _coinsRepo.deactivateCoinsSync([event.coin], notify: false); … - await _coinsRepo.activateCoinsSync([event.coin]); + await _coinsRepo.activateCoinsSync([event.coin], notify: false);Please verify whether these calls should indeed suppress notifications.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/bloc/coins_manager/coins_manager_bloc.dart (1)
327-340: Correct use of unique identifier for coin mergingUsing
coin.id.idinstead ofcoin.abbrproperly handles coins with the same ticker on different chains.
🧹 Nitpick comments (1)
lib/bloc/auth_bloc/auth_bloc.dart (1)
331-335: Track this temporary workaround as technical debtWhile this workaround prevents race conditions during login, marking it as "temporary" without a concrete plan for resolution could lead to technical debt accumulation. Consider:
- Creating a GitHub issue to track the proper solution
- Adding more context about what the "more robust solution" should look like
- Setting a target timeline for addressing this
Would you like me to help create a GitHub issue to track this technical debt with a detailed description of the problem and potential solutions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
lib/bloc/auth_bloc/auth_bloc.dart(1 hunks)lib/bloc/auth_bloc/trezor_auth_mixin.dart(1 hunks)lib/bloc/coins_bloc/coins_bloc.dart(5 hunks)lib/bloc/coins_bloc/coins_repo.dart(7 hunks)lib/bloc/coins_manager/coins_manager_bloc.dart(5 hunks)lib/bloc/coins_manager/coins_manager_event.dart(2 hunks)lib/bloc/coins_manager/coins_manager_state.dart(4 hunks)lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart(5 hunks)
🧠 Learnings (3)
📓 Common learnings
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.701Z
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.
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.
lib/bloc/auth_bloc/auth_bloc.dart (1)
Learnt from: takenagain
PR: #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.
lib/bloc/coins_manager/coins_manager_bloc.dart (7)
Learnt from: takenagain
PR: #2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.701Z
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.
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/bloc/coins_bloc/coins_bloc.dart
- lib/bloc/coins_bloc/coins_repo.dart
- lib/bloc/coins_manager/coins_manager_state.dart
- lib/views/wallet/coins_manager/coins_manager_list_wrapper.dart
- lib/bloc/coins_manager/coins_manager_event.dart
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.701Z
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.
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.
lib/bloc/auth_bloc/auth_bloc.dart (1)
Learnt from: takenagain
PR: #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.
lib/bloc/coins_manager/coins_manager_bloc.dart (7)
Learnt from: takenagain
PR: #2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.701Z
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.
Learnt from: takenagain
PR: #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: #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: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #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.
⏰ 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 Mobile (Android)
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Test web-app-macos
- GitHub Check: validate_code_guidelines
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (macos)
- GitHub Check: Build Desktop (windows)
- GitHub Check: build_and_preview
- GitHub Check: unit_tests
🔇 Additional comments (8)
lib/bloc/auth_bloc/trezor_auth_mixin.dart (1)
25-26: Good addition of loading state emissionThis provides immediate user feedback when Trezor authentication starts, improving the user experience.
lib/bloc/coins_manager/coins_manager_bloc.dart (7)
24-61: Well-structured dependency injection and event handling setupThe addition of
SettingsRepositoryandTradingEntitiesBlocdependencies is clean and follows proper dependency injection patterns. The comment about the sequential transformer provides valuable context for future maintainers.
74-105: Good implementation of wallet coins merging and filteringThe logic for merging wallet coins in add mode is well-documented and addresses the UI state management needs. The test coins filtering based on user settings is a nice touch.
178-249: Good refactoring of coin selection logicThe extraction of activation/deactivation logic into separate methods improves code organization. The immediate state emission for UI responsiveness is a good UX improvement.
Note that errors during activation/deactivation are caught and logged but don't update the UI state. This is acceptable since the selection state has already been updated, but consider if users should be notified of activation failures.
276-283: Clean sorting implementationThe sorting logic is well-structured with clear separation between sort types. The delegation to specific sort functions maintains good modularity.
Also applies to: 342-360
362-494: Excellent implementation of coin removal flow with trading checksThe removal flow is comprehensive and handles all edge cases well:
- Checks for active swaps on both parent and child coins
- Verifies open orders before allowing removal
- Includes order cancellation with proper error handling
- Re-verifies order cancellation to ensure success
- Provides clear error messages to users
The separation between add mode (deselection) and remove mode is well-handled.
285-325: Well-implemented filtering and merging logicThe filtering methods handle their responsibilities cleanly:
- Test coins filtering respects user preferences
- Phrase filtering includes alphabetical sorting for better UX
- Wallet coins merging correctly prevents duplicates using coin IDs
496-526: Clean implementation of state management helpersThe cancellation and error clearing handlers are straightforward and correct. The
_getOriginalCoinListhelper properly filters excluded assets for the add action.
when disabling parent, children should also be indicated as disabled
when logging in and out quickly the new events would be dropped in favor of the previous running event, resulting in an empty coins list
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
if user signed out, then signed back in again without refreshing, the activation state would be dropped due to a lack of listeners. _enabledCoinsSubscription should only be cancelled in the onClose or when spawning another
takenagain
changed the title
|
Though it may be too late for this PR, perhaps we could reduce this to a single page? For example, on the main wallet page which only shows active coins, we could have another toggle (hide disabled coins) similar to the "hide zero balance" toggle. When in state where disabled are shown, switch out the address expansion arrow for an activation toggle, and block "click to coin wallet" on the disabled. This would also shift the protocol filter into the main view, which currently lacks such a filter. One last thing which deviates from legacy desktop, is that there is no bulk selection to activate / deactivate a selected set of coins. I'm in two minds over this, as lazy users could soon find their machines overwhelmed with a "activate 500 coins" command. On the other hand, its inconvenient to manually enable/disable coins individually if you have more than half a dozen to get thru. A compromise which offers some convenience on the "disable" side, would be to add a "disable zero balance assets" button. This eliminates laborious single disable ops, without risk of performance issues via bulk enable. I'll push the above in to an issue for next iteration 👍 |
|
When filtering for MATIC/PLG20, I noticed that MATIC itself was halfway down the list. It would be nice if for EVM/Tendermint assets, that the parent is pinned to the top (ie. frozen row), with tokens in selected order beneath. To further reduce the potentially long lists, even when filtered, I'd propose we also include a "disabled only" / "active only" option to the filter panel. |
smk762
left a comment
smk762
left a comment
There was a problem hiding this comment.
Thanks! Functions to intended specification.
A few additional releated comments, suggestions and observations have been delegated to issues for future enhancement.
CharlVS
left a comment
CharlVS
left a comment
There was a problem hiding this comment.
LGTM. The Coins "bloc" is an ongoing refactoring effort. There is still a decent chunk of tech debt surrounding it which we still revisit.
…2976) * feat(activation): show enabled assets in Add Assets page (#2973) * feat(wallet): show enabled coins in add asset * fix(wallet): show enabled assets in add coins * fix(coins-bloc): ignore repo broadcasts when activating from bloc * refactor(coins): move sorting to bloc * fix(coins): filter test coins in bloc * refactor(coins-manager): add sequential transformer and fix warnings * refactor(coins-manager): emit state first for responsive UI * fix(coins-manager): filter and sort selected coins alongside others * refactor(coin): remove unused legacy `accounts` field * feat(coins-manager): move deactivation trading check to bloc * fix(coin-details): disable send & swap buttons if coin is not active * refactor(coins-manager): improve error handling * fix(coins-bloc): incorrect active state emitted when syncing states * fix(coins-repo): move metadata updating before coin activation * fix(coins-manager): remove children from selected coins list when disabling parent, children should also be indicated as disabled * chore(deps): bump SDK to 74d8eff * fix(coins-bloc): simplify and de-duplicate activation logic and syncing * fix(activation): broadcast and subscribe to parent if child asset * fix(activation): change login activation events to restartable when logging in and out quickly the new events would be dropped in favor of the previous running event, resulting in an empty coins list * fix(activation): do not cancel state syncing subscription on logout if user signed out, then signed back in again without refreshing, the activation state would be dropped due to a lack of listeners. _enabledCoinsSubscription should only be cancelled in the onClose or when spawning another
Fixes
The same coin deactivation logic from #2750 should apply here as well:
Coin activation failures tested with:
Coin deactivation test cases from #2750 (comment) should apply in the "Add Assets" page as well:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removals
Refactor