diff --git a/docs/feedback/FB-09-hud-title-driven-sizing.md b/docs/feedback/FB-09-hud-title-driven-sizing.md index 646ea2e..131e289 100644 --- a/docs/feedback/FB-09-hud-title-driven-sizing.md +++ b/docs/feedback/FB-09-hud-title-driven-sizing.md @@ -1,6 +1,6 @@ # FB-09 — HUD default sizing follows the title, not a fixed uniform cell -**Depends on** FB-08 (same files, sequence after it merges) · **Size** L · **Status** Not started +**Depends on** FB-08 (same files, sequence after it merges) · **Size** L · **Status** Done ## Goal A HUD widget's title must always fit at a fixed, readable size. A HUD widget's value @@ -258,3 +258,71 @@ merged, and re-run FB-08's own tests to confirm the two features compose correct `colSpan` — if `_reflowRow`'s even split would push a widget below its own minimum, that is a real interaction between the two tickets worth checking explicitly and documenting in this ticket's Outcome section). + +## Outcome +Implemented exactly as designed, with no changes to the Design section's approach: + +- `minColSpanForLabel(String label)` added to `hud_widget_layout.dart`, verbatim. +- `HudWidgetLayout.defaultFor` reworked into the left-to-right row-packing factory + from the Design section, verbatim. +- `draggable_resizable_hud_widget.dart`'s resize handle `onPanEnd` now clamps + `snappedColSpan` up to `minColSpanForLabel(layout.metric.label)` (floor) and down to + `hudMaxColSpan` (ceiling), instead of the generic `hudMinSpan` floor the grid-level + clamp (`clampedToGrid`, still called downstream in `HudLayoutController`) applies to + every other span. +- `_HudMetricValue.build()` in `record_screen.dart` now renders the title as a plain, + unwrapped `Text` (with `maxLines: 1`/`TextOverflow.ellipsis` restored as a safety + net) at its fixed reference size, and wraps only the value `Text` in + `Flexible(child: FittedBox(fit: BoxFit.scaleDown, ...))`. + +**Risk #1 (four even columns no longer even) — confirmed, as predicted, not a +regression.** With real label lengths, the default row-0 packing is now: Speed (span +1), Average speed (span 2), Distance (span 1) filling all 4 columns — Elapsed time (a +fourth default-visible metric, still marked `visible: true` by the unchanged +`index < 4` rule) wraps to row 2 alongside Max speed, rather than sharing row 0 with +the other three. This is the ticket's own predicted, intended outcome of sizing by +title rather than by a fixed 4-per-row assumption, not a bug. (Note: the worked +example table in the Design section above lists slightly different character counts +for "Average speed"/"Elevation gain"/"Points captured" than their actual `String` +lengths in `hud_metric.dart` — the real lengths are 13/14/15, one less than the +table's 14/15/16 — which changes "Elevation gain" from the table's predicted span-3 to +an actual span-2. This doesn't affect correctness of the implemented function, which +matches the Design section's code verbatim; it only means the packing result differs +slightly from the table's worked example. No grid-bounds issue results: the last +default row (`pointsCaptured` alone) lands at `row: 6`, `rowSpan: 2`, exactly at +`hudGridRows`'s (8) boundary.) + +**Risk #2 / FB-08 interaction (named in Out of scope) — confirmed real, left +unfixed per the ticket's own scope.** Checked directly: `_reflowRow`'s even split +divides `hudGridColumns` (4) across however many visible members now share a row, +with no awareness of any member's `minColSpanForLabel`. Simulating every membership +count `_reflowRow` ever handles (2, 3, or 4 -- it no-ops below 2) against every real +`HudMetric` label shows the even split pushes a member below its own title-driven +minimum in *every* case where that member's label needs more than 1 column: +- 2 members -> both get span 2 -- too small for "Points captured" (needs 3). +- 3 members -> spans of 2/1/1 -- too small for any member needing 2 (e.g. "Average + speed", "Elapsed time", "Moving time", "Elevation gain") unless it happens to be the + one member that lands on span 2, and always too small for "Points captured". +- 4 members -> all get span 1 -- too small for any label longer than "Speed", + "Distance", or "Max speed". +This is a real, reproducible gap between FB-08's `_reflowRow` and this ticket's +`minColSpanForLabel`, but per this ticket's own "Out of scope" section it is +deliberately left unfixed here — `_reflowRow` is FB-08's function, and changing its +even-split algorithm to respect a per-label minimum (e.g. giving longer-labeled +members first claim on any spare columns, falling back to letting a row simply not +fill exactly 4 columns when it can't be split evenly and legibly) is a follow-up +ticket's work, not this one's. + +Test suite: `flutter analyze` stays at the same 4 pre-existing, unrelated info-level +issues as the pre-FB-09 baseline (none in files this ticket touches). `flutter test` +went from 426 to **434** tests, all green (8 net new: 5 in +`hud_widget_layout_test.dart` for `minColSpanForLabel`/`defaultFor`'s new invariants, +3 in `hud_edit_overlay_test.dart` for the resize-floor clamp, the longest-label +minimum-width no-overflow case, and the fixed-title/flexible-value split). Two +pre-existing `hud_layout_controller_test.dart` tests (FB-08's own reflow tests) had to +be updated, not because `_reflowRow` broke, but because they asserted on *which +specific row* certain metrics defaulted into -- an assumption that no longer holds now +that `defaultFor` packs by title width instead of a fixed 4-per-row index. Both were +rewritten to explicitly position their metrics into a shared row via +`updatePosition`/`updateSize` before exercising `_reflowRow`, so they test the reflow +behavior itself rather than an incidental default-layout coincidence. diff --git a/lib/src/hud/hud_widget_layout.dart b/lib/src/hud/hud_widget_layout.dart index 6bd33a8..76aae8e 100644 --- a/lib/src/hud/hud_widget_layout.dart +++ b/lib/src/hud/hud_widget_layout.dart @@ -20,6 +20,17 @@ const int hudMaxRowSpan = 3; /// True if rectangles [a] and [b] (both in grid-cell coordinates) overlap -- edges that /// merely touch do not count as overlapping. +/// A label under 10 characters fits one column. A label under 15 characters needs +/// two. Anything longer needs three. This is a character-count proxy for "how much +/// horizontal room this title needs," not an exact pixel measurement -- `defaultFor` +/// runs with no `BuildContext` and cannot measure real text width. Adjust these +/// thresholds directly if a future label reads too cramped or too loose in practice. +int minColSpanForLabel(String label) { + if (label.length < 10) return 1; + if (label.length < 15) return 2; + return 3; +} + bool hudRectsOverlap(HudWidgetLayout a, HudWidgetLayout b) { final aColEnd = a.col + a.colSpan; final aRowEnd = a.row + a.rowSpan; @@ -113,26 +124,41 @@ class HudWidgetLayout { /// A deterministic starting grid -- a fresh install has a working, if plain, HUD /// before the rider customises anything, and a metric toggled on for the first time /// (with no saved position) lands somewhere sane rather than stacked on another - /// widget. Two rows of four, matching the Stitch mockup's row of cards for however - /// many metrics fit in the first row, with the rest continuing below it. Purely a - /// function of the metric's own index -- these 8 fixed slots never overlap each - /// other by construction, so no runtime state is needed here. + /// widget. FB-09: rather than a fixed "always 4 per row, colSpan 1" assumption, this + /// packs metrics left-to-right with each one's own title-driven width + /// ([minColSpanForLabel]), wrapping to a new row when the current one would + /// overflow. Purely a function of the metric's own index/label -- these slots never + /// overlap each other by construction, so no runtime state is needed here. factory HudWidgetLayout.defaultFor(HudMetric metric) { - const columns = 4; const rowSpan = 2; - final index = HudMetric.values.indexOf(metric); - final row = (index ~/ columns) * rowSpan; - final col = index % columns; - return HudWidgetLayout( - metric: metric, - col: col, - row: row, - colSpan: 1, - rowSpan: rowSpan, - // The Map HUD mockup's own fixed row is Speed/Avg Speed/Dist/Time -- the first - // four enum values are ordered to match, so only those start visible. - visible: index < columns, - ); + var col = 0; + var row = 0; + for (final m in HudMetric.values) { + final span = minColSpanForLabel(m.label); + if (col + span > hudGridColumns) { + col = 0; + row += rowSpan; + } + if (m == metric) { + return HudWidgetLayout( + metric: metric, + col: col, + row: row, + colSpan: span, + rowSpan: rowSpan, + // The Map HUD mockup's own fixed row is Speed/Avg Speed/Dist/Time -- the + // first four enum values are ordered to match, so only those start visible. + // Their exact column positions may no longer form four perfectly even + // columns once their individual widths differ -- that is the correct, + // intended result of sizing by title, not a bug (see FB-09's Outcome + // section). + visible: HudMetric.values.indexOf(metric) < 4, + ); + } + col += span; + } + // Unreachable: metric is always one of HudMetric.values. + throw StateError('Unknown metric: $metric'); } /// Scans row-major from (0,0) for the first [colSpan]x[rowSpan] slot that doesn't diff --git a/lib/src/ui/components/draggable_resizable_hud_widget.dart b/lib/src/ui/components/draggable_resizable_hud_widget.dart index 64e99af..6f5d1fa 100644 --- a/lib/src/ui/components/draggable_resizable_hud_widget.dart +++ b/lib/src/ui/components/draggable_resizable_hud_widget.dart @@ -146,9 +146,18 @@ class _DraggableResizableHudWidgetState extends State const FittedBox( - fit: BoxFit.scaleDown, - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Text('SPEED', style: TextStyle(fontSize: 10, letterSpacing: 1)), - SizedBox(height: 4), - Text('12.3 km/h', style: TextStyle(fontSize: 18, fontWeight: FontWeight.bold)), - ], - ), -); +/// referenced directly from outside its library. FB-09: the title is a plain, +/// unwrapped `Text` at a fixed reference size; only the value is wrapped in its own +/// `Flexible(child: FittedBox(...))`. +Widget _representativeHudChild({String label = 'SPEED', String value = '12.3 km/h'}) => + Column( + mainAxisSize: MainAxisSize.min, + children: [ + Text( + label, + style: const TextStyle(fontSize: 10, letterSpacing: 1), + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + const SizedBox(height: 4), + Flexible( + child: FittedBox( + fit: BoxFit.scaleDown, + child: Text( + value, + style: const TextStyle(fontSize: 18, fontWeight: FontWeight.bold), + ), + ), + ), + ], + ); /// UI-04: drag/toggle behaviour of the customizable HUD, simulated via `TestGesture` /// the same way other drag interactions in this codebase are tested (see @@ -263,4 +276,143 @@ void main() { expect(tester.takeException(), isNull); }); + + testWidgets( + 'resizing a widget below its title\'s minimum colSpan via the resize handle ' + 'clamps to that minimum, not the generic hudMinSpan', (tester) async { + // Points captured is the longest label -- minColSpanForLabel gives it 3, well + // above the generic hudMinSpan of 1. + const metric = HudMetric.pointsCaptured; + final expectedMin = minColSpanForLabel(metric.label); + expect(expectedMin, greaterThan(hudMinSpan)); + + int? resizedColSpan; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 400, + height: 400, + child: Stack( + children: [ + DraggableResizableHudWidget( + layout: HudWidgetLayout( + metric: metric, + col: 0, + row: 0, + colSpan: expectedMin, + rowSpan: 2, + visible: true, + ), + areaSize: const Size(400, 400), + editing: true, + onMoved: (_, _) {}, + onResized: (colSpan, _) => resizedColSpan = colSpan, + child: _representativeHudChild(label: metric.label.toUpperCase()), + ), + ], + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + final handleFinder = find.byKey(const Key('hud-resize-handle')); + final gesture = await tester.startGesture(tester.getCenter(handleFinder)); + // Drag far enough left/up to try to shrink well below every floor. + await gesture.moveBy(const Offset(-390, -390)); + await tester.pump(); + await gesture.up(); + await tester.pumpAndSettle(); + + expect(resizedColSpan, expectedMin, + reason: 'the resize handle must clamp up to the title\'s own minimum, not ' + 'the generic hudMinSpan'); + }); + + testWidgets( + 'the longest label renders in full, without overflow, at exactly its own ' + 'title-driven minimum colSpan', (tester) async { + const metric = HudMetric.pointsCaptured; + final minSpan = minColSpanForLabel(metric.label); + final cellWidth = 400 / hudGridColumns; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 400, + height: 400, + child: Stack( + children: [ + DraggableResizableHudWidget( + layout: HudWidgetLayout( + metric: metric, + col: 0, + row: 0, + colSpan: minSpan, + rowSpan: 2, + visible: true, + ), + areaSize: const Size(400, 400), + editing: false, + onMoved: (_, _) {}, + onResized: (_, _) {}, + child: _representativeHudChild(label: metric.label.toUpperCase()), + ), + ], + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(find.text('POINTS CAPTURED'), findsOneWidget, + reason: 'the full title must be present, not an ellipsized fragment, at its ' + 'own minimum width ($minSpan cols * $cellWidth px each)'); + }); + + testWidgets( + 'the title\'s rendered font size never grows past its fixed reference size, ' + 'while the value is free to size differently', (tester) async { + // Pumped directly inside a tightly-sized SizedBox (rather than through + // DraggableResizableHudWidget's GlassPanel/Center/AnimatedScale chain, which + // hands the content loose rather than tight constraints) so the Column's + // Flexible-wrapped value is actually forced to size against the box, exercising + // the fixed-title/flexible-value split this ticket adds. + Widget host(Size size) => MaterialApp( + home: Scaffold( + body: SizedBox( + width: size.width, + height: size.height, + child: _representativeHudChild(), + ), + ), + ); + + // FittedBox scales its child at paint time via a transform, not by resizing the + // child's own layout box -- so the inner value Text's RenderBox size is always + // its unscaled natural size, regardless of how much the FittedBox actually + // shrank it visually. The FittedBox's own rendered size is what reflects the + // available space, so that -- not the Text inside it -- is what's compared here. + await tester.pumpWidget(host(const Size(60, 40))); + await tester.pumpAndSettle(); + final smallTitleFontSize = tester.widget(find.text('SPEED')).style?.fontSize; + final smallValueBoxSize = tester.getSize(find.byType(FittedBox)); + + await tester.pumpWidget(host(const Size(600, 400))); + await tester.pumpAndSettle(); + final bigTitleFontSize = tester.widget(find.text('SPEED')).style?.fontSize; + final bigValueBoxSize = tester.getSize(find.byType(FittedBox)); + + expect(bigTitleFontSize, smallTitleFontSize, + reason: 'the title is a fixed-size Text, not wrapped in a FittedBox, so a ' + 'much larger widget must not change its style\'s font size'); + expect(bigValueBoxSize, isNot(smallValueBoxSize), + reason: 'the value is the flexible part and its FittedBox is free to be ' + 'sized differently as the widget grows'); + }); } diff --git a/test/hud_layout_controller_test.dart b/test/hud_layout_controller_test.dart index afb7bd8..ef36917 100644 --- a/test/hud_layout_controller_test.dart +++ b/test/hud_layout_controller_test.dart @@ -114,6 +114,22 @@ void main() { 'the row with no gap', () async { final controller = HudLayoutController(await freshConfig()); + // FB-09's title-driven defaultFor no longer puts all four default-visible + // metrics in row 0 with uniform colSpan 1 (avgSpeed and elapsedTime need 2 + // columns for their longer titles). Force that uniform row-0 arrangement + // explicitly here so this test exercises _reflowRow's own algorithm in + // isolation, independent of exactly where defaultFor happens to place things. + controller.updateSize(HudMetric.elapsedTime, 1, 2); + controller.updateSize(HudMetric.avgSpeed, 1, 2); + controller.updatePosition(HudMetric.elapsedTime, 2, 0); + expect( + [HudMetric.speed, HudMetric.avgSpeed, HudMetric.elapsedTime, HudMetric.distance] + .map((m) => controller.state[m]!.row) + .toSet(), + {0}, + reason: 'test setup assumes all four land in row 0', + ); + controller.setVisible(HudMetric.avgSpeed, false); final remaining = [HudMetric.speed, HudMetric.distance, HudMetric.elapsedTime] @@ -155,7 +171,14 @@ void main() { 'members reflows that row to fit it', () async { final controller = HudLayoutController(await freshConfig()); - // maxSpeed and movingTime both default to row 2, hidden, at adjacent columns. + // FB-09's title-driven defaultFor no longer necessarily lands maxSpeed and + // movingTime in the same row (their default rows depend on every metric's + // individual title width). Move both hidden metrics into the same row + // explicitly, before enabling either, so this test is independent of exactly + // where defaultFor happens to place them. + controller.updatePosition(HudMetric.maxSpeed, 0, 6); + controller.updatePosition(HudMetric.movingTime, 1, 6); + controller.setVisible(HudMetric.maxSpeed, true); controller.setVisible(HudMetric.movingTime, true); diff --git a/test/hud_widget_layout_test.dart b/test/hud_widget_layout_test.dart index 92ae0c7..cbe6503 100644 --- a/test/hud_widget_layout_test.dart +++ b/test/hud_widget_layout_test.dart @@ -89,6 +89,42 @@ void main() { expect(layout.clampedToGrid().rowSpan, layout.rowSpan); } }); + + test('every metric\'s colSpan is at least its own title-driven minimum', () { + for (final metric in HudMetric.values) { + final layout = HudWidgetLayout.defaultFor(metric); + expect( + layout.colSpan, + greaterThanOrEqualTo(minColSpanForLabel(metric.label)), + ); + } + }); + + test('no row\'s members ever sum past hudGridColumns (the packing ' + 'algorithm\'s own invariant)', () { + final layouts = HudMetric.values.map(HudWidgetLayout.defaultFor).toList(); + final byRow = {}; + for (final layout in layouts) { + byRow[layout.row] = (byRow[layout.row] ?? 0) + layout.colSpan; + } + for (final sum in byRow.values) { + expect(sum, lessThanOrEqualTo(hudGridColumns)); + } + }); + }); + + group('minColSpanForLabel', () { + test('a label under 10 characters fits one column', () { + expect(minColSpanForLabel(HudMetric.speed.label), 1); + }); + + test('a label under 15 characters needs two', () { + expect(minColSpanForLabel(HudMetric.elapsedTime.label), 2); + }); + + test('a label of 15 or more characters needs three', () { + expect(minColSpanForLabel(HudMetric.pointsCaptured.label), 3); + }); }); group('clampedToGrid', () {