fix: mobile seed banner, text alignment, and input dismissal#3225
fix: mobile seed banner, text alignment, and input dismissal#3225
Conversation
twas disabled by default for mobile for some reason (undocumented)
coins with long names, like BAT, would overflow and cause alignment issues. Fixed with expanded and flex
|
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR adds a new FiatOrderStatus.submitting state and a public FiatFormState.isBanxaSelected flag; FiatFormBloc emits submitting before validation and uses isBanxaSelected for Banxa checks. It also adds keyboard-dismiss-on-scroll and focus lifecycle management across several forms and updates multiple UI/layout components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Input as CustomFiatInputField
participant Form as FiatForm
participant Bloc as FiatFormBloc
participant Status as FiatOrderStatus
User->>Input: Enter amount, press done
Input->>Input: onSubmitted -> unfocus()
Input->>Form: onSubmitted(value)
Form->>Form: _scrollController.unfocus() / dismiss keyboard
Form->>Bloc: FiatFormSubmitted
Bloc->>Status: Emit submitting
Bloc->>Bloc: Validate form
alt valid
Bloc->>Status: Emit submitted / proceed
else invalid
Bloc->>Status: Emit error
end
sequenceDiagram
participant User
participant Scroll as ScrollView
participant Controller as ScrollController
participant Focus as FocusScope
User->>Scroll: Scrolls form
Scroll->>Controller: scroll event
Controller->>Focus: unfocus()
Focus->>Focus: Keyboard dismissed
Note over Focus: Inputs lose focus on scroll
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
Visit the preview URL for this PR (updated for commit d25676d): https://walletrc--pull-3225-merge-rcyauj08.web.app (expires Wed, 05 Nov 2025 07:34:18 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
takenagain
force-pushed
the
hotfix/fiat-form-and-banner
branch
from
b53bb2f to
ffe74db
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several mobile UI issues related to the seed backup banner visibility, keyboard dismissal behavior, and text alignment in coin lists and fiat forms.
Key Changes
- Added keyboard auto-dismiss on scroll for fiat onramp and swap pages
- Fixed text wrapping and alignment in expandable coin list items using
ExpandedandAutoScrollText - Made seed backup banner visible on mobile by removing the
hideOnMobiledefault
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart | Wrapped coin name and balance columns in Expanded widgets and switched to AutoScrollText for better text handling |
| lib/views/fiat/fiat_select_button.dart | Added Flexible wrapper and expanded parameter to handle flexible layout |
| lib/views/fiat/fiat_provider_web_view_settings.dart | Updated trusted domain filters with debug mode conditions and simplified code formatting |
| lib/views/fiat/fiat_inputs.dart | Added focus node management and keyboard dismissal on scroll/submit |
| lib/views/fiat/fiat_form.dart | Implemented scroll controller with keyboard dismissal listener and GestureDetector for tap-to-dismiss |
| lib/views/fiat/fiat_asset_icon.dart | Created custom coin item implementation with flexible/expanded support for proper text wrapping |
| lib/views/fiat/custom_fiat_input_field.dart | Converted to StatefulWidget with focus node support and keyboard dismissal on submit |
| lib/views/dex/simple/form/taker/taker_form_layout.dart | Added scroll listeners for keyboard dismissal in both desktop and mobile layouts |
| lib/views/dex/simple/form/maker/maker_form_layout.dart | Added scroll listeners for keyboard dismissal in both desktop and mobile layouts |
| lib/views/common/pages/page_layout.dart | Changed BackupSeedNotification to show on mobile and cleaned up formatting |
| lib/bloc/taker_form/taker_bloc.dart | Added error handling for max sell amount updates and reorganized imports |
| lib/bloc/fiat/fiat_order_status.dart | Added submitting status to track when user opens provider webview |
| lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart | Added isBanxaSelected getter for cleaner provider checks |
| lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart | Updated to emit submitting status and use new isBanxaSelected getter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (2)
168-178: Don’t leave fiatOrderStatus in submitting on validation failure.You emit
submittingbefore validation, then return without resetting, leaving the UI stuck in a loading state.Apply this fix:
Future<void> _onFormSubmitted( @@ - emit(state.copyWith(fiatOrderStatus: FiatOrderStatus.submitting)); + emit(state.copyWith(fiatOrderStatus: FiatOrderStatus.submitting)); final formValidationError = getFormIssue(); if (formValidationError != null || !state.isValid) { _log.warning('Form validation failed. Validation: ${state.isValid}'); - return; + emit(state.copyWith(fiatOrderStatus: FiatOrderStatus.initial)); + return; }
232-245: Use one Banxa detection source everywhere.you use
state.isBanxaSelectedto choose dialog mode but still check providerId == 'Banxa' for status watching. Keep it consistent to avoid mismatch.- if (state.selectedPaymentMethod.providerId != 'Banxa') { + if (!state.isBanxaSelected) { return; }Also consider centralizing Banxa constants inside FiatFormState getter to avoid stringly-typed checks. Based on learnings.
Also applies to: 350-355
lib/views/fiat/fiat_asset_icon.dart (1)
18-31: Wire unused tap handler and parameters or remove them.The fields
onTap,icon, andassetExistsare required but never used in thebuild()method. Both call sites actively pass these parameters expecting the widget to handle them:
fiat_select_button.dart:94passesonTap,icon,assetExistsfiat_currency_list_tile.dart:30passesonTap,icon,assetExistsThis creates a behavioral regression: callers expect tap handling that never fires.
Fix: Wrap both return paths with
InkWell(onTap: onTap, child: ...)to restore tap behavior, or remove/deprecate these unused fields and update all call sites.
🧹 Nitpick comments (14)
lib/views/dex/simple/form/maker/maker_form_layout.dart (1)
136-139: Consider adding a scroll threshold for smoother UX.The keyboard dismisses on every scroll event, which might feel abrupt on accidental touches. Consider dismissing only when the scroll distance exceeds a small threshold.
Example enhancement:
+ double _lastScrollPosition = 0.0; + static const double _scrollThreshold = 10.0; + void _onScroll() { - // Dismiss keyboard when user starts scrolling - FocusScope.of(context).unfocus(); + final currentPosition = _scrollController.position.pixels; + if ((currentPosition - _lastScrollPosition).abs() > _scrollThreshold) { + FocusScope.of(context).unfocus(); + _lastScrollPosition = currentPosition; + } }Note: Apply similar logic to both desktop scroll controllers if implementing this enhancement.
Also applies to: 215-218
lib/bloc/fiat/fiat_order_status.dart (1)
8-10: Disambiguate “submitting” vs “submitted”.Docs for both states read similarly (“webview opened”). Please clarify intended transition (e.g., submitting = opening/navigating; submitted = provider page fully loaded/hand-off done) to prevent UI logic drift.
lib/views/common/pages/page_layout.dart (1)
62-64: Remove redundant Column wrapper to reduce layout depth.
PagePlate(child: content)is sufficient ifcontentmanages its own sizing/scrolling. The extraColumnadds a render object without functional gain and can affect constraints.
Apply:- child: Column(mainAxisSize: MainAxisSize.max, children: [content]), + child: content,Also, desktop header change looks clean.
Also applies to: 95-96
lib/views/fiat/fiat_provider_web_view_settings.dart (1)
6-12: Trusted domains: good split by build mode.Debug-only Ramp demo and Banxa sandbox are gated; production adds app.komodoplatform.com. Consider anchoring patterns to reduce over-match risk:
^https?://(app\.ramp\.network|komodo\.banxa\.com|app\.komodoplatform\.com)/.*$.lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (1)
171-220: Avoid combining Spacer with multiple Expanded in the same Row.
Expanded(flex: 8) + Spacer() + Expanded(flex: 7)can yield unstable widths on small screens. ReplaceSpacer()with a fixedSizedBox(width: 8..12)or fold spacing into flex.
Apply:- const Spacer(), + const SizedBox(width: 12),Optional: move the right block to
Flexible(fit: FlexFit.tight, flex: 7, ...)for predictable layout.lib/bloc/taker_form/taker_bloc.dart (3)
451-469: Activation check before max-volume: good safeguard.Leaving
availableBalanceStateas loading for inactive assets avoids misleading “0.00”. Consider emitting a distinct “requiresActivation” state if UX needs to hint action.
471-477: Early return on not-logged-in for clarity.You already set
unavailable; considerreturnto signal intent and shortcut further logic.
Apply:- if (!_isLoggedIn) { + if (!_isLoggedIn) { emitter( state.copyWith( availableBalanceState: () => AvailableBalanceState.unavailable, ), ); - } else { + return; + } else {
478-499: Set failure state on unexpected errors; consider RPC timeouts.On exceptions you only log. Emit
AvailableBalanceState.failureto stop spinners and surface retriable errors. Also consider adding a timeout around RPCs.
Apply:} catch (e, s) { _log.severe('Failed to update max sell amount', e, s); + emitter( + state.copyWith( + availableBalanceState: () => AvailableBalanceState.failure, + ), + ); }Optional timeout example:
- Rational? maxSellAmount = await _dexRepo.getMaxTakerVolume( + Rational? maxSellAmount = await _dexRepo + .getMaxTakerVolume( state.sellCoin!.abbr, - ); + ) + .timeout(const Duration(seconds: 8));Also applies to: 501-502
lib/views/fiat/fiat_select_button.dart (1)
93-101: Avoid Flexible inicon:slot ofFilledButton.icon.The button already lays out the icon;
Flexiblecan cause layout quirks. Pass the icon directly.
Apply:- icon: currency == null - ? Icon(_getDefaultAssetIcon(isFiat ? 'fiat' : 'coin')) - : Flexible( - child: FiatAssetIcon( - currency: currency!, - icon: icon, - onTap: onTap, - assetExists: assetExists, - expanded: true, - ), - ), + icon: currency == null + ? Icon(_getDefaultAssetIcon(isFiat ? 'fiat' : 'coin')) + : FiatAssetIcon( + currency: currency!, + icon: icon, + onTap: onTap, + assetExists: assetExists, + expanded: true, + ),lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart (1)
123-125: Centralize the 'Banxa' provider ID constant to prevent drift across multiple comparisons.The magic string 'Banxa' is duplicated in at least 2 production code locations performing string comparisons (fiat_form_state.dart:124 and fiat_form_bloc.dart:352). Extract a static constant (e.g.,
static const String banxaProviderId = 'Banxa') inBanxaFiatProviderand use it in both locations. This eliminates the risk of inconsistent updates and improves maintainability.lib/views/fiat/fiat_asset_icon.dart (2)
43-57: Expanded inside a Row with mainAxisSize.min can overflow in unbounded widths.Expanded forces the Row to take max width; with mainAxisSize.min/unbounded parents (e.g., Wrap/IntrinsicWidth), this can throw layout exceptions. Prefer Flexible(fit: FlexFit.tight) or switch to mainAxisSize.max when expanded.
- return Row( - mainAxisSize: MainAxisSize.min, + return Row( + mainAxisSize: expanded ? MainAxisSize.max : MainAxisSize.min, crossAxisAlignment: CrossAxisAlignment.start, children: [ AssetLogo.ofId(coin.id, size: size.coinLogo), SizedBox(width: size.spacer), - expanded - ? Expanded( - child: _FiatCoinItemLabel(size: size, coin: coin), - ) - : Flexible( - child: _FiatCoinItemLabel(size: size, coin: coin), - ), + Flexible( + fit: expanded ? FlexFit.tight : FlexFit.loose, + child: _FiatCoinItemLabel(size: size, coin: coin), + ), ], );Reproduce potential issue by embedding this widget in a Wrap/IntrinsicWidth and toggling expanded=true.
82-116: Tighten nullability for internal widgets._coin is non-null at call sites; declaring Coin? forces defensive code and hides real null issues. Same for subtitle.
-class _FiatCoinItemTitle extends StatelessWidget { - const _FiatCoinItemTitle({required this.coin, required this.size}); - final Coin? coin; +class _FiatCoinItemTitle extends StatelessWidget { + const _FiatCoinItemTitle({required this.coin, required this.size}); + final Coin coin; @@ - CoinTicker( - coinId: coin?.abbr, + CoinTicker( + coinId: coin.abbr, @@ - child: _FiatCoinName( - text: coin?.displayName, + child: _FiatCoinName( + text: coin.displayName,-class _FiatCoinItemSubtitle extends StatelessWidget { - const _FiatCoinItemSubtitle({required this.coin, required this.size}); - final Coin? coin; +class _FiatCoinItemSubtitle extends StatelessWidget { + const _FiatCoinItemSubtitle({required this.coin, required this.size}); + final Coin coin; @@ - return coin?.mode == CoinMode.segwit + return coin.mode == CoinMode.segwit @@ - text: coin?.typeNameWithTestnet, + text: coin.typeNameWithTestnet,Keep _FiatCoinName.text nullable if upstream content can be absent.
Also applies to: 118-134, 136-155
lib/views/fiat/custom_fiat_input_field.dart (1)
62-83: fillColor may be ignored withoutfilled: true.Unless your theme sets
filled: trueglobally, this localfillColorwon’t apply.- final InputDecoration inputDecoration = InputDecoration( + final InputDecoration inputDecoration = InputDecoration( label: widget.label, @@ - fillColor: Theme.of(context).colorScheme.onSurface, + filled: true, + fillColor: Theme.of(context).colorScheme.onSurface,If the theme already sets
filled, ignore this change.lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (1)
41-53: Guard against double submit with a droppable transformer.The
droppable()transformer frombloc_concurrencyis available and properly imported. Apply it toFiatFormSubmittedat line 47 to prevent race conditions from multiple submissions.- on<FiatFormSubmitted>(_onFormSubmitted); + on<FiatFormSubmitted>(_onFormSubmitted, transformer: droppable());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart(2 hunks)lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart(1 hunks)lib/bloc/fiat/fiat_order_status.dart(2 hunks)lib/bloc/taker_form/taker_bloc.dart(5 hunks)lib/views/common/pages/page_layout.dart(2 hunks)lib/views/dex/simple/form/maker/maker_form_layout.dart(6 hunks)lib/views/dex/simple/form/taker/taker_form_layout.dart(3 hunks)lib/views/fiat/custom_fiat_input_field.dart(5 hunks)lib/views/fiat/fiat_asset_icon.dart(3 hunks)lib/views/fiat/fiat_form.dart(7 hunks)lib/views/fiat/fiat_inputs.dart(5 hunks)lib/views/fiat/fiat_provider_web_view_settings.dart(2 hunks)lib/views/fiat/fiat_select_button.dart(2 hunks)lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Runflutter analyzeand resolve issues before committing code
Format Dart code withdart format(on changed files) before committing
Files:
lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dartlib/bloc/fiat/fiat_order_status.dartlib/views/dex/simple/form/maker/maker_form_layout.dartlib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dartlib/views/fiat/fiat_provider_web_view_settings.dartlib/views/wallet/wallet_page/common/expandable_coin_list_item.dartlib/views/fiat/fiat_asset_icon.dartlib/views/fiat/custom_fiat_input_field.dartlib/bloc/taker_form/taker_bloc.dartlib/views/fiat/fiat_inputs.dartlib/views/fiat/fiat_form.dartlib/views/dex/simple/form/taker/taker_form_layout.dartlib/views/fiat/fiat_select_button.dartlib/views/common/pages/page_layout.dart
🧠 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/fiat/fiat_asset_icon.dartlib/bloc/taker_form/taker_bloc.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/fiat/fiat_asset_icon.dartlib/bloc/taker_form/taker_bloc.dart
🔇 Additional comments (25)
lib/views/dex/simple/form/maker/maker_form_layout.dart (2)
119-139: LGTM! Keyboard dismissal on scroll is correctly implemented.The ScrollController lifecycle management follows Flutter best practices:
- Controllers are initialized in
initState()- Listeners are registered immediately after initialization
- Listeners are removed before disposal
- Controllers are properly disposed
The keyboard dismissal using
FocusScope.of(context).unfocus()is the standard approach and aligns with the PR objectives.
202-218: LGTM! Mobile keyboard dismissal mirrors the desktop implementation.The ScrollController lifecycle management and keyboard dismissal are correctly implemented, maintaining consistency with the desktop layout pattern.
lib/views/common/pages/page_layout.dart (1)
56-56: Banner visibility on mobile: LGTM.Explicit
hideOnMobile: falserestores the seed banner as intended.lib/views/fiat/fiat_provider_web_view_settings.dart (1)
44-46: Content blockers simplification: LGTM.Default block + allowlist via IGNORE_PREVIOUS_RULES is clear and secure.
Also applies to: 50-51
lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (1)
150-168: Auto-scrolling name/balance on mobile: LGTM.Improves truncation issues for long tickers and balances.
lib/bloc/taker_form/taker_bloc.dart (1)
103-104: Logger added: LGTM.Local
_logimproves error traceability without polluting global logs.lib/bloc/fiat/fiat_order_status.dart (1)
35-38: AllisSubmittingcall sites verified—no corrections needed.The two identified usages remain logically correct after the change:
Button text (
lib/views/fiat/fiat_form.dart:151): Conditionally displays "Submitting..." whenisSubmitting=true, which now correctly includes the new pre-hand-offsubmittingphase.Form validation (
lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart:120): Disables the form whenisSubmitting=true, which appropriately prevents further input during the submitting phase.The addition of
FiatOrderStatus.submittingto theisSubmittinggetter aligns form-disable logic and button state with the new phase as intended.lib/views/fiat/fiat_select_button.dart (1)
51-55: Review comment is factually incorrect; no changes needed.The code already uses the correct API. Color.withValues was introduced in Flutter 3.27, and withOpacity was deprecated in 3.27. The codebase's minimum Flutter version is
>=3.35.3, which is well above 3.27, sowithValuesis fully available. The review's suggestion to switch towithOpacityis backwards—withOpacityis the deprecated API. The current code at lines 51–55 and 65–67 is already correct.Likely an incorrect or invalid review comment.
lib/views/fiat/custom_fiat_input_field.dart (2)
34-49: Good FocusNode ownership and disposal.Local node is created when none provided and disposed conditionally. Solid.
97-107: Rewrite to manual verification request — thedarttoolchain is unavailable in the sandbox environment.The code changes appear sound (onSubmitted, inputFormatters, keyboardType, and textInputAction all align correctly with the requirement to dismiss the keyboard on submit), but I cannot execute
flutter analyzeanddart formathere. Please verify locally before committing.lib/views/dex/simple/form/taker/taker_form_layout.dart (2)
32-59: Keyboard dismissal on scroll is implemented correctly (desktop).Controllers are lifecycle-managed, listeners unfocus, and disposal order is correct.
Also applies to: 60-64
76-88: No issues found—code structure is correct and no nested scrollables present.Verification confirms the scrollables at lines 81 and 110 are sibling widgets (Flexible and Expanded children), not nested. The Stack children within the main scrollable contain no nested scrollables, eliminating focus trap concerns. Keyboard dismissal is already implemented via
_onScroll()method. The controller wiring is consistent and mirrors the maker layout pattern correctly.lib/views/fiat/fiat_inputs.dart (4)
63-71: Solid focus lifecycle.Local focus node created/disposed correctly; aligns with keyboard dismissal goals.
Also applies to: 74-80
134-140: Submit handler correctly dismisses keyboard and syncs value.Simple and effective; mirrors desktop/mobile scroll-dismiss approach.
219-233: Responsive coin selector layout improvement.Wrapping FiatCurrencyItem in Flexible->SizedBox avoids overflow in tight widths.
262-264: Keyboard dismissal before dialogs is good UX practice.Prevents obscured dialogs and stray focus. The code at lines 263–264 correctly calls
_fiatFocusNode.unfocus()before opening the selection dialog.Note:
dart formatanddart analyzeverification cannot be performed in this environment (dart CLI unavailable). Please verify formatting and analysis per repository guidelines before committing:dart format lib/views/fiat/fiat_inputs.dart dart analyze lib/views/fiat/fiat_inputs.dartlib/views/fiat/fiat_form.dart (9)
35-35: LGTM: ScrollController declaration is appropriate.The
latekeyword is correctly used since the controller is initialized ininitState.
43-45: LGTM: ScrollController initialization is correct.The controller is properly initialized and the listener is added in
initState, following Flutter lifecycle best practices.
60-65: LGTM: Proper resource cleanup in dispose.The disposal sequence is correct: listener removal → controller disposal → super.dispose(). This prevents memory leaks.
67-70: LGTM: Keyboard dismissal on scroll is correctly implemented.Using
FocusScope.of(context).unfocus()is the standard approach for dismissing the keyboard in Flutter.
95-179: LGTM: UI changes properly implement keyboard dismissal.The GestureDetector wrapper and shared
_scrollControllercorrectly enable keyboard dismissal on both tap and scroll interactions. The button text correctly reflects the submitting state.
182-186: LGTM: Keyboard dismissal before form submission enhances UX.Dismissing the keyboard prior to dispatching
FiatFormSubmittedensures a clean submission flow.
236-238: Minor formatting improvement.This is a purely cosmetic change with no functional impact.
290-290: LGTM: Correct handling of submitting state.Adding
FiatOrderStatus.submittingto the early-return cases is appropriate, as this transient state should not trigger a dialog.
336-342: LGTM: Improved localization message formatting.The refactored error messages use the
tr()method with positional arguments, improving maintainability. Note: These are positional arguments (not named), which work with placeholders like{0},{1}in the locale files.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/views/fiat/fiat_asset_icon.dart (2)
43-59: Simplify the FlexFit assignment.On line 54, the ternary
fit: expanded ? FlexFit.tight : FlexFit.looseis redundant because it's inside theelsebranch whereexpandedis alwaysfalse. The expression will always evaluate toFlexFit.loose.Apply this diff to simplify:
expanded ? Expanded( child: _FiatCoinItemLabel(size: size, coin: coin), ) : Flexible( - fit: expanded ? FlexFit.tight : FlexFit.loose, + fit: FlexFit.loose, child: _FiatCoinItemLabel(size: size, coin: coin), ),
83-117: Consider extracting the width threshold to a named constant.Line 93 uses a magic number
75to determine whether to show the coin name. Extracting this to a named constant would improve readability and maintainability.Example:
class _FiatCoinItemTitle extends StatelessWidget { const _FiatCoinItemTitle({required this.coin, required this.size}); final Coin? coin; final CoinItemSize size; static const double _minWidthForCoinName = 75; @override Widget build(BuildContext context) { return LayoutBuilder( builder: (context, constraints) { final showCoinName = constraints.maxWidth > _minWidthForCoinName; // ... rest of implementation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/bloc/fiat/fiat_order_status.dart(3 hunks)lib/bloc/taker_form/taker_bloc.dart(5 hunks)lib/views/fiat/fiat_asset_icon.dart(3 hunks)lib/views/fiat/fiat_select_button.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/bloc/fiat/fiat_order_status.dart
- lib/views/fiat/fiat_select_button.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Runflutter analyzeand resolve issues before committing code
Format Dart code withdart format(on changed files) before committing
Files:
lib/bloc/taker_form/taker_bloc.dartlib/views/fiat/fiat_asset_icon.dart
🧠 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/bloc/taker_form/taker_bloc.dartlib/views/fiat/fiat_asset_icon.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/bloc/taker_form/taker_bloc.dartlib/views/fiat/fiat_asset_icon.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: validate_code_guidelines
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Test web-app-macos
- GitHub Check: unit_tests
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Build Desktop (macos)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (windows)
- GitHub Check: build_and_preview
🔇 Additional comments (4)
lib/bloc/taker_form/taker_bloc.dart (4)
103-103: LGTM! Logger field appropriately added.The private logger field follows best practices and is properly used for error logging in the try-catch block.
451-507: Excellent error handling improvements!The try-catch block adds comprehensive error handling with several key improvements:
- Asset activation check (lines 452-469): Prevents displaying misleading "0.00" balance for inactive assets by maintaining loading state.
- Efficient fallback logic (lines 478-498): Tries
getMaxTakerVolumefirst, then falls back to_frequentlyGetMaxTakerVolumeonly if needed, avoiding unnecessary retries.- Proper error logging (line 501): Uses structured logging with severity level to capture failures for debugging.
- Consistent state transitions: Handles all cases (inactive asset, not logged in, success, failure) with appropriate state updates.
451-507: Run static analysis and format code before merging.Per coding guidelines for Dart files, please run
flutter analyzeon the changed file and resolve any issues. Additionally, rundart formatto ensure code formatting compliance before committing.
6-6: Remove the unused importkomodo_defi_type_utilsat line 6.This import is not referenced anywhere in the file and should be removed. Per the coding guidelines, running
flutter analyzeon changed files will catch this before committing.⛔ Skipped due to 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.
|
On android:
Minor nit: With BAT-BEP20 selected in fiat 
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| Emitter<FiatFormState> emit, | ||
| ) async { | ||
| emit(state.copyWith(fiatOrderStatus: FiatOrderStatus.submitting)); | ||
|
|
There was a problem hiding this comment.
Bug: UI Stuck on Validation Fail
In _onFormSubmitted, fiatOrderStatus is set to submitting before form validation. If validation fails, the method returns early, leaving the UI stuck in a submitting state. This prevents users from retrying the form and displays misleading "Submitting..." text.
There was a problem hiding this comment.
Moved the emit below the legacy validation function as a precaution in 6e867da and deprecated the function for visibility and documentation.
Fiat onramp validation is handled by formz, which is integrated into the state and event handlers. E.g. fiat_amount_input.dart.
Confrmed resolved. |
takenagain
force-pushed
the
hotfix/fiat-form-and-banner
branch
from
b84071b to
4ff4a6e
Compare
Fixes the following UI issues on mobile:
Before and After
Seed banner
Coin list
Fiat form
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Note
Improves mobile UX (seed banner, keyboard dismissal, text alignment), refines fiat onramp flow (new submitting state, Banxa webview handling, input focus), relaxes webview CSP, removes MM bot priceUrl from settings, enhances error handling, and adds swap data export.
FiatOrderStatus.submitting; block submit while submitting and skip dialogs for non-terminal states.checkoutUrlbefore new submit.FiatFormState.isBanxaSelectedand use it in webview mode selection; adjust web dialog handling.CustomFiatInputField,FiatInputs) with focus nodes, onSubmitted, and layout tweaks; richer crypto asset display in selectors.priceUrlfrom settings/repository and related props; keeppricesUrlV3endpoint updated.onCancelAll; add simplified bot controls and hook cancel-all.RpcErrorType.fromStringnullable; minor logging/format tweaks.pricesUrlV3tohttps://prices.komodian.info/api/v2/tickers?expire_at=60.Written by Cursor Bugbot for commit 14b0157. This will update automatically on new commits. Configure here.