Add multi-address support to "Faucet" button #2533
Conversation
Remove the faucet button from the coin overview and add it as a button icon in the addresses list Move references to faucet_button.dart from the details page to the coin address list. Note that you might have to change how the page is called in coin_details.dart to pass the selected address of the coin (rather than defaultAddress) NB: Take a branch from dev and name it add/multi-address-faucet
|
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 update integrates a new faucet feature managed by a dedicated bloc. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FaucetButton
participant FaucetBloc
participant FaucetAPI
User->>FaucetButton: Click button
FaucetButton->>FaucetBloc: Dispatch FaucetRequested(coinAbbr, address)
FaucetBloc->>FaucetAPI: Call faucet API with coinAbbr & address
FaucetAPI-->>FaucetBloc: Return response (success/error)
FaucetBloc-->>FaucetButton: Emit state (FaucetLoading, then FaucetSuccess/FaucetError)
FaucetButton->>User: Update UI (e.g., show dialog or error message)
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
The button would disappear when the browser was minimized to mobile size. Also did some UI changes to make the button look more like swap address.
Faucet Button now uses Bloc. Problem: showDialog appears for each addressCard because they each have a faucet button.
Fixed multiple dialogs appearing for each faucet button by creating an activeAddress
|
@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 (7)
lib/views/wallet/coin_details/faucet/faucet_button.dart (4)
29-30: Consider instance-based address tracking instead of static variable.The static
_activeAddressvariable can cause issues if multiple FaucetButtons are active simultaneously, as they would overwrite each other's active address. This could lead to unexpected behavior, especially if users interact with multiple buttons in quick succession.class _FaucetButtonState extends State<FaucetButton> { - static String? _activeAddress; + bool _isRequesting = false;Then update the button logic accordingly:
onPressed: () { - if (_activeAddress != null) { + if (_isRequesting) { return; } - _activeAddress = widget.address.address; + setState(() => _isRequesting = true); context.read<FaucetBloc>().add(FaucetRequested( coinAbbr: widget.coinAbbr, address: widget.address.address, )); },And in the listener:
Future.delayed(Duration(milliseconds: 300), () { - _activeAddress = null; + if (mounted) setState(() => _isRequesting = false); });
36-61: Add visual indication when button is disabled.The button is functionally disabled when
_activeAddressis set, but there's no visual indication to the user. The button should show a disabled state to improve user experience.child: UiPrimaryButton( key: Key('coin-details-faucet-button-${widget.address.address}'), height: isMobile ? 24.0 : 32.0, backgroundColor: themeData.colorScheme.tertiary, + enabled: _activeAddress == null && widget.enabled, onPressed: () { if (_activeAddress != null) { return; } _activeAddress = widget.address.address; context.read<FaucetBloc>().add(FaucetRequested( coinAbbr: widget.coinAbbr, address: widget.address.address, )); },
37-40: Improve listenWhen condition for better state management.The current
listenWhencondition only considers the active address, but it should also verify relevant state transitions to avoid unnecessary dialog displays.listenWhen: (previous, current) { - return _activeAddress == widget.address.address; + return _activeAddress == widget.address.address && + (current is FaucetLoading || + (previous is FaucetLoading && current is! FaucetLoading)); },
51-55: Add error handling for Future.delayed.Using a fixed delay to reset state could lead to timing issues. Consider using the FaucetBloc state transitions to manage the button state instead of a timer.
onClose: () { Navigator.of(dialogContext).pop(); - Future.delayed(Duration(milliseconds: 300), () { - _activeAddress = null; - }); + // Reset immediately or on next frame to ensure UI consistency + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted) { + setState(() { + _activeAddress = null; + }); + } + }); },lib/views/wallet/coin_details/faucet/faucet_view.dart (3)
26-46: Consider enhancing dialog accessibility.The Dialog implementation lacks some accessibility features that would improve usability.
return Dialog( shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(16)), + insetPadding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 24.0), + clipBehavior: Clip.antiAliasWithSaveLayer, child: Container( constraints: const BoxConstraints(maxWidth: 400), padding: const EdgeInsets.all(16), child: BlocBuilder<FaucetBloc, FaucetState>( + buildWhen: (previous, current) => previous != current, builder: (context, state) { return Column( mainAxisSize: MainAxisSize.min, children: [ _DialogHeader(title: title(state), onClose: onClose), _StatesOfPage( state: state, onClose: onClose, ), ], ); }, ), ), );
63-86: Enhance dialog header with better semantics.The dialog header could be improved with additional semantics for screen readers.
class _DialogHeader extends StatelessWidget { final String title; final VoidCallback onClose; const _DialogHeader({required this.title, required this.onClose}); @override Widget build(BuildContext context) { return Row( children: [ Expanded( child: Text( title, style: Theme.of(context).textTheme.titleLarge, + semanticsLabel: title, ), ), IconButton( icon: const Icon(Icons.close), onPressed: onClose, + tooltip: MaterialLocalizations.of(context).closeButtonTooltip, + semanticsLabel: MaterialLocalizations.of(context).closeButtonTooltip, ), ], ); } }
96-98: Add loading state indicator and cancel option.The loading state doesn't provide any way for users to cancel an in-progress request, which could be problematic if the request is taking too long.
if (localState is FaucetLoading || localState is FaucetInitial) { - return const _Loading(); + return _Loading(onCancel: onClose); }Then update the _Loading class:
class _Loading extends StatelessWidget { - const _Loading(); + final VoidCallback? onCancel; + const _Loading({this.onCancel}); @override Widget build(BuildContext context) { - return const Column( + return Column( crossAxisAlignment: CrossAxisAlignment.start, mainAxisSize: MainAxisSize.min, children: [ - SizedBox(height: 28), - Center(child: UiSpinner()), - SizedBox(height: 28), + const SizedBox(height: 28), + const Center(child: UiSpinner()), + const SizedBox(height: 28), + if (onCancel != null) + Center( + child: TextButton( + onPressed: onCancel, + child: Text(LocaleKeys.cancel.tr()), + ), + ), + const SizedBox(height: 12), ], ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/bloc/app_bloc_root.dart(2 hunks)lib/bloc/faucet_button/faucet_button_bloc.dart(1 hunks)lib/bloc/faucet_button/faucet_button_event.dart(1 hunks)lib/bloc/faucet_button/faucet_button_state.dart(1 hunks)lib/views/wallet/coin_details/coin_details.dart(0 hunks)lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart(5 hunks)lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart(0 hunks)lib/views/wallet/coin_details/coin_details_info/coin_details_info.dart(2 hunks)lib/views/wallet/coin_details/coin_page_type.dart(1 hunks)lib/views/wallet/coin_details/faucet/faucet_button.dart(1 hunks)lib/views/wallet/coin_details/faucet/faucet_page.dart(2 hunks)lib/views/wallet/coin_details/faucet/faucet_view.dart(5 hunks)
💤 Files with no reviewable changes (2)
- lib/views/wallet/coin_details/coin_details.dart
- lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Test web-app-macos
- GitHub Check: Build Desktop (windows)
- GitHub Check: Build Mobile (Android)
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Build Desktop (macos)
- GitHub Check: validate_code_guidelines
- GitHub Check: Build Mobile (iOS)
- GitHub Check: unit_tests
🔇 Additional comments (19)
lib/views/wallet/coin_details/coin_page_type.dart (1)
1-1: Change aligns with the PR objective.The removal of the
faucetvalue from theCoinPageTypeenum is consistent with the PR's goal of relocating the faucet button from the coin overview section to the addresses list.lib/bloc/faucet_button/faucet_button_event.dart (1)
3-21: Well-structured BLoC event implementation.The event architecture follows good BLoC pattern practices with proper use of:
- Abstract base class extending
Equatable- Immutable fields with required parameters
- Correctly overridden
propsgetter for equality comparisonThis structure will support the faucet functionality being moved to the addresses list as described in the PR objectives.
lib/bloc/app_bloc_root.dart (2)
33-33: Appropriate import for the new faucet functionality.This import supports the integration of the new faucet button BLoC implementation.
308-310: BLoC integration follows established patterns.The
FaucetBlocprovider is correctly integrated into the application's state management architecture, using the same pattern as other BLoC providers in the file. This ensures the faucet functionality will be accessible throughout the widget tree.lib/bloc/faucet_button/faucet_button_state.dart (1)
4-35: Comprehensive state management for faucet operations.The state architecture is well-implemented with:
- A clear hierarchy with an abstract base class
- Distinct states for initial, loading, success and error scenarios
- Proper use of
Equatablefor state comparison- Appropriate props overrides in states with data
This state management approach will provide good user feedback during faucet operations and handle different outcomes effectively.
lib/views/wallet/coin_details/faucet/faucet_page.dart (2)
4-18: Improved architecture by converting to StatefulWidgetThe change from StatelessWidget to StatefulWidget provides better structure for this component, making it more maintainable as it now properly isolates state management.
20-29: Good implementation of state handlingThe implementation now directly uses FaucetView with the needed properties, which simplifies the widget hierarchy and removes unnecessary BlocProvider wrapping at this level.
lib/views/wallet/coin_details/coin_details_info/coin_details_info.dart (2)
190-191: Correctly passing setPageType to CoinAddressesThis change enables the address list to handle navigation to the faucet feature, which aligns with moving the faucet functionality from the overview to the addresses list.
294-298: Correctly passing setPageType to CoinAddresses in mobile layoutThe changes ensure consistent behavior between desktop and mobile interfaces by passing the same navigation function.
lib/bloc/faucet_button/faucet_button_bloc.dart (3)
1-8: Good imports organization for the new faucet blocThe imports are well-organized, including all necessary dependencies for the bloc implementation. The sequential import from bloc_concurrency will help prevent race conditions in faucet requests.
9-15: Well-structured FaucetBloc implementationThe bloc implementation follows best practices by:
- Storing KomodoDefiSdk as a dependency
- Correctly initializing the initial state
- Using the sequential transformer to prevent multiple simultaneous requests
16-41: Comprehensive event handling with proper error managementThe _onFaucetRequest method:
- Correctly checks if already loading to prevent duplicate requests
- Properly emits loading state at the beginning
- Has comprehensive try/catch error handling
- Properly differentiates between API errors and network errors
- Uses the addError method to properly log errors with stack traces
This implementation ensures that errors are properly handled and reported to the user with meaningful messages.
lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart (5)
21-22: Added necessary imports for faucet functionalityThe imports for the faucet button and page type are required for the new faucet integration in the address list.
28-32: Added required setPageType parameter to CoinAddressesThis parameter addition ensures the component can handle navigation to the faucet page when requested.
198-204: Added required setPageType parameter to AddressCardPassing the navigation function down to the address card allows each individual address to trigger faucet functionality.
228-238: Well-implemented faucet button in mobile layoutThe faucet button is:
- Correctly conditionally rendered based on coin.hasFaucet
- Properly constrained with appropriate size limits
- Correctly receiving the coin abbreviation and address parameters
This implementation properly integrates the faucet functionality into the mobile address list view.
261-271: Well-implemented faucet button in desktop layoutThe desktop implementation correctly mirrors the mobile implementation while considering desktop UI requirements. The button is properly positioned in the address row.
lib/views/wallet/coin_details/faucet/faucet_button.dart (1)
13-27: Good conversion to StatefulWidget to handle state changes.The change from StatelessWidget to StatefulWidget is appropriate for maintaining state during faucet operations. The constructor parameters now properly support multiple addresses, aligning with the PR objective of adding multi-address support.
lib/views/wallet/coin_details/faucet/faucet_view.dart (1)
13-18: Good API redesign for multi-address support.The constructor parameters now properly include
coinAbbrandcoinAddress, enabling the faucet to work with any selected address rather than just the default address. This aligns well with the PR objective.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Made changes of tracking active address to not use a static variable but rather handle it in the bloc and not in the button. Made some changes to the UI to show unactive buttons. Improved state management in Listenerwhen.
|
@ShantanuSharma9873 Please test that the faucet button works in HD wallet mode for test coins. |
Remove the faucet button from the coin overview and add it as a button icon in the addresses list
Move references to faucet_button.dart from the details page to the coin address list. Note that you might have to change how the page is called in coin_details.dart to pass the selected address of the coin (rather than defaultAddress)
Summary by CodeRabbit
New Features
Refactor