diff --git a/docs/feedback/FB-02-hide-idle-speed-panel.md b/docs/feedback/FB-02-hide-idle-speed-panel.md index ae0dd6f..8a52cbc 100644 --- a/docs/feedback/FB-02-hide-idle-speed-panel.md +++ b/docs/feedback/FB-02-hide-idle-speed-panel.md @@ -1,6 +1,6 @@ # FB-02 — Hide the idle Speed panel until recording starts -**Depends on** — · **Size** S · **Status** Not started +**Depends on** — · **Size** S · **Status** Done ## Goal The Record screen's idle (not-recording) state currently shows a live "SPEED" readout @@ -191,3 +191,53 @@ content, it does not touch how the recording-state HUD widgets are laid out or s 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 4ffa1a6..8330946 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