feat(us12): phase 11 — My Trades screen, list item, providers - #61
Conversation
- Implement lib/features/trades/screens/trades_screen.dart: AppBar with menu + Mostro logo + notification bell, My Trades sub-header, status filter dropdown (PopupMenuButton with all TradeStatusFilter values), sorted list of TradesListItem, empty state with bolt icon, wired to /order_book route - Implement lib/features/trades/widgets/trades_list_item.dart: card with selling/buying title, colored status chip + role chip (creator/taker), bank icon + fiat amount + time-ago + payment method, chevron, tap → trade detail; _timeAgo helper; status→chip color mapping (Pending=yellow, Active/FiatSent=blue, Success=green, Canceled=gray, Dispute=red) - Implement lib/features/trades/providers/trades_providers.dart: TradeListItem model, TradeStatusFilter enum, TradeRole enum, selectedStatusFilterProvider (StateProvider), filteredTradesWithOrderState- Provider (sorted newest-first, empty until bridge wired), orderBookNotificationCountProvider (0 until bridge wired) - Wire /order_book route to TradesScreen in app_routes.dart (replaces stub) - Mark T082, T083, T084 [x] in tasks.md
… skew guard - Clarify TradesScreen doc comment: reference AppRoute.orderBook by name so the path is self-documenting rather than repeating the string literal - Guard _timeAgo against negative Duration from future timestamps (clock skew): treat any negative diff as "just now" so the output is never "-X minutes ago"
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds a new "My Trades" feature: a TradesScreen UI, TradesListItem widget, and Riverpod providers for trade filtering and notification count; wires TradesScreen into app routes and updates task checklist documentation. Changes
Sequence DiagramsequenceDiagram
participant User
participant TradesScreen
participant StatusFilterButton
participant SelectedFilterProvider
participant FilteredTradesProvider
participant ListView
User->>StatusFilterButton: Tap to open filter
StatusFilterButton->>User: Show popup menu
User->>StatusFilterButton: Choose filter value
StatusFilterButton->>SelectedFilterProvider: Update filter state
SelectedFilterProvider->>FilteredTradesProvider: Notify state change
FilteredTradesProvider->>TradesScreen: Emit filtered trades list
TradesScreen->>ListView: Rebuild with new list
ListView->>User: Display updated trades
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/features/trades/widgets/trades_list_item.dart (1)
146-156: Consider adding a comment for theallcase.
TradeStatusFilter.allis a filter option, not an actual trade status. While the fallback tostatusInactiveis reasonable, a brief comment would clarify this is a defensive case that shouldn't occur in practice.📝 Suggested clarification
TradeStatusFilter.canceled => AppColors.statusInactive, TradeStatusFilter.dispute => AppColors.statusDispute, + // `all` is a filter option, not a real status; fallback defensively. TradeStatusFilter.all => AppColors.statusInactive,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/trades/widgets/trades_list_item.dart` around lines 146 - 156, Add a short clarifying comment for the defensive `all` branch inside the _statusColors function: next to the TradeStatusFilter.all case, note that `all` is a filter (not a real trade status) and that returning AppColors.statusInactive is a fallback that "shouldn't occur in practice" to make intent explicit; update the comment near _statusColors/TradeStatusFilter.all so reviewers know this is defensive handling rather than a real mapped status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/features/trades/widgets/trades_list_item.dart`:
- Around line 146-156: Add a short clarifying comment for the defensive `all`
branch inside the _statusColors function: next to the TradeStatusFilter.all
case, note that `all` is a filter (not a real trade status) and that returning
AppColors.statusInactive is a fallback that "shouldn't occur in practice" to
make intent explicit; update the comment near
_statusColors/TradeStatusFilter.all so reviewers know this is defensive handling
rather than a real mapped status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf906df4-000c-4827-87c1-f2917967c039
📒 Files selected for processing (5)
lib/core/app_routes.dartlib/features/trades/providers/trades_providers.dartlib/features/trades/screens/trades_screen.dartlib/features/trades/widgets/trades_list_item.dartspecs/004-mostro-p2p-client/tasks.md
Mostro logo + notification bell, My Trades sub-header, status filter
dropdown (PopupMenuButton with all TradeStatusFilter values), sorted list
of TradesListItem, empty state with bolt icon, wired to /order_book route
selling/buying title, colored status chip + role chip (creator/taker),
bank icon + fiat amount + time-ago + payment method, chevron, tap → trade
detail; _timeAgo helper; status→chip color mapping (Pending=yellow,
Active/FiatSent=blue, Success=green, Canceled=gray, Dispute=red)
TradeListItem model, TradeStatusFilter enum, TradeRole enum,
selectedStatusFilterProvider (StateProvider), filteredTradesWithOrderState-
Provider (sorted newest-first, empty until bridge wired),
orderBookNotificationCountProvider (0 until bridge wired)
Summary by CodeRabbit