UI Refinements for My Trades Screen #116
Conversation
WalkthroughThe trades screen UI was simplified by removing the filter button and related elements, replacing them with a new header and a consolidated content container. The trade list item widget was restyled to use a custom container, added role and status chips with expanded status handling, and removed unused methods and imports for a cleaner layout. Additionally, the Gradle wrapper was updated from version 8.4 to 8.9. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TradesScreen
participant TradesListItem
User->>TradesScreen: Opens Trades Screen
TradesScreen->>TradesScreen: Display new header and content container
TradesScreen->>TradesListItem: Render each trade item
TradesListItem->>TradesListItem: Determine role and status
TradesListItem->>TradesListItem: Display role/status chips and trade info
User-->>TradesScreen: Pull to refresh (optional)
TradesScreen->>TradesScreen: Reload trades list
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/trades/screens/trades_screen.dart (1)
63-64: Replace Spanish comment with English.For consistency, please use English for code comments.
- // Espacio superior + // Top spacing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/features/trades/screens/trades_screen.dart(1 hunks)lib/features/trades/widgets/trades_list_item.dart(1 hunks)
🔇 Additional comments (5)
lib/features/trades/screens/trades_screen.dart (1)
31-111: Clean UI restructuring aligns well with PR objectives.The simplified layout with a clear header and consolidated content area effectively removes the filter UI complexity while maintaining all essential functionality including error handling and refresh capability.
lib/features/trades/widgets/trades_list_item.dart (4)
47-63: Well-structured layout with clear visual hierarchy.The reorganization effectively places the trade type, status, and role information on the same row as requested in the PR objectives, creating a cleaner and more scannable interface.
76-76: Correct implementation of fiat amount display.Using
fiatAmount.minimumproperly addresses the PR objective to show the actual trade amount.
87-101: Clean implementation of payment methods display.The comma-separated list format for multiple payment methods and fallback to "Bank Transfer" provides a clear and concise presentation.
136-222: Excellent status chip styling with improved visual consistency.The updated color scheme with dark backgrounds and light text provides good contrast and visual hierarchy. The more rounded corners (borderRadius: 12) align well with the PR's styling objectives.
There was a problem hiding this comment.
Hi @AndreaDiazCorreia all orders have the label created by you even if they are not created by the user.
- In this screenshot, the 1st and last orders are taken by user, not created by user, it should have the label:
taken by you
- All payment methods that the order has must appear, not just the first one, but I suppose that when the pr Show all payment methods on order cards #115 is finished, that will be fixed, right?
There was a problem hiding this comment.
Please rebase against main
It's important that the statuses displayed on orders are not those published in events 38383 (specifically in-progress), but rather those sent to the user by mostrod in DMs. These would be the same status that were displayed before: pending, waiting-payment, waiting-buyer-invoice, active, fiat-sent, success, canceled.
|
If the order has a % premium or discount, it must also appear on the order card. |
| final role = session?.role; | ||
| final isBuying = role == Role.buyer; | ||
| // Determine if the user is the creator of the order based on available information | ||
| final isCreator = session != null && role != null; | ||
|
|
There was a problem hiding this comment.
In order to display the internal status as per:
#116 (review)
you will first need to retrieve the current OrderState:
| final orderState = ref.watch(orderNotifierProvider(trade.orderId!)); |
| ), | ||
| ), | ||
| const Spacer(), | ||
| _buildStatusChip(trade.status), |
There was a problem hiding this comment.
You can then use the OrderState.status which is the internal Mostro order status and not the one from the 38383 event
| _buildStatusChip(trade.status), | |
| _buildStatusChip(orderState.status), |
changes in main
|
@Catrya what do you think about this colors? |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/features/trades/widgets/trades_list_item.dart (2)
92-115: Duplicateddouble.parse(...)– parse once, reuse
trade.premiumis parsed three times inside this small block. Parsing once simplifies the code and avoids redundant work:- if (trade.premium != null && trade.premium != '0') + final premium = double.tryParse(trade.premium ?? ''); + if (premium != null && premium != 0) ... - color: - double.tryParse(trade.premium!) != null && - double.parse(trade.premium!) > 0 + color: (premium ?? 0) > 0 ? Colors.green.shade700 : Colors.red.shade700, ... - '${double.tryParse(trade.premium!) != null && double.parse(trade.premium!) > 0 ? '+' : ''}${trade.premium}%', + '${premium! > 0 ? '+' : ''}${premium.toStringAsFixed(2)}%',
170-238:_buildStatusChipis very verbose – consider a lookup map22 identical
switchbranches differ only by three values (background, text color, label). A simple map drives the same result with ⅓ of the code and far less maintenance overhead:static const _statusStyles = { Status.active: _ChipStyle('Active', Color(0xFF1E3A8A), Color(0xFF93C5FD)), Status.pending: _ChipStyle('Pending', Color(0xFF854D0E), Color(0xFFFCD34D)), Status.fiatSent: _ChipStyle('Fiat-sent', Color(0xFF065F46), Color(0xFF6EE7B7)), ... }; Widget _buildStatusChip(Status status) { final style = _statusStyles[status] ?? _ChipStyle(status.name, Colors.grey, Colors.white); return Container( padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 2), decoration: BoxDecoration( color: style.bg.withOpacity(0.3), borderRadius: BorderRadius.circular(12), ), child: Text( style.label, style: TextStyle(color: style.fg, fontSize: 12, fontWeight: FontWeight.w500), ), ); } class _ChipStyle { const _ChipStyle(this.label, this.bg, this.fg); final String label; final Color bg; final Color fg; }Benefits: easier to add new statuses, avoids future copy-paste errors, and reduces file size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
android/gradle/wrapper/gradle-wrapper.properties(1 hunks)lib/features/trades/screens/trades_screen.dart(2 hunks)lib/features/trades/widgets/trades_list_item.dart(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- android/gradle/wrapper/gradle-wrapper.properties
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/trades/screens/trades_screen.dart
| final session = ref.watch(sessionProvider(trade.orderId!)); | ||
| final role = session?.role; | ||
| final isBuying = role == Role.buyer; | ||
| final orderState = ref.watch(orderNotifierProvider(trade.orderId!)); | ||
|
|
There was a problem hiding this comment.
Null-safety guard for orderId is missing
trade.orderId! is force-unwrapped twice. If a corrupted/legacy event reaches this widget with a null orderId, the entire screen will crash.
- final session = ref.watch(sessionProvider(trade.orderId!));
- final role = session?.role;
- final isBuying = role == Role.buyer;
- final orderState = ref.watch(orderNotifierProvider(trade.orderId!));
+ final orderId = trade.orderId;
+ if (orderId == null) {
+ // Defensive – skip rendering instead of crashing
+ return const SizedBox.shrink();
+ }
+
+ final session = ref.watch(sessionProvider(orderId));
+ final role = session?.role;
+ final isBuying = role == Role.buyer;
+ final orderState = ref.watch(orderNotifierProvider(orderId));📝 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.
| final session = ref.watch(sessionProvider(trade.orderId!)); | |
| final role = session?.role; | |
| final isBuying = role == Role.buyer; | |
| final orderState = ref.watch(orderNotifierProvider(trade.orderId!)); | |
| final orderId = trade.orderId; | |
| if (orderId == null) { | |
| // Defensive – skip rendering instead of crashing | |
| return const SizedBox.shrink(); | |
| } | |
| final session = ref.watch(sessionProvider(orderId)); | |
| final role = session?.role; | |
| final isBuying = role == Role.buyer; | |
| final orderState = ref.watch(orderNotifierProvider(orderId)); |
🤖 Prompt for AI Agents
In lib/features/trades/widgets/trades_list_item.dart around lines 23 to 27, the
code force-unwraps trade.orderId twice without null-safety checks, risking a
crash if orderId is null. Add a null check before using trade.orderId, and
handle the null case gracefully, such as returning early or showing a fallback
UI, to prevent the screen from crashing when orderId is null.
| // Determine if the user is the creator of the order based on role and order type | ||
| final isCreator = isBuying | ||
| ? trade.orderType == OrderType.buy | ||
| : trade.orderType == OrderType.sell; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
isCreator still yields wrong results when either role or orderType is null
If role == null the ternary falls into the seller branch, which marks the order as “Created by you” whenever orderType == sell.
Guard against nulls and use a single, explicit comparison:
-final isCreator = isBuying
- ? trade.orderType == OrderType.buy
- : trade.orderType == OrderType.sell;
+final orderType = trade.orderType;
+final isCreator = role != null &&
+ orderType != null &&
+ ((role == Role.buyer && orderType == OrderType.buy) ||
+ (role == Role.seller && orderType == OrderType.sell));📝 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.
| // Determine if the user is the creator of the order based on role and order type | |
| final isCreator = isBuying | |
| ? trade.orderType == OrderType.buy | |
| : trade.orderType == OrderType.sell; | |
| // Determine if the user is the creator of the order based on role and order type | |
| final orderType = trade.orderType; | |
| final isCreator = role != null && | |
| orderType != null && | |
| ((role == Role.buyer && orderType == OrderType.buy) || | |
| (role == Role.seller && orderType == OrderType.sell)); |
🤖 Prompt for AI Agents
In lib/features/trades/widgets/trades_list_item.dart around lines 28 to 32, the
isCreator calculation does not handle null values for role or orderType, causing
incorrect results. Update the logic to explicitly check for nulls before
comparing role and orderType, and use a single clear comparison that returns
true only when the user's role matches the orderType, ensuring no false
positives when either value is null.
UI Refinements for My Trades Screen - Issue #86
This PR implements UI improvements to match the design mockups for the My Trades screen:
Changes
Notes
Summary by CodeRabbit
New Features
Refactor
Chores