feat(orderbook): pixel-exact Order Book screen from Claude Design mock #3 - #243
Conversation
…#3 Port the 'Mostro UX Redesign' mock (screen #3 · Order book with reasons to pick) to the Flutter home screen, matching colors, spacing, and type scale exactly: - Add OrderBookPalette to app_theme.dart with the mock's exact dark tokens (bg #0F151C, card #1A2029, elevated #222A35, green #8FE04A, gold #FFC940, etc.) plus a legibility mapping for light mode (the mock is dark-only). Scoped to this screen; AppColors untouched. - Restyle HomeScreen: 52px app bar (hamburger + bell, empty center, 1px hairline), BUY/SELL tabs with green 13px/700 active label and 2px full-width underline, rounded FILTER pill with offer count and 'Sort: best premium' caption, 16/8/90 list padding with 12px gaps. Scaffold background themed so the bottom nav matches seamlessly. - Rebuild OrderListItem card per the mock: reason pill + timestamp row, 26px/700 amount with currency/flag and 13%-alpha premium pill, 'Market price' caption, elevated reputation strip (gold star, bold numbers, dot separators), payment-method line. The mock carries no buy/sell pill (tabs scope the side); only a 'yours' pill is kept on own orders. Skeleton and empty state re-tinted to the palette. - l10n: replace orderReputationStats with reputationTradesLabel / reputationDaysLabel and add sortBestPremium (en/es/fr/de/it). - Regenerate order-list-item goldens.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (16)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughThe order book screen now uses dedicated palette tokens, redesigned offer cards and layout spacing, simplified app-bar behavior, palette-based loading and empty states, contrast validation, and separate localized labels for reputation statistics and newest sorting. ChangesOrder book redesign
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HomeScreen
participant OrderBookPalette
participant orderBookProvider
participant OrderListItem
HomeScreen->>OrderBookPalette: select palette from BuildContext
HomeScreen->>orderBookProvider: read order book state
orderBookProvider-->>HomeScreen: loading, error, or orders
HomeScreen->>OrderListItem: render each order with palette and localization
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8adba752dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ), | ||
| const Spacer(), | ||
| Text( | ||
| l10n.sortBestPremium, |
There was a problem hiding this comment.
The new caption tells users that offers are sorted by best premium, but filteredOrdersProvider still sorts by createdAt descending (lib/features/home/providers/home_order_providers.dart:212), and this caption has no interaction that changes the ordering. Whenever premium order differs from publication order, users may select a worse offer under the false impression that the best premium is first; implement the advertised sort or label the ordering that is actually applied. The active contract also specifies expiration-time ordering in specs/004-mostro-p2p-client/spec.md:328, so any intentional contract change needs to be reconciled there.
AGENTS.md reference: AGENTS.md:L74-L75
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99222a9 — relabeled the caption to "Sort: newest first ▾" (all five locales), which is the createdAt-descending order filteredOrdersProvider actually applies. Implementing a premium sort would change the protocol-layer ordering contract (spec 004 §ordering) and is out of this PR's UI scope; the ARB description now states the caption must describe the ordering actually applied.
| size: 16, | ||
| color: colors?.textSecondary, | ||
| padding: const EdgeInsets.fromLTRB(16, 14, 16, 6), | ||
| child: Row( |
There was a problem hiding this comment.
This Row gives both the filter pill and sort caption their full intrinsic widths, with only a Spacer between them and no flexible or wrapping child. On a common 320px-wide device, longer supported translations such as German (FILTERN · N Angebote plus Sortierung: beste Prämie ▾) exceed the available 288px and produce a horizontal RenderFlex overflow/clipping; constrain, flex, or wrap these labels so the localized screen remains usable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99222a9 — the FILTER pill and the sort caption are now both Flexible with TextOverflow.ellipsis (the offer-count inside the pill too), so long de/fr strings degrade gracefully on 320px screens instead of overflowing the RenderFlex.
| final colors = Theme.of(context).extension<AppColors>(); | ||
| final cardBg = colors?.backgroundCard ?? const Color(0xFF1E2230); | ||
| final shimmer = colors?.backgroundInput ?? const Color(0xFF252A3A); | ||
| final pal = OrderBookPalette.of(context); |
There was a problem hiding this comment.
The palette-aware skeleton being updated here is dead code: a repo-wide search for OrderListItemSkeleton finds only this declaration, while the home loading branch still creates the shared OrderListSkeleton. That rendered widget retains the old hard-coded dark colors, 12px radius, and 100px blocks, so loading never shows this redesign and remains especially incorrect in light mode; use this skeleton from the loading list or update the shared widget that is actually instantiated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99222a9 — the rendered OrderListSkeleton (shared widget actually instantiated by the loading branch) now reads OrderBookPalette for shimmer base/highlight, uses radius 16, ~172px cards, and the redesigned 16/8/90 list padding with 12px gaps, correct in both themes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/home/widgets/order_list_item.dart (1)
89-115: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReason pill is unbounded while the "yours" pill is
Flexible.In row 1 the reason
_Pillhas no width constraint, so a long localized label (German/French are noticeably longer than English) on a narrow card can overflow theRowinstead of ellipsizing — the_PillTextOverflow.ellipsisonly helps once the pill itself is bounded. Wrapping it inFlexiblematches the treatment already applied tomineLabel.♻️ Proposed change
if (reasonLabel != null) ...[ - _Pill( - label: reasonLabel, - color: reasonColor!, - background: reasonBg!, - ), + Flexible( + child: _Pill( + label: reasonLabel, + color: reasonColor!, + background: reasonBg!, + ), + ), const SizedBox(width: 6), ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/home/widgets/order_list_item.dart` around lines 89 - 115, Wrap the reasonLabel _Pill in the first Row’s reasonLabel branch with Flexible, matching the existing mineLabel handling, so its width is constrained and its internal ellipsis can apply on narrow cards. Preserve the existing label, colors, spacing, and the rest of the Row layout.
🧹 Nitpick comments (2)
lib/features/home/widgets/order_list_item.dart (1)
369-378: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEmpty-state copy is low-contrast in light theme.
textTertiaryis#888888, giving roughly 3.5:1 on the lightbg(#FFFFFF) for 14px regular text — under AA. Since this is the only content on screen,textSecondarywould be safer here.#!/bin/bash python - <<'PY' def lin(c): c/=255 return c/12.92 if c<=0.03928 else ((c+0.055)/1.055)**2.4 def L(h): r,g,b=(int(h[i:i+2],16) for i in (0,2,4)) return 0.2126*lin(r)+0.7152*lin(g)+0.0722*lin(b) def ratio(a,b): la,lb=L(a),L(b) hi,lo=max(la,lb),min(la,lb) return (hi+0.05)/(lo+0.05) for fg in ("888888","666666","AAAAAA"): print(fg, round(ratio(fg,"FFFFFF"),2)) PY🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/home/widgets/order_list_item.dart` around lines 369 - 378, Update the empty-state styling in the visible order-list widget to use OrderBookPalette.textSecondary instead of textTertiary for the no-orders Icon and Text, preserving the existing layout, copy, and palette lookup.lib/features/home/screens/home_screen.dart (1)
222-223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse palette colors for the loading skeleton shimmer.
OrderListSkeletonstill uses0xFF1E2230/0xFF2A2D35andAppSpacingpadding, whileOrderListItemSkeletonnow readsOrderBookPalette. Make the loading state follow the redesigned palette so the skeleton matches the cards, especially in light theme.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/home/screens/home_screen.dart` around lines 222 - 223, Update the OrderListSkeleton loading state used by orderBookProvider to use OrderBookPalette colors for its shimmer and replace AppSpacing padding with the redesigned skeleton/card spacing, matching OrderListItemSkeleton across light and dark themes. Leave the surrounding loading/error/data handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/home/screens/home_screen.dart`:
- Around line 170-210: Replace the FILTER pill’s GestureDetector in
lib/features/home/screens/home_screen.dart lines 170-210 with an InkWell using
BorderRadius.circular(999), preserving showOrderFilterDialog(context) and the
existing pill content. In lib/features/home/widgets/order_list_item.dart lines
76-78, replace the card GestureDetector and HitTestBehavior.opaque with an
InkWell or Semantics(button: true), preserving card activation while making each
offer keyboard-accessible and announced as a button.
- Around line 151-152: Update the inactive tab label color in the tab
configuration near labelColor to use pal.textSecondary instead of
pal.textDisabled, preserving the active pal.green color and dark-theme behavior.
In `@lib/features/home/widgets/order_list_item.dart`:
- Around line 46-47: Update the numeric formatting in the order card, including
premiumText, _formatRating, and _StatText values, to use locale-aware intl
NumberFormat based on Localizations.localeOf(context) instead of toStringAsFixed
or raw interpolation. Preserve the existing signs, precision, and displayed
values while emitting localized decimal separators and digits for the active
locale.
- Around line 189-204: Update the _StatText usages for trade and day counts to
use localized ICU plural messages, such as the generated
reputationTradesLabel(count) and reputationDaysLabel(count), instead of
concatenating raw numbers with static labels. Add or update these pluralized
messages in the Dart ARB localization files for every supported locale,
preserving the existing localized output for each count.
In `@lib/l10n/app_localizations.dart`:
- Around line 3869-3885: Replace the static getters reputationTradesLabel and
reputationDaysLabel with ICU pluralized counted getters accepting an int count,
following the existing sibling counted-getter pattern. Regenerate the
localization files and update all order-card call sites to pass the
corresponding trade and active-day counts instead of concatenating raw unit
labels.
---
Outside diff comments:
In `@lib/features/home/widgets/order_list_item.dart`:
- Around line 89-115: Wrap the reasonLabel _Pill in the first Row’s reasonLabel
branch with Flexible, matching the existing mineLabel handling, so its width is
constrained and its internal ellipsis can apply on narrow cards. Preserve the
existing label, colors, spacing, and the rest of the Row layout.
---
Nitpick comments:
In `@lib/features/home/screens/home_screen.dart`:
- Around line 222-223: Update the OrderListSkeleton loading state used by
orderBookProvider to use OrderBookPalette colors for its shimmer and replace
AppSpacing padding with the redesigned skeleton/card spacing, matching
OrderListItemSkeleton across light and dark themes. Leave the surrounding
loading/error/data handling unchanged.
In `@lib/features/home/widgets/order_list_item.dart`:
- Around line 369-378: Update the empty-state styling in the visible order-list
widget to use OrderBookPalette.textSecondary instead of textTertiary for the
no-orders Icon and Text, preserving the existing layout, copy, and palette
lookup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c963df81-dae5-4356-bf38-303f5e073442
⛔ Files ignored due to path filters (2)
test/features/home/goldens/order_list_item_dark.pngis excluded by!**/*.pngtest/features/home/goldens/order_list_item_light.pngis excluded by!**/*.png
📒 Files selected for processing (14)
lib/core/app_theme.dartlib/features/home/screens/home_screen.dartlib/features/home/widgets/order_list_item.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dart
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Strict review — changes requested
I reviewed commit 8adba752dcb7f3adfc562f593480dde50005f258 after reading the full existing conversation and current thread state.
I did not duplicate prior inline comments. The existing unresolved threads about the advertised sort not matching the actual order, the narrow localized filter row overflowing, and the redesigned loading skeleton not being wired into the rendered loading state are still valid blockers on this head.
I found one additional blocking accessibility issue in the new palette; see the inline comment. The current golden tests do not enforce contrast or cover the full HomeScreen/responsive states.
CI is green for this exact head, but those runtime/UI issues are not exercised by the current suite.
| border: Color(0x14000000), | ||
| textPrimary: Color(0xFF1A1A1A), | ||
| textSecondary: Color(0xFF666666), | ||
| textTertiary: Color(0xFF888888), |
There was a problem hiding this comment.
Blocking: the new palette's “legibility mapping” does not provide legible text contrast on the surfaces where these tokens are rendered. This is broader than the existing inactive-tab/empty-state comments. Using WCAG relative luminance against the actual composited backgrounds:
textTertiaryis only 3.39:1 on the dark card and 3.25:1 on the light card, but it renders the 11px timestamp and “Market price” text; the sort caption is 3.80:1 dark / 3.54:1 light on the page.- The light active tab (
greenon white) is 3.23:1 at 13px/700. - Light green and amber premium pills are 2.61:1 and 2.66:1 after their 13% fills are composited over
bgCard; the light green/gold reason pills are about 2.56:1 / 2.57:1 on their 15% fills. The dark red premium pill is 3.70:1.
All of these are normal-sized text and need 4.5:1. Please choose role-specific colors that meet the threshold on each real rendered surface and add deterministic contrast tests so the light/dark legibility contract cannot regress.
There was a problem hiding this comment.
Fixed in 99222a9 — role-specific colors adjusted to meet 4.5:1 on the actual composited surfaces: dark textTertiary #6B7280→#848C9A (4.83:1 on the card, 5.42:1 on the page) and dark red #E5484D→#F27D81 (5.12:1 on its 13% fill); light green→#426800, gold→#7E5C09, amber→#845010, blue→#35638F, textTertiary→#696969, red→#AE3333, and the light active tab now uses the darkened green (5.84:1). Added test/core/order_book_palette_contrast_test.dart, which composites the translucent pill fills over their surfaces and asserts AA for every text-role/surface pair in both palettes, so the legibility contract cannot regress. Dark values not flagged (green/gold/blue pills, secondary text) were verified ≥5.7:1 and are also locked by the test.
…8n plurals/numbers Addresses all Codex, CodeRabbit, and ermeme review findings on #243: - Palette (ermeme, blocking): every text role now meets WCAG AA 4.5:1 on its real composited surface. Dark textTertiary #6B7280→#848C9A and red #E5484D→#F27D81 (13% pill fill was 3.7:1); light green/gold/amber/ blue/red/textTertiary darkened. textDisabled replaced by a tabInactive role — dark keeps the mock's dimmed tab, light uses AA-compliant gray. New deterministic test locks every pair (order_book_palette_contrast_test). - Sort caption (Codex): relabeled to 'Sort: newest first' — the caption now describes the createdAt-descending order the provider actually applies (spec 004 ordering contract unchanged). - Filter row (Codex): FILTER pill and sort caption are Flexible with ellipsis so long de/fr strings fit 320px screens without overflow. - Loading skeleton (Codex + CodeRabbit nit): OrderListSkeleton now uses OrderBookPalette shimmer colors, radius 16, and the redesigned list padding, in both themes. - A11y semantics (CodeRabbit): FILTER pill and offer cards use Material+InkWell (focusable, keyboard-activatable, button semantics) instead of raw GestureDetector. - Locale numbers (CodeRabbit): premium, rating, and counts formatted via intl NumberFormat with the active locale (decimal comma in es/fr/de/it). - Plural agreement (CodeRabbit): reputationTradesLabel/reputationDaysLabel are ICU plural messages taking the count ('1 trade' / '2 trades') in all five locales. - Reason pill wrapped in Flexible (CodeRabbit, outside diff) so long localized labels ellipsize instead of overflowing. - Empty state uses textSecondary (CodeRabbit nit). - Goldens regenerated.
|
Review round 1 addressed in 99222a9 — all 9 findings from Codex, CodeRabbit (incl. nitpicks and the outside-diff comment), and ermeme's blocking a11y review are fixed: WCAG AA palette adjustments locked by a new deterministic contrast test, honest sort caption, overflow-safe filter row, palette-aware loading skeleton, InkWell semantics, locale-aware number formatting, and ICU plurals. |
1 participant
Summary
Ports screen #3 · "Order book with reasons to pick" from the Claude Design project Mostro UX Redesign EN to the Flutter home screen, matching the mock's colors, proportions, and type scale exactly. Scope is strictly this screen — no other screens or shared-widget styling were touched.
What changed
OrderBookPalette(lib/core/app_theme.dart): the mock's exact dark tokens — bg#0F151C, card#1A2029, elevated#222A35, green#8FE04A, greenDim#2A4015, gold#FFC940/#3A2D0A, blue#7BB4F0/#1E2B42, amber#E89C3C, red#E5484D, text#F2F4F7/#A8B0BC/#6B7280/#4A5060, hairlinergba(255,255,255,0.06). The mock is dark-only, so light mode gets a legibility mapping onto the existing light surfaces.AppColorsand every other screen are untouched.HomeScreen: 52px app bar (hamburger + notification bell, empty center per the mock, 1px hairline divider); BUY/SELL tabs with 13px/700/1px-letter-spacing labels, active in green with a full-width 2px underline, inactive intextDisabledover a 1px rule; rounded (999) FILTER pill (card bg, 1px border, icon + FILTER +· N offers) with the "Sort: best premium ▾" caption right-aligned; list padding 16/8/90 with 12px card gaps. The scaffold background is theme-overridden locally so the bottom nav blends with#0F151Cseam-free.OrderListItemrebuilt to the mock's card: radius 16, padding 14/12/14/14; reason pill (3×8, radius 8, 11px/600) + relative timestamp; 26px/700 amount with 14px/600 currency, 18px flag and 13%-alpha premium pill; "Market price" caption; elevated reputation strip (radius 10, 12×10 padding, gold star 16px, bold rating,·separators, bold counts with secondary unit words); payment-method line. Skeleton and empty state re-tinted.orderReputationStats→reputationTradesLabel+reputationDaysLabel(allows the mock's bold-number/secondary-word styling), newsortBestPremium.order_list_itemgoldens (dark + light).Deliberate deviations from the mock (functional, minimal)
Test plan / Manual testing guide
flutter run -d linux(or android/chrome) and open the home screen (Order Book).#0F151C; cards#1A2029; reputation strip#222A35; active tab and best-premium pill green#8FE04A; star#FFC940.#4A5060); a 1px hairline runs under the whole row.FILTER · N offers, tap opens the existing filter dialog; right side shows the sort caption.★ 4.9 · 47 trades · 312 daysstrip with bold numbers; payment methods below.flutter analyze→ no issues;flutter test→ 144/144 passing.🤖 Generated with Claude Code
https://claude.ai/code/session_01VQUKX88xRBCPsHQTXBEiYa
Summary by CodeRabbit