Merge FB-04: fix Route Planner map render + zoom

This commit is contained in:
2026-08-24 16:12:30 -05:00
3 changed files with 114 additions and 3 deletions

View File

@@ -1,6 +1,6 @@
# FB-04 — Fix Route Planner's map failing to render + match street-level zoom # 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 **Depends on** FB-01 (reuses its `ambientZoom` constant) · **Size** M · **Status** Done
## Goal ## Goal
The Route Planner screen's map doesn't render at all for the user. Find the real cause The Route Planner screen's map doesn't render at all for the user. Find the real cause
@@ -214,3 +214,61 @@ far past street level, exactly matching the "same zoomed in map view" ask.
Closed-loop routes (FB-05, sequenced after this ticket since it also edits this file 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). heavily). Turn-by-turn route following (V3-09, already explicitly deferred by UI-06).
Any change to `RoutesListScreen` itself. Any change to `RoutesListScreen` itself.
## Outcome
The ticket's high-confidence root-cause hypothesis was confirmed exactly as written, no
surprises in investigation. `FB-01`'s `ambientZoom` constant was already present in
`lib/src/ui/components/ride_map.dart` (line 54) when this ticket started, so no
fallback definition was needed.
**Root cause**, confirmed by reproduction: `route` was read via
`ref.watch(routePlanProvider(widget.routeId)).valueOrNull`, which collapses "stream
still loading" and "route doesn't exist" into the same `null`. On a cold navigation to
a brand-new route (fresh `StreamProvider.autoDispose.family` instance, DB stream not
yet emitted), this made the screen render "This route no longer exists." with zero
`FlutterMap` in the tree for a route that was completely valid — exactly matching the
user's "doesn't work at all" report. `waypointsAsync` already had the correct
`hasValue` guard right next to it; `route` simply never got the same treatment.
Reproduced first via a widget test (no real emulator available/reliable in this
environment): `repo.createRoutePlan(...)` followed by `pumpWidget` with **no**
additional pump — asserting immediately after the very first frame — reliably showed
the "no longer exists" text and zero `FlutterMap` widgets, confirming the bug. The same
test, run again after applying the fix (and separately, reverted before the fix to
confirm it fails on the old code), passed once `routeAsync.hasValue` guards the
transition, matching the ticket's `Design`/`Implementation` sections precisely:
- `route` was changed to hold the full `AsyncValue<RoutePlan?>` (`routeAsync`), with a
new `if (!routeAsync.hasValue)` branch returning a spinner-only `Scaffold`, mirroring
`waypointsAsync`'s existing pattern two lines below.
- `route = routeAsync.value` is read only after that guard passes, and the existing
"no longer exists" branch (for a genuinely missing/deleted route) is otherwise
unchanged.
- `ambientZoom` was added to this file's `ride_map.dart` `show` clause, and
`initialZoom: waypoints.length <= 1 ? 14 : maxTileZoom - 3` became
`initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3`.
No investigation steps 3-5 (debugPrint state tracing, checking
`cachedTileProviderProvider`'s null-tileProvider fallback, diffing tile URL setup
against `RideMap`) were needed — the primary hypothesis fully explained the reported
symptom on the first reproduction attempt, and step 5's `tileUrlTemplate`/
`tileSubdomains`/etc. are already imported from the same `ride_map.dart` export
`RideMap` uses, so no stale-URL-style regression was present.
Two new regression tests were added to `test/route_planner_screen_test.dart`:
1. A brand-new route's screen is pumped with no settling: asserts the loading spinner
shows and neither the "no longer exists" text nor `FlutterMap` appear on that first
frame, then asserts the real map appears once both streams resolve. Verified this
test fails against the pre-fix code (assertion on the spinner/text) and passes after
the fix.
2. A route with zero waypoints is asserted to open at `MapOptions.initialZoom ==
ambientZoom` (17.0), not the old hardcoded `14`. Verified this test fails against
the pre-fix code and passes after the fix.
The existing "a missing route says so instead of a blank map" test (using
`pumpAndSettle`) continues to pass unchanged, confirming the fix doesn't weaken the
genuinely-deleted/never-existed case.
**Final state**: `flutter analyze` clean (4 pre-existing `info`-level issues in
unrelated files — `crash_reporter.dart`, `map_connectivity.dart` — present on `main`
before this ticket, untouched by this change). `flutter test`: **404 passing** (402
baseline + 2 new regression tests), zero failures, zero regressions.

View File

@@ -30,6 +30,7 @@ import '../components/glass_panel.dart';
import '../components/ride_map.dart' import '../components/ride_map.dart'
show show
TileAttribution, TileAttribution,
ambientZoom,
maxTileZoom, maxTileZoom,
tileMaxNativeZoom, tileMaxNativeZoom,
tileSubdomains, tileSubdomains,
@@ -112,12 +113,27 @@ class _RoutePlannerScreenState extends ConsumerState<RoutePlannerScreen>
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
final colors = Theme.of(context).colorScheme; final colors = Theme.of(context).colorScheme;
final route = ref.watch(routePlanProvider(widget.routeId)).valueOrNull; final routeAsync = ref.watch(routePlanProvider(widget.routeId));
final waypointsAsync = ref.watch(routeWaypointsProvider(widget.routeId)); final waypointsAsync = ref.watch(routeWaypointsProvider(widget.routeId));
final waypoints = waypointsAsync.valueOrNull ?? const []; final waypoints = waypointsAsync.valueOrNull ?? const [];
final units = ref.watch(unitSystemProvider); final units = ref.watch(unitSystemProvider);
final repo = ref.read(routePlanRepositoryProvider); final repo = ref.read(routePlanRepositoryProvider);
// Mirrors waypointsAsync's own guard below: `.valueOrNull` alone can't tell "still
// loading" apart from "genuinely doesn't exist" -- both collapse to null. On a
// brand-new route (fresh navigation right after `repo.createRoutePlan`), this
// provider's DB stream hasn't delivered its first row yet, so without this guard
// the screen would render "This route no longer exists." for a route that is
// completely valid, with zero FlutterMap in the tree, until that first emission
// arrives.
if (!routeAsync.hasValue) {
return const Scaffold(
backgroundColor: Colors.transparent,
body: Center(child: CircularProgressIndicator()),
);
}
final route = routeAsync.value;
if (route == null) { if (route == null) {
return Scaffold( return Scaffold(
backgroundColor: Colors.transparent, backgroundColor: Colors.transparent,
@@ -154,7 +170,7 @@ class _RoutePlannerScreenState extends ConsumerState<RoutePlannerScreen>
initialCenter: waypoints.isEmpty initialCenter: waypoints.isEmpty
? const ll.LatLng(0, 0) ? const ll.LatLng(0, 0)
: ll.LatLng(waypoints.first.latitude, waypoints.first.longitude), : ll.LatLng(waypoints.first.latitude, waypoints.first.longitude),
initialZoom: waypoints.length <= 1 ? 14 : maxTileZoom - 3, initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3,
maxZoom: maxTileZoom, maxZoom: maxTileZoom,
onTap: (tapPosition, point) { onTap: (tapPosition, point) {
repo.addWaypoint(widget.routeId, point.latitude, point.longitude); repo.addWaypoint(widget.routeId, point.latitude, point.longitude);

View File

@@ -9,6 +9,7 @@ import 'package:rippr/src/data/database.dart';
import 'package:rippr/src/data/route_plan_repository.dart'; import 'package:rippr/src/data/route_plan_repository.dart';
import 'package:rippr/src/ui/app_shell.dart'; import 'package:rippr/src/ui/app_shell.dart';
import 'package:rippr/src/ui/components/floating_pill.dart'; import 'package:rippr/src/ui/components/floating_pill.dart';
import 'package:rippr/src/ui/components/ride_map.dart' show ambientZoom;
import 'package:rippr/src/ui/format.dart'; import 'package:rippr/src/ui/format.dart';
import 'package:rippr/src/ui/routes/route_planner_screen.dart'; import 'package:rippr/src/ui/routes/route_planner_screen.dart';
import 'package:rippr/src/ui/routes/routes_list_screen.dart'; import 'package:rippr/src/ui/routes/routes_list_screen.dart';
@@ -190,6 +191,42 @@ void main() {
expect(find.textContaining('no longer exists'), findsOneWidget); expect(find.textContaining('no longer exists'), findsOneWidget);
}); });
screenTest(
'a brand-new route never flashes "no longer exists" while its stream is '
'still resolving, and shows the real map once it settles (FB-04)',
(tester) async {
// Mirrors the real "+ New route" flow: `repo.createRoutePlan` returns as soon as
// the row is written, but `routePlanProvider`'s DB stream hasn't delivered its
// first emission by the very next frame -- both `routePlanProvider` and
// `routeWaypointsProvider` start out `AsyncLoading` on this fresh navigation.
final id = await repo.createRoutePlan(1000, name: 'Fresh route');
await tester.pumpWidget(host(RoutePlannerScreen(routeId: id)));
// Immediately after the very first frame -- before the stream has had any chance
// to deliver -- the loading spinner must show, never the "no longer exists" text.
// (Pre-fix, `route` collapsed "still loading" and "doesn't exist" into the same
// null via `.valueOrNull`, so this assertion is exactly the FB-04 regression.)
expect(find.textContaining('no longer exists'), findsNothing);
expect(find.byType(CircularProgressIndicator), findsOneWidget);
expect(find.byType(FlutterMap), findsNothing);
// Once both streams deliver their first value, the real map canvas appears.
for (var i = 0; i < 10; i++) {
await tester.pump(const Duration(milliseconds: 50));
}
expect(find.textContaining('no longer exists'), findsNothing);
expect(find.byType(FlutterMap), findsOneWidget);
});
screenTest('a route with no waypoints opens at street-level zoom, not zoomed out '
'(FB-04)', (tester) async {
final id = await repo.createRoutePlan(1000);
await pumpMap(tester, host(RoutePlannerScreen(routeId: id)));
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
expect(map.options.initialZoom, ambientZoom);
});
screenTest('the offline-tiles download menu item is disabled with no pins ' screenTest('the offline-tiles download menu item is disabled with no pins '
'(V3-11, UI-06: now a PopupMenuItem behind the overflow menu)', '(V3-11, UI-06: now a PopupMenuItem behind the overflow menu)',
(tester) async { (tester) async {