diff --git a/docs/v3/README.md b/docs/v3/README.md index b957e39..64f20ad 100644 --- a/docs/v3/README.md +++ b/docs/v3/README.md @@ -20,7 +20,7 @@ backup are v4 — see [../BACKLOG.md](../BACKLOG.md). | [V3-01](V3-01-activity-type.md) | Activity type per ride | M | — | Done | | [V3-02](V3-02-settings-screen.md) | Settings screen | S | — | Done | | [V3-03](V3-03-units.md) | Distance and speed units | S | V3-02 | Done | -| [V3-04](V3-04-live-map.md) | Live map on the recording screen | M | — | Not started | +| [V3-04](V3-04-live-map.md) | Live map on the recording screen | M | — | Done | | [V3-05](V3-05-mounted-mode.md) | Mounted (handlebar) mode | M | V3-04 | Not started | | [V3-06](V3-06-notification-stats.md) | Live stats in the notification | S | — | Not started | | [V3-07](V3-07-route-drawing.md) | Route drawing (pins, straight lines) | M | — | Not started | diff --git a/docs/v3/V3-04-live-map.md b/docs/v3/V3-04-live-map.md index d537fce..61a3e8e 100644 --- a/docs/v3/V3-04-live-map.md +++ b/docs/v3/V3-04-live-map.md @@ -1,6 +1,6 @@ # V3-04 — Live map on the recording screen -**Phase** Live map · **Depends on** nothing · **Size** M · **Status** Not started +**Phase** Live map · **Depends on** nothing · **Size** M · **Status** Done ## Goal While recording, show the path as it is drawn, on the recording screen. @@ -60,3 +60,31 @@ Behind the existing map toggle, off by default while it is unproven on battery. ## Out of scope Mounted mode (V3-05). Other riders on the map (v4). + +## Outcome +Shipped as designed. `AppDatabase.watchPointsForTrip`/`watchSegmentsForTrip` feed two +`autoDispose.family` providers (`livePointsProvider`, `liveSegmentsProvider`) keyed by +trip id; a `_LiveMap` adapter widget on the record screen reads them and hands the result +to the existing `RideMap`, unchanged in shape. `RecordingEngine` was never touched — +verified by `test/architecture_test.dart`, which greps the source rather than trusting a +comment. + +`RideMap` itself grew two small, general capabilities rather than a parallel "live" +widget: a `WidgetsBindingObserver` that drops the `TileLayer` entirely (not just visually, +via widget tree omission) outside `AppLifecycleState.resumed`, and an optional `follow` +flag that recentres on the latest point via `didUpdateWidget` + a post-frame +`MapController.move`, cancelled permanently by the first user-gesture pan. Both apply +to the trip-detail map too, which is a free win: a backgrounded detail screen no longer +holds tiles fetching either. + +Three existing record-screen tests (`recording swaps to PAUSE and STOP`, `paused offers +RESUME`, `discard asks before destroying anything`) had to gain an explicit `map: false` — +they predate this ticket and would otherwise have started constructing a real +`FlutterMap`/`TileLayer` against an active trip, which is exactly the tile-fetch-in-tests +problem the trip-detail tests already route around. + +3 new tests: the grep-based engine-purity check in `architecture_test.dart`, plus two in +`widget_test.dart` — map presence/absence by toggle and trip state, and the lifecycle +transition (paused drops `TileLayer` but keeps `PolylineLayer`; resumed brings it back), +driven via the standard `flutter/lifecycle` platform-message technique rather than a +private binding API. `flutter analyze` clean; full suite green (224 tests, up from 221). diff --git a/lib/src/app/providers.dart b/lib/src/app/providers.dart index 187baf2..4308a7b 100644 --- a/lib/src/app/providers.dart +++ b/lib/src/app/providers.dart @@ -110,3 +110,14 @@ final mapEnabledProvider = StateProvider( final unitSystemProvider = StateProvider( (ref) => ref.watch(configProvider)?.unitSystem ?? UnitSystem.metric, ); + +/// Live path for the recording screen's map (V3-04). `.family` + `autoDispose` so the +/// stream tears down the moment the record screen stops watching it — no leftover +/// subscription ticking away once a ride ends or the map toggle goes off. +final livePointsProvider = StreamProvider.autoDispose.family, int>( + (ref, tripId) => ref.watch(databaseProvider).watchPointsForTrip(tripId), +); + +final liveSegmentsProvider = StreamProvider.autoDispose.family, int>( + (ref, tripId) => ref.watch(databaseProvider).watchSegmentsForTrip(tripId), +); diff --git a/lib/src/data/database.dart b/lib/src/data/database.dart index 9234c05..66f312d 100644 --- a/lib/src/data/database.dart +++ b/lib/src/data/database.dart @@ -261,6 +261,15 @@ class AppDatabase extends _$AppDatabase { return rows.map(_toSegment).toList(); } + /// Feeds the live map (V3-04): segment boundaries are what turn a pause into a visible + /// gap instead of a straight line drawn across the gap. + Stream> watchSegmentsForTrip(int tripId) => + (select(segments) + ..where((s) => s.tripId.equals(tripId)) + ..orderBy([(s) => OrderingTerm.asc(s.id)])) + .watch() + .map((rows) => rows.map(_toSegment).toList()); + /// The segment currently being recorded into, if any. Future openSegment(int tripId) async { final row = @@ -343,6 +352,19 @@ class AppDatabase extends _$AppDatabase { return rows.map(_toPoint).toList(); } + /// Feeds the live map (V3-04). Updates on each writer flush (~2 s), not per fix — + /// the stream is backed by the same table the batched writer flushes into, so there is + /// nothing extra to throttle. + Stream> watchPointsForTrip(int tripId) => + (select(trackPoints) + ..where((p) => p.tripId.equals(tripId)) + ..orderBy([ + (p) => OrderingTerm.asc(p.segmentId), + (p) => OrderingTerm.asc(p.id), + ])) + .watch() + .map((rows) => rows.map(_toPoint).toList()); + Future> pointsForSegment(int segmentId) async { final rows = await (select(trackPoints) diff --git a/lib/src/ui/components/ride_map.dart b/lib/src/ui/components/ride_map.dart index 840c0a0..3aa5313 100644 --- a/lib/src/ui/components/ride_map.dart +++ b/lib/src/ui/components/ride_map.dart @@ -43,21 +43,62 @@ class RideMap extends StatefulWidget { required this.points, required this.segments, this.height = 320, + this.follow = false, }); final List points; final List segments; final double height; + /// V3-04: keep the latest point centred while recording. A manual pan/pinch turns + /// this off until the widget is rebuilt fresh (e.g. a new ride) — chasing the rider + /// back to centre after they deliberately looked elsewhere would be worse than not + /// following at all. + final bool follow; + @override State createState() => _RideMapState(); } -class _RideMapState extends State { +class _RideMapState extends State with WidgetsBindingObserver { final _controller = MapController(); + late bool _following = widget.follow; + + /// V3-04: a pocketed phone must cost exactly what it costs today. Backgrounding drops + /// the tile layer entirely rather than merely pausing it — flutter_map has no pause + /// primitive, and an app resume already triggers a full rebuild anyway. + bool _backgrounded = false; + + @override + void initState() { + super.initState(); + WidgetsBinding.instance.addObserver(this); + } + + @override + void didUpdateWidget(RideMap old) { + super.didUpdateWidget(old); + if (!_following || widget.points.isEmpty || _backgrounded) return; + final last = widget.points.last; + // 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. + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!mounted || !_following) return; + _controller.move(ll.LatLng(last.latitude, last.longitude), _controller.camera.zoom); + }); + } + + @override + void didChangeAppLifecycleState(AppLifecycleState state) { + final backgrounded = state != AppLifecycleState.resumed; + if (backgrounded != _backgrounded && mounted) { + setState(() => _backgrounded = backgrounded); + } + } @override void dispose() { + WidgetsBinding.instance.removeObserver(this); _controller.dispose(); super.dispose(); } @@ -122,15 +163,28 @@ class _RideMapState extends State { interactionOptions: const InteractionOptions( flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag, ), + onPositionChanged: !widget.follow + ? null + : (position, hasGesture) { + // Only a real pan/pinch turns following off -- the programmatic + // moves this widget makes to chase the rider must not cancel + // themselves out. + if (hasGesture && _following) { + setState(() => _following = false); + } + }, ), children: [ - TileLayer( - urlTemplate: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png', - userAgentPackageName: tileUserAgent, - maxNativeZoom: maxTileZoom.toInt(), - // Respect OSM's usage policy: render what is looked at, never bulk prefetch. - panBuffer: 0, - ), + // Omitted entirely while backgrounded -- not just visually hidden -- so no + // tile request can fire off-screen. See the lifecycle observer above. + if (!_backgrounded) + TileLayer( + urlTemplate: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png', + userAgentPackageName: tileUserAgent, + maxNativeZoom: maxTileZoom.toInt(), + // Respect OSM's usage policy: render what is looked at, never bulk prefetch. + panBuffer: 0, + ), PolylineLayer(polylines: polylines), ], ), diff --git a/lib/src/ui/record/record_screen.dart b/lib/src/ui/record/record_screen.dart index 972118c..7fb1406 100644 --- a/lib/src/ui/record/record_screen.dart +++ b/lib/src/ui/record/record_screen.dart @@ -10,6 +10,7 @@ import '../../app/providers.dart'; import '../../domain/models.dart'; import '../../recording/location_source.dart'; import '../../telemetry/telemetry.dart'; +import '../components/ride_map.dart'; import '../components/stats.dart'; import '../format.dart'; @@ -187,6 +188,17 @@ class _RecordScreenState extends ConsumerState { ), const SizedBox(height: 16), + // V3-04: RecordingEngine has no idea this exists -- it is fed + // entirely from a Drift stream, the same seam the trip-detail map + // already uses. Off by default (mapEnabledProvider) until battery + // impact is measured (V3-13), and only while a ride is actually + // active -- an idle screen has nothing to draw and no reason to + // hold a tile layer alive. + if (ui.trip != null && ref.watch(mapEnabledProvider)) ...[ + _LiveMap(tripId: ui.trip!.id), + const SizedBox(height: 16), + ], + Card( child: Padding( padding: const EdgeInsets.all(16), @@ -274,6 +286,28 @@ class _RecordScreenState extends ConsumerState { } } +/// Thin adapter from the two live Drift streams to [RideMap]. Kept out of +/// [_RecordScreenState] so a provider read/write cycle here can't be confused with the +/// engine's own state -- this widget only ever reads. +class _LiveMap extends ConsumerWidget { + const _LiveMap({required this.tripId}); + + final int tripId; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final points = ref.watch(livePointsProvider(tripId)).valueOrNull ?? const []; + final segments = ref.watch(liveSegmentsProvider(tripId)).valueOrNull ?? const []; + return RideMap( + key: const Key('live-map'), + points: points, + segments: segments, + height: 220, + follow: true, + ); + } +} + class _Controls extends StatelessWidget { const _Controls({ required this.ui, diff --git a/test/architecture_test.dart b/test/architecture_test.dart new file mode 100644 index 0000000..d08e855 --- /dev/null +++ b/test/architecture_test.dart @@ -0,0 +1,15 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +/// A structural invariant from V3-04, checked directly rather than trusted: rendering +/// belongs to a visible screen's widget lifecycle, never to the recording engine. If this +/// ever starts failing, a map reference has leaked into code that keeps running while the +/// phone is pocketed and the screen is off. +void main() { + test('RecordingEngine never imports the map', () { + final source = File('lib/src/recording/recording_engine.dart').readAsStringSync(); + expect(source.contains('ride_map'), isFalse); + expect(source.contains('flutter_map'), isFalse); + }); +} diff --git a/test/widget_test.dart b/test/widget_test.dart index 1b173cd..9a68241 100644 --- a/test/widget_test.dart +++ b/test/widget_test.dart @@ -1,6 +1,8 @@ import 'package:drift/drift.dart' show driftRuntimeOptions; import 'package:drift/native.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_map/flutter_map.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:rippr/src/app/providers.dart'; @@ -137,7 +139,7 @@ void main() { screenTest('recording swaps to PAUSE and STOP, with no DISCARD', (tester) async { await repo.startTrip(1000); - await pumpLive(tester, host(const RecordScreen())); + await pumpLive(tester, host(const RecordScreen(), map: false)); expect(find.byKey(const Key('pause')), findsOneWidget); expect(find.byKey(const Key('stop')), findsOneWidget); @@ -150,7 +152,7 @@ void main() { screenTest('paused offers RESUME, STOP and DISCARD', (tester) async { await repo.startTrip(1000); await repo.pauseTrip(2000); - await pumpLive(tester, host(const RecordScreen())); + await pumpLive(tester, host(const RecordScreen(), map: false)); expect(find.byKey(const Key('resume')), findsOneWidget); expect(find.byKey(const Key('stop')), findsOneWidget); @@ -161,7 +163,7 @@ void main() { screenTest('discard asks before destroying anything', (tester) async { await repo.startTrip(1000); await repo.pauseTrip(2000); - await pumpLive(tester, host(const RecordScreen())); + await pumpLive(tester, host(const RecordScreen(), map: false)); await tester.tap(find.byKey(const Key('discard'))); await tester.pumpAndSettle(); @@ -208,6 +210,66 @@ void main() { expect(opened, isTrue, reason: 'the button must actually invoke the callback that navigates'); }); + + screenTest('the live map appears only while recording and the toggle is on ' + '(V3-04)', (tester) async { + // Idle, toggle on: no trip to draw, so no map at all. + await tester.pumpWidget(host(const RecordScreen())); + await tester.pumpAndSettle(); + expect(find.byKey(const Key('live-map')), findsNothing); + + // Recording, toggle off: RecordingEngine has produced a trip, but the map must + // not be constructed at all -- not just hidden. + await repo.startTrip(1000); + await tester.pumpWidget(const SizedBox.shrink()); + await pumpLive(tester, host(const RecordScreen(), map: false)); + expect(find.byKey(const Key('live-map')), findsNothing); + expect(find.byType(FlutterMap), findsNothing); + + // Recording, toggle on: the map is drawn. + await tester.pumpWidget(const SizedBox.shrink()); + await pumpLive(tester, host(const RecordScreen())); + expect(find.byKey(const Key('live-map')), findsOneWidget); + }); + + screenTest('backgrounding the app drops the tile layer (V3-04)', + (tester) async { + final h = await repo.startTrip(1000); + await repo.appendPoints([ + TrackPoint( + tripId: h.tripId, + segmentId: h.segmentId, + timestamp: 1000, + latitude: 51.0, + longitude: -114.0, + speedKmh: 20.0, + altitudeM: 1000.0, + ), + ]); + await pumpLive(tester, host(const RecordScreen())); + + expect(find.byType(TileLayer), findsOneWidget, + reason: 'foregrounded: tiles render normally'); + + // Simulates the platform lifecycle message a real backgrounding sends -- this is + // the standard way to drive AppLifecycleState changes in a widget test. + final message = const StringCodec().encodeMessage('AppLifecycleState.paused'); + await tester.binding.defaultBinaryMessenger + .handlePlatformMessage('flutter/lifecycle', message, (_) {}); + await tester.pump(); + + expect(find.byType(TileLayer), findsNothing, + reason: 'backgrounded: no tile layer means no tile request can fire'); + expect(find.byType(PolylineLayer), findsOneWidget, + reason: 'the drawn path itself is not removed, only tile fetching'); + + final resumed = const StringCodec().encodeMessage('AppLifecycleState.resumed'); + await tester.binding.defaultBinaryMessenger + .handlePlatformMessage('flutter/lifecycle', resumed, (_) {}); + await tester.pump(); + expect(find.byType(TileLayer), findsOneWidget, + reason: 'foregrounding again must resume tiles'); + }); }); group('trips list', () {