Improve dispute chat details screen UX and fix keyboard overflow issues #319
Conversation
…pub from user notifications
…te payload parsing
…ad of hardcoded text
…ling in dispute model
…in en/es/it translations
WalkthroughUpdates dispute model, providers, UI widgets, and notifier/order-state logic to use session/order context and explicit userRole; prefers orderState.peer for counterparty, prevents admin as counterparty for terminal states, converts several widgets to Riverpod ConsumerWidgets, and adds dispute localization keys. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as DisputeChatScreen (ConsumerWidget)
participant RP as Riverpod
participant DP as disputeDetailsProvider
participant SN as sessionNotifierProvider
participant ON as orderNotifierProvider
participant Repo as Repository
UI->>RP: ref.watch(disputeDetailsProvider(disputeId))
RP->>DP: resolve(disputeId)
DP->>SN: inspect sessions for matching order/dispute
alt matching orderState found
DP->>ON: watch specific orderState
else
DP->>ON: watch orders broadly
end
DP->>Repo: getDispute(disputeId)
Repo-->>DP: Dispute
DP-->>UI: AsyncValue<Dispute>
UI->>UI: convert Dispute → DisputeData (orderState?, userRole?)
UI-->>UI: render info, messages, input (input only if in-progress)
sequenceDiagram
autonumber
participant Notifier as AbstractMostroNotifier
participant Session as SessionStore
participant State as OrderState
Note over Notifier: event handler (e.g., buyerTookOrder)
Notifier->>Session: fetch session by orderId
alt session missing
Notifier->>Notifier: log and abort
else
Notifier->>Session: read session.role
alt role == buyer
Notifier->>State: peerPubkey = sellerTradePubkey
else role == seller
Notifier->>State: peerPubkey = buyerTradePubkey
else
Notifier->>State: peerPubkey = null
end
alt peerPubkey exists
Notifier->>State: set Peer(pubkey)
end
Notifier->>State: set dispute.createdAt from event.timestamp if available
State-->>Notifier: updated state/session
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/features/disputes/widgets/dispute_status_content.dart (2)
16-16: Treat all resolved-like statuses as resolved UIInclude solved and seller-refunded so the green resolution box shows consistently.
- bool isResolved = dispute.status.toLowerCase() == 'resolved' || dispute.status.toLowerCase() == 'closed'; + final s = dispute.status.toLowerCase(); + bool isResolved = s == 'resolved' || s == 'closed' || s == 'solved' || s == 'seller-refunded';
151-153: Fix unknown-state messageReturning “Unknown Resolved” is contradictory. Use the dedicated string.
- // Use a generic message with the status - return "${S.of(context)!.unknown} ${S.of(context)!.disputeStatusResolved}"; + return S.of(context)!.disputeUnknownStatus;
🧹 Nitpick comments (12)
lib/data/models/dispute.dart (2)
383-416: Simplify userRole inference (remove redundant branches and add fallback)The three branches all compute the same result. Use a single source of truth and prefer any available order (OrderState or Dispute.order).
- // Determine if user is buyer or seller based on order information - if (orderState != null && orderState.order != null) { - final order = orderState.order!; - - // Try to determine user role by checking master pubkeys first - // This is the most accurate way to determine the user's role - if (order.masterBuyerPubkey != null || order.masterSellerPubkey != null) { - // We need to get the current user's pubkey to compare - // For now, we'll use the logic based on who initiated the dispute - // and the order type to infer the correct role - - // If user initiated the dispute, we can infer their role from context - if (isUserCreator == true) { - // User initiated dispute - they are likely the one who has an issue - // In a 'buy' order, if buyer has issue, they dispute with seller - // In a 'sell' order, if seller has issue, they dispute with buyer - userRole = order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller; - } else if (isUserCreator == false) { - // Peer initiated dispute against user - // In a 'buy' order, if seller disputes, user is buyer - // In a 'sell' order, if buyer disputes, user is seller - userRole = order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller; - } else { - // Unknown who initiated, use order type logic - // This is a simplified approach - in a 'buy' order, assume user is buyer - userRole = order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller; - } - } else { - // Fallback to order type logic - // In a 'buy' order, the user is typically the buyer - // In a 'sell' order, the user is typically the seller - userRole = order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller; - } - } + // Determine role from any available order + final resolvedOrder = orderState?.order ?? dispute.order; + if (resolvedOrder != null) { + final isBuy = resolvedOrder.kind.value == 'buy'; + userRole = isBuy ? UserRole.buyer : UserRole.seller; + }
316-569: Decouple UI concerns from data layerDisputeData depends on Flutter widgets and localization (S.of) within lib/data/. Consider moving DisputeData (and getLocalizedDescription) under lib/features/disputes/models or a view-model layer to keep lib/data/ UI-agnostic. As per coding guidelines.
lib/features/disputes/widgets/dispute_message_input.dart (2)
76-79: Avoid Color.withValues; use withOpacity for compatibility.
withValues(alpha: 153)is non-standard in many Flutter channels. UsewithOpacity(0.6)to match the comment and improve compatibility.Apply this diff:
- hintStyle: TextStyle( - color: AppTheme.textSecondary.withValues(alpha: 153), // 0.6 opacity - fontSize: 15), + hintStyle: TextStyle( + color: AppTheme.textSecondary.withOpacity(0.6), // 0.6 opacity + fontSize: 15, + ),
70-70: Remove redundantenabled: true.Default is enabled; drop it to reduce noise.
- enabled: true,lib/features/disputes/providers/dispute_providers.dart (3)
95-101: TypeorderStateinstead ofdynamic.Using
dynamichides errors and risks runtime crashes. Type this to the actual order state (e.g.,OrderStateor the correct model from order_notifier_provider).As per coding guidelines
Also applies to: 123-131, 133-136
96-98: Make sessions reactive in mapping.Use
ref.watchso dispute view models recompute on session changes (even if the disputes list itself hasn’t changed).- final sessions = ref.read(sessionNotifierProvider); + final sessions = ref.watch(sessionNotifierProvider);Based on learnings
65-69: Avoid aliasing an existing provider with another provider.
disputeChatProviderjust re-exportsdisputeChatNotifierProvider(...).notifier, adding indirection and potential confusion. Prefer using the original provider directly or rename clearly if both are required.Based on learnings
lib/features/disputes/screens/dispute_chat_screen.dart (3)
66-71: Normalize status before checking for in-progress.Status strings vary; normalize for consistency (you already do this elsewhere).
- if (disputeData.status == 'in-progress') + if (_normalizeStatus(disputeData.status) == 'in-progress') DisputeMessageInput(disputeId: disputeId)Add helper:
+ String _normalizeStatus(String s) => + s.trim().toLowerCase().replaceAll(RegExp(r'[\s_]+'), '-');
100-207: Avoiddynamicfor order state; add proper typing.Calls like
matchingOrderState.peerondynamicrisk runtime errors. Type to your order state model, or guard via interfaces. Consider extracting an order-state resolver utility to dedupe the repeated loops.Based on learnings
210-235: Prefer acopyWithon DisputeData.Manually reconstructing DisputeData increases breakage risk if fields evolve. Add
copyWith(counterparty: ...)and use it here.lib/features/disputes/widgets/dispute_messages_list.dart (2)
98-102: Simplify redundant ternary.Both branches are identical.
- final messageIndex = isResolvedStatus && messages.isNotEmpty - ? index - 1 // Account for info card - : index - 1; // Account for info card + final messageIndex = index - 1; // Account for info card
281-286: Status normalization utility: good.Consistent with other parts; consider centralizing this helper to avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
lib/data/models/dispute.dart(1 hunks)lib/features/disputes/providers/dispute_providers.dart(3 hunks)lib/features/disputes/screens/dispute_chat_screen.dart(3 hunks)lib/features/disputes/widgets/dispute_info_card.dart(1 hunks)lib/features/disputes/widgets/dispute_message_input.dart(1 hunks)lib/features/disputes/widgets/dispute_messages_list.dart(3 hunks)lib/features/disputes/widgets/dispute_status_content.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(2 hunks)lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(2 hunks)lib/l10n/intl_it.arb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
lib/l10n/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain ARB localization files in
lib/l10n/(en, es, it)
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all ARB files: intl_en.arb, intl_es.arb, intl_it.arb
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
**/*.arb
📄 CodeRabbit inference engine (AGENTS.md)
Keep user-facing strings in localization ARB files; do not hard-code copy in Dart code
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
lib/features/**/{screens,providers,notifiers,widgets}/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Follow feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/widgets/dispute_message_input.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Always use localized strings viaS.of(context)!.keyName; avoid hardcoded user-facing text
Useconstconstructors where possible for immutability and performance
Checkmountedbefore usingBuildContextafter async operations
UseS.of(context)!.keyNameand passBuildContextto methods needing localization
Files:
lib/features/disputes/widgets/dispute_status_content.dartlib/data/models/dispute.dartlib/features/disputes/widgets/dispute_message_input.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place feature-specific Dart code under lib/features// (screens, providers, widgets)
Files:
lib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/widgets/dispute_message_input.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Format code with the Dart formatter (two-space indentation, trailing commas where beneficial)
Keep the analyzer (flutter_lints) at zero warnings across the codebase
Files:
lib/features/disputes/widgets/dispute_status_content.dartlib/data/models/dispute.dartlib/features/disputes/widgets/dispute_message_input.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/features/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier within their feature directory
Files:
lib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/widgets/dispute_message_input.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Place persistence and API abstractions under lib/data/
Files:
lib/data/models/dispute.dart
lib/features/**/providers/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under
features/{feature}/providers/
Files:
lib/features/disputes/providers/dispute_providers.dart
🧠 Learnings (1)
📚 Learning: 2025-06-08T23:54:09.987Z
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.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.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 (7)
lib/features/disputes/widgets/dispute_status_content.dart (1)
137-141: LGTM for clearer in-progress copyReplacing “No messages yet” with an explicit in-progress status improves UX.
lib/l10n/intl_es.arb (2)
704-706: LGTM: adds disputeChatClosedString aligns with new closed-chat UX.
723-734: LGTM: intl_it.arb includes the new disputeOpenedByYouAgainstSeller and disputeOpenedByYouAgainstBuyer keys.lib/l10n/intl_en.arb (2)
698-699: LGTM: adds disputeChatClosedConsistent with closed-chat handling.
717-728: Approve: locale parity confirmed
All ARB files include the new keys and parse as valid JSON—run gen-l10n to regenerate localization classes.lib/l10n/intl_it.arb (2)
777-788: LGTM: disputeOpenedByYouAgainstSeller/Buyer keys added to intl_en, intl_es, intl_it with correct placeholders.All locales include both keys with consistent
{counterparty}placeholders.
758-760: disputeChatClosed localized string present in all locales Verified in intl_it.arb, intl_es.arb, and intl_en.arb.
…ty resolution logic
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/data/models/dispute.dart (2)
372-387: Expand terminal status list for consistency.The
terminalStatusListincludes only'resolved'and'closed', but other parts of the codebase (e.g.,_isResolvedStatusinlib/features/disputes/widgets/dispute_messages_list.dartlines 286-293) also treat'solved'and'seller-refunded'as terminal/resolved states. For consistency, consider including all resolved synonyms.Apply this diff to align with the broader codebase:
- // Terminal dispute states where admin should not be used as counterparty - final terminalStatusList = ['resolved', 'closed']; + // Terminal dispute states where admin should not be used as counterparty + final terminalStatusList = ['resolved', 'closed', 'solved', 'seller-refunded'];
402-416: Misleading comments: user role doesn't depend on isUserCreator as stated.The comments at lines 403-405 and 408-410 suggest that the user's role differs based on who initiated the dispute, but the code assigns the same role (
order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller) regardless ofisUserCreator. This is correct—user role is determined by order type, not by who initiated the dispute—but the comments imply otherwise.Clarify the comments to reflect the actual logic:
- // If user initiated the dispute, we can infer their role from context if (isUserCreator == true) { - // User initiated dispute - they are likely the one who has an issue - // In a 'buy' order, if buyer has issue, they dispute with seller - // In a 'sell' order, if seller has issue, they dispute with buyer + // User's role is determined by order type, not by who initiated userRole = order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller; } else if (isUserCreator == false) { - // Peer initiated dispute against user - // In a 'buy' order, if seller disputes, user is buyer - // In a 'sell' order, if buyer disputes, user is seller + // User's role is determined by order type, not by who initiated userRole = order.kind.value == 'buy' ? UserRole.buyer : UserRole.seller;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/data/models/dispute.dart(1 hunks)lib/features/disputes/screens/dispute_chat_screen.dart(3 hunks)lib/features/disputes/widgets/dispute_info_card.dart(1 hunks)lib/features/disputes/widgets/dispute_messages_list.dart(7 hunks)lib/features/disputes/widgets/dispute_status_content.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/features/disputes/widgets/dispute_info_card.dart
- lib/features/disputes/widgets/dispute_status_content.dart
🧰 Additional context used
📓 Path-based instructions (6)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Always use localized strings viaS.of(context)!.keyName; avoid hardcoded user-facing text
Useconstconstructors where possible for immutability and performance
Checkmountedbefore usingBuildContextafter async operations
UseS.of(context)!.keyNameand passBuildContextto methods needing localization
Files:
lib/data/models/dispute.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Place persistence and API abstractions under lib/data/
Files:
lib/data/models/dispute.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Format code with the Dart formatter (two-space indentation, trailing commas where beneficial)
Keep the analyzer (flutter_lints) at zero warnings across the codebase
Files:
lib/data/models/dispute.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/features/**/{screens,providers,notifiers,widgets}/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Follow feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place feature-specific Dart code under lib/features// (screens, providers, widgets)
Files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/screens/dispute_chat_screen.dart
lib/features/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier within their feature directory
Files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/screens/dispute_chat_screen.dart
🧠 Learnings (3)
📚 Learning: 2025-06-08T23:54:09.987Z
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.
Applied to files:
lib/data/models/dispute.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-09-22T18:48:23.369Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T18:48:23.369Z
Learning: Applies to lib/**/*.dart : Always use localized strings via `S.of(context)!.keyName`; avoid hardcoded user-facing text
Applied to files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.dart
📚 Learning: 2025-09-22T18:48:23.369Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T18:48:23.369Z
Learning: Applies to lib/**/*.dart : Use `S.of(context)!.keyName` and pass `BuildContext` to methods needing localization
Applied to files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/screens/dispute_chat_screen.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/features/order/notfiers/abstract_mostro_notifier.dart (2)
150-172: Critical issue resolved: session guard now in place.The code now re-fetches the session and guards against null before accessing
session.role, eliminating theLateInitializationErrorrisk flagged in the previous review. The role-based peer resolution logic is correct.
198-221: Critical issue resolved: session guard applied consistently.The same session initialization guard is correctly applied here. Both blocks now safely resolve the peer based on the current user's role.
lib/features/disputes/screens/dispute_chat_screen.dart (3)
32-32: Remove hardcoded fallback in AppBar title.Use
S.of(context)!.disputeDetailsconsistently per coding guidelines.As per coding guidelines
- S.of(context)?.disputeDetails ?? 'Dispute Details', + S.of(context)!.disputeDetails,
49-49: Remove hardcoded fallback for "Dispute not found".Use
S.of(context)!.disputeNotFoundconsistently per coding guidelines.As per coding guidelines
- S.of(context)?.disputeNotFound ?? 'Dispute not found', + S.of(context)!.disputeNotFound,
82-82: Remove hardcoded fallback in error message.Use
S.of(context)!.errorLoadingDispute(error.toString())consistently per coding guidelines.As per coding guidelines
- S.of(context)?.errorLoadingDispute(error.toString()) ?? 'Error loading dispute: ${error.toString()}', + S.of(context)!.errorLoadingDispute(error.toString()),
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
- Si es una orden de venta, tanto al comprador como al vendedor le dice siempre: "Dispute with Buyer"
- Si es una orden de Compra le dice a los 2 usuarios: "Disputes with Seller"
-
No creo que deba poner la pubkey de la contraparte, sino el nym que se usa para que sea mas facil para el usuario y se corresponda con las pantallas de order details que no dicen las pubkeys sino los nyms
-
Tuve una disputa que a pesar de ser settle por el admin se quedó en la pantalla del vendedor como in-progress, nunca se le actualizó a ese usuario. A la contraparte sí se le actualizó a Resolved. Las demás veces sí se actualizaron bien
- Al seller siempre le aparecía la disputa más reciente al final de la lista de disputas, pero al buyer sí le aparecia bien de 1era arriba
- Cuando una disputa es terminada, si fue canceled o si fue settled se ven muy diferentes, pues a una le deja el mensaje con las instrucciones de la disputa y a la otra no, deberían verse parecidas.
- Hay superposición en el chat de disputas con los botones del telefono, eso se arregló en las demás pantallas añadiendo un margen al final
…reserve message timestamps
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/features/disputes/screens/dispute_chat_screen.dart (2)
33-51: Remove hard-coded fallbacks in localized strings.
S.of(context)already yields a non-null localization instance when the widget is underMaterialApp. Keeping?? 'Dispute Details'/'Dispute not found'leaves hard-coded English strings and breaks the localization rule. Please drop the null-aware operator and fallbacks so we always use the localized copy.- title: Text( - S.of(context)?.disputeDetails ?? 'Dispute Details', + title: Text( + S.of(context).disputeDetails, ... - return Center( - child: Text( - S.of(context)?.disputeNotFound ?? 'Dispute not found', + return Center( + child: Text( + S.of(context).disputeNotFound,
83-84: Also localize the error string without fallback.Same issue for the error state: drop the fallback and rely solely on the localized entry.
- child: Text( - S.of(context)?.errorLoadingDispute(error.toString()) ?? 'Error loading dispute: ${error.toString()}', + child: Text( + S.of(context).errorLoadingDispute(error.toString()),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
lib/data/models/dispute.dart(3 hunks)lib/features/disputes/providers/dispute_providers.dart(3 hunks)lib/features/disputes/screens/dispute_chat_screen.dart(3 hunks)lib/features/disputes/widgets/dispute_messages_list.dart(4 hunks)lib/features/disputes/widgets/dispute_status_content.dart(6 hunks)lib/features/order/models/order_state.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(5 hunks)lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(2 hunks)lib/l10n/intl_it.arb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/l10n/intl_es.arb
🧰 Additional context used
📓 Path-based instructions (9)
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Place persistence and API abstractions under lib/data/
Files:
lib/data/models/dispute.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Format code with the Dart formatter (two-space indentation, trailing commas where beneficial)
Keep the analyzer (flutter_lints) at zero warnings across the codebase
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
Files:
lib/data/models/dispute.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/models/order_state.dartlib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/screens/dispute_chat_screen.dartlib/features/disputes/widgets/dispute_messages_list.dart
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possible
Files:
lib/data/models/dispute.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/models/order_state.dartlib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/screens/dispute_chat_screen.dartlib/features/disputes/widgets/dispute_messages_list.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place feature-specific Dart code under lib/features// (screens, providers, widgets)
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/models/order_state.dartlib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/screens/dispute_chat_screen.dartlib/features/disputes/widgets/dispute_messages_list.dart
lib/features/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier within their feature directory
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/models/order_state.dartlib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/providers/dispute_providers.dartlib/features/disputes/screens/dispute_chat_screen.dartlib/features/disputes/widgets/dispute_messages_list.dart
**/*.arb
📄 CodeRabbit inference engine (AGENTS.md)
Keep user-facing strings in localization ARB files; do not hard-code copy in Dart code
Files:
lib/l10n/intl_it.arblib/l10n/intl_en.arb
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it)
Files:
lib/l10n/intl_it.arblib/l10n/intl_en.arb
lib/l10n/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
For parameterized strings, include proper ARB metadata
Files:
lib/l10n/intl_it.arblib/l10n/intl_en.arb
lib/features/**/providers/**
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under
features/{feature}/providers/
Files:
lib/features/disputes/providers/dispute_providers.dart
🧠 Learnings (1)
📚 Learning: 2025-06-08T23:54:09.987Z
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.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.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
…tates for admin counterparty (coderabbit comments)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/data/models/dispute.dart (1)
380-407: Consider using aconstset for terminal statuses.
SwitchingterminalStatusListto aconstSet<String>avoids repeated list allocations on every factory call and gives O(1) membership checks.Apply:
- final terminalStatusList = [ - 'resolved', - 'closed', - 'seller-refunded', - 'seller_refunded', - 'admin-canceled', - 'admin_canceled', - 'admin-settled', - 'admin_settled', - 'solved', - 'completed', - ]; + const terminalStatusSet = { + 'resolved', + 'closed', + 'seller-refunded', + 'seller_refunded', + 'admin-canceled', + 'admin_canceled', + 'admin-settled', + 'admin_settled', + 'solved', + 'completed', + }; ... - if (orderState?.peer == null && - dispute.adminPubkey != null && - !terminalStatusList.contains(normalizedStatus)) { + if (orderState?.peer == null && + dispute.adminPubkey != null && + !terminalStatusSet.contains(normalizedStatus)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/data/models/dispute.dart(3 hunks)lib/features/order/models/order_state.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place feature-specific Dart code under lib/features// (screens, providers, widgets)
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/models/order_state.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Format code with the Dart formatter (two-space indentation, trailing commas where beneficial)
Keep the analyzer (flutter_lints) at zero warnings across the codebase
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/dispute.dartlib/features/order/models/order_state.dart
lib/features/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier within their feature directory
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/models/order_state.dart
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possible
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/dispute.dartlib/features/order/models/order_state.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Place persistence and API abstractions under lib/data/
Files:
lib/data/models/dispute.dart
🧠 Learnings (1)
📚 Learning: 2025-06-08T23:54:09.987Z
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.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.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 (3)
lib/features/order/models/order_state.dart (1)
136-148: Timestamp normalization looks good.
EnsuringcreatedAtfollows the message timestamp keeps dispute ordering consistent downstream.lib/features/order/notfiers/abstract_mostro_notifier.dart (2)
150-171: Great guard around session reuse.
Re-fetching the session before touchingsession.roleremoves the late-init risk and keeps peer resolution dependable in both flows.Also applies to: 198-219
291-377: Created-at propagation is spot on.
Copying disputes with the event timestamp plugged intocreatedAtensures downstream sorting stays stable for both self- and peer-initiated disputes.
|
@AndreaDiazCorreia ya funcionan bien la mayoría de los errores de la review anterior, genial. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/disputes/widgets/dispute_messages_list.dart (1)
68-120: Consider simplifying the index mapping logic for chat closed messages.The current logic correctly handles all cases (empty messages, non-empty messages, resolved/non-resolved), but the index mapping on lines 89-104 is complex with multiple conditions and calculations.
Consider extracting the item type determination into a helper method:
enum ListItemType { infoCard, message, chatClosed } ({ListItemType type, int? messageIndex}) _getItemType(int index, List<DisputeChat> messages) { if (index == 0) { return (type: ListItemType.infoCard, messageIndex: null); } final messageIndex = index - 1; final isResolvedStatus = _isResolvedStatus(widget.status); if (messageIndex >= messages.length) { // Beyond messages, must be chat closed (only added for resolved) return (type: ListItemType.chatClosed, messageIndex: null); } return (type: ListItemType.message, messageIndex: messageIndex); }This would make the builder more readable and easier to verify.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/features/disputes/widgets/dispute_messages_list.dart(3 hunks)lib/features/disputes/widgets/dispute_status_content.dart(6 hunks)lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(2 hunks)lib/l10n/intl_it.arb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it)
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
lib/l10n/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
For parameterized strings, include proper ARB metadata
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/widgets/dispute_status_content.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/widgets/dispute_status_content.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/disputes/widgets/dispute_messages_list.dartlib/features/disputes/widgets/dispute_status_content.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 (11)
lib/l10n/intl_es.arb (1)
703-738: LGTM! Localization keys are complete and consistent.All new dispute-related keys are present in Spanish, English, and Italian ARB files with proper placeholder metadata. The messages correctly distinguish between buyer and seller perspectives for canceled and settled disputes.
lib/l10n/intl_en.arb (1)
697-732: LGTM! English localization additions are correct.The new dispute keys follow the established pattern and are properly documented with placeholder metadata.
lib/l10n/intl_it.arb (1)
757-792: LGTM! Italian localization is complete.All new dispute keys are present with correct placeholder metadata, maintaining consistency across all three locale files.
lib/features/disputes/widgets/dispute_status_content.dart (3)
1-24: LGTM! Proper migration to ConsumerWidget with Riverpod.The widget correctly uses
ConsumerWidgetandWidgetRefto resolve the counterparty pubkey to a readable nym, with a fallback to theunknownlocalized string when the counterparty is not available.As per coding guidelines
164-200: LGTM! Role-based messages with proper unknown fallback.The
_getDisputeStatusTextmethod correctly handles buyer/seller/unknown roles for theinitiatedByUsercase (lines 168-177), addressing the previous review feedback by providing a generic fallback message when the role is unknown.
38-62: Verifyactionandstatusare always populated for resolved disputes
Inspect theDisputeDatamodel (lib/data/models/dispute.dart) and its JSON mapping to ensureactionandstatusare non-nullable and always set; if they can be missing, update the fallback to a neutral resolution message or add defensive logging.lib/features/disputes/widgets/dispute_messages_list.dart (5)
241-249: LGTM! Mock data removed as per PR objectives.The mock message generation now returns an empty list, removing the confusing mock data from the UI. Real dispute chat messages will be loaded from the dispute chat provider in future iterations.
252-266: LGTM! Status normalization handles variations correctly.The
_normalizeStatusand_isResolvedStatusmethods properly handle status variations (spaces, underscores, case) and identify all terminal states: resolved, closed, solved, and seller-refunded.
158-200: LGTM! Empty state content builders are clear and well-structured.The three content builders (
_buildWaitingForAdmin,_buildEmptyChatArea,_buildChatClosedArea) correctly use localized strings without hardcoded fallbacks, as per the resolved review feedback.As per coding guidelines
203-238: LGTM! Admin notification correctly limited to in-progress status.The notification now only appears for
in-progressstatus (line 206), avoiding confusion duringinitiated(no admin yet) andresolved(dispute finished) states.
318-351: LGTM! Chat closed message provides clear visual closure.The lock icon and localized
disputeChatClosedmessage (line 340) clearly indicate that messaging is disabled for resolved disputes, addressing the PR objective to add localized chat-closed messages.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/disputes/widgets/dispute_messages_list.dart (1)
66-359: Consider adding more const constructors where possible.While some widgets use
const(e.g., lines 156, 181, 215), other immutable widgets could benefit from const constructors for minor performance improvements. For example:
- Line 169:
const EdgeInsets.all(32)- Line 198:
const EdgeInsets.all(32)- Line 220:
const EdgeInsets.all(12)- Line 329:
const EdgeInsets.all(16)As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/disputes/widgets/dispute_messages_list.dart(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/disputes/widgets/dispute_messages_list.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/disputes/widgets/dispute_messages_list.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/disputes/widgets/dispute_messages_list.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 (12)
lib/features/disputes/widgets/dispute_messages_list.dart (12)
9-10: LGTM! Clean enum design.The private enum clearly categorizes the three types of items in the dispute list.
113-128: LGTM! Item type logic is correct.The method correctly maps indices to item types, with proper handling of the info card (index 0), messages, and the chat-closed item for resolved disputes.
130-164: LGTM! Well-structured empty state with proper scroll handling.The use of
LayoutBuilder,SingleChildScrollView, andConstrainedBoxensures the empty state fills available space while remaining scrollable when needed. The conditional spacer for resolved states is a nice touch.
166-208: LGTM! Proper localization without hardcoded fallbacks.Both empty state helpers correctly use
S.of(context)!for all user-facing strings, adhering to the coding guidelines.As per coding guidelines
210-246: LGTM! Correct status-conditional notification.The notification properly shows only for in-progress disputes, uses normalized status checks, and correctly localizes the message.
249-257: Appropriate placeholder for future implementation.The empty return and clear comments align with the PR objective that messaging functionality will be added in future iterations.
259-274: LGTM! Robust status normalization and terminal state detection.The normalization handles various status formats consistently, and the terminal state list covers all expected resolved statuses.
276-288: LGTM! Proper status-based content routing.The method correctly routes to different UI states based on normalized status, with a safe default for in-progress disputes.
290-301: LGTM! Item count calculation matches item type logic.The count correctly includes the info card, messages, and conditionally adds the chat-closed item for resolved disputes.
303-323: LGTM! Properly localized closed chat area.The empty state for resolved disputes correctly uses localized strings without hardcoded fallbacks.
As per coding guidelines
325-359: LGTM! Clear visual indication of closed chat state.The message correctly uses localized text and provides clear visual feedback with appropriate styling and iconography.
As per coding guidelines
69-110: No action needed for the unreachable CustomScrollView branch.
The stubbed_getMockMessages()intentionally returns an empty list and is documented for future real-message loading.
|
@Catrya Ya solo queda probar. Quité el mensaje verde que aparecía abajo cuando se resolvía, porque decía lo mismo que el de arriba y daba la sensación de ser una notificación que no se podía cerrar. Dejé solo el de arriba, que ya incluye la corrección que pediste 
|
| "disputeAdminSettledMessage": "El administrador resolvió la orden a favor de una de las partes. Revisa tu billetera para ver los pagos.", | ||
| "disputeSellerRefundedMessage": "El administrador canceló la orden y reembolsó al vendedor. La disputa está ahora cerrada.", |
There was a problem hiding this comment.
Estos mensajes ya no se usan con los nuevos que pusiste para cada caso en la resolución de disputas. Se pueden eliminar, pero no imprescindible hacerlo para mergear el PR, se puede hacer después
🎯 Overview
Enhanced the existing dispute chat screen by removing mock data, implementing proper keyboard overflow handling, fixing layout issues, and improving the overall user experience. Key improvements include resolving the "BOTTOM OVERFLOW BY X PIXELS" error when the keyboard appears, ensuring the resolution message is always visible without scrolling for resolved disputes, removing confusing status messages from in-progress disputes, adding proper dispute list ordering (most recent first), and implementing localized chat closed messages. The layout architecture was refactored to use proper scrollable structures with SafeArea handling for better cross-device compatibility.
Important
Chat functionality is not yet implemented - this PR focuses on UI/UX improvements and layout fixes. Real-time messaging will be added in future iterations.
🧪 Testing
Summary by CodeRabbit
New Features
Bug Fixes