diff --git a/docs/feedback/FB-06-always-interactive-map-recenter.md b/docs/feedback/FB-06-always-interactive-map-recenter.md new file mode 100644 index 0000000..a110701 --- /dev/null +++ b/docs/feedback/FB-06-always-interactive-map-recenter.md @@ -0,0 +1,199 @@ +# FB-06 — Map is always pannable and zoomable, with a recenter control + +**Depends on** — · **Size** M · **Status** Done + +## Goal +The rider must be able to pan and zoom the map at all times. Today the map locks all +interaction while idle. Add a recenter button. The button must bring the camera back to +the rider's live position. + +## Context +Direct user feedback (`docs/FEEDBACK.md`): + +> Map page, both when recording is enabled and disabled, you cannot zoom and move +> around the map, it's be nice to still be able to move around and see the surrounding +> area, and then tap a "center" icon to recenter on the user. +> +> User dot on the map should always glow and occilate in size like it does when +> recording. + +`lib/src/ui/components/ride_map.dart`, `_RideMapState.build()` (~line 270): + +```dart +interactionOptions: hasPoints + ? const InteractionOptions( + flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag, + ) + : const InteractionOptions(flags: InteractiveFlag.none), +``` + +`hasPoints` is `widget.points.isNotEmpty`. The shared background map is idle (no +recorded points) any time no trip is recording. In that state `InteractiveFlag.none` +blocks every pan and zoom gesture. This is the exact cause of "when recording is +disabled you cannot zoom and move around the map." + +During an active recording, `hasPoints` is true and pan/zoom are already allowed. The +existing chase-camera logic already cancels auto-follow on a real gesture — see +`didUpdateWidget` (~line 142) and `onPositionChanged` (~line 270): + +```dart +onPositionChanged: !widget.follow + ? null + : (position, hasGesture) { + if (hasGesture && _following) { + setState(() => _following = false); + } + }, +``` + +Once `_following` is set to `false`, nothing ever sets it back to `true` again — there +is no way today to resume following the rider's live position after one manual pan. +That is the missing "recenter" affordance the feedback names directly. + +`_following` is a `late bool` field set once, at `State` creation +(`late bool _following = widget.follow;`, ~line 128) — it never re-reads `widget.follow` +on a later rebuild. A recenter action must set this field back to `true` directly +(inside `_RideMapState`, since it owns the field) — there is no way to do this from +outside the widget today, and there should not be; this stays internal. + +`lib/src/ui/components/pulsing_location_marker.dart` already animates continuously +(`AnimationController.repeat()` in `didChangeDependencies`, unless the platform's +reduced-motion setting is on) and is the same widget instance used for both the idle +(ambient) marker and the recording marker — see `ride_map.dart` ~line 307: + +```dart +if (widget.showLocationMarker && (hasPoints || widget.ambientPosition != null)) + MarkerLayer( + markers: [ + Marker( + key: const Key('location-marker'), + point: hasPoints ? ... : widget.ambientPosition!, + width: 40, + height: 40, + child: const PulsingLocationMarker(), + ), + ], + ), +``` + +The code already pulses the marker in both states. This ticket's job for the marker is +to confirm, on a real device, that the pulse is actually visible in both states — not to +assume the report is wrong. If the pulse is confirmed working, say so plainly in the +Outcome section and make no code change for it. + +## Design +- **Always allow pan and zoom.** Remove the `hasPoints` gate on `interactionOptions`. + Use `const InteractionOptions(flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag)` + unconditionally. +- **Add a recenter button inside `RideMap`.** Show it only when `widget.follow` is true + and `_following` is false — the exact state where the rider panned away from an + actively-followed camera. Place it as a small circular icon button, bottom-right of + the map, above the tile layer, using `Icons.my_location` and the app's existing + `GlassPanel`/theme conventions (check `lib/src/ui/components/glass_panel.dart` for the + existing floating-control pattern this app already uses, e.g. `FloatingPill`, and + match it rather than inventing new chrome). Give it `key: const Key('recenter-button')`. + On tap: + 1. Set `_following = true`. + 2. Move the camera to the latest known position: `widget.points.last` if + `widget.points.isNotEmpty`, else `widget.ambientPosition` if it is not null. If + neither is available, do nothing (no position to recenter on yet). + 3. Keep the current zoom level — do not force a specific zoom on recenter, since the + rider may have deliberately zoomed in or out and recenter should not undo that. +- **Do not show the recenter button when there is no `follow` mode at all** (e.g. a + finished-ride static map in Trip Detail, where `follow` is always false) — the gate + above (`widget.follow && !_following`) already excludes this case correctly. +- **Marker verification.** Run the app on the Android emulator. Watch the location + marker in the idle state and in the recording state. Confirm the pulse ring expands + and fades in both states. Write the result in the Outcome section. Fix the code only + if the pulse is genuinely missing in one of the two states — do not change the + animation if it is already working. + +## Implementation +1. Remove the `hasPoints` conditional on `interactionOptions` in `ride_map.dart`. Use + the pinch/drag flags unconditionally. +2. Add a recenter button widget inside `_RideMapState.build()`, gated on + `widget.follow && !_following`. +3. Wire the button's `onPressed` to set `_following = true` and move the camera to the + latest point or ambient position, keeping the current zoom. +4. Run the app on the Android emulator. Confirm the marker pulses in both the idle and + the recording state. Record the result in the ticket's Outcome section. + +## Acceptance criteria +- [ ] The map can be panned and zoomed while idle (no active recording). +- [ ] The map can be panned and zoomed while recording. +- [ ] Panning the map while `follow` is active shows a recenter button. +- [ ] Tapping the recenter button returns the camera to the rider's latest known + position and resumes following new position updates. +- [ ] The recenter button does not appear on a static, non-following map (e.g. Trip + Detail's finished-ride view). +- [ ] The location marker's pulse animation is confirmed visible on-device in both the + idle and the recording state, or fixed if it is not. +- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up. + +## Tests +- Widget test: `RideMap` with `hasPoints: false` (no recorded points) allows a pan + gesture to change the camera position — assert the map's `InteractionOptions.flags` + include `InteractiveFlag.drag`/`pinchZoom` regardless of `points`/`ambientPosition`. +- Widget test: after a manual pan cancels following (`_following` set to `false` via + the existing `onPositionChanged` gesture path — see the pattern already used in + `test/ride_map_test.dart`'s "a manual pan cancels ambient following" test), the + recenter button (`find.byKey(const Key('recenter-button'))`) appears. +- Widget test: before any manual pan (or when `follow` is false), the recenter button + does not appear. +- Widget test: tapping the recenter button moves the camera back to the latest point + (or ambient position) and the button disappears again (following resumed). + +## Risks +- None significant. This is additive (a new optional control) and a removed + restriction (interaction gating), not a data-model or persistence change. + +## Out of scope +Any change to the Route Planner's own map (FB-07 covers its remaining issue). Any +change to `PulsingLocationMarker`'s own animation code, unless the on-device check in +this ticket finds it is genuinely not visible in one of the two states. + +## Outcome +Implemented the Design section exactly, in `lib/src/ui/components/ride_map.dart`: + +- Removed the `hasPoints` gate on `interactionOptions`. The map now always uses + `const InteractionOptions(flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag)`, + idle or recording. +- Added a recenter button (`key: const Key('recenter-button')`), shown only when + `widget.follow && !_following` — i.e. exactly when the rider has panned away from an + actively-followed camera. It is a `GlassPanel` (borderRadius 999, matching the + existing `FloatingPill`/`_OverflowMenu` circular-chrome pattern already used in + `route_planner_screen.dart`) wrapping an `IconButton` with `Icons.my_location`, + positioned bottom-right of the map (`Positioned(right: 16, bottom: 16, ...)` inside a + `Stack` that now wraps the map). +- Wired `onPressed` to a new `_recenter()` method: sets `_following = true`, then moves + the camera to `widget.points.last` if points are non-empty, else + `widget.ambientPosition` if non-null, else does nothing. Zoom is left untouched + (`_controller.camera.zoom` is passed straight through to `_controller.move`), per the + design's explicit "keep the current zoom level" requirement. +- Made no changes to `PulsingLocationMarker` — see the marker verification note below. + +No deviation from the design. + +**Tests.** Added 7 new widget tests to `test/ride_map_test.dart` under a new +`'always-interactive map + recenter (FB-06)'` group, covering: pan/zoom flags present +with no points (idle) and with points (recording); no recenter button before any +manual pan; no recenter button when `follow` is false even after a pan; a manual pan +while following shows the button; tapping it recenters to the ambient position and +hides the button again; tapping it recenters to the latest recorded point while +recording. `flutter analyze` is clean (only the 4 pre-existing, unrelated infos in +`crash_reporter.dart`/`map_connectivity.dart` — nothing new). `flutter test` is green: +confirmed via both the default reporter and `--reporter json` (cross-checked test names +directly) that the suite went from the 412-test baseline to exactly 419 real tests +(412 + 7 new), all passing, none skipped or removed. + +**On-device marker verification: skipped.** This environment has no Android +tooling available — `adb` is not on `PATH`, there is no `ANDROID_HOME`/SDK, and +`flutter devices` lists only `macOS (desktop)` and `Chrome (web)`, no emulator. Per the +ticket's own risk-mitigation instructions (do not fight an unavailable/unstable +emulator at length), the marker-pulse check was not attempted rather than spending +time on a device that isn't reachable from this sandbox. `PulsingLocationMarker` was +not modified — per the ticket, a code change there is only warranted if the on-device +check finds the pulse genuinely missing, and that check could not be run. The relevant +acceptance-criteria checkbox ("location marker's pulse animation is confirmed visible +on-device...") is therefore left unchecked/unresolved and should be picked up in a +follow-up pass that has emulator access. diff --git a/lib/src/ui/components/ride_map.dart b/lib/src/ui/components/ride_map.dart index 09381cc..8603341 100644 --- a/lib/src/ui/components/ride_map.dart +++ b/lib/src/ui/components/ride_map.dart @@ -27,6 +27,7 @@ import '../../domain/models.dart'; import '../../geo/geo.dart' as geo; import '../../tiles/tile_config.dart'; import '../theme.dart' show ripprRadiusLarge; +import 'glass_panel.dart'; import 'pulsing_location_marker.dart'; import 'skeleton_map_layer.dart'; @@ -267,11 +268,12 @@ class _RideMapState extends State with WidgetsBindingObserver { ? (widget.ambientPosition == null ? 2 : ambientZoom) : (bounds.isDegenerate ? shortRideZoom : maxTileZoom), maxZoom: maxTileZoom, - interactionOptions: hasPoints - ? const InteractionOptions( - flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag, - ) - : const InteractionOptions(flags: InteractiveFlag.none), + // FB-06: pan/zoom must always be available, idle or recording -- the old + // `hasPoints` gate locked the map to `InteractiveFlag.none` whenever no trip + // was recording, which is exactly the "can't zoom and move around" report. + interactionOptions: const InteractionOptions( + flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag, + ), onPositionChanged: !widget.follow ? null : (position, hasGesture) { @@ -326,7 +328,43 @@ class _RideMapState extends State with WidgetsBindingObserver { ), ); - return _sized(child: map); + // FB-06: the recenter control only ever makes sense once there is a `follow` + // mode to return to and the rider has actually panned away from it -- a static + // (non-following) map, like a finished ride in Trip Detail, never shows this. + final showRecenter = widget.follow && !_following; + + return _sized( + child: Stack( + children: [ + map, + if (showRecenter) + Positioned( + right: 16, + bottom: 16, + child: GlassPanel( + borderRadius: const BorderRadius.all(Radius.circular(999)), + child: IconButton( + key: const Key('recenter-button'), + icon: const Icon(Icons.my_location), + onPressed: _recenter, + ), + ), + ), + ], + ), + ); + } + + /// FB-06: return the camera to the rider's latest known position and resume + /// following new updates. Zoom is left untouched -- the rider may have + /// deliberately zoomed in or out, and recenter should not undo that. + void _recenter() { + final ll.LatLng? target = widget.points.isNotEmpty + ? ll.LatLng(widget.points.last.latitude, widget.points.last.longitude) + : widget.ambientPosition; + if (target == null) return; + setState(() => _following = true); + _controller.move(target, _controller.camera.zoom); } /// One polyline per speed run within each segment. diff --git a/test/ride_map_test.dart b/test/ride_map_test.dart index f782005..cbfba68 100644 --- a/test/ride_map_test.dart +++ b/test/ride_map_test.dart @@ -367,4 +367,192 @@ void main() { expect(find.byKey(const Key('location-marker')), findsNothing); }); }); + + group('always-interactive map + recenter (FB-06)', () { + testWidgets('pan/zoom is allowed even with no recorded points (idle map)', + (tester) async { + await tester.pumpWidget(MaterialApp( + theme: ripprTheme(), + home: const Scaffold( + body: RideMap(points: [], segments: [], showEmptyLabel: false), + ), + )); + await tester.pump(); + + final map = tester.widget(find.byType(FlutterMap)); + expect( + map.options.interactionOptions.flags & InteractiveFlag.drag, + InteractiveFlag.drag, + ); + expect( + map.options.interactionOptions.flags & InteractiveFlag.pinchZoom, + InteractiveFlag.pinchZoom, + ); + }); + + testWidgets('pan/zoom is allowed while recording (has points)', (tester) async { + final points = [for (var i = 0; i < 4; i++) p(1, i)]; + const segments = [Segment(id: 1, tripId: 1, startedAt: 0, endedAt: 1)]; + + await tester.pumpWidget(MaterialApp( + theme: ripprTheme(), + home: Scaffold(body: RideMap(points: points, segments: segments)), + )); + await tester.pump(); + + final map = tester.widget(find.byType(FlutterMap)); + expect( + map.options.interactionOptions.flags & InteractiveFlag.drag, + InteractiveFlag.drag, + ); + expect( + map.options.interactionOptions.flags & InteractiveFlag.pinchZoom, + InteractiveFlag.pinchZoom, + ); + }); + + testWidgets('no recenter button before any manual pan', (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, + follow: true, + ambientPosition: fix, + ), + ), + )); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsNothing); + }); + + testWidgets('no recenter button when follow is false, even after a pan', + (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, + ambientPosition: fix, + ), + ), + )); + await tester.pump(); + + final map = tester.widget(find.byType(FlutterMap)); + map.options.onPositionChanged?.call(map.mapController!.camera, true); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsNothing); + }); + + testWidgets('a manual pan while following shows the recenter button', + (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, + follow: true, + ambientPosition: fix, + ), + ), + )); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsNothing); + + final map = tester.widget(find.byType(FlutterMap)); + map.options.onPositionChanged!(map.mapController!.camera, true); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsOneWidget); + }); + + testWidgets( + 'tapping recenter moves the camera back to the ambient position and ' + 'hides the button again (following resumed)', (tester) async { + const fix1 = ll.LatLng(51.0, -114.0); + const fix2 = ll.LatLng(52.0, -115.0); + + await tester.pumpWidget(MaterialApp( + theme: ripprTheme(), + home: const Scaffold( + body: RideMap( + points: [], + segments: [], + showEmptyLabel: false, + follow: true, + ambientPosition: fix1, + ), + ), + )); + await tester.pump(); + + var map = tester.widget(find.byType(FlutterMap)); + // Simulate the rider manually panning away, then a fresh ambient fix + // arriving while following is off (so the camera does not auto-chase it). + map.options.onPositionChanged!(map.mapController!.camera, true); + map.mapController!.move(fix2, map.mapController!.camera.zoom); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsOneWidget); + + await tester.tap(find.byKey(const Key('recenter-button'))); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsNothing); + map = tester.widget(find.byType(FlutterMap)); + final center = map.mapController!.camera.center; + expect(center.latitude, closeTo(fix1.latitude, 1e-9), + reason: 'recenter should move back to the latest known ambient ' + 'position, not stay at the panned-to location'); + expect(center.longitude, closeTo(fix1.longitude, 1e-9)); + }); + + testWidgets('tapping recenter moves the camera to the latest recorded ' + 'point when recording', (tester) async { + final points = [for (var i = 0; i < 4; i++) p(1, i)]; + const segments = [Segment(id: 1, tripId: 1, startedAt: 0, endedAt: 1)]; + + await tester.pumpWidget(MaterialApp( + theme: ripprTheme(), + home: Scaffold( + body: RideMap( + points: points, + segments: segments, + follow: true, + ), + ), + )); + await tester.pump(); + + var map = tester.widget(find.byType(FlutterMap)); + map.options.onPositionChanged!(map.mapController!.camera, true); + map.mapController!.move(const ll.LatLng(0, 0), map.mapController!.camera.zoom); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsOneWidget); + + await tester.tap(find.byKey(const Key('recenter-button'))); + await tester.pump(); + + expect(find.byKey(const Key('recenter-button')), findsNothing); + map = tester.widget(find.byType(FlutterMap)); + final last = points.last; + final center = map.mapController!.camera.center; + expect(center.latitude, closeTo(last.latitude, 1e-9)); + expect(center.longitude, closeTo(last.longitude, 1e-9)); + }); + }); }