dilly/rippr
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Write FB-01..FB-05 feedback tickets

Browse Source 
Turns docs/FEEDBACK.md into five self-contained tickets, each grounded in
exact current code and a concrete implementation approach, so they can be
handed to independent subagents with no prior context.
This commit is contained in:
uhryniuk
2026-08-24 15:38:44 -05:00
parent fe4476596d
commit cf6b9d1d73
6 changed files with 1799 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

335
docs/feedback/FB-01-live-street-level-map.md Normal file
View File

@@ -0,0 +1,335 @@
# FB-01 — Live, street-level map everywhere
**Depends on** — · **Size** L · **Status** Not started
## Goal
The map is too zoomed out on every screen, and the shared background map has zero
location awareness while idle — it should behave like Google/Apple Maps: zoomed in
enough to see the street you're on and the streets around it, and continuously tracking
the device's real position in real time, whether or not a ride is being recorded.
## Context
Direct user feedback (`docs/FEEDBACK.md`, "All pages" section):
> The map is too zoomed out. Start by having the map zoomed in enough where you could
> easily see what street the user is on and the streets around it. This goes for all
> screens that use the map, and even if the map is a background it should be updating
> real time to the person moving so if they aren't recording but they are riding in a
> car, it will update just like google maps. Essentially we want something that looks
> identical to google maps or apple maps.
Today's behavior, exactly:
`lib/src/ui/components/ride_map.dart`, `_RideMapState.build()` (~lines 200-237):
```dart
geo.Bounds? bounds;
if (hasPoints) {
bounds = geo.bounds([
for (final p in widget.points) geo.LatLon(p.latitude, p.longitude),
]);
}
...
options: MapOptions(
initialCameraFit: (bounds == null || bounds.isDegenerate)
? null
: CameraFit.bounds(
bounds: LatLngBounds(...),
padding: const EdgeInsets.all(24),
maxZoom: maxTileZoom,
),
initialCenter: bounds == null
? const ll.LatLng(0, 0)
: ll.LatLng(bounds.centerLat, bounds.centerLon),
initialZoom: bounds == null
? 2
: (bounds.isDegenerate ? shortRideZoom : maxTileZoom),
maxZoom: maxTileZoom,
...
```
`bounds == null` happens whenever `widget.points` is empty — which is exactly the state
of the shared background map (`ShellScaffold` in `lib/src/ui/app_shell.dart`) any time
no trip is actively recording. In that state the map centers on **`(0, 0)` (Null
Island) at zoom 2** — the literal opposite of "zoomed in enough to see your street."
The shared background map's data source, `ShellScaffold.build()` (`app_shell.dart`
~lines 59-72):
```dart
final trip = ref.watch(activeTripProvider).valueOrNull;
final points = trip == null
? const <TrackPoint>[]
: ref.watch(livePointsProvider(trip.id)).valueOrNull ?? const <TrackPoint>[];
final segments = trip == null
? const <Segment>[]
: ref.watch(liveSegmentsProvider(trip.id)).valueOrNull ?? const <Segment>[];
```
`livePointsProvider`/`liveSegmentsProvider` are DB-backed streams of a trip's *stored*
points — they only ever produce data while `RecordingEngine` is actively writing to a
trip. There is no code path anywhere that feeds the background map a raw GPS position
independent of an active recording — confirmed by grep: `locationSourceProvider`
(`lib/src/app/providers.dart` ~line 54) is only ever watched by `recordingEngineProvider`
(~lines 79-89). **The background map is blind while idle**, which is the root cause of
both complaints at once (wrong zoom AND no live tracking) — there's simply no location
signal reaching it until a trip starts.
`RideMap` already has exactly the chase-camera logic this needs, just scoped to
recorded points — `_RideMapState` (`ride_map.dart` ~lines 113-147):
```dart
class _RideMapState extends State<RideMap> with WidgetsBindingObserver {
final _controller = MapController();
late bool _following = widget.follow;
bool _backgrounded = false;
@override
void didUpdateWidget(RideMap old) {
super.didUpdateWidget(old);
if (!_following || widget.points.isEmpty || _backgrounded) return;
final last = widget.points.last;
WidgetsBinding.instance.addPostFrameCallback((_) {
if (!mounted || !_following) return;
_controller.move(ll.LatLng(last.latitude, last.longitude), _controller.camera.zoom);
});
}
...
```
And the `onPositionChanged` cancel-on-manual-pan guard (`ride_map.dart` ~lines 243-252):
```dart
onPositionChanged: !widget.follow
? null
: (position, hasGesture) {
if (hasGesture && _following) {
setState(() => _following = false);
}
},
```
This is the exact "chase the rider, but a real pan/pinch cancels it" behavior the
feedback is asking for — it just needs a second data source (ambient GPS) wired in for
when there are no recorded points to chase.
**The shared `LocationSource` instance is process-wide — do not call `stop()` on it
from ambient code.** `lib/src/app/providers.dart`:
```dart
final locationSourceProvider = Provider<LocationSource>((ref) {
final source = GeolocatorLocationSource();
ref.onDispose(source.dispose);
return source;
});
```
One `GeolocatorLocationSource` for the whole app. Its `start()` is idempotent (safe to
call from two places — `lib/src/recording/geolocator_location_source.dart`:
`if (_subscription != null) return; // idempotent`), but its `stop()` is **not**
reference-counted — it unconditionally cancels the one underlying platform subscription:
```dart
@override
Future<void> stop() async {
await _subscription?.cancel();
_subscription = null;
}
```
If ambient-mode code ever calls `locationSource.stop()` (e.g. from a provider's
`ref.onDispose`, or when the Map tab becomes invisible), and a real recording happens to
be in progress at that moment, **it would silently kill GPS delivery to the active
recording** — the engine has no way to know its location source was just stopped out
from under it by an unrelated consumer. This must not be possible. See Design below.
## Design
- **New constant** in `lib/src/ui/components/ride_map.dart`, near `maxTileZoom`/
`shortRideZoom`: `const double ambientZoom = 17.0;` — a distinct name from
`shortRideZoom` even though the value happens to match, since they mean different
things (one is "a very short recorded ride," the other is "no ride at all, just
ambient GPS").
- **New provider**, in `lib/src/app/providers.dart`:
```dart
/// The device's current position when nothing is being recorded — drives the shared
/// background map's "look like Google Maps while idle" behavior. Deliberately NOT
/// gated through `recordingEngineProvider`/`RecordingEngine.start()` — this must work
/// whether or not a ride is ever recorded. Never calls `LocationSource.stop()`: the
/// underlying `locationSourceProvider` instance is shared with the recording engine,
/// and `stop()` is not reference-counted (see FB-01's ticket for why).
final ambientPositionProvider = StreamProvider.autoDispose<LocationFix?>((ref) async* {
if (!ref.watch(mapEnabledProvider)) {
yield null;
return;
}
final source = ref.watch(locationSourceProvider);
try {
await source.start(); // idempotent; safe even if a recording already started it
} on LocationException {
yield null; // permission denied / service disabled — ambient mode is best-effort
return;
}
yield* source.fixes.map<LocationFix?>((fix) => fix);
// No `stop()` call, ever, on dispose — see the doc comment above. Only this
// provider's own subscription to the broadcast `fixes` stream ends; the shared
// platform subscription is left exactly as it was.
});
```
`LocationFix` and `LocationException` are both already imported/available via
`lib/src/recording/location_source.dart` (already imported in `providers.dart`).
`autoDispose` is correct here — this should stop listening the moment nothing watches
it (e.g. the app backgrounded, or a trip starts and `ShellScaffold` stops watching
this provider — see next bullet), same lifecycle discipline as every other
`StreamProvider.autoDispose` in this file.
- **`ShellScaffold.build()`** (`lib/src/ui/app_shell.dart`): only watch
`ambientPositionProvider` while idle, so it's never even subscribed during an active
recording:
```dart
final trip = ref.watch(activeTripProvider).valueOrNull;
final ambientFix = trip == null ? ref.watch(ambientPositionProvider).valueOrNull : null;
final ambientPosition = ambientFix == null
? null
: ll.LatLng(ambientFix.latitude, ambientFix.longitude);
```
(needs `import 'package:latlong2/latlong.dart' as ll;`) then pass `ambientPosition:
ambientPosition` into the `RideMap(...)` constructor call alongside the existing
`points`/`segments`/`follow`/etc. Leave `follow: isMapTab` and `showLocationMarker:
isMapTab` exactly as they are — ambient following should only run on the visible Map
tab, matching the existing recording-follow rationale already in that file's comments.
- **`RideMap`** (`lib/src/ui/components/ride_map.dart`):
- Add `final ll.LatLng? ambientPosition;` to the widget's fields (with a doc comment
explaining it's only meaningful when `points` is empty — a recorded path always
takes priority) and thread it through the constructor.
- In `build()`, change the `bounds == null` branch to use it:
```dart
initialCenter: bounds == null
? (widget.ambientPosition ?? const ll.LatLng(0, 0))
: ll.LatLng(bounds.centerLat, bounds.centerLon),
initialZoom: bounds == null
? (widget.ambientPosition == null ? 2 : ambientZoom)
: (bounds.isDegenerate ? shortRideZoom : maxTileZoom),
```
(`initialCenter`/`initialZoom` are read once at `FlutterMap` construction by
flutter_map — this only fixes the *first* placement; live tracking needs the
`didUpdateWidget` change below.)
- In `didUpdateWidget`, extend the chase logic to fall back to ambient position when
there are no recorded points:
```dart
@override
void didUpdateWidget(RideMap old) {
super.didUpdateWidget(old);
if (!_following || _backgrounded) return;
if (widget.points.isNotEmpty) {
final last = widget.points.last;
WidgetsBinding.instance.addPostFrameCallback((_) {
if (!mounted || !_following) return;
_controller.move(ll.LatLng(last.latitude, last.longitude), _controller.camera.zoom);
});
} else if (widget.ambientPosition != null &&
widget.ambientPosition != old.ambientPosition) {
WidgetsBinding.instance.addPostFrameCallback((_) {
if (!mounted || !_following) return;
_controller.move(widget.ambientPosition!, _controller.camera.zoom);
});
}
}
```
- `showLocationMarker`: today it only ever reads `widget.points.last`
(`MarkerLayer`/`PulsingLocationMarker` at ~lines 275-289). Add a fallback so the
pulsing marker also shows on ambient position:
```dart
if (widget.showLocationMarker && (hasPoints || widget.ambientPosition != null))
MarkerLayer(
markers: [
Marker(
key: const Key('location-marker'),
point: hasPoints
? ll.LatLng(widget.points.last.latitude, widget.points.last.longitude)
: widget.ambientPosition!,
width: 40,
height: 40,
child: const PulsingLocationMarker(),
),
],
),
```
- **Permission prompt timing**: `LocationSource.start()` requests permission if not
already granted (`_ensurePermission` in `geolocator_location_source.dart`). This means
the very first time a user opens the app (or opens the Map tab) they may see a real OS
location-permission dialog before ever pressing Start — this is intentional per the
feedback ("even if they aren't recording... it will update just like Google Maps") and
matches how a real maps app behaves. A denial must degrade gracefully: catch
`LocationException` and yield `null` (already shown above) so the map simply falls
back to today's `(0,0)`/zoom-2 behavior rather than crashing or showing an error.
## Implementation
1. Add `ambientZoom` constant to `ride_map.dart`.
2. Add `ambientPosition` field + constructor param to `RideMap`.
3. Update the `bounds == null` branch of `initialCenter`/`initialZoom`.
4. Update `didUpdateWidget` to chase `ambientPosition` when there are no recorded
points.
5. Update the `showLocationMarker` `MarkerLayer` to fall back to `ambientPosition`.
6. Add `ambientPositionProvider` to `lib/src/app/providers.dart`.
7. Wire it into `ShellScaffold.build()` in `app_shell.dart`, gated on `trip == null`.
## Acceptance criteria
- [ ] With no active trip and no ambient fix yet available, the background map still
falls back to today's `(0,0)`/zoom-2 (no regression / no crash on first frame
before permission resolves).
- [ ] Once an ambient fix arrives (simulated via `FakeLocationSource.emit` in tests),
the background map centers on it at `ambientZoom` (street level) and continues to
re-center as new fixes arrive, exactly like the existing recorded-path chase
behavior.
- [ ] A manual pan/pinch on the Map tab cancels ambient following the same way it
cancels recording-follow today (reuses `_following`/`onPositionChanged` unchanged).
- [ ] The moment a trip starts recording, the background map switches to following the
trip's own recorded points (unchanged priority — `hasPoints` already wins in
`didUpdateWidget`), and `ShellScaffold` stops watching `ambientPositionProvider`
entirely (`trip == null` gate) so there's no duplicate GPS consumer during a ride.
- [ ] Turning `mapEnabledProvider` off stops the ambient GPS subscription (via the
provider's own `ref.watch(mapEnabledProvider)` short-circuit) — no tile or
location request fires while the map is disabled, matching the existing map-toggle
guarantee.
- [ ] `LocationSource.stop()` is never called by any code this ticket adds — grep the
diff to confirm.
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
## Tests
- Widget test: `ShellScaffold` with `activeTripProvider` returning `null` and
`ambientPositionProvider` overridden/fed a fake `LocationFix` — assert the background
`RideMap`'s effective camera center/zoom reflects the ambient fix (read via
`MapController.camera` or by asserting the constructor args passed to `RideMap`,
whichever is more direct given the existing shell test patterns in
`test/widget_test.dart`'s `shell nav bar` group).
- Widget test: manually panning the map while an ambient fix is active stops further
auto-recentering on subsequent fixes (mirrors any existing recording-follow-cancel
test, if one exists — check `test/widget_test.dart`/`test/ride_map_test.dart`).
- Widget/provider test: `ambientPositionProvider` never calls `LocationSource.stop()` —
drive a `FakeLocationSource`, dispose the provider (e.g. via `container.dispose()` in
a `ProviderContainer`-based test), and assert `fakeSource.stopCalls == 0`.
- Widget test: `activeTripProvider` returning a non-null trip means the shell never
reads from `ambientPositionProvider` (e.g. assert no permission/`start()` call
happens when a trip is active and the fake source's `startCalls` was already
incremented by the recording path only).
- Existing `RideMap`/shell background tests continue to pass unmodified except where
they need a new `ambientPosition: null` default (should be a no-op given it's an
optional/nullable constructor param).
## Risks
- **Battery**: `ambientPositionProvider` calling `start()` means GPS may run continuously
any time the Map tab (or the app in general, given the shell map is always mounted)
is open and the map is enabled, even with no ride ever recorded. This ticket
deliberately does not add a "stop after N minutes idle" or reference-counted shutdown
— that's a real product decision better made with actual battery data, not guessed at
here. Flag it in the Outcome section rather than solving it silently.
- **Permission timing**: as noted above, this may surface a permission dialog earlier
in the app's lifecycle than before (first Map-tab view rather than first Start press).
Confirmed intentional per the feedback; call out if it feels wrong in practice.
## Out of scope
Route Planner's own map (it manages its own `FlutterMap` directly, not through
`RideMap`) — covered by FB-04, which should reuse `ambientZoom` from this ticket for
zoom-consistency but does not need ambient location following (Route Planner already
centers on the route's own waypoints, which is correct). HUD widget changes (FB-03).
The idle Speed panel (FB-02) — unrelated, but note FB-02's idle screen will now show a
genuinely live, moving map underneath once this ticket lands, which is the whole point.

193
docs/feedback/FB-02-hide-idle-speed-panel.md Normal file
View File

@@ -0,0 +1,193 @@
# FB-02 — Hide the idle Speed panel until recording starts
**Depends on** — · **Size** S · **Status** Not started
## 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.

603
docs/feedback/FB-03-grid-snapped-hud-widgets.md Normal file
View File

@@ -0,0 +1,603 @@
# FB-03 — Grid-snapped HUD widgets with auto-fit, centered text
**Depends on** — · **Size** L · **Status** Not started
## Goal
Rework the customizable HUD telemetry widgets (built in UI-04) from free-form fractional
positioning into an Android-home-screen-style snapped grid — drag and resize stick to
grid cells, a placement that would overlap another widget is rejected rather than
silently stacking — and make each widget's label/value text scale to fill its own
current size instead of a fixed font that can overflow when small or look lost when
large.
## Context
Direct user feedback (`docs/FEEDBACK.md`, "Map Page" section):
> Speed and other widgets should only appear when recording starts. When toggling off
> different statistics, the flow and arrangement of the widgets goes crazy. This should
> be "drag and drop" but they stick to a grid much like the home screen on android, and
> they can be resizable like android widgets too. Make sure the font and values are
> centered in the widgets as well, and scale to the size of the widget (no over or
> underflow).
(The "only appear when recording starts" half is FB-02, already scoped separately —
this ticket is the grid/drag/resize/text-fit half.)
### Today's model — free-form fractions, no grid at all
`lib/src/hud/hud_widget_layout.dart` (full file, 122 lines):
```dart
const double hudMinWidthFraction = 0.20;
const double hudMaxWidthFraction = 0.70;
const double hudMinHeightFraction = 0.08;
const double hudMaxHeightFraction = 0.40;
class HudWidgetLayout {
const HudWidgetLayout({
required this.metric,
required this.x,
required this.y,
required this.width,
required this.height,
required this.visible,
});
final HudMetric metric;
final double x; // top-left, fraction of the HUD area
final double y;
final double width;
final double height;
final bool visible;
HudWidgetLayout copyWith({double? x, double? y, double? width, double? height, bool? visible}) => ...;
HudWidgetLayout clamped() {
final clampedWidth = width.clamp(hudMinWidthFraction, hudMaxWidthFraction);
final clampedHeight = height.clamp(hudMinHeightFraction, hudMaxHeightFraction);
final clampedX = x.clamp(0.0, 1.0 - clampedWidth);
final clampedY = y.clamp(0.0, 1.0 - clampedHeight);
return HudWidgetLayout(metric: metric, x: clampedX, y: clampedY, width: clampedWidth, height: clampedHeight, visible: visible);
}
Map<String, dynamic> toJson() => {'x': x, 'y': y, 'width': width, 'height': height, 'visible': visible};
static HudWidgetLayout fromJson(HudMetric metric, Map<String, dynamic> json) {
try {
return HudWidgetLayout(metric: metric, x: (json['x'] as num).toDouble(), y: (json['y'] as num).toDouble(),
width: (json['width'] as num).toDouble(), height: (json['height'] as num).toDouble(), visible: json['visible'] as bool).clamped();
} catch (_) {
return HudWidgetLayout.defaultFor(metric);
}
}
factory HudWidgetLayout.defaultFor(HudMetric metric) {
const columns = 4;
const cellWidth = 0.22;
const cellHeight = 0.12;
const gap = 0.02;
final index = HudMetric.values.indexOf(metric);
final row = index ~/ columns;
final col = index % columns;
return HudWidgetLayout(
metric: metric,
x: 0.02 + col * (cellWidth + gap),
y: 0.06 + row * (cellHeight + gap),
width: cellWidth,
height: cellHeight,
visible: index < columns, // only the first 4 (Speed/Avg Speed/Dist/Time) start visible
);
}
}
```
`HudMetric.values` order (`lib/src/hud/hud_metric.dart`, load-bearing — `defaultFor` uses
this indexing directly): `speed, avgSpeed, distance, elapsedTime, maxSpeed, movingTime,
elevationGain, pointsCaptured` (8 total; first 4 start visible).
`lib/src/hud/hud_layout_controller.dart` (full file, 52 lines) — the in-memory,
authoritative layout during an edit session, persisted to `Config` only on exit:
```dart
class HudLayoutController extends StateNotifier<Map<HudMetric, HudWidgetLayout>> {
HudLayoutController(this._config)
: super(_config?.hudLayout ?? {for (final m in HudMetric.values) m: HudWidgetLayout.defaultFor(m)});
final Config? _config;
void updatePosition(HudMetric metric, double x, double y) {
final current = state[metric];
if (current == null) return;
state = {...state, metric: current.copyWith(x: x, y: y).clamped()};
}
void updateSize(HudMetric metric, double width, double height) {
final current = state[metric];
if (current == null) return;
state = {...state, metric: current.copyWith(width: width, height: height).clamped()};
}
void setVisible(HudMetric metric, bool visible) {
final current = state[metric] ?? HudWidgetLayout.defaultFor(metric);
state = {...state, metric: current.copyWith(visible: visible)};
}
Future<void> persist() async => _config?.setHudLayout(state);
}
```
**Why "toggling widgets goes crazy" happens today**: `Config.hudLayout`'s getter
(`lib/src/config/config.dart` ~lines 95-113) always returns an entry for *every*
`HudMetric`, falling back to `HudWidgetLayout.defaultFor` for any metric never
explicitly saved — so `setVisible`'s `state[metric] ?? defaultFor(metric)` fallback is
essentially dead code; every metric already has a stored layout the moment the
controller exists. That stored layout is whatever it was the last time that metric was
visible (or its untouched `defaultFor` position if it's never been shown/moved). Nothing
about `setVisible`, `defaultFor`, or `clamped()` is aware of what other widgets
currently occupy — `clamped()` only keeps a single widget's own rectangle inside the
0.0–1.0 area, it has no concept of a sibling. So: drag widget A to overlap where widget
B's (currently hidden) default position sits, then re-enable B in Settings — B pops up
directly on top of A. Resize A larger (up to `hudMaxWidthFraction = 0.70`) and it can
swallow B's or C's default slot the same way. This reads as "the whole layout goes
crazy" even though, strictly, no *other* widget's own stored position ever actually
changes — the illusion is caused by newly-visible widgets landing on top of
already-visible ones with zero collision awareness.
### Drag/resize gesture wiring (continuous, per-frame, into shared state)
`lib/src/ui/components/draggable_resizable_hud_widget.dart` (`_DraggableResizableHudWidgetState.build`,
~lines 54-132) computes pixel position/size from `widget.layout`'s fractions × `widget.areaSize`
every build, and the drag/resize gesture handlers call the parent's callbacks **on every
frame of movement**, not just at gesture end:
```dart
onLongPressMoveUpdate: (details) {
widget.onMoved(
layout.x + details.offsetFromOrigin.dx / widget.areaSize.width,
layout.y + details.offsetFromOrigin.dy / widget.areaSize.height,
);
},
...
onPanUpdate: (details) { // the resize handle
widget.onResized(
layout.width + details.delta.dx / widget.areaSize.width,
layout.height + details.delta.dy / widget.areaSize.height,
);
},
```
`onMoved`/`onResized` are wired straight to `HudLayoutController.updatePosition`/
`updateSize` in `lib/src/ui/components/hud_edit_overlay.dart` (~lines 62-69):
```dart
DraggableResizableHudWidget(
key: ValueKey(entry.key),
layout: entry.value,
areaSize: areaSize,
editing: _editing,
onMoved: (x, y) => notifier.updatePosition(entry.key, x, y),
onResized: (w, h) => notifier.updateSize(entry.key, w, h),
child: widget.metricBuilder(context, entry.key),
),
```
So every pixel of drag movement recomputes and commits a new fraction to the shared
`StateNotifier` (though `persist()` to `SharedPreferences` still only happens on exit —
that part is fine and unchanged). This continuous-commit model does not translate
directly to a grid: an integer `col`/`row` can't represent "40% of the way toward the
next cell," so this ticket changes drag/resize to track a **local, uncommitted pixel
offset** during the gesture and only calls the parent callback once, with final snapped
grid coordinates, at gesture end. See Design below.
### Fixed-size text, no auto-fit
`lib/src/ui/record/record_screen.dart`, `_HudMetricValue.build()` (~lines 321-346):
```dart
@override
Widget build(BuildContext context) {
final colors = Theme.of(context).colorScheme;
return Column(
mainAxisSize: MainAxisSize.min,
children: [
Text(
metric.label.toUpperCase(),
style: TextStyle(fontSize: 10, letterSpacing: 1, color: colors.onSurfaceVariant),
maxLines: 1,
overflow: TextOverflow.ellipsis,
),
const SizedBox(height: 4),
Text(
_value,
style: monoDigits.copyWith(fontSize: 18, fontWeight: FontWeight.bold, color: _valueColor(colors)),
maxLines: 1,
overflow: TextOverflow.ellipsis,
),
],
);
}
```
Font sizes are hardcoded regardless of the widget's actual current size — at the
smallest allowed size this can ellipsize; at the largest allowed size the same small
text looks lost in a big glass panel. `DraggableResizableHudWidget` just centers
whatever child it's given (`GlassPanel(child: Center(child: widget.child))`,
~line 64) — no text-scaling logic exists anywhere in this chain.
### Persistence — no migration needed
`HudWidgetLayout.fromJson`'s existing `try { ... } catch (_) { return
HudWidgetLayout.defaultFor(metric); }` already handles a field-shape change for free: an
old saved JSON blob has `x`/`y`/`width`/`height` keys (doubles); this ticket's new
`fromJson` reads `col`/`row`/`colSpan`/`rowSpan` (ints) and will throw on old data
(missing key, or a cast failure), falling back to `defaultFor` automatically — no schema
version, no explicit migration code. Confirmed reasonable: this is local
`SharedPreferences`, not a shared/synced format, and there's no production data to
preserve.
## Design
### Grid model
Fixed **4 columns × 8 rows** spanning the HUD area (matches the current 4-column
default-row assumption and gives ample vertical room for the 8-metric list). Replace
`HudWidgetLayout`'s `x`/`y`/`width`/`height` doubles with:
```dart
const int hudGridColumns = 4;
const int hudGridRows = 8;
const int hudMinSpan = 1;
const int hudMaxColSpan = 4;
const int hudMaxRowSpan = 3;
class HudWidgetLayout {
const HudWidgetLayout({
required this.metric,
required this.col,
required this.row,
required this.colSpan,
required this.rowSpan,
required this.visible,
});
final HudMetric metric;
final int col;
final int row;
final int colSpan;
final int rowSpan;
final bool visible;
...
}
```
- **`clampedToGrid()`** replaces `clamped()` — a *self-contained* bounds check with no
awareness of siblings (kept as its own method because `fromJson` and
`defaultFor`/`nextFreeSlot` all still need "is this rectangle even inside the grid"
independent of collision):
```dart
HudWidgetLayout clampedToGrid() {
final clampedColSpan = colSpan.clamp(hudMinSpan, hudMaxColSpan);
final clampedRowSpan = rowSpan.clamp(hudMinSpan, hudMaxRowSpan);
final clampedCol = col.clamp(0, hudGridColumns - clampedColSpan);
final clampedRow = row.clamp(0, hudGridRows - clampedRowSpan);
return HudWidgetLayout(metric: metric, col: clampedCol, row: clampedRow,
colSpan: clampedColSpan, rowSpan: clampedRowSpan, visible: visible);
}
```
- **Collision helper**, a static/top-level function usable by both the layout file and
the controller:
```dart
bool hudRectsOverlap(HudWidgetLayout a, HudWidgetLayout b) {
final aColEnd = a.col + a.colSpan;
final aRowEnd = a.row + a.rowSpan;
final bColEnd = b.col + b.colSpan;
final bRowEnd = b.row + b.rowSpan;
return a.col < bColEnd && aColEnd > b.col && a.row < bRowEnd && aRowEnd > b.row;
}
```
- **`defaultFor`/`nextFreeSlot`**: keep `defaultFor(metric)` for the *static* initial
layout (unchanged conceptually — still purely a function of the metric's own index,
no runtime state needed, since the 8 fixed default slots below never overlap each
other by construction):
```dart
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,
visible: index < columns,
);
}
```
(Speed/AvgSpeed/Distance/ElapsedTime → row 0, cols 0-3, visible; MaxSpeed/MovingTime/
ElevationGain/PointsCaptured → row 2, cols 0-3, hidden — 8 distinct, non-overlapping
slots, same invariant the existing `defaultFor` test already checks.)
Add a **new** function for the actual re-enable-without-collision fix:
```dart
/// Scans row-major from (0,0) for the first colSpan×rowSpan slot that doesn't
/// overlap any `visible` entry in [occupied]. Falls back to (0,0) unconditionally
/// if the grid is fully packed -- a stacked default is better than a crash or an
/// exception the rider can't do anything about.
static HudWidgetLayout nextFreeSlot(
HudMetric metric, {
required Map<HudMetric, HudWidgetLayout> occupied,
int colSpan = 1,
int rowSpan = 2,
}) {
final candidate = (int col, int row) => HudWidgetLayout(
metric: metric, col: col, row: row, colSpan: colSpan, rowSpan: rowSpan, visible: true,
);
for (var row = 0; row <= hudGridRows - rowSpan; row++) {
for (var col = 0; col <= hudGridColumns - colSpan; col++) {
final c = candidate(col, row);
final overlapsAny = occupied.values
.where((l) => l.visible && l.metric != metric)
.any((other) => hudRectsOverlap(c, other));
if (!overlapsAny) return c;
}
}
return candidate(0, 0);
}
```
### Controller — collision-aware position/size updates, free-slot re-enable
`lib/src/hud/hud_layout_controller.dart`:
```dart
void updatePosition(HudMetric metric, int col, int row) {
final current = state[metric];
if (current == null) return;
final candidate = current.copyWith(col: col, row: row).clampedToGrid();
if (_overlapsAnyOther(metric, candidate)) return; // reject: no-op, widget stays put
state = {...state, metric: candidate};
}
void updateSize(HudMetric metric, int colSpan, int rowSpan) {
final current = state[metric];
if (current == null) return;
final candidate = current.copyWith(colSpan: colSpan, rowSpan: rowSpan).clampedToGrid();
if (_overlapsAnyOther(metric, candidate)) return;
state = {...state, metric: candidate};
}
bool _overlapsAnyOther(HudMetric metric, HudWidgetLayout candidate) => state.values
.where((l) => l.visible && l.metric != metric)
.any((other) => hudRectsOverlap(candidate, other));
void setVisible(HudMetric metric, bool visible) {
var current = state[metric] ?? HudWidgetLayout.defaultFor(metric);
if (visible && _overlapsAnyOther(metric, current.copyWith(visible: true))) {
// The metric's own saved/default slot is now occupied by something else (the
// "goes crazy" bug this ticket exists to fix) -- find a genuinely free one
// instead of popping up on top of whatever's there.
current = HudWidgetLayout.nextFreeSlot(
metric, occupied: state, colSpan: current.colSpan, rowSpan: current.rowSpan,
);
}
state = {...state, metric: current.copyWith(visible: visible)};
}
```
**"Reject and no-op" is the collision rule for drag/resize** — simplest correct
behavior, and it's the same operation as "revert to last valid" here since `state` is
never mutated until the candidate passes the check (there's nothing to revert *from*,
the old value was never overwritten). A drag/resize that would overlap another visible
widget simply has no effect for that frame/gesture; the widget stays at its last valid
position. `setVisible` gets the one exception — it actively finds a free slot rather
than rejecting, since "the metric just doesn't turn on" would be a much worse user
experience than briefly landing somewhere else on the grid.
### Drag/resize gesture — snap on gesture end, not live
`lib/src/ui/components/draggable_resizable_hud_widget.dart`: track a **local, transient
pixel offset** in `_DraggableResizableHudWidgetState` during the gesture (not committed
to the controller), paint the widget at `layout position (from grid) + local offset`
so the drag still feels smooth and continuous under the finger — then on
`onLongPressEnd`/`onPanEnd`, convert the final raw pixel position/size to the nearest
grid cell and call `widget.onMoved`/`onResized` exactly once with the snapped integer
coordinates, then reset the local offset to zero.
```dart
class _DraggableResizableHudWidgetState extends State<DraggableResizableHudWidget> {
bool _grabbed = false;
Offset _dragOffset = Offset.zero; // pixels, uncommitted, live during a move gesture
Size _resizeDelta = Size.zero; // pixels, uncommitted, live during a resize gesture
double get _cellWidth => widget.areaSize.width / hudGridColumns;
double get _cellHeight => widget.areaSize.height / hudGridRows;
@override
Widget build(BuildContext context) {
final layout = widget.layout;
final left = layout.col * _cellWidth + _dragOffset.dx;
final top = layout.row * _cellHeight + _dragOffset.dy;
final width = layout.colSpan * _cellWidth + _resizeDelta.width;
final height = layout.rowSpan * _cellHeight + _resizeDelta.height;
...
onLongPressMoveUpdate: (details) => setState(() => _dragOffset = details.offsetFromOrigin),
onLongPressEnd: (_) {
final snappedCol = ((layout.col * _cellWidth + _dragOffset.dx) / _cellWidth).round();
final snappedRow = ((layout.row * _cellHeight + _dragOffset.dy) / _cellHeight).round();
widget.onMoved(snappedCol, snappedRow);
setState(() { _grabbed = false; _dragOffset = Offset.zero; });
},
onLongPressCancel: () => setState(() { _grabbed = false; _dragOffset = Offset.zero; }),
...
// resize handle:
onPanUpdate: (details) => setState(() =>
_resizeDelta = Size(_resizeDelta.width + details.delta.dx, _resizeDelta.height + details.delta.dy)),
onPanEnd: (_) {
final snappedColSpan = ((layout.colSpan * _cellWidth + _resizeDelta.width) / _cellWidth).round();
final snappedRowSpan = ((layout.rowSpan * _cellHeight + _resizeDelta.height) / _cellHeight).round();
widget.onResized(snappedColSpan, snappedRowSpan);
setState(() => _resizeDelta = Size.zero);
},
```
If the parent rejects the move/resize (collision), the widget simply rebuilds at its
unchanged `layout` — since `_dragOffset`/`_resizeDelta` are reset to zero right after
calling `onMoved`/`onResized` regardless of whether the controller accepted it, the
widget visually snaps back to wherever it actually ended up (its last accepted grid
cell) the instant the gesture ends. No separate "was it accepted?" callback needed.
`widget.onMoved`/`onResized` function signatures change to `void Function(int col, int
row)` / `void Function(int colSpan, int rowSpan)`.
`lib/src/ui/components/hud_edit_overlay.dart`: update the two callback wire-ups
(`onMoved: (col, row) => notifier.updatePosition(entry.key, col, row)`, `onResized:
(colSpan, rowSpan) => notifier.updateSize(entry.key, colSpan, rowSpan)`) — no other
change needed in this file; `areaSize` is still the raw pixel `Size` from its own
`LayoutBuilder`, cell-size division now happens inside
`DraggableResizableHudWidget` as shown above.
### Text auto-fit — `FittedBox`, not a computed font size
`lib/src/ui/record/record_screen.dart`, `_HudMetricValue.build()`: wrap the existing
label+value `Column` in `FittedBox(fit: BoxFit.scaleDown)`, drop `maxLines`/
`TextOverflow.ellipsis` on both `Text`s (structurally impossible to overflow once
`FittedBox` owns sizing), and keep the existing `fontSize: 10`/`18` values as the
"reference" size `FittedBox` scales down from — the ratio between label and value size
is preserved automatically as it scales:
```dart
@override
Widget build(BuildContext context) {
final colors = Theme.of(context).colorScheme;
return FittedBox(
fit: BoxFit.scaleDown,
child: Column(
mainAxisSize: MainAxisSize.min,
children: [
Text(
metric.label.toUpperCase(),
style: TextStyle(fontSize: 10, letterSpacing: 1, color: colors.onSurfaceVariant),
),
const SizedBox(height: 4),
Text(
_value,
style: monoDigits.copyWith(fontSize: 18, fontWeight: FontWeight.bold, color: _valueColor(colors)),
),
],
),
);
}
```
`FittedBox` only scales *down* (`BoxFit.scaleDown` never enlarges past the reference
size) — so a widget resized to the grid's maximum span (4 cols × 3 rows) shows text at
its natural 10/18px reference size, not blown up to fill the space. That's an
intentional, reasonable simplification for this ticket (matching "no overflow" exactly
as asked; "looks proportionally larger in a bigger widget" is a nice-to-have, not named
in the feedback) — call this trade-off out plainly in the Outcome section rather than
silently under-delivering on it.
`DraggableResizableHudWidget`'s own centering (`GlassPanel(child: Center(child:
widget.child))`) already satisfies "centered" — no change needed there.
## Implementation
1. Rewrite `lib/src/hud/hud_widget_layout.dart`: grid constants, `col`/`row`/`colSpan`/
`rowSpan` fields, `clampedToGrid()`, `hudRectsOverlap()`, updated `toJson`/
`fromJson`, updated `defaultFor`, new `nextFreeSlot`.
2. Rewrite `lib/src/hud/hud_layout_controller.dart`: `updatePosition`/`updateSize` take
ints and reject-on-collision; `setVisible` uses `nextFreeSlot` when the metric's
current slot collides.
3. Rewrite `DraggableResizableHudWidget`: local pixel-offset drag state, snap-on-end,
updated callback signatures, pixel math from `areaSize / hudGridColumns` /
`hudGridRows`.
4. Update `HudEditOverlay`'s two callback closures to the new int signatures.
5. Wrap `_HudMetricValue`'s `Column` in `FittedBox(fit: BoxFit.scaleDown)`, drop
`maxLines`/`overflow` on its two `Text`s.
6. `flutter analyze`, fix any resulting type errors elsewhere that referenced the old
`x`/`y`/`width`/`height` fields (grep for `.x`/`.y`/`.width`/`.height` on
`HudWidgetLayout` instances across `lib/` and `test/` to be thorough).
## Acceptance criteria
- [ ] Dragging a HUD widget in edit mode snaps to a grid cell on release; the widget
tracks the finger smoothly during the drag itself (no per-frame jump/snap while
still moving).
- [ ] Resizing via the handle snaps to whole grid cells on release, same smooth-during-
drag behavior.
- [ ] Dragging or resizing a widget onto a cell already occupied by another *visible*
widget rejects the move — the widget returns to its last valid position/size,
no crash, no silent overlap.
- [ ] Re-enabling a hidden metric whose saved/default slot is now occupied by another
visible widget places it in the next free grid cell instead of stacking on top of
the occupier — this is the concrete fix for the "goes crazy" feedback.
- [ ] Re-enabling a hidden metric whose slot is still free keeps its exact prior
position (unchanged from today's guarantee, still worth re-verifying under the
new model).
- [ ] HUD widget text (label + value) never overflows or gets ellipsized at any allowed
grid size, and stays visually centered within its widget, at both the smallest
(`1×1`) and largest (`hudMaxColSpan × hudMaxRowSpan`) allowed sizes.
- [ ] An old-shaped saved layout (pre-this-ticket JSON with `x`/`y`/`width`/`height`
keys) loads without crashing — falls back to `defaultFor` per metric, exactly as
`fromJson`'s existing try/catch already guarantees.
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
## Tests
`test/hud_widget_layout_test.dart` needs a full rewrite for the new field names/types —
keep the same test *intents*, translated to grid coordinates:
- JSON round trip (encode/decode a `col`/`row`/`colSpan`/`rowSpan`/`visible` layout
exactly).
- A malformed/missing-field JSON entry falls back to `defaultFor` (unchanged intent,
new field names in the malformed input).
- `defaultFor`: every metric gets a distinct (non-overlapping, via `hudRectsOverlap`)
default rect; the first four metrics start visible, the rest hidden (unchanged
intent).
- `clampedToGrid`: a position/span pushed outside `[0, hudGridColumns)`/
`[0, hudGridRows)` or `[hudMinSpan, hudMaxColSpan/RowSpan]` is corrected back inside
(parallel structure to the old `clamped` tests: past the right/bottom edge, past the
left/top edge, below the legibility floor, above the ceiling, shrink-before-reposition,
already-valid-is-unchanged).
- New: `hudRectsOverlap` — two identical rects overlap; two rects sharing only an edge
(touching, not overlapping) do not; two disjoint rects don't; a rect fully containing
another does.
- New: `nextFreeSlot` — given a set of `occupied` visible layouts that collide with a
metric's own `defaultFor` position, returns some other rect that doesn't collide with
any of them; given an empty/all-hidden `occupied` map, returns the same rect
`defaultFor` would have (or at least a valid non-colliding one at `(0,0)`).
`test/hud_layout_controller_test.dart` needs a parallel rewrite for the new int
signatures and the new collision behavior:
- `updatePosition`/`updateSize` update only the given metric, still true.
- New: `updatePosition` to a cell already occupied by another visible metric is a
no-op (state for the target metric is unchanged).
- New: `updateSize` that would make a widget overlap a sibling is a no-op.
- `setVisible(false)` then `setVisible(true)` restores the last position **when that
position is still free** (keep this test, using coordinates guaranteed not to
collide with anything else default-visible).
- New: `setVisible(true)` when the metric's last position now collides with another
visible widget places it somewhere else instead (assert the resulting `state[metric]`
doesn't overlap anything, not a specific coordinate).
- `persist`/pre-persist-not-visible-to-Config tests carry over unchanged in spirit
(just using int fields).
Widget-level: extend or add to `test/hud_edit_overlay_test.dart` — a drag gesture ending
over an occupied cell leaves the dragged widget's rendered position unchanged from
before the gesture; a resize past the point of colliding with a sibling leaves the
widget at its pre-gesture size. Also add a widget test putting `_HudMetricValue` (or a
representative HUD child) inside a very small `DraggableResizableHudWidget` (1×1 grid
cell) and a very large one (`hudMaxColSpan`×`hudMaxRowSpan`) and asserting no
`RenderFlex overflowed` exception/error is recorded by `FlutterError.onError` during
the pump (the standard way to assert "no overflow" in a widget test — check
`test/widget_test.dart`/other existing tests in this repo for the established pattern,
if any, otherwise use `tester.takeException()` after pumping and assert it's `null`).
## Risks
- `FittedBox(fit: BoxFit.scaleDown)` never enlarges text past its reference size, so a
widget resized to the grid's maximum span shows the same 10/18px reference text as a
1×1 widget, just with more empty space around it — not larger text filling the space.
This satisfies "no overflow" and "centered" exactly as asked; "text should grow to
fill a bigger widget" is not explicitly requested and is treated as future work, not a
silent gap — document this trade-off in the Outcome section.
- Existing saved HUD layouts (from anyone who ran a build before this ticket) reset to
defaults the first time they're loaded post-update, per the fromJson fallback. No
user-facing warning is added for this — acceptable given there's no real user base
yet; flag it anyway in Outcome for completeness.
## Out of scope
FB-02 (hiding the idle Speed panel — separate ticket, this one only touches the
recording-state HUD widgets themselves). Making widget text grow to actually fill a
larger grid span (see Risks). Any change to which metrics exist or their default
visibility set — `HudMetric`'s own enum and `label`s are untouched.

216
docs/feedback/FB-04-route-planner-map-render-fix.md Normal file
View File

@@ -0,0 +1,216 @@
# FB-04 — Fix Route Planner's map failing to render + match street-level zoom
**Depends on** FB-01 (reuses its `ambientZoom` constant) · **Size** M · **Status** Not started
## Goal
The Route Planner screen's map doesn't render at all for the user. Find the real cause
and fix it, and bring this screen's zoom in line with the rest of the app's new
street-level default (FB-01).
## Context
Direct user feedback (`docs/FEEDBACK.md`, "Route" section):
> This doesn't work at all, the map doesn't render at all... I want it to be the same
> zoomed in map view, then the user can zoom out and create the route.
### High-confidence root cause
`lib/src/ui/routes/route_planner_screen.dart`, `build()` (~lines 112-139):
```dart
@override
Widget build(BuildContext context) {
final colors = Theme.of(context).colorScheme;
final route = ref.watch(routePlanProvider(widget.routeId)).valueOrNull;
final waypointsAsync = ref.watch(routeWaypointsProvider(widget.routeId));
final waypoints = waypointsAsync.valueOrNull ?? const [];
final units = ref.watch(unitSystemProvider);
final repo = ref.read(routePlanRepositoryProvider);
if (route == null) {
return Scaffold(
backgroundColor: Colors.transparent,
body: SafeArea(
child: Center(
child: Text(
'This route no longer exists.',
style: TextStyle(color: colors.outline),
),
),
),
);
}
return Scaffold(
backgroundColor: Colors.transparent,
body: SafeArea(
child: !waypointsAsync.hasValue
? const Center(child: CircularProgressIndicator())
: Stack(
children: [
Positioned.fill(
child: FlutterMap(
...
```
Both `route` and `waypointsAsync` come from the same kind of provider —
`routePlanProvider`/`routeWaypointsProvider` are both `StreamProvider.autoDispose.family`
(`lib/src/app/providers.dart` ~lines 198-203) — so both start in `AsyncLoading` on a
fresh navigation (e.g. tapping "+ New route" on `RoutesListScreen`, which calls
`repo.createRoutePlan` then immediately `onOpenRoute?.call(id)` — a brand-new
`routePlanProvider(id)` family instance with a DB stream that hasn't delivered its
first row yet). **`waypointsAsync` is correctly guarded** — `!waypointsAsync.hasValue`
shows a spinner instead of building `FlutterMap` prematurely, with a comment explaining
exactly why (`initialCenter`/`initialZoom` are read once at construction; building too
early freezes the camera at null-island forever). **`route` has no equivalent guard** —
`.valueOrNull` collapses "still loading" and "genuinely doesn't exist" into the same
`null`, so on that same cold navigation, this screen renders the **text-only "This route
no longer exists." fallback with zero `FlutterMap` in the tree** — for a route that is
completely valid, simply because its stream's first emission hasn't arrived yet. This is
a real, reproducible-on-a-slow-cold-start bug, and it would read to a user as exactly
"the map doesn't render at all" — because for however long that loading window lasts,
there is no map, no pins, nothing to interact with.
This is the same shape of bug UI-06's own documented Outcome already found once in this
same file (a hardcoded, stale tile URL in `_DownloadDialog`): a place where near-
identical logic exists twice nearby and only one copy got the correct guard.
### Zoom special-cases to replace
Same `build()` method, ~line 157:
```dart
initialZoom: waypoints.length <= 1 ? 14 : maxTileZoom - 3,
```
`initialZoom` is only ever used by flutter_map when `initialCameraFit` is null.
`initialCameraFit: _initialFit(waypoints)` (line ~153) is non-null for 2+ waypoints
(see `_initialFit`, ~lines 92-102, which returns `null` only for `waypoints.length < 2`
or degenerate bounds) — so `maxTileZoom - 3` is **dead, misleading code today**: it can
only ever apply when `_initialFit` already returned null, which for 2+ waypoints only
happens on a degenerate (single-point) bounds, in which case `waypoints.length <= 1` is
false but the fit is still null — meaning `maxTileZoom - 3` actually *does* fire for
that one edge case (all waypoints at the same spot). Still, `14` for 0-1 waypoints is
the literal reason a fresh, empty route (or one with a single pin) opens zoomed out
far past street level, exactly matching the "same zoomed in map view" ask.
## Design
- **Fix the render bug**: change `route` from `.valueOrNull` to keeping the full
`AsyncValue<RoutePlan?>`, and add a `hasValue` guard mirroring `waypointsAsync`'s
existing pattern:
```dart
final routeAsync = ref.watch(routePlanProvider(widget.routeId));
final waypointsAsync = ref.watch(routeWaypointsProvider(widget.routeId));
final waypoints = waypointsAsync.valueOrNull ?? const [];
final units = ref.watch(unitSystemProvider);
final repo = ref.read(routePlanRepositoryProvider);
if (!routeAsync.hasValue) {
return const Scaffold(
backgroundColor: Colors.transparent,
body: Center(child: CircularProgressIndicator()),
);
}
final route = routeAsync.value;
if (route == null) {
return Scaffold( /* unchanged "This route no longer exists." body */ );
}
```
This exactly mirrors the two-step pattern `waypointsAsync` already uses two lines
below it in the same method, just applied consistently to both providers.
- **Zoom fix**: replace the `initialZoom` line with FB-01's shared constant. FB-01 adds
`const double ambientZoom = 17.0;` to `lib/src/ui/components/ride_map.dart` — this
screen already imports named constants from that file (`show TileAttribution,
maxTileZoom, tileMaxNativeZoom, tileSubdomains, tileUrlTemplate, tileUserAgent`, near
the top of the file) — add `ambientZoom` to that same `show` clause and use it:
```dart
initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3,
```
**If FB-01 has not landed yet when this ticket is implemented**, add the same
constant directly to `ride_map.dart` yourself first (`const double ambientZoom =
17.0;` near `maxTileZoom`/`shortRideZoom`) so this ticket doesn't block on ordering —
whichever of FB-01/FB-04 lands second will find the constant already defined and
should just reuse it rather than redefining it (grep for `ambientZoom` before adding
it, to avoid a duplicate-constant compile error).
- **Investigation order, if the above turns out not to be the whole story** (do this
first, before assuming the fix above is sufficient — reproduce, then fix):
1. `flutter run` on the Android emulator (or a device), tap "+ New route" from
`RoutesListScreen` (`lib/src/ui/routes/routes_list_screen.dart`, `Key('new-route')`
button), watch closely for a flash of "This route no longer exists." text or a
fully blank screen right after navigation.
2. If reproduced and the `hasValue` fix above resolves it, done — write up the root
cause and fix in the Outcome section.
3. If it's *not* resolved (map still doesn't render even with the `hasValue` guard in
place), add a temporary `debugPrint('route=${routeAsync.runtimeType} '
'waypoints=${waypointsAsync.runtimeType}')` at the top of `build()` and watch the
console across a fresh navigation to see the actual state sequence — remove it
before committing.
4. Check whether the map mounts (i.e. `FlutterMap` is in the tree, visible in the
Flutter inspector / a screenshot) but tiles themselves are blank — if so, look at
`cachedTileProviderProvider` (`lib/src/app/providers.dart` ~lines 234-238): it
returns `null` while `tileCacheProvider`'s underlying `FutureProvider` hasn't
resolved yet, and `TileLayer(tileProvider: null)` falls back to flutter_map's own
default network fetcher — verify that fallback actually fetches (it should; if it
silently doesn't, that's the bug).
5. Diff this screen's tile setup token-for-token against `RideMap`'s
(`tileUrlTemplate`, `tileSubdomains`, `tileMaxNativeZoom`, `tileUserAgent` — all
already imported from the same `ride_map.dart` export, so a stale-URL-style
regression is unlikely, but `grep -n "tile.openstreetmap\|cartocdn"` across this
file to be certain nothing reintroduced UI-06's already-fixed bug).
## Implementation
1. Change `route` to `routeAsync` (full `AsyncValue`), add the `hasValue` guard, keep
the rest of the "no longer exists" branch's body unchanged.
2. Add `ambientZoom` to this file's `ride_map.dart` import `show` clause (adding the
constant to `ride_map.dart` first if FB-01 hasn't landed yet — see Design).
3. Replace `waypoints.length <= 1 ? 14 : maxTileZoom - 3` with
`waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3`.
4. Reproduce on an emulator per the Investigation order above; if the root cause is
something other than the `hasValue` gap, document the real cause and its fix in the
Outcome section instead of (or in addition to) the above.
## Acceptance criteria
- [ ] Opening a brand-new route ("+ New route" from `RoutesListScreen`) always shows a
loading spinner briefly (if the stream hasn't emitted yet) and then the real map
canvas — never the "This route no longer exists." text for a route that
genuinely exists.
- [ ] Opening an existing, previously-created route with no waypoints shows the map
canvas at `ambientZoom` (street level), not zoom 14.
- [ ] A route that is actually deleted (or never existed — e.g. a stale/invalid id)
still correctly shows "This route no longer exists." — the fix must not weaken
this case, only stop it from firing on a merely-still-loading valid route.
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
## Tests
- Widget test: pump `RoutePlannerScreen` for a route id that has a pending (not-yet-
resolved) `routePlanProvider` stream — assert a `CircularProgressIndicator` shows,
never the "no longer exists" text, then once the stream emits the real row, assert
the map (`FlutterMap`) appears. (Check how existing tests in
`test/route_planner_screen_test.dart` seed/await the repository — likely via
`repo.createRoutePlan` then pumping a frame or two before the provider's stream
delivers, similar to the existing `pumpMap` helper in that file's own comments about
needing a handful of frames for the waypoints stream's first value.)
- Widget test: pump `RoutePlannerScreen` for an id that was never created (or was
deleted) — assert "This route no longer exists." still shows once the stream settles
(not immediately, if the provider briefly reports loading first).
- Widget test: a route with zero waypoints renders at `ambientZoom`, not `14` — read
the `FlutterMap`'s `options.initialZoom` via `tester.widget<FlutterMap>(...)` the same
way other zoom-related assertions in this test suite already inspect `MapOptions`
(check existing patterns in `test/ride_map_test.dart`/
`test/route_planner_screen_test.dart`).
- If the investigation reveals a different root cause than the `hasValue` gap, write a
regression test for the actual bug found, not just the hypothesis above.
## Risks
- If the `hasValue` fix doesn't fully explain what the user saw, this ticket's
Outcome section must say so plainly and document whatever the actual root cause
turned out to be — don't claim a fix that wasn't verified to address the real
symptom. Reproducing on a real emulator/device before declaring this done is
important precisely because the original bug report is "doesn't work at all," a
strong signal, and a subtle timing bug is an easy thing to fix on paper without
confirming it was actually the cause.
## Out of scope
Closed-loop routes (FB-05, sequenced after this ticket since it also edits this file
heavily). Turn-by-turn route following (V3-09, already explicitly deferred by UI-06).
Any change to `RoutesListScreen` itself.

411
docs/feedback/FB-05-closed-loop-routes.md Normal file
View File

@@ -0,0 +1,411 @@
# FB-05 — Closed-loop routes in Route Planner
**Depends on** FB-04 (same file, heavily edited — sequence after it merges) · **Size** M · **Status** Not started
## Goal
Route planning currently only supports a one-way A→B→C path. Add the ability to close
the route into a loop that returns to its starting pin, updating the drawn polyline and
the distance stat to include the closing segment.
## Context
Direct user feedback (`docs/FEEDBACK.md`, "Route" section):
> ...there is no way to create a loop with the pins, it only creates a unidirectional
> path.
### Data model — no loop concept exists today
`lib/src/domain/models.dart`, `RoutePlan` (~lines 229-276):
```dart
class RoutePlan {
const RoutePlan({
this.id = 0,
required this.name,
required this.createdAt,
this.activity = Activity.motorcycle,
this.distanceM = 0.0,
this.estimatedMillis,
this.geometry,
});
final int id;
final String name;
final int createdAt;
final Activity activity;
final double distanceM;
final int? estimatedMillis;
final String? geometry;
RoutePlan copyWith({
int? id, String? name, int? createdAt, Activity? activity,
double? distanceM, int? estimatedMillis, String? geometry,
}) => RoutePlan(
id: id ?? this.id, name: name ?? this.name, createdAt: createdAt ?? this.createdAt,
activity: activity ?? this.activity, distanceM: distanceM ?? this.distanceM,
estimatedMillis: estimatedMillis ?? this.estimatedMillis, geometry: geometry ?? this.geometry,
);
}
```
`Waypoint` (~lines 278-309) has an `ordinal` for ordering, and waypoints are only ever
appended at the end (`RoutePlanRepository.addWaypoint`, below) — there is no "return to
start" field or concept anywhere in the model.
### Polyline and distance — strictly open, no wraparound
`lib/src/ui/routes/route_planner_screen.dart`, the `PolylineLayer` (~lines 182-201):
```dart
if (waypoints.length >= 2)
PolylineLayer(
polylines: [
Polyline(
points: [
for (final w in waypoints) ll.LatLng(w.latitude, w.longitude),
],
strokeWidth: 4,
pattern: StrokePattern.dashed(segments: const [8, 6]),
color: colors.primary,
),
],
),
```
`lib/src/data/route_plan_repository.dart`, distance recomputation (full relevant
section):
```dart
Future<void> addWaypoint(int routeId, double latitude, double longitude) async {
final existing = await _db.waypointsForRoute(routeId);
await _db.insertWaypoint(
Waypoint(routeId: routeId, ordinal: existing.length, latitude: latitude, longitude: longitude),
);
await _recomputeDistance(routeId);
}
Future<void> _recomputeDistance(int routeId) async {
final waypoints = await _db.waypointsForRoute(routeId);
final distance = geo.pathLengthMeters([
for (final w in waypoints) geo.LatLon(w.latitude, w.longitude),
]);
await _db.setRoutePlanDistance(routeId, distance);
}
```
`geo.pathLengthMeters` (`lib/src/geo/geo.dart` ~line 185) sums consecutive-pair
distances generically over whatever point list it's given — it needs no changes itself,
just a point list that includes the closing segment when appropriate.
`FloatingPill`'s Distance/Est. Time/Pins stats read straight off `route.distanceM`/
`route.estimatedMillis`/`waypoints.length` (`route_planner_screen.dart` ~lines 280-292)
— Distance updates for free once `_recomputeDistance` accounts for the closing segment;
Pins and Est. Time are unaffected by this ticket.
### Schema — current version and migration pattern to follow
`lib/src/data/database.dart`:
```dart
@DataClassName('RoutePlanRow')
class RoutePlans extends Table {
@override
String get tableName => 'route_plans';
IntColumn get id => integer().autoIncrement()();
TextColumn get name => text()();
IntColumn get createdAt => integer()();
TextColumn get activity => textEnum<domain.Activity>().withDefault(const Constant('motorcycle'))();
RealColumn get distanceM => real().withDefault(const Constant(0))();
IntColumn get estimatedMillis => integer().nullable()();
TextColumn get geometry => text().nullable()();
}
@DriftDatabase(tables: [Trips, Segments, TrackPoints, RoutePlans, Waypoints])
class AppDatabase extends _$AppDatabase {
@override
int get schemaVersion => 3;
@override
MigrationStrategy get migration => MigrationStrategy(
onCreate: (m) => m.createAll(),
onUpgrade: (m, from, to) async {
if (from < 2) {
await m.addColumn(trips, trips.activity);
}
if (from < 3) {
await m.createTable(routePlans);
await m.createTable(waypoints);
}
},
...
```
The existing `if (from < 2) { await m.addColumn(trips, trips.activity); }` is the exact
pattern to follow for a new nullable/defaulted column.
`RoutePlanRepository` insert/conversion glue, in `database.dart`:
```dart
Future<int> insertRoutePlan(domain.RoutePlan route) => into(routePlans).insert(
RoutePlansCompanion.insert(
name: route.name, createdAt: route.createdAt, activity: Value(route.activity),
distanceM: Value(route.distanceM), estimatedMillis: Value(route.estimatedMillis),
geometry: Value(route.geometry),
),
);
Future<void> setRoutePlanDistance(int id, double distanceM) => (update(routePlans)
..where((r) => r.id.equals(id))).write(RoutePlansCompanion(distanceM: Value(distanceM)));
domain.RoutePlan _toRoutePlan(RoutePlanRow r) => domain.RoutePlan(
id: r.id, name: r.name, createdAt: r.createdAt, activity: r.activity,
distanceM: r.distanceM, estimatedMillis: r.estimatedMillis, geometry: r.geometry,
);
```
### Overflow menu — where the toggle belongs
`route_planner_screen.dart`, `_OverflowMenu` (~lines 493-530-ish) already hosts
Rename/Download/Delete as a `PopupMenuButton` inside a `GlassPanel`:
```dart
class _OverflowMenu extends StatelessWidget {
const _OverflowMenu({
required this.hasWaypoints, required this.onRename, required this.onDownload, required this.onDelete,
});
final bool hasWaypoints;
final VoidCallback onRename;
final VoidCallback? onDownload;
final VoidCallback onDelete;
@override
Widget build(BuildContext context) => GlassPanel(
borderRadius: const BorderRadius.all(Radius.circular(999)),
child: PopupMenuButton<String>(
key: const Key('route-overflow-menu'),
icon: const Icon(Icons.more_vert),
onSelected: (value) => switch (value) {
'rename' => onRename(),
'download' => onDownload?.call(),
'delete' => onDelete(),
_ => null,
},
itemBuilder: (context) => [
const PopupMenuItem(key: Key('rename-route'), value: 'rename', child: Text('Rename')),
...
```
And it's constructed at the call site (~lines 298-310):
```dart
_OverflowMenu(
hasWaypoints: waypoints.isNotEmpty,
onRename: () => _rename(context, repo, route),
onDownload: waypoints.isEmpty ? null : () => _downloadOfflineTiles(context, waypoints),
onDelete: () async {
await repo.deleteRoutePlan(widget.routeId);
widget.onBack?.call();
},
),
```
## Design
- **Schema**: add `BoolColumn get isClosedLoop => boolean().withDefault(const
Constant(false))();` to `RoutePlans` in `database.dart`. Bump `schemaVersion` from
`3` to `4`; add `if (from < 4) { await m.addColumn(routePlans, routePlans.isClosedLoop); }`
to `onUpgrade`.
- **Domain model**: add `final bool isClosedLoop;` to `RoutePlan` (default `false` in
the constructor), thread it through `copyWith`.
- **DB glue**: add `isClosedLoop: Value(route.isClosedLoop)` to `insertRoutePlan`'s
`RoutePlansCompanion.insert(...)` call, `isClosedLoop: r.isClosedLoop` to
`_toRoutePlan`, and a new method mirroring `setRoutePlanDistance`:
```dart
Future<void> setRoutePlanClosedLoop(int id, bool value) => (update(routePlans)
..where((r) => r.id.equals(id))).write(RoutePlansCompanion(isClosedLoop: Value(value)));
```
- **Repository**: add to `RoutePlanRepository`:
```dart
Future<void> setClosedLoop(int routeId, bool value) async {
await _db.setRoutePlanClosedLoop(routeId, value);
await _recomputeDistance(routeId); // the closing segment changes the total
}
```
Update `_recomputeDistance` to fetch the route's own flag and append the first
waypoint's coordinates when closed, before summing:
```dart
Future<void> _recomputeDistance(int routeId) async {
final route = await _db.getRoutePlan(routeId);
final waypoints = await _db.waypointsForRoute(routeId);
final points = [for (final w in waypoints) geo.LatLon(w.latitude, w.longitude)];
if (route?.isClosedLoop == true && points.length >= 2) {
points.add(points.first);
}
final distance = geo.pathLengthMeters(points);
await _db.setRoutePlanDistance(routeId, distance);
}
```
(`_recomputeDistance` is already called from every waypoint-mutating method —
`addWaypoint`, `moveWaypoint`, `deleteWaypoint`, `reorderWaypoint` — so the closing
segment is kept correct automatically as pins are edited, with no other call site
changes needed.)
- **UI toggle**: add a 4th item to `_OverflowMenu`, enabled only when there are enough
waypoints for a loop to mean anything (`hasWaypoints` alone isn't enough — need at
least 2 to form any segment at all; reuse the existing `hasWaypoints` bool but also
thread through whether there are `>= 2` waypoints, or simplify by passing a new
`canCloseLoop: waypoints.length >= 2` param):
```dart
class _OverflowMenu extends StatelessWidget {
const _OverflowMenu({
required this.hasWaypoints,
required this.canCloseLoop,
required this.isClosedLoop,
required this.onRename,
required this.onDownload,
required this.onToggleLoop,
required this.onDelete,
});
...
final bool canCloseLoop;
final bool isClosedLoop;
final VoidCallback onToggleLoop;
@override
Widget build(BuildContext context) => GlassPanel(
...
child: PopupMenuButton<String>(
...
onSelected: (value) => switch (value) {
'rename' => onRename(),
'download' => onDownload?.call(),
'closeLoop' => onToggleLoop(),
'delete' => onDelete(),
_ => null,
},
itemBuilder: (context) => [
const PopupMenuItem(key: Key('rename-route'), value: 'rename', child: Text('Rename')),
...
PopupMenuItem(
key: const Key('toggle-closed-loop'),
value: 'closeLoop',
enabled: canCloseLoop,
child: Text(isClosedLoop ? 'Open the loop' : 'Close the loop'),
),
...
],
),
);
}
```
Wire it at the call site:
```dart
_OverflowMenu(
hasWaypoints: waypoints.isNotEmpty,
canCloseLoop: waypoints.length >= 2,
isClosedLoop: route.isClosedLoop,
onRename: () => _rename(context, repo, route),
onDownload: waypoints.isEmpty ? null : () => _downloadOfflineTiles(context, waypoints),
onToggleLoop: () => repo.setClosedLoop(widget.routeId, !route.isClosedLoop),
onDelete: () async { ... },
),
```
- **Polyline**: append the closing point when the route is closed and there are at
least 2 waypoints:
```dart
if (waypoints.length >= 2)
PolylineLayer(
polylines: [
Polyline(
points: [
for (final w in waypoints) ll.LatLng(w.latitude, w.longitude),
if (route.isClosedLoop)
ll.LatLng(waypoints.first.latitude, waypoints.first.longitude),
],
strokeWidth: 4,
pattern: StrokePattern.dashed(segments: const [8, 6]),
color: colors.primary,
),
],
),
```
Same `Polyline` object, same styling — just one more point in the list, so the closing
segment renders with identical dashed styling to the rest of the route.
- **`FloatingPill`/`_initialFit`**: no changes needed — Distance updates automatically
via `_recomputeDistance`; Pins/Est. Time are correctly unaffected; `_initialFit`'s
bounds-fitting is based on waypoint positions regardless of whether the last segment
loops back (the closing point is always one of the existing waypoints' own
coordinates, already inside the fitted bounds).
## Implementation
1. Add `isClosedLoop` column to `RoutePlans` in `database.dart`, bump `schemaVersion`
to `4`, add the `onUpgrade` migration step.
2. Add `isClosedLoop` field to `RoutePlan` domain model + `copyWith`.
3. Add `isClosedLoop` to `insertRoutePlan`'s companion and `_toRoutePlan` in
`database.dart`; add `setRoutePlanClosedLoop`.
4. Add `RoutePlanRepository.setClosedLoop`; update `_recomputeDistance` to append the
closing point when `isClosedLoop`.
5. Extend `_OverflowMenu` with the new toggle item and params; wire it at the call
site in `route_planner_screen.dart`.
6. Extend the `PolylineLayer`'s point list with the conditional closing point.
## Acceptance criteria
- [ ] "Close the loop" appears in the overflow menu, disabled when there are fewer than
2 waypoints, enabled otherwise.
- [ ] Toggling it on redraws the polyline with a visible segment from the last waypoint
back to the first, same dashed styling as the rest of the route.
- [ ] Toggling it on increases the Distance stat by the length of the new closing
segment; toggling it back off returns Distance to the open-path total.
- [ ] The menu label reflects current state ("Close the loop" when open, "Open the
loop" when already closed).
- [ ] Adding/moving/deleting a waypoint on a closed-loop route keeps the closing segment
correct automatically (it's recomputed via the existing `_recomputeDistance` call
already present in every waypoint-mutating method).
- [ ] Deleting waypoints down to fewer than 2 while closed doesn't crash — the
`waypoints.length >= 2` guards in both the polyline and distance code make the
closing point simply disappear along with the rest of the route drawing, same as
today's behavior for an open path with 0-1 waypoints.
- [ ] A pre-existing (schema v3) database opens cleanly post-migration with
`isClosedLoop` defaulting to `false` on every existing route.
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
## Tests
- **Migration test** (`test/migration_test.dart`, following the exact pattern of the
existing `'a v2 database (V3-07) gains route_plans/waypoints and keeps its trips'`
test): seed a v3 database (v1 seed + `activity` column + `route_plans`/`waypoints`
tables created via raw SQL, `PRAGMA user_version = 3;`), open it with `AppDatabase`,
insert a route via `db.insertRoutePlan`, and assert `isClosedLoop` reads back
`false` by default, and that toggling it via `setRoutePlanClosedLoop` persists.
- **Repository tests** (`test/route_plan_repository_test.dart`, alongside the existing
`'adding waypoints appends in order and updates distance live'` etc.):
- `setClosedLoop(true)` on a route with 2+ waypoints increases `distanceM` by
exactly the closing segment's length (compute the expected delta directly with
`geo.pathLengthMeters`/a manual haversine call between the last and first
waypoint, matching how other distance tests in this file already assert exact
values).
- `setClosedLoop(false)` after `true` returns `distanceM` to the pre-toggle value.
- Adding a waypoint to an already-closed route keeps the distance correct
(recomputed including the new closing segment against the new last-added point,
not the old one).
- **Widget tests** (`test/route_planner_screen_test.dart`):
- The "Close the loop" menu item is disabled with 0-1 waypoints, enabled with 2+
(mirrors the existing "offline-tiles download menu item is disabled with no pins"
test's structure — open the overflow menu, inspect the `PopupMenuItem.enabled`
property via its key).
- Tapping it toggles `route.isClosedLoop` (assert via
`repo.routePlanById(id)?.isClosedLoop`, the same pattern the existing rename test
uses to verify writes through to the repository) and the label switches to "Open
the loop".
- With the loop closed, `find.byType(Polyline)` (or inspecting the `PolylineLayer`'s
`polylines` list directly, matching whatever existing pattern this test file uses
to inspect polyline data) has one more point than the waypoint count.
## Risks
- None significant — this is additive (new column, new optional toggle) and every
touched call site (`_recomputeDistance`) already runs on every mutation, so there's
no new code path that could silently skip recomputation.
## Out of scope
Turn-by-turn following of a closed loop (V3-09, already deferred). Any UI indication of
loop direction/rotation. Road-snapped routing for the closing segment (V3-08) — it stays
a straight line like every other segment in this ticket's scope.

41
docs/feedback/README.md Normal file
View File

@@ -0,0 +1,41 @@
# Post-launch feedback tickets
Same shape as `docs/ui-redesign/`: one file per ticket, Goal · Context · Design ·
Implementation · Acceptance criteria · Tests · Risks · Out of scope, written before
implementing, with an Outcome section appended after.
Source: `docs/FEEDBACK.md` — hands-on feedback after using the redesigned app. Turned
into 5 tickets, each independently completable (self-contained enough for a fresh
subagent with no prior context to implement correctly).
## The tickets
| # | Ticket | Size | Depends on | Status |
|---|---|---|---|---|
| [FB-01](FB-01-live-street-level-map.md) | Live, street-level map everywhere | L | — | Not started |
| [FB-02](FB-02-hide-idle-speed-panel.md) | Hide the idle Speed panel until recording starts | S | — | Not started |
| [FB-03](FB-03-grid-snapped-hud-widgets.md) | Grid-snapped HUD widgets with auto-fit text | L | — | Not started |
| [FB-04](FB-04-route-planner-map-render-fix.md) | Fix Route Planner's map failing to render + zoom | M | FB-01 (shared zoom constant) | Not started |
| [FB-05](FB-05-closed-loop-routes.md) | Closed-loop routes in Route Planner | M | FB-04 (same file) | Not started |
## Dependencies / dispatch order
```
Wave 1 (parallel): FB-01, FB-02, FB-03
Wave 2 (after Wave 1 lands): FB-04 — reuses FB-01's ambientZoom constant
Wave 3 (after Wave 2 lands): FB-05 — heavily edits the same file FB-04 just touched
```
FB-02 and FB-03 both touch `lib/src/ui/record/record_screen.dart`, but disjoint
regions (the idle-state `Column` vs. `_HudMetricValue`) — low conflict risk.
## Working method, per ticket
1. Implement against the ticket's own Implementation section.
2. `flutter analyze` clean, `flutter test` green — count must not regress (374 passing
before this batch starts).
3. Outcome section appended to the ticket file: what shipped, deviations, test counts.
4. Commit, status flipped to Done in both the ticket file and this table.
Android-emulator verification is best-effort only for this batch — lean on widget
tests as the primary bar; don't get stuck fighting emulator flakiness.