diff --git a/docs/feedback/FB-02-hide-idle-speed-panel.md b/docs/feedback/FB-02-hide-idle-speed-panel.md new file mode 100644 index 0000000..8a52cbc --- /dev/null +++ b/docs/feedback/FB-02-hide-idle-speed-panel.md @@ -0,0 +1,243 @@ +# FB-02 — Hide the idle Speed panel until recording starts + +**Depends on** — · **Size** S · **Status** Done + +## Goal +The Record screen's idle (not-recording) state currently shows a live "SPEED" readout +plus status text before the rider has pressed Start at all. It should show nothing but +the (live, per FB-01) map and the START control until a ride actually begins. + +## Context +Direct user feedback (`docs/FEEDBACK.md`, "Map Page" section): + +> Remove the "Speed" widget from the map screen when "start" hasn't been pressed yet. It +> blocks the entire screen, and you can't see the map at all. Speed and other widgets +> should only appear when recording starts. + +Today's idle branch — `lib/src/ui/record/record_screen.dart`, inside `build()`'s +`Stack` (~lines 196-223): + +```dart +child: Stack( + children: [ + if (ui.isIdle) + Center( + child: GlassPanel( + padding: const EdgeInsets.all(24), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + // Current speed, not max: a max figure only moves when you beat + // it, which reads as a frozen screen while riding steadily. + // This is the 2.0.1 fix and it must not regress. + BigStat( + key: const Key('speed'), + label: 'SPEED', + value: speedValue, + unit: speedUnit, + scale: mountedMode ? mountedTextScale : 1.0, + ), + const SizedBox(height: 16), + // A resting state, not a zeroed ride -- otherwise the screen + // looks like a recording that is going nowhere. + const StatRow(label: 'Status', value: 'Ready'), + const StatRow(label: 'Last ride', value: 'See Rides'), + ], + ), + ), + ) + else + Positioned.fill( + bottom: _controlBarHeight(mountedMode), + child: HudEditOverlay( + metricBuilder: (context, metric) => + _HudMetricValue(metric: metric, ui: ui, units: units), + ), + ), + ... +``` + +`ui.isIdle` is `trip == null` (`RecordUiState.isIdle`, ~line 42). The `GlassPanel` sizes +to its content (`mainAxisSize: MainAxisSize.min`) so it isn't literally full-screen, but +it's centered, opaque-ish (`GlassPanel` is a near-opaque translucent fill per its own +doc comment elsewhere in this codebase), and shows a real live speed figure +(`_liveSpeed`, updated every GPS tick via `liveTelemetryProvider` — see `initState` +~lines 86-88) before any ride exists — which is exactly what the feedback is describing +as "blocking the screen" and confusing to see before pressing Start. + +## Design +Remove the entire `if (ui.isIdle) ... else ...` branching for HUD content — always +render the `HudEditOverlay` branch, and have the idle state simply show nothing there +(the HUD overlay area is empty when there's no trip, since there's nothing to edit/no +metrics to place — confirm `HudEditOverlay` degrades gracefully with an idle +`RecordUiState`, or gate it explicitly: only mount `HudEditOverlay` when `!ui.isIdle`, +and render an empty `SizedBox.shrink()` (or nothing at all) for the idle case, so the +full-bleed live map (per FB-01) is the entire idle screen behind the START control bar. + +Concretely: + +```dart +child: Stack( + children: [ + if (!ui.isIdle) + Positioned.fill( + bottom: _controlBarHeight(mountedMode), + child: HudEditOverlay( + metricBuilder: (context, metric) => + _HudMetricValue(metric: metric, ui: ui, units: units), + ), + ), + if (ui.isPaused) + ... +``` + +(i.e. drop the idle `Center(child: GlassPanel(...))` block entirely — no replacement +content, not even a "Ready" label. The now-live background map, per FB-01, is the idle +screen's entire content above the control bar.) + +`speedValue`/`speedUnit` (`formatSpeedParts(_liveSpeed, unit: units)`, ~line 176) become +unused once the idle panel is removed — check whether `_HudMetricValue`'s own +`HudMetric.speed` case (`formatSpeedParts(ui.currentSpeedKmh, unit: units).$1` at +~line 310) already computes this independently (it does — `ui.currentSpeedKmh` is +`_liveSpeed`), so the `final (speedValue, speedUnit) = ...` line in `build()` should +simply be deleted rather than left as dead code. Run `flutter analyze` to confirm no +other lingering unused-variable warnings from this removal. + +`BigStat`/`StatRow` imports in `record_screen.dart` (from `../components/stats.dart`) +may become unused if nothing else in this file still uses them — check before removing +the import; `confirmDialog` (also from `stats.dart`) is used elsewhere in this file for +the discard confirmation, so the import itself likely stays, just drop `BigStat`/ +`StatRow` specifically if nothing else references them. + +## Implementation +1. Remove the idle `Center(child: GlassPanel(...))` block from `record_screen.dart`'s + `build()`. +2. Gate `HudEditOverlay` on `!ui.isIdle` (it was previously the `else` branch of the + idle check — now it's the only branch, wrapped in its own `if`). +3. Delete the now-unused `final (speedValue, speedUnit) = formatSpeedParts(...)` line. +4. Run `flutter analyze` and remove any import that's now unused as a result. + +## Acceptance criteria +- [ ] Idle state (no active trip) shows no Speed/Status/"Last ride" content at all — + just the live background map and the control bar with only "START RECORDING" + visible (unchanged from today). +- [ ] The moment a trip starts recording, the full HUD (`HudEditOverlay` with the + default-visible metrics: Speed, Avg Speed, Distance, Elapsed Time) appears exactly + as it does today — this ticket only changes the idle state, not recording/paused. +- [ ] `flutter analyze` clean (no unused-variable/import warnings from the removal). +- [ ] `flutter test` green, test count only goes up (net effect after the test + migrations below should still be positive or flat, never negative). + +## Tests +Several existing tests assert against the idle Speed panel specifically and must be +migrated — not deleted — to seed an active recording and assert against the HUD's own +`Speed` widget instead, since the underlying regressions they guard (black-on-black +text, mounted-mode contrast/scale) are real risks that still apply once the same figure +lives inside `_HudMetricValue` instead of the idle `BigStat`. The HUD's per-metric +widgets are keyed `ValueKey(entry.key)` where `entry.key` is the `HudMetric` +(`lib/src/ui/components/hud_edit_overlay.dart` ~line 63) — use +`find.byKey(const ValueKey(HudMetric.speed))` to locate the speed widget in a recording +state, then descend into it the same way the old tests descended into `Key('speed')`. + +In `test/widget_test.dart`: + +- **`'idle shows Ready and only START'`** (~line 126): remove the + `expect(find.text('Ready'), findsOneWidget);` assertion (the "Ready"/"Status" text no + longer exists at all); keep the START/PAUSE/STOP key assertions and the + `find.text('Distance')` findsNothing check — both remain valid (this test's title may + want updating to `'idle shows only START'`). +- **`'the headline is live speed, not max speed'`** (~line 138): change to seed a + recording trip first (`await repo.startTrip(1000); await pumpLive(tester, + host(const RecordScreen(), map: false));` — see the identical pattern already used at + ~line 205-206 in `'tapping PAUSE and then STOP...'`), then assert + `find.text('SPEED')` findsOneWidget / `find.text('MAX SPEED')` findsNothing exactly as + before (Max Speed is not one of the four default-visible metrics — + `HudWidgetLayout.defaultFor`'s `visible: index < columns` with `columns = 4` and + `HudMetric.values` ordered `speed, avgSpeed, distance, elapsedTime, maxSpeed, ...` — + so this assertion still holds true in the recording state). +- **`'text is legible against the dark ground'`** (~line 235): seed a recording trip the + same way, then replace + `find.descendant(of: find.byKey(const Key('speed')), matching: find.text('0'))` with + `find.descendant(of: find.byKey(const ValueKey(HudMetric.speed)), matching: + find.text('0'))`. Same downstream assertions (`colour != ripprBackground`, + `colour != Colors.black`) unchanged. +- **`'the mounted theme is high-contrast and text scales up (V3-05)'`** (~line 380) and + **`'the mounted-mode speed digit is legible against GlassPanel's own translucent + surface...'`** (~line 400): both currently pump `RecordScreen()` idle with + `mountedMode: true` and inspect `Key('speed')`. Migrate both to seed a recording trip + first, then use `ValueKey(HudMetric.speed)` the same way. Confirm `_HudMetricValue`'s + value `Text` actually honors `mountedMode`'s scale/theme the same way the old idle + `BigStat` did (check `_HudMetricValue.build()` — if it currently has no mounted-mode + awareness at all, that's a **pre-existing gap this migration would newly expose**, not + something to paper over: if the mounted-mode font-size/contrast assertions fail + against the HUD widget, that's a real, separate bug worth calling out plainly in this + ticket's Outcome section rather than weakening the assertion to make it pass. +- **`'HUD widgets render over the map, not replacing it (UI-05)'`** (~line 219) already + seeds a recording trip and asserts `find.text('SPEED')`/`find.text('DISTANCE')` — no + change needed, but worth a quick sanity check that it still passes unmodified. + +## Risks +- The mounted-mode migration above may surface that `_HudMetricValue` never actually + had mounted-mode-aware styling (it was only ever exercised via the idle `BigStat` + before now) — if so, this ticket's Outcome section must say so explicitly rather than + silently deleting or weakening the assertion. Fixing that gap (if real) is reasonable + to fold into this ticket since it's directly caused by this change, but keep the fix + minimal (reuse whatever scale/theme mechanism `BigStat` already used) rather than + redesigning `_HudMetricValue`. + +## Out of scope +The HUD grid/drag-resize/text-auto-fit rework (FB-03) — this ticket only removes idle +content, it does not touch how the recording-state HUD widgets are laid out or sized. +The live-map-while-idle behavior itself (FB-01) — this ticket assumes it lands +separately; if FB-01 hasn't merged yet, the idle screen will just show today's +`(0,0)`/zoom-2 map, which is still a strict improvement over a blocking Speed panel. + +## Outcome +Implemented exactly as designed. `record_screen.dart`'s idle `Center(child: GlassPanel(...))` +block (Speed/Status/Last ride) is gone; `HudEditOverlay` is now gated on `!ui.isIdle` and is +the Stack's only HUD-content branch. The dead `final (speedValue, speedUnit) = ...` line was +deleted; `BigStat`/`StatRow` are no longer referenced in the file (the `stats.dart` import +stays, since `confirmDialog` from the same file is still used for the discard dialog). + +All five named tests were migrated per the ticket's Tests section: +- `'idle shows Ready and only START'` → renamed to `'idle shows only START'`, the `Ready` + assertion removed, START/PAUSE/STOP and `find.text('Distance')` checks kept. +- `'the headline is live speed, not max speed'` → now seeds a recording trip and asserts + against the HUD, same SPEED/MAX SPEED expectations. +- `'text is legible against the dark ground'` → now seeds a recording trip and descends into + `find.byKey(const ValueKey(HudMetric.speed))` instead of `Key('speed')`. +- The two mounted-mode tests (V3-05 scale/contrast, and UI-05 GlassPanel contrast) were + migrated to seed a recording trip and use `ValueKey(HudMetric.speed)`. The V3-05 test's + unrelated `Key('start')` height check was swapped for `Key('pause')`, since a recording + state has no START button. The GlassPanel-lookup test switched from + `find.byType(GlassPanel).first` (an ancestor of `Key('speed')` in the old layout) to + `find.descendant(of: find.byKey(const ValueKey(HudMetric.speed)), matching: + find.byType(GlassPanel))`, since in the new HUD tree the per-metric `GlassPanel` is a + *descendant* of the keyed widget (`DraggableResizableHudWidget`), not an ancestor. +- `'HUD widgets render over the map, not replacing it (UI-05)'` needed no change and still + passes. + +**The risk the ticket flagged was real, in two parts, both now fixed:** +1. `_HudMetricValue` had no `scale` parameter at all — its label/value `fontSize`s were + hardcoded (10/18), so the old `BigStat`-based mounted-mode text-scale assertion + (`64 * mountedTextScale`) had no equivalent to migrate to. Added a `scale` field to + `_HudMetricValue`, threaded from `record_screen.dart`'s existing `mountedMode` local the + same way `BigStat` was driven (`mountedMode ? mountedTextScale : 1.0`), and multiplied + both font sizes by it. The migrated test now asserts `18 * mountedTextScale`. +2. Once (1) was fixed, the *second* mounted-mode test (GlassPanel contrast, UI-05) still + failed: `_HudMetricValue._valueColor`'s per-metric accent colours (`colors.primary` for + Speed, tuned for the dark HUD mockup) measure 4.43:1 against the mounted theme's + `GlassPanel` surface — just under the 4.5:1 AA threshold the test enforces. This is a + real, separate, pre-existing gap this migration exposed: `_HudMetricValue` never + accounted for mounted mode's own colour scheme in `_valueColor` at all (unlike `BigStat`, + which always named its headline colour explicitly as `colors.onSurface` rather than + trusting an accent). Fixed minimally by adding a `mountedMode` bool to `_HudMetricValue` + that, when true, forces `_valueColor` to `colors.onSurface` for every metric — reusing + `BigStat`'s own explicit-ink fallback — while leaving the dark-theme mockup's per-metric + accent colours (Speed: primary, Distance: secondary) untouched for the normal (non-mounted) + case. This is a colour-only override scoped to mounted mode; no other `_HudMetricValue` + behavior changed. + +`flutter analyze`: clean (same 4 pre-existing `info`-level issues as baseline, none new). +`flutter test`: 374 passing (unchanged from baseline — no tests were added or removed, five +were migrated in place per the ticket). diff --git a/lib/src/ui/record/record_screen.dart b/lib/src/ui/record/record_screen.dart index 3130f3f..7db02d3 100644 --- a/lib/src/ui/record/record_screen.dart +++ b/lib/src/ui/record/record_screen.dart @@ -173,7 +173,6 @@ class _RecordScreenState extends ConsumerState { final engine = ref.watch(recordingEngineProvider); final trip = ref.watch(activeTripProvider).valueOrNull; final units = ref.watch(unitSystemProvider); - final (speedValue, speedUnit) = formatSpeedParts(_liveSpeed, unit: units); final ui = RecordUiState( trip: trip, @@ -195,38 +194,17 @@ class _RecordScreenState extends ConsumerState { body: SafeArea( child: Stack( children: [ - if (ui.isIdle) - Center( - child: GlassPanel( - padding: const EdgeInsets.all(24), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - // Current speed, not max: a max figure only moves when you beat - // it, which reads as a frozen screen while riding steadily. - // This is the 2.0.1 fix and it must not regress. - BigStat( - key: const Key('speed'), - label: 'SPEED', - value: speedValue, - unit: speedUnit, - scale: mountedMode ? mountedTextScale : 1.0, - ), - const SizedBox(height: 16), - // A resting state, not a zeroed ride -- otherwise the screen - // looks like a recording that is going nowhere. - const StatRow(label: 'Status', value: 'Ready'), - const StatRow(label: 'Last ride', value: 'See Rides'), - ], - ), - ), - ) - else + if (!ui.isIdle) Positioned.fill( bottom: _controlBarHeight(mountedMode), child: HudEditOverlay( - metricBuilder: (context, metric) => - _HudMetricValue(metric: metric, ui: ui, units: units), + metricBuilder: (context, metric) => _HudMetricValue( + metric: metric, + ui: ui, + units: units, + scale: mountedMode ? mountedTextScale : 1.0, + mountedMode: mountedMode, + ), ), ), if (ui.isPaused) @@ -288,21 +266,50 @@ class _RecordScreenState extends ConsumerState { /// same `RecordUiState`/units that powered the old fixed stats card. No new data /// plumbing, only new presentation, per the ticket's own Implementation section. class _HudMetricValue extends StatelessWidget { - const _HudMetricValue({required this.metric, required this.ui, required this.units}); + const _HudMetricValue({ + required this.metric, + required this.ui, + required this.units, + this.scale = 1.0, + this.mountedMode = false, + }); final HudMetric metric; final RecordUiState ui; final UnitSystem units; + /// V3-05: mounted mode reads bigger, at a glance, at speed. Same mechanism `BigStat` + /// used for the old idle Speed panel -- fixed pixel sizes below (not the ambient text + /// theme) are what a glove-and-visor readout actually is, so scaling has to happen + /// here rather than through `Theme.of(context).textTheme`. + final double scale; + + /// FB-02: `_HudMetricValue` had no mounted-mode awareness at all before the idle + /// `BigStat` (which did) was removed as the idle panel's speed readout -- this + /// migration is what surfaced that gap. [scale] above covers the font-size half of + /// it; this covers the other half `BigStat` also handled by naming its headline + /// colour explicitly (`colors.onSurface`, never a metric accent) rather than trusting + /// the ambient theme: in the mounted theme specifically, `_valueColor`'s per-metric + /// accent colours (tuned for the dark theme's own contrast) fall a hair short of AA + /// (4.43:1, not 4.5:1) against `GlassPanel`'s mounted-theme surface. Reusing + /// `BigStat`'s own fallback -- plain, guaranteed-legible ink whenever mounted mode is + /// on -- fixes it without touching the accent colours the dark-theme HUD mockup + /// deliberately specifies. + final bool mountedMode; + /// Matches the Map HUD mockup's own colour choices for its default four metrics /// exactly (Speed: primary, Avg Speed/Time: plain ink, Dist: secondary) rather than /// this app's usual reference-vs-live convention -- this HUD is a new visual context - /// the mockup already specifies directly, colour by colour. - Color _valueColor(ColorScheme colors) => switch (metric) { - HudMetric.speed => colors.primary, - HudMetric.distance => colors.secondary, - _ => colors.onSurface, - }; + /// the mockup already specifies directly, colour by colour. Mounted mode overrides + /// this with plain ink -- see [mountedMode] doc above. + Color _valueColor(ColorScheme colors) { + if (mountedMode) return colors.onSurface; + return switch (metric) { + HudMetric.speed => colors.primary, + HudMetric.distance => colors.secondary, + _ => colors.onSurface, + }; + } String get _value { final trip = ui.trip; @@ -326,7 +333,11 @@ class _HudMetricValue extends StatelessWidget { children: [ Text( metric.label.toUpperCase(), - style: TextStyle(fontSize: 10, letterSpacing: 1, color: colors.onSurfaceVariant), + style: TextStyle( + fontSize: 10 * scale, + letterSpacing: 1, + color: colors.onSurfaceVariant, + ), maxLines: 1, overflow: TextOverflow.ellipsis, ), @@ -334,7 +345,7 @@ class _HudMetricValue extends StatelessWidget { Text( _value, style: monoDigits.copyWith( - fontSize: 18, + fontSize: 18 * scale, fontWeight: FontWeight.bold, color: _valueColor(colors), ), diff --git a/test/widget_test.dart b/test/widget_test.dart index dbf2efd..aeb2c0b 100644 --- a/test/widget_test.dart +++ b/test/widget_test.dart @@ -9,6 +9,7 @@ import 'package:rippr/src/app/providers.dart'; import 'package:rippr/src/data/database.dart'; import 'package:rippr/src/data/trip_repository.dart'; import 'package:rippr/src/domain/models.dart'; +import 'package:rippr/src/hud/hud_metric.dart'; import 'package:rippr/src/ui/activity_display.dart'; import 'package:rippr/src/recording/location_source.dart'; import 'package:rippr/src/recording/wakelock_controller.dart'; @@ -123,11 +124,10 @@ void main() { } group('record screen', () { - screenTest('idle shows Ready and only START', (tester) async { + screenTest('idle shows only START', (tester) async { await tester.pumpWidget(host(const RecordScreen())); await tester.pumpAndSettle(); - expect(find.text('Ready'), findsOneWidget); expect(find.byKey(const Key('start')), findsOneWidget); expect(find.byKey(const Key('pause')), findsNothing); expect(find.byKey(const Key('stop')), findsNothing); @@ -139,8 +139,8 @@ void main() { // v2.0 shipped max speed as the headline and it read as a frozen, broken screen on // a real ride, because a max figure only moves when you beat it. This guards the // 2.0.1 fix. - await tester.pumpWidget(host(const RecordScreen())); - await tester.pumpAndSettle(); + await repo.startTrip(1000); + await pumpLive(tester, host(const RecordScreen(), map: false)); expect(find.text('SPEED'), findsOneWidget); expect(find.text('MAX SPEED'), findsNothing); @@ -236,12 +236,12 @@ void main() { // The regression this exists for: removing Compose's Surface left LocalContentColor // black, and a 64sp figure rendered invisibly on a near-black background. No logic // test could catch it. This asserts the rendered colour differs from the ground. - await tester.pumpWidget(host(const RecordScreen())); - await tester.pumpAndSettle(); + await repo.startTrip(1000); + await pumpLive(tester, host(const RecordScreen(), map: false)); final speed = tester.widget( find.descendant( - of: find.byKey(const Key('speed')), + of: find.byKey(const ValueKey(HudMetric.speed)), matching: find.text('0'), ), ); @@ -379,21 +379,24 @@ void main() { screenTest('the mounted theme is high-contrast and text scales up (V3-05)', (tester) async { - await tester.pumpWidget(host(const RecordScreen(), mountedMode: true)); - await tester.pumpAndSettle(); + await repo.startTrip(1000); + await pumpLive( + tester, + host(const RecordScreen(), map: false, mountedMode: true), + ); final speed = tester.widget( find.descendant( - of: find.byKey(const Key('speed')), + of: find.byKey(const ValueKey(HudMetric.speed)), matching: find.text('0'), ), ); - expect(speed.style?.fontSize, 64 * mountedTextScale); + expect(speed.style?.fontSize, 18 * mountedTextScale); expect(speed.style?.color, isNot(ripprBackground), reason: 'still legible, just against a different (lighter) ground'); - final start = tester.getSize(find.byKey(const Key('start'))); - expect(start.height, 96, + final pause = tester.getSize(find.byKey(const Key('pause'))); + expect(pause.height, 96, reason: 'V3-05: 72dp is not enough at speed, with gloves'); }); @@ -405,13 +408,24 @@ void main() { // content behaves differently than the dark-on-dark case V3-05 originally // guarded against. This asserts the real contrast ratio, not just "differs from // the wrong ground" the way the test above does. - await tester.pumpWidget(host(const RecordScreen(), mountedMode: true)); - await tester.pumpAndSettle(); + await repo.startTrip(1000); + await pumpLive( + tester, + host(const RecordScreen(), map: false, mountedMode: true), + ); final speed = tester.widget( - find.descendant(of: find.byKey(const Key('speed')), matching: find.text('0')), + find.descendant( + of: find.byKey(const ValueKey(HudMetric.speed)), + matching: find.text('0'), + ), + ); + final panel = tester.widget( + find.descendant( + of: find.byKey(const ValueKey(HudMetric.speed)), + matching: find.byType(GlassPanel), + ), ); - final panel = tester.widget(find.byType(GlassPanel).first); final mountedColors = ripprMountedTheme().colorScheme; // GlassPanel fills with `colors.surface` at `GlassPanel.fillOpacity` -- since it // is a solid, near-opaque fill (not a transparency composited over unknown