New add order buttom and new add order screens #103
Conversation
main changes
## Walkthrough
This update introduces an expandable floating action button for quick BUY/SELL actions, refactors the order creation UI into modular widgets, and adds new providers and data files for payment methods. UI color constants are updated, and asset declarations are broadened. The changes focus on enhancing user interaction and code modularity.
## Changes
| File(s) | Change Summary |
|-------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| assets/data/payment_methods.json, pubspec.yaml | Added a new JSON data file mapping currencies to payment methods; updated asset declaration to include the whole data directory. |
| lib/core/app_theme.dart | Updated several color constants for green and red tones to new hex values. |
| lib/features/home/screens/home_screen.dart | Repositioned the AddOrderButton from the Scaffold's floatingActionButton to a manually positioned widget inside a Stack. |
| lib/features/order/providers/payment_methods_provider.dart | Added new Riverpod providers to load, select, and customize payment methods per currency. |
| lib/features/order/screens/add_order_screen.dart | Major refactor: unified buy/sell order forms, modularized UI into separate widgets, streamlined state management, improved validation, and enhanced order submission logic. |
| lib/features/order/widgets/action_buttons.dart | Added new ActionButtons widget with Cancel and Submit buttons, integrating with order state and actions. |
| lib/features/order/widgets/amount_section.dart, .../currency_section.dart, .../form_section.dart, .../lightning_address_section.dart, .../order_type_header.dart, .../payment_methods_section.dart, .../premium_section.dart, .../price_type_section.dart | Added modular widgets for each order form section: amount input, currency selection, form section layout, lightning address input, order type header, payment methods selection, premium slider, and price type toggle. |
| lib/shared/widgets/add_order_button.dart | Refactored AddOrderButton from a stateless to a stateful widget with animated expandable menu for BUY/SELL, including navigation and appear/disappear animations. |
| lib/shared/providers/exchange_service_provider.dart | Changed selectedFiatCodeProvider to non-nullable with default 'USD' initial value. |
| lib/shared/widgets/currency_combo_box.dart | Simplified null checks when accessing currencyCodes with selectedFiatCode. |
| lib/data/models/order.dart | Modified Order model to support a list of payment methods instead of a single string, updated serialization/deserialization, and added copyWith method. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant AddOrderButton
participant Navigator
User->>AddOrderButton: Tap floating action button
AddOrderButton->>AddOrderButton: Animate menu open/close
User->>AddOrderButton: Tap BUY or SELL button
AddOrderButton->>Navigator: Navigate to /add_order with order typesequenceDiagram
participant User
participant AddOrderScreen
participant Providers
participant OrderNotifier
User->>AddOrderScreen: Fill out order form sections
AddOrderScreen->>Providers: Fetch currencies, payment methods, etc.
User->>AddOrderScreen: Tap Submit
AddOrderScreen->>OrderNotifier: Validate and submit order
OrderNotifier-->>AddOrderScreen: Success or error response
Assessment against linked issues
Poem
|
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (17)
assets/data/payment_methods.json (1)
1-20: Well-structured payment methods dataThe JSON structure provides a comprehensive mapping of currency codes to their respective payment methods, with a sensible default fallback. This organization allows for easy extensibility and maintenance.
Consider adding comments in the JSON to document when this data was last updated and potentially where to find canonical sources for payment methods by region, which could help with future maintenance.
lib/features/order/widgets/order_type_header.dart (1)
4-36: Clean, reusable header componentThe
OrderTypeHeaderwidget is well-implemented with appropriate styling and text conditionally based on the order type. The use of a dedicated widget improves reusability and code organization.Consider extracting the color value
Color(0xFF1E2230)to a constant in your theme file for better maintainability and consistency.- color: Color(0xFF1E2230), + color: AppTheme.headerBackgroundDark, // Define this in your themelib/features/order/widgets/lightning_address_section.dart (1)
4-29: Well-structured form section componentThe
LightningAddressSectionwidget is well-implemented with clear separation of concerns and appropriate reuse of theFormSectionwidget. The optional nature of the field is clearly communicated to users.Consider adding validation for Lightning addresses to ensure they follow the correct format (usually in the form of
user@domain.com).child: TextField( controller: controller, style: const TextStyle(color: Colors.white), + onChanged: (value) { + // Basic Lightning address validation + final isValid = value.isEmpty || value.contains('@'); + // Update validation state if needed + }, decoration: const InputDecoration( border: InputBorder.none, hintText: 'Enter lightning address', hintStyle: TextStyle(color: Colors.grey), ), ),lib/features/home/screens/home_screen.dart (1)
80-84: Consider device adaptability for button positioningThe fixed positioning may not adapt well to different screen sizes or when the keyboard is shown. Consider using MediaQuery to adjust positioning based on screen dimensions.
Positioned( - bottom: 100, + bottom: MediaQuery.of(context).size.height > 700 ? 100 : 80, right: 16, child: const AddOrderButton(), ),lib/features/order/widgets/form_section.dart (1)
1-68: Good job creating this reusable FormSection widget!The widget provides a consistent UI container for form sections with a title, icon, and child content. This will help maintain visual consistency across the order creation screens.
A few suggestions to improve:
- Consider extracting hardcoded colors like
Color(0xFF1E2230)andColors.white.withOpacity(0.7)to theme constants for better maintainability- Add documentation comments to explain the purpose and parameters of this widget
import 'package:flutter/material.dart'; +import 'package:mostro_mobile/core/app_theme.dart'; // Assuming this is where theme colors are defined +/// A reusable section container for form elements. +/// +/// Displays a styled container with a title, icon in a colored circle, +/// main content, and optional extra content below. class FormSection extends StatelessWidget { final String title; final Widget icon; final Color iconBackgroundColor; final Widget child; final Widget? extraContent; const FormSection({ super.key, required this.title, required this.icon, required this.iconBackgroundColor, required this.child, this.extraContent, }); @override Widget build(BuildContext context) { return Container( decoration: BoxDecoration( - color: const Color(0xFF1E2230), + color: Theme.of(context).colorScheme.surface, // Or appropriate theme color borderRadius: BorderRadius.circular(8), ), child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ Padding( padding: const EdgeInsets.fromLTRB(16, 16, 16, 8), child: Text( title, style: TextStyle( - color: Colors.white.withOpacity(0.7), + color: Theme.of(context).colorScheme.onSurface.withOpacity(0.7), fontSize: 14, ), ), ),lib/features/order/widgets/action_buttons.dart (2)
5-15: Review the currentRequestId parameter necessityThe
currentRequestIdparameter is marked as nullable (int?) but is required in the constructor. If it's truly optional, consider making it optional in the constructor as well.class ActionButtons extends StatelessWidget { final VoidCallback onCancel; final VoidCallback onSubmit; - final int? currentRequestId; + final int? currentRequestId; // ID to track button state const ActionButtons({ super.key, required this.onCancel, required this.onSubmit, - required this.currentRequestId, + this.currentRequestId, });
1-55: Extract hardcoded colors to theme constantsConsider using theme colors or defined constants instead of hardcoded hex values for better maintainability and consistency.
import 'package:flutter/material.dart'; import 'package:mostro_mobile/data/models/enums/action.dart' as nostr_action; import 'package:mostro_mobile/shared/widgets/mostro_reactive_button.dart'; +import 'package:mostro_mobile/core/app_theme.dart'; // Assuming this is where theme colors are defined +/// A widget that displays Cancel and Submit buttons for order creation. class ActionButtons extends StatelessWidget { // ... existing code @override Widget build(BuildContext context) { + final theme = Theme.of(context); + return Row( mainAxisAlignment: MainAxisAlignment.spaceBetween, children: [ Expanded( child: ElevatedButton( onPressed: onCancel, style: ElevatedButton.styleFrom( - backgroundColor: const Color(0xFF1E2230), + backgroundColor: theme.colorScheme.surface, // Or appropriate theme color foregroundColor: Colors.white, padding: const EdgeInsets.symmetric(vertical: 12), shape: RoundedRectangleBorder( borderRadius: BorderRadius.circular(24), ), ), child: const Text('Cancel'), ), ), // ... rest of the code timeout: const Duration(seconds: 5), showSuccessIndicator: true, - backgroundColor: const Color(0xFF7856AF), + backgroundColor: theme.colorScheme.primary, // Or appropriate theme color ),lib/features/order/widgets/premium_section.dart (1)
78-79: Consider fewer slider divisions for smoother interactionUsing 200 divisions for a range of -10 to 10 results in increments of 0.1, which might be unnecessarily precise for premium percentages and could make the slider less smooth. Consider reducing the number of divisions.
child: Slider( value: value, min: -10, max: 10, - divisions: 200, + divisions: 40, // Increments of 0.5% are likely sufficient onChanged: onChanged, ),lib/features/order/widgets/price_type_section.dart (1)
52-68: Improve Switch and Label arrangementThe current arrangement has the "Market" label before the switch, which might be confusing. Consider placing the label after the switch or using a more descriptive approach to clearly indicate what each state represents.
Row( children: [ + Switch( + value: isMarketRate, + activeColor: const Color(0xFF764BA2), + onChanged: onToggle, + ), Text( - 'Market', + isMarketRate ? 'Market' : 'Custom', style: TextStyle( color: isMarketRate ? const Color(0xFF8CC63F) : Colors.grey, fontSize: 14, ), ), - Switch( - value: isMarketRate, - activeColor: const Color(0xFF764BA2), - onChanged: onToggle, - ), ], ),lib/shared/widgets/add_order_button.dart (3)
55-57: Hard-coded size limits flexibility on tablets & landscape
SizedBox(height: 130, width: 200)forces a fixed footprint that may clip or float awkwardly on large or very small screens.
PreferIntrinsicHeight,IntrinsicWidth, orConstrainedBoxwithmaxconstraints (or remove the wrapper and rely onAlign/Padding) so the widget adapts naturally to its surroundings.
64-70: Minor layout nit – residual gap when the menu is closed
margin: const EdgeInsets.only(bottom: 10)is applied even when the menu height is0, leaving a visible 10 px gap beneath the collapsed FAB. Consider animating the margin together with the height:- margin: const EdgeInsets.only(bottom: 10), + margin: EdgeInsets.only(bottom: _isMenuOpen ? 10 : 0),
120-130: Accessibility: expose a semantic label for the FABScreen-reader users cannot know what the FAB does because no semantics are provided. Wrap the
FloatingActionButtonin aSemanticswidget or settooltipon the FAB so assistive technologies announce “Add order menu”.FloatingActionButton( onPressed: _toggleMenu, + tooltip: 'Create order',lib/features/order/widgets/currency_section.dart (1)
16-18: Minor: defaulting silently to USD can hide configuration errorsIf
selectedFiatCodeProviderunexpectedly returnsnull, the widget silently falls back to"USD", masking the underlying state problem.
Consider asserting or logging a warning so the issue surfaces during development.lib/features/order/widgets/amount_section.dart (1)
33-34: Keyboard type:numberimproves UX
TextInputType.textshows the full keyboard, yet only digits (and an optional dash) are expected.
UsingTextInputType.numberWithOptions(signed: true)keeps the numeric keypad while still allowing “-” on iOS.- keyboardType: TextInputType.text, + keyboardType: const TextInputType.numberWithOptions(signed: true),lib/core/app_theme.dart (1)
7-16: Duplicate colour constants make maintenance harder
mostroGreen,buyColor,activeColor, andstatusActiveall carry the same hex value (0xFF9CD651).
Keeping multiple aliases for the exact same colour increases the chance that future tweaks forget to update all of them.Consider picking a single source-of-truth constant (e.g.
mostroGreen) and alias the others:static const Color buyColor = mostroGreen; static const Color activeColor = mostroGreen; static const Color statusActive = mostroGreen;That way, a palette change only needs to be performed in one place.
lib/features/order/widgets/payment_methods_section.dart (1)
101-112: Minor: nestedStatefulBuilderis unnecessaryThe outer
StatefulBuilderalready gives yousetDialogState.
Embedding a secondStatefulBuilderfor the custom text field adds complexity without benefit.You can safely remove the inner builder and call
setDialogStatefrom the outer scope if needed.lib/features/order/screens/add_order_screen.dart (1)
245-251: Duplicate “Other” token in the concatenated payment methodsWhen the user selects “Other” and provides a custom value, the final string becomes
"Bank Transfer, Other, MyCustomMethod".Consider dropping
"Other"if a concrete custom method is supplied:List<String> paymentMethods = List<String>.from(_selectedPaymentMethods); if (_showCustomPaymentMethod && _customPaymentMethodController.text.isNotEmpty) { - paymentMethods.add(_customPaymentMethodController.text); + paymentMethods + ..remove('Other') + ..add(_customPaymentMethodController.text); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
assets/data/payment_methods.json(1 hunks)lib/core/app_theme.dart(2 hunks)lib/features/home/screens/home_screen.dart(1 hunks)lib/features/order/providers/payment_methods_provider.dart(1 hunks)lib/features/order/screens/add_order_screen.dart(2 hunks)lib/features/order/widgets/action_buttons.dart(1 hunks)lib/features/order/widgets/amount_section.dart(1 hunks)lib/features/order/widgets/currency_section.dart(1 hunks)lib/features/order/widgets/form_section.dart(1 hunks)lib/features/order/widgets/lightning_address_section.dart(1 hunks)lib/features/order/widgets/order_type_header.dart(1 hunks)lib/features/order/widgets/payment_methods_section.dart(1 hunks)lib/features/order/widgets/premium_section.dart(1 hunks)lib/features/order/widgets/price_type_section.dart(1 hunks)lib/shared/widgets/add_order_button.dart(1 hunks)pubspec.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build & Release
🔇 Additional comments (2)
pubspec.yaml (1)
116-116: Asset declaration improvementThe asset declaration has been broadened to include the entire
assets/data/directory rather than listing individual files. This is a good practice that simplifies the addition of new data files without requiring pubspec.yaml modifications each time.lib/features/home/screens/home_screen.dart (1)
24-86: Improved button positioning for enhanced UI interactionsThe floating action button has been repositioned using a Stack and Positioned widget instead of the Scaffold's floatingActionButton property. This change supports the new expandable menu behavior and provides precise control over button placement.
grunch
left a comment
There was a problem hiding this comment.
Great work @AndreaDiazCorreia I would like to change some things like, when the user mark Other in payment method, the payment method doesn't include the word Other, but I think we can merge this and fix it later.
This is a tACK !
|
Another issue I found on this implementation is that when you create an order USD is the default fiat currency apparently selected but if you want to use USD and just leave that dropdown and keep filling the other elements of the form when submit the app will tell you that you didn't select a fiat currency. Also we need to add a filter to select a currency, we have 137 fiat currencies to show and we can't filter as we can do with the current version of the app. @AndreaDiazCorreia Can you fix this before merge please? |
Catrya
left a comment
There was a problem hiding this comment.
Hola @AndreaDiazCorreia, buen trabajo!
-
El botón de Market/Fixed no funciona, ese es importante. Por defecto está Market pero si el usuario lo desactiva y pasa a Fixed, se debe cambiar la barra del Premium % por la de "Enter the Sats amount you want to Sell" como está en lovable
-
En los métodos de pago de USD no debe estar Cash, LA meetup, eso fue solo un ejemplo que puse en lovable para que se viera que un usuario podía poner lo que quisiera en una orden
-
Faltan los textos de los botones de info, creo que si no se van a poner ahora entonces se debería quitar el icono de info, pero lo idel es que sí esté todo eso.
-
Para seleccionar el fiat debería dejarse además de la lista, que arriba el usuario pudiera escribirla para no scrolear mucho, como está actualmente en la app. (disculpa que en lovable no estuviera exactamente así)
|
Something that I just realize, it is really nice that the app allows to the user to select payment methods, but if the user select various methods we should follow NIP-69 which is an standard attempt For an event with three methods You can read the NIP 69 for a better understanding and a complete example |
| return jsonDecode(response) as Map<String, dynamic>; | ||
| }); | ||
|
|
||
| // Proveedor para obtener los métodos de pago basados en la moneda seleccionada |
There was a problem hiding this comment.
Hi @AndreaDiazCorreia remember always write comments in english please
|
Hola @AndreaDiazCorreia esto no tiene prioridad alta, no es imprescindible para mergear este PR, pero si te da tiempo hacerlo estaría genial, si no, no hay problema y se deja para luego:
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
lib/features/order/screens/add_order_screen.dart (1)
233-244: False “no currency selected” error when the user keeps the defaultAs discussed above,
selectedFiatCodeProvidermay still holdnulleven though USD is visibly selected by default, causing this guard to block form submission.Once the provider default is fixed (see
CurrencySection.dartreview), remove this defensive branch or change the condition to:- if (selectedFiatCode == null || selectedFiatCode.isEmpty) { + if (selectedFiatCode.trim().isEmpty) {lib/features/order/providers/payment_methods_provider.dart (1)
38-44: Initial empty list instance is now recreated per notifier – previous concern resolved
SelectedPaymentMethodsNotifierconstructs a new list in its constructor, so the shared-mutable-list problem highlighted earlier no longer applies. 👍
🧹 Nitpick comments (5)
lib/features/order/widgets/currency_section.dart (1)
20-24: Inconsistent capitalisation (“Fiat” vs “fiat”)Minor, but user-facing strings should be consistent to avoid looking like typos.
Suggestion: keep “fiat” lower-case in both sentences.
lib/shared/widgets/add_order_button.dart (2)
60-66: Code comments are in Spanish – stick to project language (English)Maintain consistency so every contributor can understand inline documentation.
- height: 130, // Altura suficiente para mostrar ambos elementos - width: 200, // Ancho suficiente para los botones + height: 130, // Enough height to display both menu items + width: 200, // Enough width for the buttons
60-71: Hard-coded size may clip on very small or tablet devicesPinning the FAB container to
height: 130, width: 200risks overflow or an odd amount of empty space depending on text scale and layout.Consider using
IntrinsicHeight/IntrinsicWidthorSizedBox.shrink()together withAnimatedSizeso the widget adapts to its content:- return SizedBox( - height: 130, - width: 200, + return AnimatedSize( + duration: const Duration(milliseconds: 200), + child: ConstrainedBox( + constraints: const BoxConstraints(minWidth: 56, minHeight: 56),lib/features/order/screens/add_order_screen.dart (1)
170-193: Numeric validation is too lax for sats amount
int.tryParsesilently converts “-1”, “0”, or huge values; the validator only checks presence.Enhance validation:
validator: (value) { if (!_marketRate) { - if (value == null || value.isEmpty) { - return 'Please enter sats amount'; - } + final parsed = int.tryParse(value ?? ''); + if (parsed == null || parsed <= 0) { + return 'Enter a positive integer amount'; + } } return null; },Also guard against huge numbers (
> 2.1e15) to avoidintoverflow on 32-bit platforms.lib/features/order/providers/payment_methods_provider.dart (1)
18-26: Provider returns a mutable list – callers can accidentally tamper with shared stateAlthough you clone the JSON array (
List.from(...)), you thenadd('Other'), returning a growable list that widgets might mutate in-place.To keep data immutable:
- final methods = List<String>.from(data[currencyCode]); - if (!methods.contains('Other')) { - methods.add('Other'); - } - return methods; + final raw = List<String>.from(data[currencyCode]); + final withOther = raw.contains('Other') ? raw : [...raw, 'Other']; + return List.unmodifiable(withOther);Same applies to the default branch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
lib/generated/l10n.dartis excluded by!**/generated/**
📒 Files selected for processing (8)
lib/features/order/providers/payment_methods_provider.dart(1 hunks)lib/features/order/screens/add_order_screen.dart(2 hunks)lib/features/order/widgets/amount_section.dart(1 hunks)lib/features/order/widgets/currency_section.dart(1 hunks)lib/features/order/widgets/payment_methods_section.dart(1 hunks)lib/features/order/widgets/premium_section.dart(1 hunks)lib/features/order/widgets/price_type_section.dart(1 hunks)lib/shared/widgets/add_order_button.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/features/order/widgets/price_type_section.dart
- lib/features/order/widgets/premium_section.dart
- lib/features/order/widgets/amount_section.dart
- lib/features/order/widgets/payment_methods_section.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build & Release
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/features/order/screens/add_order_screen.dart (1)
274-282:⚠️ Potential issueFix payment methods encoding for NIP-69 compliance.
The current implementation joins payment methods into a single comma-separated string, which doesn't comply with the NIP-69 standard mentioned by reviewer @grunch. According to NIP-69, payment methods should be encoded as separate tags.
-final paymentMethod = paymentMethods.join(', '); - -final order = Order( - kind: _orderType, - fiatCode: selectedFiatCode, - fiatAmount: fiatAmount!, - minAmount: minAmount, - maxAmount: maxAmount, - paymentMethod: paymentMethod, +// Store payment methods as a list for proper NIP-69 encoding + +final order = Order( + kind: _orderType, + fiatCode: selectedFiatCode, + fiatAmount: fiatAmount!, + minAmount: minAmount, + maxAmount: maxAmount, + paymentMethods: paymentMethods, // Order model needs to be updated to accept List<String>This will require updating the
Ordermodel to accept aList<String>for payment methods, and ensuring the event creation logic properly encodes them according to NIP-69.
🧹 Nitpick comments (2)
lib/features/order/screens/add_order_screen.dart (2)
160-169: Restrict premium slider to integer values only.According to reviewer @Catrya's suggestion, the percentage slider for premium or discount should restrict input to integer values only, avoiding fractional numbers when the user slides.
The PremiumSection widget should be modified to ensure only integer values are accepted. This is a non-blocking UI refinement.
235-243: Remove redundant empty check for selectedFiatCode.Since
selectedFiatCodeProvidernow defaults to 'USD' and is non-nullable, this check for an empty string is redundant and can be removed.-if (selectedFiatCode.isEmpty) { - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Please select a currency'), - duration: Duration(seconds: 2), - ), - ); - return; -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/features/order/screens/add_order_screen.dart(2 hunks)lib/features/order/widgets/currency_section.dart(1 hunks)lib/features/order/widgets/payment_methods_section.dart(1 hunks)lib/shared/providers/exchange_service_provider.dart(1 hunks)lib/shared/widgets/currency_combo_box.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/order/widgets/currency_section.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build & Release
🔇 Additional comments (6)
lib/shared/providers/exchange_service_provider.dart (1)
30-30: LGTM: Default fiat code implemented correctly.The change from nullable
StateProvider<String?>to non-nullableStateProvider<String>with a default value of 'USD' addresses the currency selection issue mentioned in PR comments. This ensures a currency is always selected.lib/shared/widgets/currency_combo_box.dart (1)
71-73: Null check simplification is appropriate.The removal of the null check for
selectedFiatCodealigns with the provider change making it non-nullable. This simplification improves code readability while maintaining the same functionality.lib/features/order/widgets/payment_methods_section.dart (3)
84-89: LGTM: Overflow property is correctly placed.The Text widget correctly uses overflow property. This addresses the previously identified issue mentioned in the past review comments.
158-174: LGTM: Controller usage is correct.The TextField now directly uses the existing controller that's passed from the parent, avoiding memory leaks and cursor position issues that were flagged in previous reviews.
50-66:Details
❓ Verification inconclusive
Add filtering for fiat currencies dropdown.
The reviewer @grunch pointed out that a filtering mechanism for the currency selection dropdown is needed, as the app currently lists 137 fiat currencies without any filter. This component doesn't appear to implement that filtering functionality.
🏁 Script executed:
#!/bin/bash # Check if there's a separate currency filtering implementation elsewhere rg "currency.*filter" --type dartLength of output: 35
Ensure filtering for the fiat‐currency dropdown
I wasn’t able to find any existing search or filter logic for the fiat currency selector—users currently have to scroll through all 137 currencies. Please add a text‐based filter (or searchable dropdown) so users can quickly narrow the list.
• Location to update:
- lib/features/order/widgets/payment_methods_section.dart (around the
DropdownButton/currency selector that setsselectedFiatCode)• Suggested approach:
- Wrap the currency list in a
List<String>and apply.where(...)based on a search field.- Consider using a package like
dropdown_searchor a customTextField+ListViewcombo.lib/features/order/screens/add_order_screen.dart (1)
177-192: LGTM: Fixed sats amount controller now properly connected.The
_satsAmountControlleris now correctly attached to a TextFormField with proper validation, resolving the previous issue where it was declared but never used.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/features/order/widgets/currency_section.dart (1)
32-39:⚠️ Potential issueUI shows USD by default but provider may be null
The code shows 'USD' as default in the UI even when
selectedFiatCodeis null, which creates inconsistency between what the user sees and what the system stores. To fix this, initialize the provider with 'USD' or set it when building this widget.- final selectedFiatCode = ref.watch(selectedFiatCodeProvider); + final selectedFiatCodeNotifier = ref.read(selectedFiatCodeProvider.notifier); + final selectedFiatCode = ref.watch(selectedFiatCodeProvider) ?? 'USD'; + + // Initialize with default if null + if (selectedFiatCodeNotifier.state == null) { + selectedFiatCodeNotifier.state = 'USD'; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/data/models/order.dart(11 hunks)lib/features/order/screens/add_order_screen.dart(2 hunks)lib/features/order/widgets/currency_section.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build & Release
🔇 Additional comments (15)
lib/data/models/order.dart (9)
16-16: Changed the paymentMethod field to a list of stringsThe change from a single string to a List for payment methods aligns with the requirement to support multiple payment methods selection. This modification follows good practices by properly updating all related methods and maintaining backward compatibility.
37-48: Constructor updated to handle payment methods listThe constructor now properly accepts and defensively copies the payment methods list, which is good practice. This ensures that modifications to the original list won't affect the Order instance.
61-61: JSON serialization updated for payment methods listThe
toJsonmethod now correctly serializes the payment methods as a list rather than a single string.
93-94: Updated validation for required fieldsThe validation list now includes 'payment_methods' instead of 'payment_method', which is consistent with the model changes.
96-107: Robust backward compatibility for payment methodsThe implementation effectively handles multiple formats for payment methods:
- List of strings
- Single comma-separated string
- Legacy 'payment_method' field
This ensures smooth transitions from older app versions and prevents data loss.
140-140: Updated fromEvent factory to use payment methods listThe factory constructor now correctly uses the event's payment methods list property.
157-157: Updated toMap to handle payment methods listThe toMap method now properly includes the payment methods list, maintaining consistency across all serialization methods.
171-182: Consistent deserialization in fromMap methodThe fromMap method applies the same robust backward compatibility logic as fromJson, handling various formats of payment methods data. This ensures consistency across all deserialization paths.
208-230: Updated copyWith method to handle payment methods listThe copyWith method now properly includes the payment methods list, ensuring all model properties are maintained during partial updates.
lib/features/order/widgets/currency_section.dart (3)
67-70: Currency search implemented correctlyThe search functionality for currencies has been properly implemented with a text field that dynamically filters the currency list, addressing @grunch's concern about navigating through 137 currencies.
179-185: Currency selection callback executed correctlyThe onCurrencySelected callback is now executed after updating the provider state and before closing the dialog, ensuring that listeners receive the new selection. This addresses the issue mentioned in previous reviews.
16-17:Details
❌ Incorrect review comment
Currency selection issue may persist
Although the UI shows 'USD' as default, the provider may still be null initially. If a user leaves the dropdown unchanged, this could cause the validation failure mentioned by @grunch in the PR comments.
🏁 Script executed:
#!/bin/bash # Check how selectedFiatCodeProvider is defined in the provider file rg "selectedFiatCodeProvider" lib/shared/providers/exchange_service_provider.dartLength of output: 153
selectedFiatCodeProvider always initialized to ‘USD’
TheStateProvider<String>forselectedFiatCodeProvideris defined with an initial value of'USD', soref.watch(selectedFiatCodeProvider)can never be null. The previous concern about a null provider can be disregarded.Likely an incorrect or invalid review comment.
lib/features/order/screens/add_order_screen.dart (3)
31-32: Sats amount controller now properly connectedThe _satsAmountController is now correctly wired to a TextFormField (lines 177-192) that appears when fixed price is selected. This resolves the issue from the previous review.
177-193: Fixed price sats amount input implemented correctlyThe TextFormField for entering sats amount when using fixed price is properly implemented with validation that ensures a value is entered only when market rate is disabled.
290-300: Order creation with multiple payment methodsThe Order constructor is now called with the payment methods list directly, which is consistent with the updated Order model. This ensures that multiple payment methods are properly stored and can be encoded according to NIP-69 later.
| // Preparar la lista de métodos de pago para cumplir con NIP-69 | ||
| List<String> paymentMethods = | ||
| List<String>.from(_selectedPaymentMethods); | ||
| if (_showCustomPaymentMethod && | ||
| _customPaymentMethodController.text.isNotEmpty) { | ||
| paymentMethods.add(_customPaymentMethodController.text); | ||
| } | ||
|
|
||
| // Cada método de pago se mantiene como un elemento separado en la lista | ||
| // en lugar de concatenarlos en una cadena | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
NIP-69 compliance for payment methods
The code now properly maintains payment methods as a list of separate items rather than concatenating them into a string. This is the first step toward NIP-69 compliance, which requires each payment method to be emitted as a separate ["pm", "<method>"] tag.
🏁 Script executed:
#!/bin/bash
# Check if event creation code properly handles NIP-69 tagging
rg -A 10 "createEvent|pm" lib/ --glob "**event**"Length of output: 947
Update NIP-69 payment method tagging in both parsing and creation
To achieve full NIP-69 compliance you need to:
-
lib/data/models/nostr_event.dart
• Replace the comma-based getterList<String> get paymentMethods => _getTagValue('pm')?.split(',') ?? [];
with a multi-tag reader, e.g.:
List<String> get paymentMethods => _getTagValues('pm') ?? [];
(ensure you implement or expose a
_getTagValues('pm')that returns all “pm” tag values.) -
Your event builder (wherever you call
createEventor assemble thetagslist)
• Emit each payment method as its own tag instead of joining them:tags: [ ..._selectedPaymentMethods.map((m) => ['pm', m]), if (_showCustomPaymentMethod && _customPaymentMethodController.text.isNotEmpty) ['pm', _customPaymentMethodController.text], // …other tags ],
Remove any logic that concatenates payment methods into a single comma-separated string.
🤖 Prompt for AI Agents
In lib/features/order/screens/add_order_screen.dart around lines 274 to 284,
update the event creation code to emit each payment method as a separate ['pm',
method] tag instead of concatenating them into a single string. Replace any
logic that joins payment methods with commas by mapping each selected payment
method to its own tag array, and include the custom payment method similarly if
present. Also, ensure the event model's getter for payment methods uses a
multi-tag reader like _getTagValues('pm') to retrieve all 'pm' tags
individually.
| void _submitOrder() { | ||
| if (_formKey.currentState?.validate() ?? false) { | ||
| final selectedFiatCode = ref.read(selectedFiatCodeProvider); | ||
|
|
||
| // 1) Currency dropdown always enabled | ||
| CurrencyComboBox( | ||
| key: const Key('fiatCodeDropdown'), | ||
| label: 'Fiat code', | ||
| onSelected: (String fiatCode) { | ||
| setState(() { | ||
| _isEnabled = true; | ||
| }); | ||
| }, | ||
| if (selectedFiatCode.isEmpty) { | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar( | ||
| content: Text('Please select a currency'), | ||
| duration: Duration(seconds: 2), | ||
| ), | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Currency selection validation still has an issue
The validation checks if selectedFiatCode is empty but doesn't initialize it with a default value. This likely causes the bug reported by @grunch where users are prompted about missing currency selection even though USD appears selected in the UI.
- final selectedFiatCode = ref.read(selectedFiatCodeProvider);
+ // Ensure we have a selected currency, defaulting to USD if none is set
+ final selectedFiatCodeNotifier = ref.read(selectedFiatCodeProvider.notifier);
+ if (selectedFiatCodeNotifier.state == null || selectedFiatCodeNotifier.state!.isEmpty) {
+ selectedFiatCodeNotifier.state = 'USD';
+ }
+ final selectedFiatCode = selectedFiatCodeNotifier.state!;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void _submitOrder() { | |
| if (_formKey.currentState?.validate() ?? false) { | |
| final selectedFiatCode = ref.read(selectedFiatCodeProvider); | |
| // 1) Currency dropdown always enabled | |
| CurrencyComboBox( | |
| key: const Key('fiatCodeDropdown'), | |
| label: 'Fiat code', | |
| onSelected: (String fiatCode) { | |
| setState(() { | |
| _isEnabled = true; | |
| }); | |
| }, | |
| if (selectedFiatCode.isEmpty) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar( | |
| content: Text('Please select a currency'), | |
| duration: Duration(seconds: 2), | |
| ), | |
| ); | |
| return; | |
| } | |
| void _submitOrder() { | |
| if (_formKey.currentState?.validate() ?? false) { | |
| // Ensure we have a selected currency, defaulting to USD if none is set | |
| final selectedFiatCodeNotifier = ref.read(selectedFiatCodeProvider.notifier); | |
| if (selectedFiatCodeNotifier.state == null || selectedFiatCodeNotifier.state!.isEmpty) { | |
| selectedFiatCodeNotifier.state = 'USD'; | |
| } | |
| final selectedFiatCode = selectedFiatCodeNotifier.state!; | |
| if (selectedFiatCode.isEmpty) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar( | |
| content: Text('Please select a currency'), | |
| duration: Duration(seconds: 2), | |
| ), | |
| ); | |
| return; | |
| } | |
| // …rest of _submitOrder() | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/features/order/screens/add_order_screen.dart around lines 231 to 243, the
validation checks if selectedFiatCode is empty but does not ensure it has a
default value. To fix this, initialize selectedFiatCodeProvider with a default
currency code such as 'USD' so that the validation reflects the actual UI state
and prevents incorrect prompts about missing currency selection.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
146-146: Consider adding null check for consistency.While there's a null check for
orderat line 138 that breaks out of the switch statement, for consistency with the implementation at lines 172-174, consider adding an explicit null check here as well.- 'payment_method': _formatPaymentMethods(order.paymentMethods), + 'payment_method': order != null + ? _formatPaymentMethods(order.paymentMethods) + : 'No payment method',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
assets/data/payment_methods.json(1 hunks)lib/core/app_theme.dart(2 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(3 hunks)lib/features/order/screens/add_order_screen.dart(2 hunks)lib/features/order/widgets/action_buttons.dart(1 hunks)lib/features/order/widgets/amount_section.dart(1 hunks)lib/features/order/widgets/form_section.dart(1 hunks)lib/features/order/widgets/premium_section.dart(1 hunks)lib/features/order/widgets/price_type_section.dart(1 hunks)lib/features/trades/screens/trade_detail_screen.dart(4 hunks)lib/features/trades/widgets/mostro_message_detail_widget.dart(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- assets/data/payment_methods.json
- lib/features/order/widgets/form_section.dart
- lib/features/order/widgets/premium_section.dart
- lib/features/order/widgets/action_buttons.dart
- lib/features/order/widgets/price_type_section.dart
- lib/features/order/widgets/amount_section.dart
- lib/core/app_theme.dart
- lib/features/order/screens/add_order_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build & Release
🔇 Additional comments (7)
lib/features/trades/screens/trade_detail_screen.dart (2)
104-109: Multiple payment methods support is implemented correctly.The code now properly handles a list of payment methods, joining them with commas for display, and provides a fallback message when the list is empty.
140-140: Label updated for multiple payment methods.Label has been correctly updated from singular "Payment method" to plural "Payment methods" to reflect the support for multiple payment methods.
lib/features/trades/widgets/mostro_message_detail_widget.dart (3)
21-34: Good implementation of payment methods formatting helper.The helper method properly handles different scenarios:
- Returns a clear message for empty lists
- Returns just the method name for single payment methods
- Returns a concise format for multiple methods showing count
This approach aligns with NIP-69 requirements mentioned in the PR comments for handling multiple payment methods.
141-144: Proper null checking for payment methods formatting.The code correctly handles potential null values for orderPayload when formatting payment methods.
152-155: Consistent implementation of payment methods formatting.This change correctly applies the same pattern for payment methods formatting and null checking as used elsewhere.
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)
37-50: Well-implemented payment methods formatting helper.The helper method properly handles different scenarios for payment methods formatting in notifications, consistent with the implementation in other files.
172-175: Proper null handling for payment methods in notifications.The code correctly handles potential null values for the order object when formatting payment methods for notifications.
|
This shouldn't be merged, creating an order doesn't work, this is the log on my Mostro: |
New Order Flow Implementation
This PR implements the new order creation flow and refactors several UI components to improve consistency across the app.
Changes
Related Issues
Closes #88, #89, #90
Notes
The cancel/submit buttons in the order creation screens don't exactly match the Lovable designs. I opted to reuse existing button components for consistency, but I can update them to match the designs precisely if needed.
Summary by CodeRabbit
New Features
Enhancements
Refactor