feat: reactivate and fully implement order book filter functionality #270
Conversation
- Implement comprehensive filtering logic for currencies, payment methods, premium/discount, and reputation - Implement real-time order filtering based on Nostr event tags - Convert rating filter to range-based selection with min/max values - Integrate dynamic data sources from exchange service and payment methods providers - Apply consistent Material Design styling across all filter components - Add proper async data handling with loading and error states - Ensure visual consistency with app theme and touch interactions
WalkthroughAdds Riverpod-backed UI filters (currency, payment method, rating, premium range), refactors filteredOrdersProvider to consume them, replaces HomeScreen filter control with an interactive InkWell that opens an OrderFilter dialog, and converts OrderFilter to a ConsumerStatefulWidget with async data, multi-selects, and range sliders. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant HS as HomeScreen
participant D as Dialog
participant OF as OrderFilter
participant RP as Riverpod Providers
participant FO as filteredOrdersProvider
U->>HS: Tap filter control
HS->>D: showDialog(OrderFilter)
D->>OF: build
OF->>RP: watch(currencyCodesProvider, paymentMethodsDataProvider, filter providers)
RP-->>OF: async data + current filter state
U->>OF: change selections/ranges
U->>OF: tap Apply
OF->>RP: update(filter providers)
RP->>FO: trigger recompute
FO-->>HS: updated filtered list
HS-->>U: updated offers count/list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
lib/features/home/screens/home_screen.dart (1)
34-36: Fix RefreshIndicator callback: refreshing the wrong provider and returning wrong type.
ref.refresh(filteredOrdersProvider)returns a List, not a Future; returning it fromonRefreshis incorrect.- You should invalidate/refresh the source orders provider instead.
Apply this diff:
- onRefresh: () async { - return await ref.refresh(filteredOrdersProvider); - }, + onRefresh: () async { + // Re-fetch the underlying source; filtered list will update automatically. + ref.invalidate(orderEventsProvider); + },lib/shared/widgets/order_filter.dart (1)
31-37: Fix: avoid double-managed TextEditingController (leak) in MultiSelectAutocompleteYou create a controller in initState and then replace it with the one provided by Autocomplete. The provided controller is owned/disposed by Autocomplete; the extra one you created leaks. Make the field nullable, don’t instantiate it yourself, and null-check when clearing.
Apply this diff:
-class MultiSelectAutocompleteState extends State<MultiSelectAutocomplete> { - late TextEditingController _controller; +class MultiSelectAutocompleteState extends State<MultiSelectAutocomplete> { + TextEditingController? _controller; @@ @override void initState() { super.initState(); - _controller = TextEditingController(); } @@ @override void dispose() { - //_controller.dispose(); super.dispose(); } @@ onSelected: (String selection) { final updated = List<String>.from(widget.selectedValues) ..add(selection); widget.onChanged(updated); - _controller.clear(); + _controller?.clear(); },Also applies to: 39-43, 111-116
🧹 Nitpick comments (4)
lib/features/home/providers/home_order_providers.dart (1)
29-71: Non-blocking: avoid mutating the source orders list before reversing.
sortmutatesallOrders. If other consumers share the same instance, consider copying before sorting for isolation.Outside this range, consider:
final sorted = [...allOrders]..sort((o1, o2) => o1.expirationDate.compareTo(o2.expirationDate)); var filtered = sorted.reversed .where((o) => o.orderType == orderType) .where((o) => o.status == Status.pending);lib/shared/widgets/order_filter.dart (3)
71-110: Autocomplete options dropdown may overflow on small screens; make width responsiveHard-coding maxWidth: 300 can clip or overflow in narrow layouts. Prefer a width derived from screen size.
Apply this diff:
- child: ConstrainedBox( - constraints: const BoxConstraints(maxHeight: 200, maxWidth: 300), + child: ConstrainedBox( + constraints: BoxConstraints( + maxHeight: 200, + maxWidth: MediaQuery.of(context).size.width - 48, + ),
209-227: Accessibility: use IconButton with semantics for chip removalGestureDetector on a small tap target isn’t accessible. Use IconButton with tooltip for better a11y and focus handling.
Apply this diff:
- GestureDetector( - onTap: () { + IconButton( + onPressed: () { final updated = List<String>.from(widget.selectedValues) ..remove(value); widget.onChanged(updated); }, - child: Container( - padding: const EdgeInsets.all(2), - decoration: BoxDecoration( - color: AppTheme.textSecondary.withOpacity(0.2), - shape: BoxShape.circle, - ), - child: const Icon( - Icons.close, - size: 14, - color: AppTheme.textSecondary, - ), - ), + constraints: const BoxConstraints(minWidth: 24, minHeight: 24), + padding: const EdgeInsets.all(2), + tooltip: S.of(context)!.clear, + icon: const Icon(Icons.close, size: 14, color: AppTheme.textSecondary),
259-274: Robustness: remove “Other” by ID or case-insensitive matchIf upstream localizes “Other” or changes casing, this removal will fail. Better to filter by a stable ID or normalize case.
Example:
- allMethods.remove('Other'); + allMethods.removeWhere((m) => m.toLowerCase() == 'other');Ideally remove by a canonical ID provided by the API, not the display label.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
lib/features/home/providers/home_order_providers.dart(1 hunks)lib/features/home/screens/home_screen.dart(2 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/shared/widgets/order_filter.dart(7 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
lib/features/*/providers/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Providers are organized by feature in
features/{feature}/providers/
Files:
lib/features/home/providers/home_order_providers.dart
lib/features/*/{screens,providers,notifiers,widgets}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/home/providers/home_order_providers.dartlib/features/home/screens/home_screen.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/home/providers/home_order_providers.dartlib/features/home/screens/home_screen.dartlib/shared/widgets/order_filter.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/home/providers/home_order_providers.dartlib/features/home/screens/home_screen.dartlib/shared/widgets/order_filter.dart
lib/shared/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared utilities and widgets must be placed in
shared/
Files:
lib/shared/widgets/order_filter.dart
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/l10n/*.arb: Internationalization ARB files must be located inlib/l10n/
Add new localization keys to all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
🧠 Learnings (1)
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/l10n/*.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
🪛 RuboCop (1.76.1)
lib/l10n/intl_it.arb
[warning] 837-837: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 838-838: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 845-845: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 846-846: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_es.arb
[warning] 829-829: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 830-830: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 837-837: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 838-838: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_en.arb
[warning] 804-804: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 805-805: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 812-812: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 813-813: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (6)
lib/features/home/providers/home_order_providers.dart (2)
10-15: Good addition: dedicated filter state providers.The providers are typed and scoped appropriately for filter state. Records for ranges are a clean choice.
29-32: Confirm that filtering to Status.pending only is intended.Current logic excludes non-pending orders (e.g., Active). If the “order book” should include other visible statuses, adjust here.
Would you like me to update this to a configurable set (e.g., {pending, active})?
lib/features/home/screens/home_screen.dart (2)
241-263: LGTM: localized label and count in the filter button.Using
S.of(context)!.filterandoffersCountaligns with the localization guideline. Ripple feedback with InkWell is a nice UX touch.
220-223: Fix const mismatch: make OrderFilter const inside DialogDialog is declared const but its child isn't — this is a compile error. OrderFilter already exposes a const constructor at lib/shared/widgets/order_filter.dart:243, so make the child a const expression.
- Files to change:
- lib/features/home/screens/home_screen.dart — around lines 220–223
- lib/shared/widgets/order_filter.dart — confirms const constructor at line 243
Suggested fix (apply the child as const):
- return const Dialog( - child: OrderFilter(), - ); + return const Dialog( + child: const OrderFilter(), + );⛔ Skipped due to learnings
Learnt from: CR PR: MostroP2P/mobile#0 File: CLAUDE.md:0-0 Timestamp: 2025-08-03T09:43:38.727Z Learning: Applies to **/*.dart : Use `const` constructors where possiblelib/shared/widgets/order_filter.dart (2)
242-249: LGTM: Solid Riverpod integration and provider updates on Apply/Clear
- Correct use of ConsumerStatefulWidget and ref.watch/ref.read.
- Apply and Clear correctly propagate to providers and close the dialog.
- UI theming is consistent with AppTheme.
Also applies to: 299-301, 637-721
130-151: Keep withValues(...) — repo already standardises on the new API; do not apply the localized withOpacity() diffsI searched the codebase: Color.withValues(...) is used widely (examples below) and CLAUDE.md documents preferring latest APIs (withValues). Replacing only the snippets in order_filter.dart would introduce inconsistency.
- Examples of confirmed occurrences (partial):
- lib/shared/widgets/order_filter.dart — lines: 133, 140, 193, 218, 309, 344, 386, 462, 531, 603, 665
- lib/shared/widgets/mostro_app_bar.dart — line 80
- lib/core/app_theme.dart — lines ~222–230
- many other files under lib/features/* and lib/shared/widgets/* (repo-wide)
Action: Remove/ignore the suggested diffs in the original comment. If you intend to support older Flutter channels instead of following the repository standard, apply a repo-wide, consistent replacement (not a per-file patch) and verify the minimum Flutter/Dart SDK in pubspec.yaml.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
lib/shared/widgets/order_filter.dart (1)
31-31: Fix controller lifecycle: don’t create or dispose a controller you don’t own; clear safely
RawAutocompleteowns theTextEditingControllerprovided tofieldViewBuilder. Creating your own controller (and then not disposing it) leaks memory; disposing it risks double-dispose. Keep only a nullable reference to the controller handed in by Autocomplete and never dispose it. Clear it safely on selection.-class MultiSelectAutocompleteState extends State<MultiSelectAutocomplete> { - late TextEditingController _controller; +class MultiSelectAutocompleteState extends State<MultiSelectAutocomplete> { + TextEditingController? _controller; @@ - @override - void initState() { - super.initState(); - _controller = TextEditingController(); - } + // No initState needed; RawAutocomplete provides the controller @@ - @override - void dispose() { - //_controller.dispose(); - super.dispose(); - } + // Do not dispose _controller; RawAutocomplete owns it @@ onSelected: (String selection) { final updated = List<String>.from(widget.selectedValues) ..add(selection); widget.onChanged(updated); - _controller.clear(); + _controller?.clear(); },Also applies to: 33-37, 39-43, 111-116
♻️ Duplicate comments (5)
lib/shared/widgets/order_filter.dart (5)
276-295: Guard post-frame setState with mounted; destructure typed rating record instead of legacy double handlingAs previously noted, the post-frame callback can run after disposal, causing “setState() called after dispose()”. Also,
ratingFilterProvideris a typed record; remove the legacydoublehandling and destructure the record.WidgetsBinding.instance.addPostFrameCallback((_) { final currencies = ref.read(currencyFilterProvider); final paymentMethods = ref.read(paymentMethodFilterProvider); final currentRatingRange = ref.read(ratingFilterProvider); final currentPremiumRange = ref.read(premiumRangeFilterProvider); - - setState(() { + if (!mounted) return; + setState(() { selectedFiatCurrencies = List.from(currencies); selectedPaymentMethods = List.from(paymentMethods); - ratingMin = currentRatingRange is double ? 0.0 : currentRatingRange.min; - ratingMax = currentRatingRange is double ? currentRatingRange as double : currentRatingRange.max; + final (min: rMin, max: rMax) = currentRatingRange; + ratingMin = rMin; + ratingMax = rMax; premiumMin = currentPremiumRange.min; premiumMax = currentPremiumRange.max; }); });
390-397: Localize “Loading currencies…” per i18n guidelineUser-facing strings must use
S.of(context). Replace the hardcoded loading label.- child: const Center( - child: Text( - 'Loading currencies...', - style: TextStyle( - color: AppTheme.textInactive, - fontSize: 14, - ), - ), - ), + child: Center( + child: Text( + S.of(context)!.loadingCurrencies, + style: const TextStyle( + color: AppTheme.textInactive, + fontSize: 14, + ), + ), + ),
466-473: Localize “Loading payment methods…” per i18n guidelineSame as currencies — use localized string.
- child: const Center( - child: Text( - 'Loading payment methods...', - style: TextStyle( - color: AppTheme.textInactive, - fontSize: 14, - ), - ), - ), + child: Center( + child: Text( + S.of(context)!.loadingPaymentMethods, + style: const TextStyle( + color: AppTheme.textInactive, + fontSize: 14, + ), + ), + ),
578-590: Localize “Min/Max” labels for reputation sliderUse localized keys for “Min”/“Max”.
- Text( - "Min: ${ratingMin.toInt()}", + Text( + "${S.of(context)!.min}: ${ratingMin.toInt()}", @@ - Text( - "Max: ${ratingMax.toInt()}", + Text( + "${S.of(context)!.max}: ${ratingMax.toInt()}",
479-482: Localize fallback payment method labels and prefer canonical IDs to avoid mismatchesThese hardcoded English labels violate i18n and risk not matching provider/emitted IDs used in filtering. At minimum, localize them; ideally, use canonical method IDs and map to localized labels at render-time.
Minimal i18n fix:
- options: ['Bank Transfer', 'Cash in person', 'PayPal', 'Zelle'], // Fallback options + options: [ + S.of(context)!.bankTransfer, + S.of(context)!.cashInPerson, + S.of(context)!.paypal, + S.of(context)!.zelle, + ], // Fallback optionsStronger recommendation (follow-up): have
paymentMethodsDataProviderexpose canonical IDs (e.g., "BANK_TRANSFER") and pass options as IDs; extendMultiSelectAutocompleteto accept aString Function(String id) labelForto render localized labels. I can provide a patch if you want to go this route.
🧹 Nitpick comments (3)
lib/shared/widgets/order_filter.dart (3)
8-8: Remove unused import to satisfy repo guidelines and avoid warningsThe
exchange_service_provider.dartimport is not used in this file.-import 'package:mostro_mobile/shared/providers/exchange_service_provider.dart';
259-274: Strengthen types for payment method aggregation to catch errors at compile timeUsing
Map<String, dynamic>and runtime casts hides shape mismatches. PreferMap<String, List<String>>.- List<String> _getAllPaymentMethods(Map<String, dynamic> paymentMethodsData) { + List<String> _getAllPaymentMethods(Map<String, List<String>> paymentMethodsData) { final Set<String> allMethods = {}; - - // Add all payment methods from all currencies - for (final methods in paymentMethodsData.values) { - if (methods is List) { - allMethods.addAll(methods.cast<String>()); - } - } + // Add all payment methods from all currencies + for (final List<String> methods in paymentMethodsData.values) { + allMethods.addAll(methods); + } // Remove "Other" since it's for custom input, not filtering allMethods.remove('Other'); final sortedMethods = allMethods.toList()..sort(); return sortedMethods; }If the provider currently returns a looser shape, I can help adjust its typing and usages first.
303-305: Make dialog width responsive to avoid overflows on narrow devicesHard-coding
width: 320can overflow on very small screens. Use a maxWidth constraint instead so it shrinks if needed.- return Container( - width: 320, + return Container( + constraints: const BoxConstraints(maxWidth: 320), padding: const EdgeInsets.all(20),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/shared/widgets/order_filter.dart(7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/shared/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared utilities and widgets must be placed in
shared/
Files:
lib/shared/widgets/order_filter.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/shared/widgets/order_filter.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/shared/widgets/order_filter.dart
🧠 Learnings (1)
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Applied to files:
lib/shared/widgets/order_filter.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). (1)
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lib/l10n/intl_en.arb (1)
799-816: Action required — remove duplicate ARB keys in lib/l10nAll new keys are present in intl_en.arb, intl_es.arb and intl_it.arb, but duplicate top-level keys were detected. Please remove/merge the duplicates before merging.
Files with duplicates:
- lib/l10n/intl_en.arb — forAmount, createdOn, proofOfWork, yourSharedKey
- lib/l10n/intl_es.arb — createdOn, proofOfWork, cancelPendingButton, completePurchaseButton, forAmount, @forAmount
- lib/l10n/intl_it.arb — createdOn, proofOfWork, yourSharedKey, cancelPendingButton, completePurchaseButton, forAmount, @forAmount
Recommended fix:
- Remove duplicate root-level entries so each message key appears only once (also dedupe any duplicated metadata keys that start with '@').
- Ensure message keys and their corresponding "@key" metadata are present exactly once and correctly paired.
- Regenerate localization artifacts after editing the ARB files.
♻️ Duplicate comments (1)
lib/l10n/intl_en.arb (1)
803-810: Duplicate keys resolved in the filter block — good cleanupThe previously duplicated keys (fiatCurrencies, paymentMethods, noneSelected, typeToAdd) are no longer redefined here. Thanks for fixing.
🧹 Nitpick comments (1)
lib/l10n/intl_en.arb (1)
799-799: Use uppercase casing for action labels to match existing UI buttonsMost action buttons are uppercase (e.g., CLOSE, TAKE, BACK, RATE). Consider aligning Apply for consistency.
Apply this diff:
- "apply": "Apply", + "apply": "APPLY",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/l10n/intl_it.arb
- lib/l10n/intl_es.arb
🧰 Additional context used
📓 Path-based instructions (1)
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/l10n/*.arb: Internationalization ARB files must be located inlib/l10n/
Add new localization keys to all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
Files:
lib/l10n/intl_en.arb
🧠 Learnings (1)
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/l10n/*.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_en.arb
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
lib/l10n/intl_en.arb (1)
811-815: Timeout message keys added — LGTMThe new timeout-related strings are clear, concise, and consistent with existing tone and spelling (“canceled”). No issues found.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lib/shared/widgets/order_filter.dart (1)
33-43: Avoid allocating your own TextEditingController (leak/double-dispose risk)Autocomplete supplies and disposes its own controller via fieldViewBuilder. Creating another in initState leaks it, and disposing the provided one can cause double-dispose. Remove the manual init and keep not disposing.
Apply this diff:
@override void initState() { super.initState(); - _controller = TextEditingController(); + // Controller is provided by Autocomplete's fieldViewBuilder; no manual init. } @override void dispose() { - //_controller.dispose(); + // Do not dispose _controller here — Autocomplete owns it. super.dispose(); }
♻️ Duplicate comments (3)
lib/shared/widgets/order_filter.dart (3)
482-491: Fallback payment method labels should be localized or derived from canonical IDsHardcoded English labels violate i18n and risk mismatch with provider data. Prefer localized strings (ARB keys) or derive from canonical method IDs.
Apply this diff once the ARB keys exist:
- error: (error, stack) => MultiSelectAutocomplete( - label: S.of(context)!.paymentMethods, - options: ['Bank Transfer', 'Cash in person', 'PayPal', 'Zelle'], // Fallback options + error: (error, stack) => MultiSelectAutocomplete( + label: S.of(context)!.paymentMethods, + options: [ + S.of(context)!.paymentMethodBankTransfer, + S.of(context)!.paymentMethodCashInPerson, + S.of(context)!.paymentMethodPayPal, + S.of(context)!.paymentMethodZelle, + ], // Localized fallback optionsWould you like me to add these ARB keys and translations? Also, if the provider returns canonical IDs, we can map IDs -> localized strings instead.
To confirm what we need, run this to inspect payment_methods_provider for available IDs:
#!/bin/bash set -euo pipefail rg -n -C3 --type=dart "paymentMethodsDataProvider|payment_method|method" || true
133-135: Replace Color.withValues(...) with withOpacity(...) to avoid SDK incompatibilityColor.withValues isn’t available on stable Flutter channels (and no local extension exists per prior scan). This will break builds. Use withOpacity.
Apply this diff:
- color: Colors.white.withValues(alpha: 0.2), + color: Colors.white.withOpacity(0.2), @@ - color: Colors.white.withValues(alpha: 0.2), + color: Colors.white.withOpacity(0.2), @@ - color: AppTheme.textSecondary.withValues(alpha: 0.6), + color: AppTheme.textSecondary.withOpacity(0.6), @@ - color: AppTheme.textSecondary.withValues(alpha: 0.2), + color: AppTheme.textSecondary.withOpacity(0.2), @@ - color: Colors.white.withValues(alpha: 0.1), + color: Colors.white.withOpacity(0.1), @@ - backgroundColor: Colors.white.withValues(alpha: 0.1), + backgroundColor: Colors.white.withOpacity(0.1), @@ - color: Colors.white.withValues(alpha: 0.2), + color: Colors.white.withOpacity(0.2), @@ - color: Colors.white.withValues(alpha: 0.2), + color: Colors.white.withOpacity(0.2), @@ - overlayColor: AppTheme.textSecondary.withValues(alpha: 0.2), + overlayColor: AppTheme.textSecondary.withOpacity(0.2), @@ - overlayColor: AppTheme.textSecondary.withValues(alpha: 0.2), + overlayColor: AppTheme.textSecondary.withOpacity(0.2), @@ - color: AppTheme.textSecondary.withValues(alpha: 0.3), + color: AppTheme.textSecondary.withOpacity(0.3),Also applies to: 140-142, 193-195, 218-221, 312-314, 347-348, 389-391, 465-467, 534-535, 606-607, 668-670
394-401: Localize remaining user-facing strings (“Loading…”, “Min:”, “Max:”)Per guidelines and retrieved learnings, use S.of(context) for all user-visible text.
Apply these diffs:
- child: const Center( - child: Text( - 'Loading currencies...', - style: TextStyle( + child: Center( + child: Text( + S.of(context)!.loadingCurrencies, + style: const TextStyle( color: AppTheme.textInactive, fontSize: 14, ), ), ),- child: const Center( - child: Text( - 'Loading payment methods...', - style: TextStyle( + child: Center( + child: Text( + S.of(context)!.loadingPaymentMethods, + style: const TextStyle( color: AppTheme.textInactive, fontSize: 14, ), ), ),- Text( - "Min: ${ratingMin.toInt()}", + Text( + "${S.of(context)!.min}: ${ratingMin.toInt()}",- Text( - "Max: ${ratingMax.toInt()}", + Text( + "${S.of(context)!.max}: ${ratingMax.toInt()}",If these ARB keys aren’t present yet, I can add them (en/es/it).
Also applies to: 470-476, 583-583, 592-592
🧹 Nitpick comments (5)
lib/shared/widgets/order_filter.dart (5)
72-110: Avoid repeatedly traversing Iterable options; convert to a List onceThe ListView uses options.length and options.elementAt(index) on an Iterable, which can be O(n) per call. Convert to a list once for performance and clarity.
Apply this diff:
- optionsViewBuilder: (context, onSelected, options) { - return Align( + optionsViewBuilder: (context, onSelected, options) { + final items = options.toList(growable: false); + return Align( alignment: Alignment.topLeft, child: Material( color: AppTheme.backgroundCard, elevation: 8, borderRadius: BorderRadius.circular(8), child: ConstrainedBox( constraints: const BoxConstraints(maxHeight: 200, maxWidth: 300), child: ListView.builder( padding: const EdgeInsets.all(4), shrinkWrap: true, - itemCount: options.length, + itemCount: items.length, itemBuilder: (context, index) { - final option = options.elementAt(index); + final option = items[index]; return InkWell( onTap: () => onSelected(option),
259-274: Flatten nested payment-method structures and avoid relying on the display label “Other”If paymentMethodsData is nested (e.g., by country/currency/provider), only handling List values will miss entries. Also, filtering “Other” by label is brittle.
Apply this diff to collect recursively and skip “Other” case-insensitively:
- List<String> _getAllPaymentMethods(Map<String, dynamic> paymentMethodsData) { - final Set<String> allMethods = {}; - - // Add all payment methods from all currencies - for (final methods in paymentMethodsData.values) { - if (methods is List) { - allMethods.addAll(methods.cast<String>()); - } - } - - // Remove "Other" since it's for custom input, not filtering - allMethods.remove('Other'); - - final sortedMethods = allMethods.toList()..sort(); - return sortedMethods; - } + List<String> _getAllPaymentMethods(Map<String, dynamic> paymentMethodsData) { + final Set<String> allMethods = {}; + void collect(dynamic node) { + if (node is List) { + allMethods.addAll(node.cast<String>()); + } else if (node is Map) { + for (final v in node.values) { + collect(v); + } + } + } + collect(paymentMethodsData); + allMethods.removeWhere((m) => m.toLowerCase() == 'other'); + final sortedMethods = allMethods.toList()..sort(); + return sortedMethods; + }If the provider exposes canonical method IDs, prefer filtering/removal by ID not by localized label. I can adjust once we verify the provider shape.
305-311: Avoid fixed width; constrain to maxWidth for smaller screensA hard-coded width: 320 can overflow on compact devices. Prefer a maxWidth constraint so the dialog remains responsive.
Apply this diff:
- return Container( - width: 320, + return Container( + constraints: const BoxConstraints(maxWidth: 320), padding: const EdgeInsets.all(20),
209-227: Use IconButton for the “remove” affordance to improve accessibility and tap feedbackGestureDetector + Container + Icon lacks semantics/tooltip and ink ripple in Material contexts. IconButton is better for a11y and consistency.
Apply this diff:
- GestureDetector( - onTap: () { + IconButton( + onPressed: () { final updated = List<String>.from(widget.selectedValues) ..remove(value); widget.onChanged(updated); }, - child: Container( - padding: const EdgeInsets.all(2), - decoration: BoxDecoration( - color: AppTheme.textSecondary.withOpacity(0.2), - shape: BoxShape.circle, - ), - child: const Icon( - Icons.close, - size: 14, - color: AppTheme.textSecondary, - ), - ), + icon: const Icon(Icons.close, size: 14, color: AppTheme.textSecondary), + tooltip: S.of(context)!.remove, // add ARB key if missing + style: IconButton.styleFrom( + padding: const EdgeInsets.all(2), + backgroundColor: AppTheme.textSecondary.withOpacity(0.2), + minimumSize: const Size(28, 28), + shape: const CircleBorder(), + ), ),I can add the S.remove key if needed.
331-337: Avoid uppercasing localized strings (locale-insensitive)toUpperCase() isn’t locale-aware and can produce odd results (e.g., Turkish). Prefer styling (letterSpacing/weight) without forcing uppercase, or provide already-uppercased variants in translations.
- S.of(context)!.filter.toUpperCase(), + S.of(context)!.filter, @@ - S.of(context)!.clear.toUpperCase(), + S.of(context)!.clear, @@ - S.of(context)!.apply.toUpperCase(), + S.of(context)!.apply,If uppercase is a design requirement, consider having translators provide the desired casing per locale.
Also applies to: 676-683, 714-719
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/features/home/providers/home_order_providers.dart(1 hunks)lib/shared/widgets/order_filter.dart(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/home/providers/home_order_providers.dart
🧰 Additional context used
📓 Path-based instructions (3)
lib/shared/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared utilities and widgets must be placed in
shared/
Files:
lib/shared/widgets/order_filter.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/shared/widgets/order_filter.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/shared/widgets/order_filter.dart
🧠 Learnings (1)
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Applied to files:
lib/shared/widgets/order_filter.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). (1)
- GitHub Check: build
🔇 Additional comments (1)
lib/shared/widgets/order_filter.dart (1)
242-256: Solid Riverpod integration and typed record usageGood move converting OrderFilter to ConsumerStatefulWidget, reading/writing through providers, and using typed records for rating/premium ranges. Initialization via a post-frame callback with mounted guard is correct.
Also applies to: 300-304
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
lib/shared/widgets/order_filter.dart (1)
31-37: Avoid leaking a TextEditingController; don’t create/dispose controllers owned by AutocompleteYou create an extra controller in initState and never dispose it; later you reassign _controller to Autocomplete’s controller (which you shouldn’t dispose). Make _controller nullable, don’t instantiate one yourself, and use null-aware clear.
Apply this diff:
-class MultiSelectAutocompleteState extends State<MultiSelectAutocomplete> { - late TextEditingController _controller; +class MultiSelectAutocompleteState extends State<MultiSelectAutocomplete> { + TextEditingController? _controller; @@ @override void initState() { super.initState(); - _controller = TextEditingController(); } @@ @override void dispose() { - //_controller.dispose(); super.dispose(); } @@ widget.onChanged(updated); - _controller.clear(); + _controller?.clear();Also applies to: 39-43, 115-116
♻️ Duplicate comments (1)
lib/shared/widgets/order_filter.dart (1)
482-491: Fallback payment method labels should be localized (or derived from canonical IDs)These strings are user-facing. Prior discussion deferred this to a follow-up PR; leaving a note to track.
Non-blocking for this PR.
🧹 Nitpick comments (1)
lib/shared/widgets/order_filter.dart (1)
259-274: Consider removing “Other” by canonical ID rather than labelRelying on the literal "Other" can break if upstream data localizes or changes casing. If the provider exposes stable IDs, filter by ID; otherwise, make the comparison case-insensitive.
Example:
// If labels only, guard against casing allMethods.removeWhere((m) => m.toLowerCase() == 'other');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/shared/widgets/order_filter.dart(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
lib/shared/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared utilities and widgets must be placed in
shared/
Files:
lib/shared/widgets/order_filter.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/shared/widgets/order_filter.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/shared/widgets/order_filter.dart
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/l10n/*.arb: Internationalization ARB files must be located inlib/l10n/
Add new localization keys to all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
Files:
lib/l10n/intl_es.arblib/l10n/intl_en.arblib/l10n/intl_it.arb
🧠 Learnings (5)
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-15T01:37:12.194Z
Learnt from: Catrya
PR: MostroP2P/mobile#270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.194Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: All changes should pass `flutter analyze` before commit
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Zero `flutter analyze` issues must be maintained
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/l10n/*.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_es.arblib/l10n/intl_en.arblib/l10n/intl_it.arb
🪛 RuboCop (1.76.1)
lib/l10n/intl_es.arb
[warning] 835-835: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 836-836: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_en.arb
[warning] 810-810: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 811-811: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 843-843: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 844-844: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (7)
lib/l10n/intl_es.arb (1)
826-827: Added "apply" key looks goodThe new "apply" translation aligns with UI usage in OrderFilter.
lib/l10n/intl_it.arb (1)
834-835: Added "apply" key looks goodMatches new UI action text and parity with EN/ES.
lib/l10n/intl_en.arb (2)
798-800: Added "apply" key is correctThis supports the new Apply button in the filter dialog.
815-820: Timeout message additions look consistentNew timeout strings align with usage and mirror ES/IT entries.
lib/shared/widgets/order_filter.dart (3)
276-298: Good: post-frame setState guarded by mounted; record destructuring for ratingThis avoids “setState() called after dispose()” and uses typed range values cleanly.
358-405: Nice: currencies async handling with localized loading stateLoading skeleton uses S.of(context)!.loadingCurrencies and consistent styling. Looks good.
446-481: Nice: payment methods async handling with localized loading textConsistent with currencies block and app theme.
… action buttons and remove duplicate ARB keys for loading messages
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
lib/shared/widgets/order_filter.dart (2)
41-41: Fix potential memory leak: uncomment controller disposalThe TextEditingController disposal is commented out, which will cause a memory leak. Controllers must be disposed when the widget is destroyed.
Apply this diff to fix the memory leak:
@override void dispose() { - //_controller.dispose(); + _controller.dispose(); super.dispose(); }
117-120: Avoid reassigning the field-built controllerThe
_controllermember is reassigned to the field-built controller on line 119, which interferes with the controller lifecycle management. The field builder provides its own controller that shouldn't be stored in the state.Remove the reassignment to prevent lifecycle issues:
fieldViewBuilder: (context, textEditingController, focusNode, onFieldSubmitted) { - _controller = textEditingController; return TextFormField( controller: textEditingController,Since you need to clear the field after selection, modify the
onSelectedcallback instead:onSelected: (String selection) { final updated = List<String>.from(widget.selectedValues) ..add(selection); widget.onChanged(updated); - _controller.clear(); + // Field will auto-clear after selection },
♻️ Duplicate comments (1)
lib/shared/widgets/order_filter.dart (1)
488-497: Consider using provider's fallback data instead of hardcoded optionsWhen the payment methods provider fails, consider using cached data or deriving fallback options from the currencies provider rather than hardcoding English payment method names.
This aligns with the previous review comment about localizing or deriving fallback payment method labels from canonical IDs.
🧹 Nitpick comments (1)
lib/shared/widgets/order_filter.dart (1)
425-426: Remove duplicate label parameter in error stateThe
MultiSelectAutocompletewidget already receives the label through the parent Column's Text widget. The duplicate label parameter in line 426 is redundant.MultiSelectAutocomplete( - label: S.of(context)!.fiatCurrencies, options: ['USD', 'EUR', 'VES'], // Fallback options selectedValues: selectedFiatCurrencies,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/shared/widgets/order_filter.dart(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/l10n/intl_es.arb
- lib/l10n/intl_it.arb
- lib/l10n/intl_en.arb
🧰 Additional context used
📓 Path-based instructions (3)
lib/shared/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared utilities and widgets must be placed in
shared/
Files:
lib/shared/widgets/order_filter.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/shared/widgets/order_filter.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/shared/widgets/order_filter.dart
🧠 Learnings (4)
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-15T01:37:12.194Z
Learnt from: Catrya
PR: MostroP2P/mobile#270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.194Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: All changes should pass `flutter analyze` before commit
Applied to files:
lib/shared/widgets/order_filter.dart
📚 Learning: 2025-08-03T09:43:38.727Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Zero `flutter analyze` issues must be maintained
Applied to files:
lib/shared/widgets/order_filter.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). (1)
- GitHub Check: build
🔇 Additional comments (5)
lib/shared/widgets/order_filter.dart (5)
259-274: LGTM! Clean helper method for aggregating payment methodsThe implementation correctly aggregates payment methods from all currencies, removes the "Other" option (which is for custom input), and returns a sorted list. Good separation of concerns.
276-298: LGTM! Proper lifecycle management with mounted checkThe implementation correctly uses
addPostFrameCallbackto access providers after the first frame, includes the mounted check before setState to prevent calling setState after disposal, and properly destructures the rating range tuple. This addresses the previous review comment about guarding setState with mounted.
365-375: LGTM! Well-structured async data handlingThe implementation properly handles the async state with loading and error states, providing appropriate fallbacks. The localized label and sorting of currency options enhance usability.
657-674: LGTM! Clear action properly resets all filtersThe clear button implementation correctly resets both local state and all provider states to their defaults, then closes the dialog. Good UX pattern.
704-712: LGTM! Apply action correctly syncs state to providersThe apply button properly updates all filter providers with the current local state values before closing the dialog. The state management flow is clean and follows React/Flutter patterns.
Summary by CodeRabbit
New Features
UI
Internationalization
Refactor