Merge FB-02: hide idle Speed panel
# Conflicts: # docs/feedback/FB-02-hide-idle-speed-panel.md
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# FB-02 — Hide the idle Speed panel until recording starts
|
# FB-02 — Hide the idle Speed panel until recording starts
|
||||||
|
|
||||||
**Depends on** — · **Size** S · **Status** Not started
|
**Depends on** — · **Size** S · **Status** Done
|
||||||
|
|
||||||
## Goal
|
## Goal
|
||||||
The Record screen's idle (not-recording) state currently shows a live "SPEED" readout
|
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
|
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
|
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.
|
`(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).
|
||||||
|
|||||||
@@ -173,7 +173,6 @@ class _RecordScreenState extends ConsumerState<RecordScreen> {
|
|||||||
final engine = ref.watch(recordingEngineProvider);
|
final engine = ref.watch(recordingEngineProvider);
|
||||||
final trip = ref.watch(activeTripProvider).valueOrNull;
|
final trip = ref.watch(activeTripProvider).valueOrNull;
|
||||||
final units = ref.watch(unitSystemProvider);
|
final units = ref.watch(unitSystemProvider);
|
||||||
final (speedValue, speedUnit) = formatSpeedParts(_liveSpeed, unit: units);
|
|
||||||
|
|
||||||
final ui = RecordUiState(
|
final ui = RecordUiState(
|
||||||
trip: trip,
|
trip: trip,
|
||||||
@@ -195,38 +194,17 @@ class _RecordScreenState extends ConsumerState<RecordScreen> {
|
|||||||
body: SafeArea(
|
body: SafeArea(
|
||||||
child: Stack(
|
child: Stack(
|
||||||
children: [
|
children: [
|
||||||
if (ui.isIdle)
|
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(
|
Positioned.fill(
|
||||||
bottom: _controlBarHeight(mountedMode),
|
bottom: _controlBarHeight(mountedMode),
|
||||||
child: HudEditOverlay(
|
child: HudEditOverlay(
|
||||||
metricBuilder: (context, metric) =>
|
metricBuilder: (context, metric) => _HudMetricValue(
|
||||||
_HudMetricValue(metric: metric, ui: ui, units: units),
|
metric: metric,
|
||||||
|
ui: ui,
|
||||||
|
units: units,
|
||||||
|
scale: mountedMode ? mountedTextScale : 1.0,
|
||||||
|
mountedMode: mountedMode,
|
||||||
|
),
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
if (ui.isPaused)
|
if (ui.isPaused)
|
||||||
@@ -288,21 +266,50 @@ class _RecordScreenState extends ConsumerState<RecordScreen> {
|
|||||||
/// same `RecordUiState`/units that powered the old fixed stats card. No new data
|
/// same `RecordUiState`/units that powered the old fixed stats card. No new data
|
||||||
/// plumbing, only new presentation, per the ticket's own Implementation section.
|
/// plumbing, only new presentation, per the ticket's own Implementation section.
|
||||||
class _HudMetricValue extends StatelessWidget {
|
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 HudMetric metric;
|
||||||
final RecordUiState ui;
|
final RecordUiState ui;
|
||||||
final UnitSystem units;
|
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
|
/// 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
|
/// 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
|
/// this app's usual reference-vs-live convention -- this HUD is a new visual context
|
||||||
/// the mockup already specifies directly, colour by colour.
|
/// the mockup already specifies directly, colour by colour. Mounted mode overrides
|
||||||
Color _valueColor(ColorScheme colors) => switch (metric) {
|
/// this with plain ink -- see [mountedMode] doc above.
|
||||||
HudMetric.speed => colors.primary,
|
Color _valueColor(ColorScheme colors) {
|
||||||
HudMetric.distance => colors.secondary,
|
if (mountedMode) return colors.onSurface;
|
||||||
_ => colors.onSurface,
|
return switch (metric) {
|
||||||
};
|
HudMetric.speed => colors.primary,
|
||||||
|
HudMetric.distance => colors.secondary,
|
||||||
|
_ => colors.onSurface,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
String get _value {
|
String get _value {
|
||||||
final trip = ui.trip;
|
final trip = ui.trip;
|
||||||
@@ -326,7 +333,11 @@ class _HudMetricValue extends StatelessWidget {
|
|||||||
children: [
|
children: [
|
||||||
Text(
|
Text(
|
||||||
metric.label.toUpperCase(),
|
metric.label.toUpperCase(),
|
||||||
style: TextStyle(fontSize: 10, letterSpacing: 1, color: colors.onSurfaceVariant),
|
style: TextStyle(
|
||||||
|
fontSize: 10 * scale,
|
||||||
|
letterSpacing: 1,
|
||||||
|
color: colors.onSurfaceVariant,
|
||||||
|
),
|
||||||
maxLines: 1,
|
maxLines: 1,
|
||||||
overflow: TextOverflow.ellipsis,
|
overflow: TextOverflow.ellipsis,
|
||||||
),
|
),
|
||||||
@@ -334,7 +345,7 @@ class _HudMetricValue extends StatelessWidget {
|
|||||||
Text(
|
Text(
|
||||||
_value,
|
_value,
|
||||||
style: monoDigits.copyWith(
|
style: monoDigits.copyWith(
|
||||||
fontSize: 18,
|
fontSize: 18 * scale,
|
||||||
fontWeight: FontWeight.bold,
|
fontWeight: FontWeight.bold,
|
||||||
color: _valueColor(colors),
|
color: _valueColor(colors),
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import 'package:rippr/src/app/providers.dart';
|
|||||||
import 'package:rippr/src/data/database.dart';
|
import 'package:rippr/src/data/database.dart';
|
||||||
import 'package:rippr/src/data/trip_repository.dart';
|
import 'package:rippr/src/data/trip_repository.dart';
|
||||||
import 'package:rippr/src/domain/models.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/ui/activity_display.dart';
|
||||||
import 'package:rippr/src/recording/location_source.dart';
|
import 'package:rippr/src/recording/location_source.dart';
|
||||||
import 'package:rippr/src/recording/wakelock_controller.dart';
|
import 'package:rippr/src/recording/wakelock_controller.dart';
|
||||||
@@ -123,11 +124,10 @@ void main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
group('record screen', () {
|
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.pumpWidget(host(const RecordScreen()));
|
||||||
await tester.pumpAndSettle();
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
expect(find.text('Ready'), findsOneWidget);
|
|
||||||
expect(find.byKey(const Key('start')), findsOneWidget);
|
expect(find.byKey(const Key('start')), findsOneWidget);
|
||||||
expect(find.byKey(const Key('pause')), findsNothing);
|
expect(find.byKey(const Key('pause')), findsNothing);
|
||||||
expect(find.byKey(const Key('stop')), 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
|
// 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
|
// a real ride, because a max figure only moves when you beat it. This guards the
|
||||||
// 2.0.1 fix.
|
// 2.0.1 fix.
|
||||||
await tester.pumpWidget(host(const RecordScreen()));
|
await repo.startTrip(1000);
|
||||||
await tester.pumpAndSettle();
|
await pumpLive(tester, host(const RecordScreen(), map: false));
|
||||||
|
|
||||||
expect(find.text('SPEED'), findsOneWidget);
|
expect(find.text('SPEED'), findsOneWidget);
|
||||||
expect(find.text('MAX SPEED'), findsNothing);
|
expect(find.text('MAX SPEED'), findsNothing);
|
||||||
@@ -236,12 +236,12 @@ void main() {
|
|||||||
// The regression this exists for: removing Compose's Surface left LocalContentColor
|
// 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
|
// 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.
|
// test could catch it. This asserts the rendered colour differs from the ground.
|
||||||
await tester.pumpWidget(host(const RecordScreen()));
|
await repo.startTrip(1000);
|
||||||
await tester.pumpAndSettle();
|
await pumpLive(tester, host(const RecordScreen(), map: false));
|
||||||
|
|
||||||
final speed = tester.widget<Text>(
|
final speed = tester.widget<Text>(
|
||||||
find.descendant(
|
find.descendant(
|
||||||
of: find.byKey(const Key('speed')),
|
of: find.byKey(const ValueKey(HudMetric.speed)),
|
||||||
matching: find.text('0'),
|
matching: find.text('0'),
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
@@ -379,21 +379,24 @@ void main() {
|
|||||||
|
|
||||||
screenTest('the mounted theme is high-contrast and text scales up (V3-05)',
|
screenTest('the mounted theme is high-contrast and text scales up (V3-05)',
|
||||||
(tester) async {
|
(tester) async {
|
||||||
await tester.pumpWidget(host(const RecordScreen(), mountedMode: true));
|
await repo.startTrip(1000);
|
||||||
await tester.pumpAndSettle();
|
await pumpLive(
|
||||||
|
tester,
|
||||||
|
host(const RecordScreen(), map: false, mountedMode: true),
|
||||||
|
);
|
||||||
|
|
||||||
final speed = tester.widget<Text>(
|
final speed = tester.widget<Text>(
|
||||||
find.descendant(
|
find.descendant(
|
||||||
of: find.byKey(const Key('speed')),
|
of: find.byKey(const ValueKey(HudMetric.speed)),
|
||||||
matching: find.text('0'),
|
matching: find.text('0'),
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
expect(speed.style?.fontSize, 64 * mountedTextScale);
|
expect(speed.style?.fontSize, 18 * mountedTextScale);
|
||||||
expect(speed.style?.color, isNot(ripprBackground),
|
expect(speed.style?.color, isNot(ripprBackground),
|
||||||
reason: 'still legible, just against a different (lighter) ground');
|
reason: 'still legible, just against a different (lighter) ground');
|
||||||
|
|
||||||
final start = tester.getSize(find.byKey(const Key('start')));
|
final pause = tester.getSize(find.byKey(const Key('pause')));
|
||||||
expect(start.height, 96,
|
expect(pause.height, 96,
|
||||||
reason: 'V3-05: 72dp is not enough at speed, with gloves');
|
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
|
// content behaves differently than the dark-on-dark case V3-05 originally
|
||||||
// guarded against. This asserts the real contrast ratio, not just "differs from
|
// guarded against. This asserts the real contrast ratio, not just "differs from
|
||||||
// the wrong ground" the way the test above does.
|
// the wrong ground" the way the test above does.
|
||||||
await tester.pumpWidget(host(const RecordScreen(), mountedMode: true));
|
await repo.startTrip(1000);
|
||||||
await tester.pumpAndSettle();
|
await pumpLive(
|
||||||
|
tester,
|
||||||
|
host(const RecordScreen(), map: false, mountedMode: true),
|
||||||
|
);
|
||||||
|
|
||||||
final speed = tester.widget<Text>(
|
final speed = tester.widget<Text>(
|
||||||
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<GlassPanel>(
|
||||||
|
find.descendant(
|
||||||
|
of: find.byKey(const ValueKey(HudMetric.speed)),
|
||||||
|
matching: find.byType(GlassPanel),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
final panel = tester.widget<GlassPanel>(find.byType(GlassPanel).first);
|
|
||||||
final mountedColors = ripprMountedTheme().colorScheme;
|
final mountedColors = ripprMountedTheme().colorScheme;
|
||||||
// GlassPanel fills with `colors.surface` at `GlassPanel.fillOpacity` -- since it
|
// GlassPanel fills with `colors.surface` at `GlassPanel.fillOpacity` -- since it
|
||||||
// is a solid, near-opaque fill (not a transparency composited over unknown
|
// is a solid, near-opaque fill (not a transparency composited over unknown
|
||||||
|
|||||||
Reference in New Issue
Block a user