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. The order state update logic was refined to derive status from actions via a new helper method. The trade detail screen's action buttons were improved with conditional labeling and unified rate button handling. Notifiers were updated to accept optional initial state and to sync order states by applying messages chronologically. Minor formatting improvements were made in trade providers. 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
sequenceDiagram
participant TradeDetailScreen
participant ActionButtons
TradeDetailScreen->>ActionButtons: Build action buttons based on status and action
alt Trade status is success and action in [purchaseCompleted, released, rate] without RATE button
ActionButtons->>TradeDetailScreen: Insert RATE button
else Trade status is settledHoldInvoice and action is rate
ActionButtons->>TradeDetailScreen: Show RATE button
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (4)
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 #115 is finished, that will be fixed, right?
Catrya
left a comment
Catrya
left a comment
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.
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
It must comply with the aesthetics of lovable, the colors must be darker.
Now:
In Lovable:
|
If the order has a % premium or discount, it must also appear on the order card. |
chebizarro
left a comment
chebizarro
left a comment
There was a problem hiding this comment.
Suggestions follow to address comments on order creator/owner and 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
There was a problem hiding this comment.
Hola @AndreaDiazCorreia está mucho mejor pero aun quedan algunas cosas por arreglar:
- Cuando el usuario crea una orden que es tomada por otro usuario, mientras aun es el turno del otro usuario y está en
waiting-paymentowaiting-buyer-invoicese sigue mostrando la etiqueta depending, pero debería mostrar el status correcto - Cuando la orden pasa a status
fiat-sentdebería mostrarse la etiquetafiat-sent, pero se sigue mostrandoactive - Cuando la orden es finalizada debería mostrar la etiqueta
successpero se sigue mostrandoactive
ahi lo reviso |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/order/models/order_state.dart (1)
105-149: Longswitchin_getStatusFromActionis brittle – extract a constant map
The 40-lineswitchis hard to scan, error-prone for future additions, and duplicates knowledge already encoded in enums. A static immutable map keeps the logic declarative and testable.- Status _getStatusFromAction(Action action, Status? payloadStatus) { - switch (action) { - ... - default: - return payloadStatus ?? status; - } - } + // One-stop lookup; falls back to current status when absent. + static const Map<Action, Status> _actionToStatus = { + Action.waitingSellerToPay: Status.waitingPayment, + Action.waitingBuyerInvoice: Status.waitingBuyerInvoice, + Action.addInvoice: Status.waitingBuyerInvoice, + Action.buyerTookOrder: Status.active, + Action.holdInvoicePaymentAccepted: Status.active, + Action.holdInvoicePaymentSettled: Status.active, + Action.fiatSent: Status.fiatSent, + Action.fiatSentOk: Status.fiatSent, + Action.purchaseCompleted: Status.success, + Action.released: Status.success, + Action.rateReceived: Status.success, + Action.canceled: Status.canceled, + Action.adminCanceled: Status.canceled, + Action.cooperativeCancelAccepted: Status.canceled, + }; + + Status _getStatusFromAction(Action action, Status? payloadStatus) => + _actionToStatus[action] ?? payloadStatus ?? status;This shrinks cognitive load and makes missing mappings stand out immediately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/order/models/order_state.dart(2 hunks)
🔇 Additional comments (1)
lib/features/order/models/order_state.dart (1)
152-154: Nice null-safety addition ingetActions
Using the cascade of?[]and?? []prevents accidentalNoSuchMethodErrorcrashes when a combination is unmapped.
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
-
Cuando es el turno del maker y está en
waiting-paymentowaiting-buyer-invoicetiene la etiquetapending, debería tener el status correcto -
Cuando la orden pasa a
fiatsent, al vendedor no le aparece el botón derelease
-
Cuando la orden pasa a
successdebe salir el botón derate, pero ahora solo sale el declose
-
No esta saliendo el botón de cancel en esta pantalla pero debe salir, es cuando el taker hace su parte y espera por el maker, pero debe tener la opción de cancelar si quiere
-
Cuando el usuario toma una orden de compra, que está en status
waiting-paymentdicepending
…screen and order state
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/features/trades/widgets/trades_list_item.dart (1)
23-31: Null-safety / creator-logic issues persist
Past review already covered the crash risk fromtrade.orderId!and the inaccurateisCreatorderivation whenroleisnull. These problems are still present in the current diff.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/features/order/models/order_state.dart(6 hunks)lib/features/trades/screens/trade_detail_screen.dart(2 hunks)lib/features/trades/widgets/trades_list_item.dart(2 hunks)
refactor: expand order state transitions to advance to the next status
Adjust visible buttons
|
@Catrya ayer subí 2 commits que resuelvan el tema de las etiquetas, podés revisarlos y confirmar que habian solucionado el problema? |
Buenísimo, lo voy a testear |
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
Hola @AndreaDiazCorreia @chebizarro
-
Cuando el maker de una orden de venta recibe el QR para pagarlo, si sale de esa pantalla sin pagar todavia, la etiqueta que sale es
Pending, cuando abre la orden, salepending - pay-invoicey no tiene la opcion de ver el QR, solo el boton deClose
-
Al cerrar la app y volver a abrirla, los status cambian, todas las ordenes que estan en la imagen, estaban
successantes de cerrar la app
-
Cuando una orden creada por el usuario pasa a
waiting-paymentsale comopendingno muestra el QR y ademas sale el boton deDisputeque no debe ir ahi
-
Cuando el taker de una ordn de venta cumple su parte y pasa a
waiting-seller to payy tiene que esperar por la contraparte, ahora salen los botones deDisputayCooperative cancel, que ninguno tiene que salir en ese paso
-
Cuando el taker de una orden de venta (el buyer) selecciona
fiatsent, sale un boton rojo para cancelar que no esta bien pues no diceCancel, sino que tiene un simbolo de check
-
Cuando el usuario toma una orden de compra, que está en status waiting-payment dice pending (Se mantiene de la review anterior)
- Ademas cuando vuelve a entrar entonces no sale el boton de pagar la invoice
- Ademas cuando vuelve a entrar entonces no sale el boton de pagar la invoice
-
Cuando el taker seller paga la invoice, pasa a
waiting buyer invoicepero sale el boton de disputa que no debe salir ahi
|
Hi @AndreaDiazCorreia @chebizarro please let's finish this PR ASAP 🙏🏽 |
|
@grunch Im working on that |
|
@Catrya Prueba de nuevo, los botones estan de acuerdo a la documentacion de mostro (los de cancel y dispute), ahora si cierras la app actualiza bien el estado.
Ya ahi no esta el boton de dispute pero va a seguir saliendo como estado pending -new -order porque por alguna razon es el estado que esta recibiendo la app aun cuando mostro manda el correcto, eso lo puedes abrir como otro issue porque en mostro si cambia el status asi que el flujo se mantiene, sobre el qr, si sale el qr cuando la persona que toma la orden agrega la invoice que es el flujo correcto pero igual prueba. |
|
please @AndreaDiazCorreia mark as solved those rabbit requests if you solved it, if they are not necessaries comment them telling that change is not necessary explaining the reason |
@grunch all the suggestions was solved |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
4 participants
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