-
Notifications
You must be signed in to change notification settings - Fork 16
Refactoring and Styling Order Screens (Issue #119) #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Feat/issue 86
main changes
feat: improve cooperative cancel flow and order state management
main changes
|
""" WalkthroughThese changes refactor and enhance order and trade detail screens by introducing modular card widgets for displaying order information, updating background colors, improving date formatting, and refining the UI for pending orders. A new shared file provides reusable cards for order amount, payment method, creation date, order ID, creator reputation, and notifications, streamlining the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TakeOrderScreen
participant OrderCards
participant OrderIdCard
participant Clipboard
participant Snackbar
participant Notifier
User->>TakeOrderScreen: View order details
TakeOrderScreen->>OrderCards: Display order info (amount, payment, date, reputation, ID)
User->>OrderIdCard: Tap copy button
OrderIdCard->>Clipboard: Copy order ID
OrderIdCard->>Snackbar: Show "Copied" notification
User->>TakeOrderScreen: Tap action button
TakeOrderScreen->>TakeOrderScreen: If range order, prompt for amount input dialog
User->>TakeOrderScreen: Enter amount (if prompted)
TakeOrderScreen->>Notifier: takeBuyOrder/takeSellOrder(amount)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
lib/shared/widgets/order_cards.dart (1)
267-271: Add const keyword for consistency.The
Iconwidget should useconstfor consistency with other icons in this file and for performance optimization.- Icon( + const Icon( Icons.star, color: AppTheme.mostroGreen, size: 16, ),lib/features/trades/screens/trade_detail_screen.dart (1)
645-650: Address TODO: Implement creator rating extraction.The hardcoded values should be replaced with actual data extraction from the creator's information.
Would you like me to help implement the rating data extraction logic or create an issue to track this TODO?
lib/features/order/screens/take_order_screen.dart (1)
132-133: Consider adding null safety checks.The code accesses order properties without checking for null values. While these might be guaranteed by the data model, adding defensive checks would make the code more robust.
- final methods = order.paymentMethods.isNotEmpty - ? order.paymentMethods.join(', ') + final methods = order.paymentMethods?.isNotEmpty == true + ? order.paymentMethods!.join(', ') : 'No payment method';- createdDate: formatDateTime(order.createdAt!), + createdDate: formatDateTime(order.createdAt ?? DateTime.now()),Also applies to: 142-143
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/features/order/screens/take_order_screen.dart(4 hunks)lib/features/trades/screens/trade_detail_screen.dart(6 hunks)lib/shared/widgets/custom_card.dart(1 hunks)lib/shared/widgets/order_cards.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
lib/shared/widgets/custom_card.dart (1)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/shared/widgets/mostro_reactive_button.dart:22-32
Timestamp: 2025-05-08T15:39:07.322Z
Learning: In the Mostro Mobile codebase, WidgetStateProperty is used instead of the deprecated MaterialStateProperty for styling buttons with state-dependent properties.
lib/features/trades/screens/trade_detail_screen.dart (6)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
lib/features/order/screens/take_order_screen.dart (3)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
grunch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-write comments to english
grunch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pay attention to rabbit comments and apply changes if needed, fix conflicts and rebase
Catrya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
… platform plugin registrants
grunch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since now we are not having hardcoded text strings, text strings are now moved to ARB files in l10n directory, please with each new text string you need to add, also add a key in ARB files in spanish, italian and english
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/trades/screens/trade_detail_screen.dart (1)
518-532: TODO comment needs attention for creator reputation.The creator reputation display currently uses placeholder values with a TODO comment indicating the need to implement extraction of creator rating data.
The TODO comment indicates that extraction of creator rating data from Nostr events needs to be implemented. This is important for displaying actual reputation data instead of hardcoded values.
Would you like me to help implement the creator rating extraction logic or open a new issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/features/order/screens/take_order_screen.dart(3 hunks)lib/features/trades/screens/trade_detail_screen.dart(19 hunks)lib/shared/widgets/order_cards.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/order/screens/take_order_screen.dart
🧰 Additional context used
🧠 Learnings (1)
lib/features/trades/screens/trade_detail_screen.dart (3)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
🔇 Additional comments (16)
lib/shared/widgets/order_cards.dart (7)
1-6: Good import organization and comprehensive dependencies.The imports are well-organized with proper separation of Flutter core, services, and app-specific modules. All necessary dependencies are included for the widgets' functionality.
8-89: Well-structured OrderAmountCard with good flexibility.The
OrderAmountCardwidget is well-designed with:
- Clear parameter interface with appropriate optional fields
- Proper null safety handling with conditional rendering
- Consistent text styling and layout
- Good use of Flutter's conditional widget rendering with spread operators
The implementation follows Flutter best practices and provides good flexibility for different use cases.
92-135: Clean and consistent PaymentMethodCard implementation.The widget follows the same design pattern as other cards with:
- Proper use of localization (
S.of(context)!.paymentMethod)- Consistent styling and layout
- Clear visual hierarchy with icon and text
138-181: Consistent CreatedDateCard implementation.The widget maintains consistency with other cards and properly uses localization for the label text.
184-237: OrderIdCard with good UX for copy functionality.The widget provides excellent user experience with:
- Clear visual indication of the order ID using
AppTheme.mostroGreen- Proper clipboard functionality
- User feedback via SnackBar with localized message
- Clean layout with copy button
240-380: CreatorReputationCard with well-structured layout.The widget provides a comprehensive reputation display with:
- Three-column layout for rating, reviews, and days
- Proper use of icons for visual context
- Consistent text styling and spacing
- Good use of
MainAxisAlignment.spaceAroundfor even distribution
383-421: Flexible NotificationMessageCard with good defaults.The widget provides good flexibility with:
- Customizable icon and color with sensible defaults
- Proper layout with icon and expandable text
- Consistent padding and styling
lib/features/trades/screens/trade_detail_screen.dart (9)
8-20: Good import organization with new dependencies.The imports are properly organized with the addition of necessary dependencies for the new card widgets and utilities.
35-47: Consistent background color update.The background color has been updated to use
AppTheme.backgroundDarkfor consistency with other screens, and proper localization is used for the app bar title.
40-44: Clean logic for creator determination.The logic to determine if the current user is the creator of a pending order is clear and properly separated into variables for readability.
61-68: Good conditional rendering for creator reputation.The conditional rendering properly handles the different UI states for pending orders created by the user versus other states.
98-153: Excellent refactoring with reusable card widgets.The refactoring successfully replaces inline card implementations with the new reusable card widgets. The implementation properly handles:
- Conditional logic for pending orders created by the user
- Proper data formatting and preparation
- Clean separation of concerns
- Consistent spacing and layout
This addresses the previous review comment about refactoring to use reusable card widgets.
185-207: Consistent use of new card widgets for non-pending orders.The implementation maintains consistency by using the new card widgets even for non-pending orders, ensuring a unified UI experience.
212-214: Simplified OrderIdCard integration.The order ID display has been successfully simplified by using the new
OrderIdCardwidget, removing the need for inline card implementation.
534-543: Excellent helper method for DRY principle.The
_isUserCreatorhelper method successfully eliminates code duplication and provides a clean, reusable way to determine creator status. This addresses the previous review comment about extracting duplicate creator check logic.
547-557: Improved date formatting for better readability.The date formatting has been updated to provide a clearer, more user-friendly format with separate date and time components and simplified timezone offset display.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/features/trades/screens/trade_detail_screen.dart (3)
186-188: Remove unused variableThe
amountStringvariable is declared but never used, as indicated by the pipeline failure.- final amountString = - '${tradeState.order!.fiatAmount} ${tradeState.order!.fiatCode} $currencyFlag';
196-196: Fix dead null-aware expressionThe session is already null-checked above (line 165), so the null-aware operator is unnecessary here.
- : session.startTime ?? DateTime.now(), + : session!.startTime ?? DateTime.now(),
538-552: Creator reputation placeholder needs implementationThe TODO comment indicates that the creator rating data extraction is not yet implemented. The placeholder values (3.1, 15, 7) should be replaced with actual data when available.
The static values are acceptable for now, but consider implementing the actual creator rating extraction from Nostr events as mentioned in the PR objectives. Do you want me to help implement this functionality or create an issue to track this work?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/features/order/screens/take_order_screen.dart(3 hunks)lib/features/trades/screens/trade_detail_screen.dart(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/order/screens/take_order_screen.dart
🧰 Additional context used
🧠 Learnings (1)
lib/features/trades/screens/trade_detail_screen.dart (5)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
🪛 GitHub Actions: Flutter
lib/features/trades/screens/trade_detail_screen.dart
[error] 140-140: The getter 'youCreatedThisOffer' isn't defined for the type 'S' (undefined_getter)
[error] 145-145: The method 'someoneIsSelling' isn't defined for the type 'S' (undefined_method)
[warning] 186-186: The value of the local variable 'amountString' isn't used (unused_local_variable)
[warning] 196-196: The left operand can't be null, so the right operand is never executed (dead_null_aware_expression)
[error] 204-204: The getter 'youAreSelling' isn't defined for the type 'S' (undefined_getter)
[error] 205-205: The getter 'youAreBuying' isn't defined for the type 'S' (undefined_getter)
🔇 Additional comments (10)
lib/features/trades/screens/trade_detail_screen.dart (10)
8-8: Import organization looks goodThe new imports for
OrderType,order_cards.dart,currency_utils.dart, andSessionare properly organized and necessary for the new functionality.Also applies to: 14-14, 19-21
37-37: Background color update aligns with theme consistencyThe change from
AppTheme.dark1toAppTheme.backgroundDarkmaintains consistency with the app's dark theme as mentioned in the PR objectives.Also applies to: 49-49
42-46: Creator logic implementation is correctThe logic to determine if the user is the creator and if the order is pending is well-implemented and uses the extracted
_isUserCreatorhelper method.
65-72: Conditional rendering logic is appropriateThe use of spread operators and conditional rendering for showing creator reputation vs. message details based on pending status and creator role is well-structured.
75-80: Countdown time formatting is properly handledThe null check for
expiresAtand proper timestamp conversion ensures safe date handling.
116-162: Excellent refactoring to use reusable card widgetsThe refactoring to use
NotificationMessageCard,OrderAmountCard,PaymentMethodCard, andCreatedDateCardfrom the sharedorder_cards.dartgreatly improves code maintainability and consistency. The conditional rendering for pending orders created by the user provides a better user experience.
200-223: Good refactoring to use reusable card widgetsThe non-pending order display has been successfully refactored to use the new reusable card widgets, improving consistency with the rest of the application.
229-232: OrderIdCard refactoring is cleanThe replacement of the manually constructed card with the reusable
OrderIdCardwidget improves maintainability and consistency.
554-563: Helper method implementation is correctThe
_isUserCreatorhelper method properly addresses the DRY principle concern from the past review comment. The null checks ensure safe access to session and order properties.
566-577: Date formatting improvement is user-friendlyThe simplified date/time format with timezone offset provides better clarity as mentioned in the PR objectives. The format "Thu, Jul 03 2025 at 23:56 (GMT-03)" is much more readable than the previous verbose format.
621f7cf to
5870f84
Compare
Hola @AndreaDiazCorreia ya reviso. Si te refieresa esto:
No es cosa de mi emulador, lo veía también en mi móvil con la ultima release que se lanzó, pasa con frecuencia, y varios usuarios lo han reportado en el grupo de soporte de Mostro. No tiene exactamente que ver con los diseños, pero como es un error que ocurre en esta pantalla quizás sea bueno ya resolverlo aquí, no sé si @chebizarro @grunch crean mejor revisarlo luego y resolverlo en otro PR, a mí me parece bien cualquiera |
|
@AndreaDiazCorreia mira esta orden, sale el reloj con tiempo restante 0, pero la orden esta pending, id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
♻️ Duplicate comments (2)
lib/l10n/intl_es.arb (1)
449-449: Remove duplicate "done" key.The "done" key is duplicated (already exists at line 125:
"done": "Hecho"). This will cause JSON parsing issues and potential runtime errors.- "done": "HECHO" + "doneButton": "HECHO"lib/l10n/intl_it.arb (1)
449-449: Remove duplicate "done" key.The "done" key is duplicated (already exists at line 125:
"done": "Fatto"). This will cause JSON parsing issues and potential runtime errors.- "done": "FATTO" + "doneButton": "FATTO"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 (1)
- lib/l10n/intl_en.arb
🧰 Additional context used
📓 Path-based instructions (2)
`lib/l10n/intl_{en,es,it}.arb`: Add new localization keys to all three ARB files (en, es, it)
lib/l10n/intl_{en,es,it}.arb: Add new localization keys to all three ARB files (en, es, it)
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
lib/l10n/intl_es.arblib/l10n/intl_it.arb
`lib/l10n/*.arb`: Use proper ARB metadata for strings with parameters
lib/l10n/*.arb: Use proper ARB metadata for strings with parameters
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
lib/l10n/intl_es.arblib/l10n/intl_it.arb
🧠 Learnings (2)
lib/l10n/intl_es.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.501Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.501Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for strings with parameters
lib/l10n/intl_it.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.501Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.501Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for strings with parameters
🪛 RuboCop (1.75.5)
lib/l10n/intl_es.arb
[fatal] 443-443: unexpected token tSTRING
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)
(Lint/Syntax)
lib/l10n/intl_it.arb
[fatal] 443-443: unexpected token tSTRING
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)
(Lint/Syntax)
🪛 GitHub Actions: Flutter
lib/l10n/intl_es.arb
[error] 443-443: Flutter pub get failed due to formatting issue in ARB file: FormatException: Unexpected character at line 443, character 3.
lib/l10n/intl_es.arb
Outdated
| "youAreSellingTitle": "Estás vendiendo{sats} sats", | ||
| "youAreBuyingTitle": "Estás comprando{sats} sats", | ||
| "forAmount": "por {amount}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add missing ARB metadata for additional parameterized strings.
These strings also contain placeholders but lack the required metadata.
"youAreSellingTitle": "Estás vendiendo{sats} sats",
+ "@youAreSellingTitle": {
+ "placeholders": {
+ "sats": {
+ "type": "String"
+ }
+ }
+ },
"youAreBuyingTitle": "Estás comprando{sats} sats",
+ "@youAreBuyingTitle": {
+ "placeholders": {
+ "sats": {
+ "type": "String"
+ }
+ }
+ },
"forAmount": "por {amount}",
+ "@forAmount": {
+ "placeholders": {
+ "amount": {
+ "type": "String"
+ }
+ }
+ },📝 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.
| "youAreSellingTitle": "Estás vendiendo{sats} sats", | |
| "youAreBuyingTitle": "Estás comprando{sats} sats", | |
| "forAmount": "por {amount}", | |
| "youAreSellingTitle": "Estás vendiendo{sats} sats", | |
| "@youAreSellingTitle": { | |
| "placeholders": { | |
| "sats": { | |
| "type": "String" | |
| } | |
| } | |
| }, | |
| "youAreBuyingTitle": "Estás comprando{sats} sats", | |
| "@youAreBuyingTitle": { | |
| "placeholders": { | |
| "sats": { | |
| "type": "String" | |
| } | |
| } | |
| }, | |
| "forAmount": "por {amount}", | |
| "@forAmount": { | |
| "placeholders": { | |
| "amount": { | |
| "type": "String" | |
| } | |
| } | |
| }, |
🤖 Prompt for AI Agents
In lib/l10n/intl_es.arb around lines 413 to 415, the parameterized strings
"youAreSellingTitle" and "youAreBuyingTitle" are missing the necessary ARB
metadata that describes their placeholders. Add metadata entries for these keys
specifying the placeholders used (e.g., "sats") and their types to ensure proper
localization handling.
|
@Catrya le agregue algo para que haga auto size con la tipografia y que los botones se vean bien pero prueba porque en mi emulador y en mi celu se ven bien los botones |
sigue saliendo igual en mi telefono 😓 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
lib/l10n/intl_it.arb (1)
492-492: Remove duplicate localization key.The "done" key at line 492 conflicts with the existing key at line 125. This duplicate will cause JSON parsing errors and potential runtime issues.
Remove the duplicate key:
- "done": "FATTO",lib/l10n/intl_es.arb (1)
484-484: Remove duplicate localization key.The "done" key at line 484 conflicts with the existing key at line 125. This duplicate will cause JSON parsing errors and potential runtime issues.
Remove the duplicate key:
- "done": "HECHO",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
lib/features/order/screens/take_order_screen.dart(2 hunks)lib/features/trades/screens/trade_detail_screen.dart(14 hunks)lib/features/trades/widgets/trades_list_item.dart(9 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)pubspec.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- pubspec.yaml
- lib/features/trades/widgets/trades_list_item.dart
- lib/l10n/intl_en.arb
- lib/features/trades/screens/trade_detail_screen.dart
- lib/features/order/screens/take_order_screen.dart
🧰 Additional context used
📓 Path-based instructions (2)
`lib/l10n/intl_{en,es,it}.arb`: Add new localization keys to all three ARB files (en, es, it)
lib/l10n/intl_{en,es,it}.arb: Add new localization keys to all three ARB files (en, es, it)
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
lib/l10n/intl_es.arblib/l10n/intl_it.arb
`lib/l10n/*.arb`: Use proper ARB metadata for strings with parameters
lib/l10n/*.arb: Use proper ARB metadata for strings with parameters
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
lib/l10n/intl_es.arblib/l10n/intl_it.arb
🧠 Learnings (2)
lib/l10n/intl_es.arb (5)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for strings with parameters
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Regenerate localization files after ARB modifications by running `dart run build_runner build -d`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Always maintain zero Flutter analyze issues
lib/l10n/intl_it.arb (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for strings with parameters
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Regenerate localization files after ARB modifications by running `dart run build_runner build -d`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
🪛 RuboCop (1.75.5)
lib/l10n/intl_es.arb
[warning] 484-484: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 492-492: 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 (2)
lib/l10n/intl_it.arb (1)
411-483: LGTM for new localization keys.The comprehensive set of new Italian localization keys properly supports the trade detail screen UI enhancements. The translations are appropriate and maintain consistency with the existing Italian translations.
lib/l10n/intl_es.arb (1)
411-473: LGTM for new localization keys.The comprehensive set of new Spanish localization keys properly supports the trade detail screen UI enhancements. The translations are appropriate and maintain consistency with the existing Spanish translations.
Catrya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grunch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK








Changes Made:
1.Visual Consistency:
Benefits:
-Display of real creator reputation data when available
Summary by CodeRabbit
New Features
Improvements
Bug Fixes