Files
rippr/docs/feedback/FB-01-live-street-level-map.md
uhryniuk 756e3a9f53 FB-01: ambient GPS position drives the shared background map while idle
Adds ambientPositionProvider (never calls LocationSource.stop(), only the
idempotent start()) and wires it into ShellScaffold whenever no trip is
active, so the persistent background map centers and follows the device's
live GPS position at street-level zoom instead of sitting at (0,0)/zoom 2
until a recording starts. RideMap gains an ambientPosition param that the
existing chase-camera/pan-cancel/location-marker logic falls back to
whenever there are no recorded points, with recorded points always taking
priority. Also brings in the docs/feedback ticket set (FB-01..FB-05, README,
FEEDBACK.md) that this worktree's branch point predated.

Adds 10 tests (374 -> 384): RideMap-level ambient centering/chase/pan-cancel/
marker coverage in test/ride_map_test.dart, plus shell-wiring and
provider-level (never-calls-stop, mapEnabledProvider-off) coverage in
test/widget_test.dart. flutter analyze remains clean.
2026-08-24 15:58:45 -05:00

384 lines
19 KiB
Markdown

# FB-01 — Live, street-level map everywhere
**Depends on** — · **Size** L · **Status** Done
## 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.
## Outcome
Implemented exactly as designed, with no deviations from the Design section:
- `ambientZoom` constant added to `ride_map.dart`.
- `RideMap.ambientPosition` field/constructor param added; `bounds == null` branch of
`initialCenter`/`initialZoom`, the `didUpdateWidget` chase logic, and the
`showLocationMarker` `MarkerLayer` all updated exactly per the ticket's diffs.
- `ambientPositionProvider` added to `providers.dart`, verbatim from the ticket's own
code block — never calls `LocationSource.stop()`, only `start()` (idempotent) and its
own subscription's implicit cancellation of the broadcast `fixes` stream on
`autoDispose`.
- `ShellScaffold.build()` wired to watch `ambientPositionProvider` only while
`activeTripProvider` is null, passing the resulting `ll.LatLng?` into `RideMap` as
`ambientPosition`, alongside the unchanged `follow: isMapTab` /
`showLocationMarker: isMapTab`.
Confirmed via `grep -n "\.stop()" $(git diff --name-only -- lib)` that no code this
ticket added calls `LocationSource.stop()` anywhere.
One nuance surfaced by testing, not a deviation from the design but worth recording:
`activeTripProvider` is a `StreamProvider`, so on the very first frame after the shell
mounts its `valueOrNull` is `null` (still loading) even when a trip is already active in
the database — this is pre-existing behavior identical to how `points`/`segments`
already fell back to empty lists before their own streams' first emission. In practice
this means `ambientPositionProvider` may be watched, and `LocationSource.start()` called,
for one transient frame during cold start even if a recording turns out to already be
active. It self-corrects the instant the trip stream emits (the shell stops watching
`ambientPositionProvider` from then on), and `start()` is idempotent regardless, so this
has no observable effect on recording — but it means the "no duplicate GPS consumer"
guarantee is a steady-state guarantee, not literally true for the first frame. Tests were
written to reflect this (see `test/widget_test.dart`'s "once a trip starts recording, the
shell stops watching ambient GPS" test, which starts idle, lets the ambient provider
engage, then starts a trip and asserts no further `start()` calls — rather than asserting
zero calls from t=0).
Both risks flagged in the ticket (continuous background GPS while idle with the Map tab
mounted, and an earlier permission-dialog timing) were left unresolved as instructed —
no reference-counted shutdown or idle timeout was added.
**Tests**: `flutter analyze` clean (same 4 pre-existing infos as baseline, no new
issues). `flutter test` green, 384 passing (up from 374 baseline; +10 new tests: 6 in
`test/ride_map_test.dart`'s new "ambient position (FB-01)" group covering
`RideMap`-level centering/chase/pan-cancel/marker-fallback behavior, and 4 in
`test/widget_test.dart` — two widget-level (`ShellScaffold` following an ambient fix
with no trip active; ambient watching stopping once a trip starts) and two
provider-level, driven directly against a `ProviderContainer` (never calls `stop()`;
`mapEnabledProvider` off never starts location and yields null)).