Markets: readable hero on a phone, timeframe tabs that actually fetch, and a table that yields to cards when it doesn't fit - #226
Merged
Conversation
braianxde
marked this pull request as ready for review
August 7, 2026 19:30
braianxde
added a commit
that referenced
this pull request
Aug 7, 2026
EduMenges
approved these changes
Aug 7, 2026
…e narrow chart its axes At phone width the GENIUS AI hero price no longer runs the full card width -- it now renders at the numericDisplay type token (32px) instead of a hardcoded 48px, and the coin name steps down to titleMd. The wide card keeps its 48px price and titleLg name exactly as measured (367.0 / 301.0 unchanged). The narrow hero chart now earns its trading frame -- money labels down the right edge and a date row along the bottom, the same as the wide chart already has -- by growing its box to kChartFrameMinHeight rather than lowering the frame's global threshold. Layout branch and type step now read one shared "wide" bool (box width AND desktop-layout window) so they can no longer disagree.
…e-plot Tapping 24H, 30D or 1Y on the Markets hero now fetches a real series for that range and re-plots it; tapping 7D still spends no network call and plots the already-bundled sparkline. A fetch that comes back empty (the only failure signal fetchHistoricalPrices ever surfaces) shows a retry affordance in the chart box instead of a silent blank chart. The bottom axis picks clock time, day-and-month or month-and-year from the real plotted window instead of always printing dates. fetchHistoricalPrices gained an optional days parameter with a per-range Hive cache key, so a 30D/1Y Markets fetch cannot collide with the dashboard chart's 1D cache entry. GWTimeframeSegment gained optional labels/onChanged parameters (both default to the existing behaviour, so the dashboard and coin-page call sites are untouched), and the Markets hero's own private timeframe segment is deleted in favour of it -- one shared component instead of two, closing the long-standing unify todo.
The All Markets table is replaced by a card per coin -- icon, name, symbol, price, 24h change, market cap and 24h volume, with no chart on any card. One card per row at phone width, two per row on desktop, ordered rank ascending (the table's own default order, via the existing compareMarketRows rather than a hardcoded comparison). Tap-to-sort by price/market cap/volume/name is gone with the table headers -- markets_sort.dart and its comparator stay live so a sort control can be added back above the list without rework. The 1h % and 7d % columns, which only ever rendered a placeholder "-", are dropped by the card design rather than carried forward. markets_table.dart is renamed to markets_cards.dart; MarketRow and its Flutter-free sort projection are unchanged.
…s only when it doesn't fit Corrects the same-day 280f9b4 change, which read "cards" as a full-width replacement for the table. Braian's actual ask was cards only when the table genuinely does not fit: All Markets is the table -- restored verbatim from 280f9b4^, sort-on-header-tap included -- whenever the box handed to it clears kMarketsTableMinWidth (the sum of the table's own column widths), and the card grid only below that. The gate is the table's own measured fit, not useDesktopLayout or any other device-class check, so a narrow desktop window correctly falls back to cards and a wide mobile-app window does not falsely get the table. Per a second, later instruction from Braian, the restored table drops its 1h %/7d % columns rather than bringing them back -- both always rendered a placeholder "-", and that range filter belongs on the hero graph's timeframe tabs (which now genuinely fetch), not a table column. kMarketsTableMinWidth is recomputed from the surviving seven columns (846 vs the original nine-column 1054), so the table falls back to cards only when it actually stops fitting, not one column early. MarketRow moves back to markets_table.dart, its original home, as the single shared definition both renderings import. MarketsCards is replaced by MarketsAllSection (the fit-gate) wrapping a private _MarketsCardGrid (today's card design, unchanged) -- one call site in markets_screen.dart, reading its own box width, which is correct here because it is the one page-width surface for All Markets.
The Rank/Market Cap/Volume 24h/All-Time High stat block was two hardcoded Rows of two Expanded(GWStatTile) -- always two columns regardless of available width. It is now a Wrap of fixed-width tiles (kMarketsHeroStatTileWidth, 152 -- the measured threshold below which "All-Time High" wraps its own GWKicker label to two lines, plus 1px margin), so as many tiles fit per row as the card's actual width allows: two at 402px (unchanged from before), three at the file's usual 1400px wide surface, four above roughly 1700px. Wrap, not LayoutBuilder -- the stat block sits inside an IntrinsicHeight Row on the wide branch, and LayoutBuilder throws on intrinsic measurement. The pinned 367.0/301.0 measurements were re-measured, not assumed, and came back numerically unchanged -- but the RIGHT column (timeframe segment + fixed-height chart) is now the one driving them unconditionally, not the left column's old fixed 2x2 grid. The left column's own height now varies between 242 and 301 depending on the stat block's row count and can no longer exceed the right's fixed 301.0. The test's reason strings and the file's doc comments are rewritten to record that derivation rather than just re-asserting the same numbers. The wide-layout chart-alignment comment (previously "lines up with the Volume 24h / All-Time High stat row") is rewritten: the Spacer mechanism it describes never cared what specifically sat at the bottom of the left column, only that something did, so the comment now describes the actual invariant instead of a row that no longer exists.
… padding
The hero card's stat tiles were a uniform SizedBox width sized to the
widest LABEL ("All-Time High"), so every tile carried dead space next
to its much narrower VALUE ("$45.26") -- four of those read as one
loose block. Dropped the fixed width entirely: GWStatTile now sizes to
its own content inside the Wrap, and kMarketsHeroStatTileWidth is
deleted rather than tuned. spacing tightens from space10 to space6,
reusing the same tight horizontal gap this card already uses for
icon-to-name and pill-to-text; runSpacing stays at space10 so wrapped
rows still read as separate rows.
Direct cost, stated rather than hidden: tiles no longer align into a
column grid -- values sit at ragged x-positions per their own content,
so the row reads as a chip line, not a table. That trade is Braian's
to keep or revert.
Knock-on, confirmed by direct measurement rather than assumed: without
the fixed-width floor, four tiles now share one row starting around
1217px instead of ~1700px, so the file's usual 1400px surface shows
all four abreast where it showed three before -- at 402px, 3-then-1
across two rows, same as before this change.
Also halves the card's own outer padding (space16 to space8, a
separate request) -- an existing 4-pt-scale token, not a new number.
Two other insets were swept and deliberately left alone: the change
pill's own padding (a component's shape, not the card's frame) and the
chart's axis gutter (load-bearing for the narrow-width axis labels
this same task added).
The pinned wide-card height moves to 335.0 (32px of padding removed);
IntrinsicHeight stays at 301.0, confirmed unaffected -- the padding
sits outside that row. Test literals and reason strings updated to
match, and the "all four share a row" geometric test now pins 1400px
directly instead of a wider surface, since content-sizing makes that
true at the width that matters.
The tiles lost their uniform-width slack when they started sizing to their own content, and 12px between ragged neighbours read as cramped. 20px is the existing space10 token, so the 4-pt scale is intact, and the four tiles still fit on one row at 1400.
The card grid's column count was a binary useDesktopLayout(context) ?
2 : 1, so cards divided whatever width was left over -- card width was
a leftover, not a decision, and read too wide for what a card holds.
Inverted: MarketsAllSection's LayoutBuilder now passes its own
c.maxWidth straight into _MarketsCardGrid (no second LayoutBuilder --
this file is not the IntrinsicHeight trap the hero card's left column
is), and the grid derives its column count from a fixed max card width
instead: columns = max(1, (available + gap) ~/ (maxCardWidth + gap)).
Continuous, no device-class check anywhere in it, consistent with
MarketsAllSection's own table-vs-cards gate.
kMarketsCardMaxWidth (287) is measured, not the 256 originally
guessed: two independent constraints converge at the same width. The
two GWStatTiles' labels ("Market Cap"/"Volume 24h") wrap to two lines
below 116px of per-tile width; separately, the price/change-pill
column is a fixed, non-shrinking 142.5px at the fixture's price, which
crushes the coin NAME column toward zero width well before that --
15.5px at a 256px card, effectively invisible. 287 is the measured
286px no-wrap threshold plus 1px of margin.
This conflicts with the stated target of 3 columns near
kMarketsTableMinWidth (846): 3 columns at 287px would need roughly
909px, past the table's own 846px fit-gate, so a 3rd column is
structurally unreachable by the card grid at any width -- reported
rather than shipped as a crowded card. Measured column counts: 402px
-> 1, 600px -> 2, 840px (just under the table threshold) -> 2, not 3.
Cards keep their width rather than stretching into leftover row space
-- SizedBox(width: kMarketsCardMaxWidth), not Expanded -- left-aligned
with the remainder empty. Found and fixed one real bug building this:
Row.spacing inserts a gap before every child including the trailing
Spacer, which the column-count budget never accounted for and caused
a genuine 22px overflow at 600px width (caught by the widget test,
never shipped) -- gaps are now inserted by hand between cards only.
kMarketsCardMaxWidth now only decides how many columns fit; the cards themselves expand to share the row evenly. Pinning each card at 287 left the division's remainder as dead space on the right, which read as a ragged edge against the full-width table one breakpoint up. A short final row pads to the full column count with empty flex slots, so its cards keep the same width as the rows above instead of one last card stretching across.
…aims Closed against the tree, not against the 2026-08-05 triage: the two control-track accessibility items (fixed by defe3b4), the dead TokenDetailHero, phase 19's redesign, and the dev fault injector. Two could not close. The five-private-segmented-controls count is three now, not five. And the chart raw-colours item stays open: the triage called it resolved on 'zero remain in lib/chart', but three Colors.transparent sentinels are still there.
braianxde
force-pushed
the
feat/markets-page-cards
branch
from
August 7, 2026 21:45
9f47225 to
3a0b253
Compare
EduMenges
pushed a commit
that referenced
this pull request
Aug 10, 2026
The old guidance said "lead with what a user hits", which is right and did not work. #226 still opened with a hardcoded fontSize, a 180px box and chartUsesFrame() needing 220 — true, and useless to anyone deciding whether to open the diff. So the skill now shows rather than tells: that exact paragraph sits next to the version a reviewer needed ("on a phone the price ran the whole width of the card, and the chart under it showed no prices and no dates"). Same change, one of them actionable. Adds a shape and a budget — a screenful, past which you have started explaining the implementation — and a list of what stays out of the summary: pixel values, function and class names, file paths, widget internals. A number earns a mention only when someone will argue about it. Everything else goes in the commit message, where whoever wants it is already reading. Also reframes "deliberately not here" as product writing, and asks for what is unverified to be said out loud: a reviewer can accept a gap they can see and cannot forgive one they find themselves. Opening switches to /gsd-ship --draft, now that git.base_branch is pinned to develop. It generates its body from PLAN and SUMMARY, which is the machine summary this commit is trying to get away from, so the skill says to keep the verification numbers and rewrite the rest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four Markets defects found by walking the running app.
Fixed
fontSize: 48— ran the full card width on a phone. Now a type token on the narrow branch; desktop unchanged.chartUsesFrame()needs 220, so it silently used the axis-free variant. Height is now derived from that threshold.fetchHistoricalPriceshaddayshardcoded to 1. Now 24H/7D/30D/1Y fetch for real, with the day count in the cache key so Markets can't overwrite the dashboard chart's series. 7D reuses the bundled sparkline and spends no request.1h %and7d %rendered-on every row. Removed.Below the width where the table's columns fit, All Markets renders as cards instead. The gate is a computed sum of the table's column widths, not a device breakpoint, so it tracks the columns automatically.
Also: the hero stat block flows instead of being a fixed 2×2; card padding halved; the hero's private timeframe segment deleted in favour of the shared
GWTimeframeSegment, which also gives that surface the keyboard and AA-contrast fixes from #221.Verification
flutter analyzeexit 0 ·flutter test1036/1036 (develop: 1019) · brace, raw-colour, seed-safety and key-logging gates exit 0.check_agent_rules_sync.shfails — pre-existing, identical on develop at545d0a3, and neitherAGENTS.mdnor.github/copilot-instructions.mdis in this diff. Left for its own commit.Deliberately not here
markets_sort.dartstays live for a future control.CryptoLiveChart's own timeframe wiring is untouched.