Merge FB-01: live street-level map

# Conflicts:
#	docs/feedback/FB-01-live-street-level-map.md
This commit is contained in:
2026-08-24 16:00:06 -05:00
6 changed files with 403 additions and 16 deletions

View File

@@ -1,6 +1,6 @@
# FB-01 — Live, street-level map everywhere # FB-01 — Live, street-level map everywhere
**Depends on** — · **Size** L · **Status** Not started **Depends on** — · **Size** L · **Status** Done
## Goal ## Goal
The map is too zoomed out on every screen, and the shared background map has zero The map is too zoomed out on every screen, and the shared background map has zero
@@ -333,3 +333,51 @@ zoom-consistency but does not need ambient location following (Route Planner alr
centers on the route's own waypoints, which is correct). HUD widget changes (FB-03). 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 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. 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)).

View File

@@ -57,6 +57,30 @@ final locationSourceProvider = Provider<LocationSource>((ref) {
return source; return source;
}); });
/// 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.
});
/// Loaded once at startup; null until then so nothing blocks the first frame. /// Loaded once at startup; null until then so nothing blocks the first frame.
final configProvider = StateProvider<Config?>((ref) => null); final configProvider = StateProvider<Config?>((ref) => null);

View File

@@ -14,6 +14,7 @@ library;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:go_router/go_router.dart'; import 'package:go_router/go_router.dart';
import 'package:latlong2/latlong.dart' as ll;
import '../app/providers.dart'; import '../app/providers.dart';
import '../domain/models.dart'; import '../domain/models.dart';
@@ -70,6 +71,13 @@ class ShellScaffold extends ConsumerWidget {
final segments = trip == null final segments = trip == null
? const <Segment>[] ? const <Segment>[]
: ref.watch(liveSegmentsProvider(trip.id)).valueOrNull ?? const <Segment>[]; : ref.watch(liveSegmentsProvider(trip.id)).valueOrNull ?? const <Segment>[];
// FB-01: only watched while idle, so there's never a duplicate GPS consumer during
// an active recording -- the moment a trip starts, this stops being watched at all.
final ambientFix =
trip == null ? ref.watch(ambientPositionProvider).valueOrNull : null;
final ambientPosition = ambientFix == null
? null
: ll.LatLng(ambientFix.latitude, ambientFix.longitude);
return Scaffold( return Scaffold(
// Falls back to the ordinary theme background when the map is disabled -- the // Falls back to the ordinary theme background when the map is disabled -- the
@@ -90,6 +98,7 @@ class ShellScaffold extends ConsumerWidget {
key: const Key('shell-background-map'), key: const Key('shell-background-map'),
points: points, points: points,
segments: segments, segments: segments,
ambientPosition: ambientPosition,
follow: isMapTab, follow: isMapTab,
fill: true, fill: true,
showEmptyLabel: false, showEmptyLabel: false,

View File

@@ -47,6 +47,12 @@ const double maxTileZoom = 19.0;
/// What a very short ride falls back to, so streets stay visible. /// What a very short ride falls back to, so streets stay visible.
const double shortRideZoom = 17.0; const double shortRideZoom = 17.0;
/// Street-level zoom for the idle, no-recorded-points ambient position (FB-01). A
/// distinct name from [shortRideZoom] even though the value happens to match -- one
/// means "a very short recorded ride," the other means "no ride at all, just ambient
/// GPS."
const double ambientZoom = 17.0;
class RideMap extends StatefulWidget { class RideMap extends StatefulWidget {
const RideMap({ const RideMap({
super.key, super.key,
@@ -60,12 +66,19 @@ class RideMap extends StatefulWidget {
this.skeletonMode = false, this.skeletonMode = false,
this.showLocationMarker = false, this.showLocationMarker = false,
this.showAttribution = true, this.showAttribution = true,
this.ambientPosition,
}); });
final List<TrackPoint> points; final List<TrackPoint> points;
final List<Segment> segments; final List<Segment> segments;
final double height; final double height;
/// FB-01: the device's current GPS position when there are no recorded [points] to
/// chase -- drives the shared background map's "look like Google Maps while idle"
/// behaviour. Only meaningful when [points] is empty; a recorded path always takes
/// priority over ambient position.
final ll.LatLng? ambientPosition;
/// UI-02: true when `MapConnectivityState` has decided neither the offline cache nor /// UI-02: true when `MapConnectivityState` has decided neither the offline cache nor
/// the network can currently produce tiles. Swaps `TileLayer` for `SkeletonMapLayer` /// the network can currently produce tiles. Swaps `TileLayer` for `SkeletonMapLayer`
/// -- markers/polylines are unaffected, since those come from local data, not tiles. /// -- markers/polylines are unaffected, since those come from local data, not tiles.
@@ -128,7 +141,8 @@ class _RideMapState extends State<RideMap> with WidgetsBindingObserver {
@override @override
void didUpdateWidget(RideMap old) { void didUpdateWidget(RideMap old) {
super.didUpdateWidget(old); super.didUpdateWidget(old);
if (!_following || widget.points.isEmpty || _backgrounded) return; if (!_following || _backgrounded) return;
if (widget.points.isNotEmpty) {
final last = widget.points.last; final last = widget.points.last;
// A manual camera move already exists once the controller has been used; jumping // A manual camera move already exists once the controller has been used; jumping
// straight to the latest fix keeps the map from ever being one flush behind. // straight to the latest fix keeps the map from ever being one flush behind.
@@ -136,6 +150,15 @@ class _RideMapState extends State<RideMap> with WidgetsBindingObserver {
if (!mounted || !_following) return; if (!mounted || !_following) return;
_controller.move(ll.LatLng(last.latitude, last.longitude), _controller.camera.zoom); _controller.move(ll.LatLng(last.latitude, last.longitude), _controller.camera.zoom);
}); });
} else if (widget.ambientPosition != null &&
widget.ambientPosition != old.ambientPosition) {
// FB-01: no recorded path yet -- chase the ambient GPS fix instead, the same way
// a recording is chased above.
WidgetsBinding.instance.addPostFrameCallback((_) {
if (!mounted || !_following) return;
_controller.move(widget.ambientPosition!, _controller.camera.zoom);
});
}
} }
@override @override
@@ -229,10 +252,10 @@ class _RideMapState extends State<RideMap> with WidgetsBindingObserver {
maxZoom: maxTileZoom, maxZoom: maxTileZoom,
), ),
initialCenter: bounds == null initialCenter: bounds == null
? const ll.LatLng(0, 0) ? (widget.ambientPosition ?? const ll.LatLng(0, 0))
: ll.LatLng(bounds.centerLat, bounds.centerLon), : ll.LatLng(bounds.centerLat, bounds.centerLon),
initialZoom: bounds == null initialZoom: bounds == null
? 2 ? (widget.ambientPosition == null ? 2 : ambientZoom)
: (bounds.isDegenerate ? shortRideZoom : maxTileZoom), : (bounds.isDegenerate ? shortRideZoom : maxTileZoom),
maxZoom: maxTileZoom, maxZoom: maxTileZoom,
interactionOptions: hasPoints interactionOptions: hasPoints
@@ -272,15 +295,17 @@ class _RideMapState extends State<RideMap> with WidgetsBindingObserver {
tileProvider: widget.tileProvider, tileProvider: widget.tileProvider,
), ),
PolylineLayer(polylines: polylines), PolylineLayer(polylines: polylines),
if (widget.showLocationMarker && hasPoints) if (widget.showLocationMarker && (hasPoints || widget.ambientPosition != null))
MarkerLayer( MarkerLayer(
markers: [ markers: [
Marker( Marker(
key: const Key('location-marker'), key: const Key('location-marker'),
point: ll.LatLng( point: hasPoints
? ll.LatLng(
widget.points.last.latitude, widget.points.last.latitude,
widget.points.last.longitude, widget.points.last.longitude,
), )
: widget.ambientPosition!,
width: 40, width: 40,
height: 40, height: 40,
child: const PulsingLocationMarker(), child: const PulsingLocationMarker(),

View File

@@ -1,6 +1,7 @@
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_map/flutter_map.dart'; import 'package:flutter_map/flutter_map.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:latlong2/latlong.dart' as ll;
import 'package:rippr/src/domain/models.dart'; import 'package:rippr/src/domain/models.dart';
import 'package:rippr/src/ui/components/ride_map.dart'; import 'package:rippr/src/ui/components/ride_map.dart';
import 'package:rippr/src/ui/components/skeleton_map_layer.dart'; import 'package:rippr/src/ui/components/skeleton_map_layer.dart';
@@ -181,4 +182,150 @@ void main() {
'just because the tile fetch is failing'); 'just because the tile fetch is failing');
}); });
}); });
group('ambient position (FB-01)', () {
testWidgets('with no recorded points, an ambient position centers the map at '
'street level', (tester) async {
const fix = ll.LatLng(51.05, -114.05);
await tester.pumpWidget(MaterialApp(
theme: ripprTheme(),
home: const Scaffold(
body: RideMap(
points: [],
segments: [],
showEmptyLabel: false,
ambientPosition: fix,
),
),
));
await tester.pump();
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
expect(map.options.initialCenter, fix);
expect(map.options.initialZoom, ambientZoom);
});
testWidgets('with neither recorded points nor an ambient fix, the map still '
'falls back to (0, 0) at zoom 2 (no regression, no crash)', (tester) async {
await tester.pumpWidget(MaterialApp(
theme: ripprTheme(),
home: const Scaffold(
body: RideMap(points: [], segments: [], showEmptyLabel: false),
),
));
await tester.pump();
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
expect(map.options.initialCenter, const ll.LatLng(0, 0));
expect(map.options.initialZoom, 2);
});
testWidgets('while following, a new ambient fix re-centers the map, exactly '
'like the recorded-path chase', (tester) async {
const fix1 = ll.LatLng(51.0, -114.0);
const fix2 = ll.LatLng(51.01, -114.01);
Widget build(ll.LatLng ambient) => MaterialApp(
theme: ripprTheme(),
home: Scaffold(
body: RideMap(
points: const [],
segments: const [],
showEmptyLabel: false,
follow: true,
ambientPosition: ambient,
),
),
);
await tester.pumpWidget(build(fix1));
await tester.pump();
await tester.pumpWidget(build(fix2));
await tester.pump();
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
final center = map.mapController!.camera.center;
expect(center.latitude, closeTo(fix2.latitude, 1e-9));
expect(center.longitude, closeTo(fix2.longitude, 1e-9));
});
testWidgets('a manual pan cancels ambient following the same way it cancels '
'recording-follow', (tester) async {
const fix1 = ll.LatLng(51.0, -114.0);
const fix2 = ll.LatLng(51.01, -114.01);
Widget build(ll.LatLng ambient) => MaterialApp(
theme: ripprTheme(),
home: Scaffold(
body: RideMap(
points: const [],
segments: const [],
showEmptyLabel: false,
follow: true,
ambientPosition: ambient,
),
),
);
await tester.pumpWidget(build(fix1));
await tester.pump();
// Simulate a real user gesture the same way flutter_map itself would report
// one to `onPositionChanged` -- calling the callback directly with
// `hasGesture: true` exercises the exact guard in `_RideMapState` without
// needing a real pointer gesture to get past `InteractionOptions`.
var map = tester.widget<FlutterMap>(find.byType(FlutterMap));
map.options.onPositionChanged!(map.mapController!.camera, true);
await tester.pump();
await tester.pumpWidget(build(fix2));
await tester.pump();
map = tester.widget<FlutterMap>(find.byType(FlutterMap));
final center = map.mapController!.camera.center;
expect(center.latitude, closeTo(fix1.latitude, 1e-9),
reason: 'following was cancelled by the manual pan; a later ambient fix '
'must not move the camera');
expect(center.longitude, closeTo(fix1.longitude, 1e-9));
});
testWidgets('the pulsing location marker falls back to the ambient position '
'when there are no recorded points', (tester) async {
const fix = ll.LatLng(51.0, -114.0);
await tester.pumpWidget(MaterialApp(
theme: ripprTheme(),
home: const Scaffold(
body: RideMap(
points: [],
segments: [],
showEmptyLabel: false,
showLocationMarker: true,
ambientPosition: fix,
),
),
));
await tester.pump();
expect(find.byKey(const Key('location-marker')), findsOneWidget);
});
testWidgets('no marker is shown when there is neither a recorded point nor an '
'ambient position', (tester) async {
await tester.pumpWidget(MaterialApp(
theme: ripprTheme(),
home: const Scaffold(
body: RideMap(
points: [],
segments: [],
showEmptyLabel: false,
showLocationMarker: true,
),
),
));
await tester.pump();
expect(find.byKey(const Key('location-marker')), findsNothing);
});
});
} }

View File

@@ -461,6 +461,85 @@ void main() {
}); });
}); });
group('ambient position (FB-01)', () {
screenTest('with no active trip, an ambient GPS fix drives the shared '
'background map', (tester) async {
await tester.pumpWidget(host(
ShellScaffold(
currentIndex: 0,
onDestinationSelected: (_) {},
child: const SizedBox.shrink(),
),
));
// A frame to build the tree, then time for `ambientPositionProvider`'s
// `await source.start()` to resolve and subscribe to `source.fixes`.
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
// No fix yet: still today's neutral fallback, no marker.
expect(find.byKey(const Key('location-marker')), findsNothing);
source.emitAt(timestamp: 1000, latitude: 51.2, longitude: -114.2);
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
expect(find.byKey(const Key('location-marker')), findsOneWidget,
reason: 'an ambient fix drives the pulsing marker even with no trip '
'ever recorded');
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
final center = map.mapController!.camera.center;
expect(center.latitude, closeTo(51.2, 1e-9));
expect(center.longitude, closeTo(-114.2, 1e-9));
// A second fix keeps the map following, exactly like the recorded-path chase.
source.emitAt(timestamp: 2000, latitude: 51.21, longitude: -114.21);
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
final moved = tester.widget<FlutterMap>(find.byType(FlutterMap));
final movedCenter = moved.mapController!.camera.center;
expect(movedCenter.latitude, closeTo(51.21, 1e-9));
expect(movedCenter.longitude, closeTo(-114.21, 1e-9));
});
screenTest('once a trip starts recording, the shell stops watching ambient '
'GPS -- no duplicate location consumer during a ride', (tester) async {
// Idle first: the ambient provider engages and calls start() at least once
// (activeTripProvider's own first, async emission of `null` briefly reads
// as "idle" too, which is expected -- same shape as `points`/`segments`
// falling back to empty lists until their streams first emit).
await tester.pumpWidget(host(
ShellScaffold(
currentIndex: 0,
onDestinationSelected: (_) {},
child: const SizedBox.shrink(),
),
));
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
source.emitAt(timestamp: 1000, latitude: 51.0, longitude: -114.0);
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
expect(find.byKey(const Key('location-marker')), findsOneWidget,
reason: 'sanity check: ambient mode is actually engaged before the '
'trip starts');
final callsWhileIdle = source.startCalls;
expect(callsWhileIdle, greaterThanOrEqualTo(1));
// Now a trip starts recording. Nothing in this test ever taps Start, so the
// only thing that could call `LocationSource.start()` again is
// `ambientPositionProvider` -- and it must not, because `ShellScaffold`
// stops watching it entirely the moment `activeTripProvider` is non-null.
await repo.startTrip(2000);
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
expect(source.startCalls, callsWhileIdle,
reason: 'once a trip is active, ambientPositionProvider must no '
'longer be watched at all');
});
});
group('trips list', () { group('trips list', () {
screenTest('empty state explains what to do', (tester) async { screenTest('empty state explains what to do', (tester) async {
await tester.pumpWidget(host(const TripsScreen())); await tester.pumpWidget(host(const TripsScreen()));
@@ -829,4 +908,59 @@ void main() {
expect(await repo.segmentsForTrip(h.tripId), hasLength(1)); expect(await repo.segmentsForTrip(h.tripId), hasLength(1));
}); });
}); });
group('ambientPositionProvider (FB-01)', () {
// Provider-level, not widget-level: exercises the provider directly against a
// `ProviderContainer` so its lifecycle (including disposal) can be driven
// precisely, per the ticket's own suggested test shape.
test('never calls LocationSource.stop() -- the underlying source is shared '
'with an active recording and stop() is not reference-counted', () async {
final fakeSource = FakeLocationSource();
final container = ProviderContainer(
overrides: [locationSourceProvider.overrideWithValue(fakeSource)],
);
addTearDown(container.dispose);
final sub = container.listen(ambientPositionProvider, (_, _) {});
// Let the provider's `await source.start()` resolve and subscribe.
await Future<void>.delayed(Duration.zero);
expect(fakeSource.startCalls, 1);
sub.close();
container.dispose();
expect(fakeSource.stopCalls, 0,
reason: 'ambientPositionProvider must only ever call start(), never '
'stop(), on the shared LocationSource');
await fakeSource.dispose();
});
test('turning mapEnabledProvider off never starts location and yields null',
() async {
final fakeSource = FakeLocationSource();
final container = ProviderContainer(
overrides: [
locationSourceProvider.overrideWithValue(fakeSource),
mapEnabledProvider.overrideWith((ref) => false),
],
);
addTearDown(container.dispose);
final values = <AsyncValue<LocationFix?>>[];
final sub = container.listen(
ambientPositionProvider,
(_, next) => values.add(next),
fireImmediately: true,
);
await Future<void>.delayed(Duration.zero);
expect(fakeSource.startCalls, 0,
reason: 'no tile or location request should fire while the map is '
'disabled, matching the existing map-toggle guarantee');
expect(values.last.valueOrNull, isNull);
sub.close();
await fakeSource.dispose();
});
});
} }