Files
rippr/docs/feedback/FB-04-route-planner-map-render-fix.md
uhryniuk e491c9ee65 FB-04: fix Route Planner map failing to render on cold navigation
routePlanProvider's AsyncLoading state collapsed with "route doesn't exist"
via .valueOrNull, so a brand-new route's screen briefly rendered "This route
no longer exists." with no FlutterMap in the tree before the DB stream's
first emission arrived. Add a hasValue guard mirroring waypointsAsync's
existing pattern, and replace the route planner's hardcoded initialZoom of
14 with FB-01's ambientZoom constant for street-level parity with the rest
of the app.
2026-08-24 16:12:06 -05:00

275 lines
15 KiB
Markdown

# 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** Done
## Goal
The Route Planner screen's map doesn't render at all for the user. Find the real cause
and fix it, and bring this screen's zoom in line with the rest of the app's new
street-level default (FB-01).
## Context
Direct user feedback (`docs/FEEDBACK.md`, "Route" section):
> This doesn't work at all, the map doesn't render at all... I want it to be the same
> zoomed in map view, then the user can zoom out and create the route.
### High-confidence root cause
`lib/src/ui/routes/route_planner_screen.dart`, `build()` (~lines 112-139):
```dart
@override
Widget build(BuildContext context) {
final colors = Theme.of(context).colorScheme;
final route = ref.watch(routePlanProvider(widget.routeId)).valueOrNull;
final waypointsAsync = ref.watch(routeWaypointsProvider(widget.routeId));
final waypoints = waypointsAsync.valueOrNull ?? const [];
final units = ref.watch(unitSystemProvider);
final repo = ref.read(routePlanRepositoryProvider);
if (route == null) {
return Scaffold(
backgroundColor: Colors.transparent,
body: SafeArea(
child: Center(
child: Text(
'This route no longer exists.',
style: TextStyle(color: colors.outline),
),
),
),
);
}
return Scaffold(
backgroundColor: Colors.transparent,
body: SafeArea(
child: !waypointsAsync.hasValue
? const Center(child: CircularProgressIndicator())
: Stack(
children: [
Positioned.fill(
child: FlutterMap(
...
```
Both `route` and `waypointsAsync` come from the same kind of provider —
`routePlanProvider`/`routeWaypointsProvider` are both `StreamProvider.autoDispose.family`
(`lib/src/app/providers.dart` ~lines 198-203) — so both start in `AsyncLoading` on a
fresh navigation (e.g. tapping "+ New route" on `RoutesListScreen`, which calls
`repo.createRoutePlan` then immediately `onOpenRoute?.call(id)` — a brand-new
`routePlanProvider(id)` family instance with a DB stream that hasn't delivered its
first row yet). **`waypointsAsync` is correctly guarded** — `!waypointsAsync.hasValue`
shows a spinner instead of building `FlutterMap` prematurely, with a comment explaining
exactly why (`initialCenter`/`initialZoom` are read once at construction; building too
early freezes the camera at null-island forever). **`route` has no equivalent guard** —
`.valueOrNull` collapses "still loading" and "genuinely doesn't exist" into the same
`null`, so on that same cold navigation, this screen renders the **text-only "This route
no longer exists." fallback with zero `FlutterMap` in the tree** — for a route that is
completely valid, simply because its stream's first emission hasn't arrived yet. This is
a real, reproducible-on-a-slow-cold-start bug, and it would read to a user as exactly
"the map doesn't render at all" — because for however long that loading window lasts,
there is no map, no pins, nothing to interact with.
This is the same shape of bug UI-06's own documented Outcome already found once in this
same file (a hardcoded, stale tile URL in `_DownloadDialog`): a place where near-
identical logic exists twice nearby and only one copy got the correct guard.
### Zoom special-cases to replace
Same `build()` method, ~line 157:
```dart
initialZoom: waypoints.length <= 1 ? 14 : maxTileZoom - 3,
```
`initialZoom` is only ever used by flutter_map when `initialCameraFit` is null.
`initialCameraFit: _initialFit(waypoints)` (line ~153) is non-null for 2+ waypoints
(see `_initialFit`, ~lines 92-102, which returns `null` only for `waypoints.length < 2`
or degenerate bounds) — so `maxTileZoom - 3` is **dead, misleading code today**: it can
only ever apply when `_initialFit` already returned null, which for 2+ waypoints only
happens on a degenerate (single-point) bounds, in which case `waypoints.length <= 1` is
false but the fit is still null — meaning `maxTileZoom - 3` actually *does* fire for
that one edge case (all waypoints at the same spot). Still, `14` for 0-1 waypoints is
the literal reason a fresh, empty route (or one with a single pin) opens zoomed out
far past street level, exactly matching the "same zoomed in map view" ask.
## Design
- **Fix the render bug**: change `route` from `.valueOrNull` to keeping the full
`AsyncValue<RoutePlan?>`, and add a `hasValue` guard mirroring `waypointsAsync`'s
existing pattern:
```dart
final routeAsync = ref.watch(routePlanProvider(widget.routeId));
final waypointsAsync = ref.watch(routeWaypointsProvider(widget.routeId));
final waypoints = waypointsAsync.valueOrNull ?? const [];
final units = ref.watch(unitSystemProvider);
final repo = ref.read(routePlanRepositoryProvider);
if (!routeAsync.hasValue) {
return const Scaffold(
backgroundColor: Colors.transparent,
body: Center(child: CircularProgressIndicator()),
);
}
final route = routeAsync.value;
if (route == null) {
return Scaffold( /* unchanged "This route no longer exists." body */ );
}
```
This exactly mirrors the two-step pattern `waypointsAsync` already uses two lines
below it in the same method, just applied consistently to both providers.
- **Zoom fix**: replace the `initialZoom` line with FB-01's shared constant. FB-01 adds
`const double ambientZoom = 17.0;` to `lib/src/ui/components/ride_map.dart` — this
screen already imports named constants from that file (`show TileAttribution,
maxTileZoom, tileMaxNativeZoom, tileSubdomains, tileUrlTemplate, tileUserAgent`, near
the top of the file) — add `ambientZoom` to that same `show` clause and use it:
```dart
initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3,
```
**If FB-01 has not landed yet when this ticket is implemented**, add the same
constant directly to `ride_map.dart` yourself first (`const double ambientZoom =
17.0;` near `maxTileZoom`/`shortRideZoom`) so this ticket doesn't block on ordering —
whichever of FB-01/FB-04 lands second will find the constant already defined and
should just reuse it rather than redefining it (grep for `ambientZoom` before adding
it, to avoid a duplicate-constant compile error).
- **Investigation order, if the above turns out not to be the whole story** (do this
first, before assuming the fix above is sufficient — reproduce, then fix):
1. `flutter run` on the Android emulator (or a device), tap "+ New route" from
`RoutesListScreen` (`lib/src/ui/routes/routes_list_screen.dart`, `Key('new-route')`
button), watch closely for a flash of "This route no longer exists." text or a
fully blank screen right after navigation.
2. If reproduced and the `hasValue` fix above resolves it, done — write up the root
cause and fix in the Outcome section.
3. If it's *not* resolved (map still doesn't render even with the `hasValue` guard in
place), add a temporary `debugPrint('route=${routeAsync.runtimeType} '
'waypoints=${waypointsAsync.runtimeType}')` at the top of `build()` and watch the
console across a fresh navigation to see the actual state sequence — remove it
before committing.
4. Check whether the map mounts (i.e. `FlutterMap` is in the tree, visible in the
Flutter inspector / a screenshot) but tiles themselves are blank — if so, look at
`cachedTileProviderProvider` (`lib/src/app/providers.dart` ~lines 234-238): it
returns `null` while `tileCacheProvider`'s underlying `FutureProvider` hasn't
resolved yet, and `TileLayer(tileProvider: null)` falls back to flutter_map's own
default network fetcher — verify that fallback actually fetches (it should; if it
silently doesn't, that's the bug).
5. Diff this screen's tile setup token-for-token against `RideMap`'s
(`tileUrlTemplate`, `tileSubdomains`, `tileMaxNativeZoom`, `tileUserAgent` — all
already imported from the same `ride_map.dart` export, so a stale-URL-style
regression is unlikely, but `grep -n "tile.openstreetmap\|cartocdn"` across this
file to be certain nothing reintroduced UI-06's already-fixed bug).
## Implementation
1. Change `route` to `routeAsync` (full `AsyncValue`), add the `hasValue` guard, keep
the rest of the "no longer exists" branch's body unchanged.
2. Add `ambientZoom` to this file's `ride_map.dart` import `show` clause (adding the
constant to `ride_map.dart` first if FB-01 hasn't landed yet — see Design).
3. Replace `waypoints.length <= 1 ? 14 : maxTileZoom - 3` with
`waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3`.
4. Reproduce on an emulator per the Investigation order above; if the root cause is
something other than the `hasValue` gap, document the real cause and its fix in the
Outcome section instead of (or in addition to) the above.
## Acceptance criteria
- [ ] Opening a brand-new route ("+ New route" from `RoutesListScreen`) always shows a
loading spinner briefly (if the stream hasn't emitted yet) and then the real map
canvas — never the "This route no longer exists." text for a route that
genuinely exists.
- [ ] Opening an existing, previously-created route with no waypoints shows the map
canvas at `ambientZoom` (street level), not zoom 14.
- [ ] A route that is actually deleted (or never existed — e.g. a stale/invalid id)
still correctly shows "This route no longer exists." — the fix must not weaken
this case, only stop it from firing on a merely-still-loading valid route.
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
## Tests
- Widget test: pump `RoutePlannerScreen` for a route id that has a pending (not-yet-
resolved) `routePlanProvider` stream — assert a `CircularProgressIndicator` shows,
never the "no longer exists" text, then once the stream emits the real row, assert
the map (`FlutterMap`) appears. (Check how existing tests in
`test/route_planner_screen_test.dart` seed/await the repository — likely via
`repo.createRoutePlan` then pumping a frame or two before the provider's stream
delivers, similar to the existing `pumpMap` helper in that file's own comments about
needing a handful of frames for the waypoints stream's first value.)
- Widget test: pump `RoutePlannerScreen` for an id that was never created (or was
deleted) — assert "This route no longer exists." still shows once the stream settles
(not immediately, if the provider briefly reports loading first).
- Widget test: a route with zero waypoints renders at `ambientZoom`, not `14` — read
the `FlutterMap`'s `options.initialZoom` via `tester.widget<FlutterMap>(...)` the same
way other zoom-related assertions in this test suite already inspect `MapOptions`
(check existing patterns in `test/ride_map_test.dart`/
`test/route_planner_screen_test.dart`).
- If the investigation reveals a different root cause than the `hasValue` gap, write a
regression test for the actual bug found, not just the hypothesis above.
## Risks
- If the `hasValue` fix doesn't fully explain what the user saw, this ticket's
Outcome section must say so plainly and document whatever the actual root cause
turned out to be — don't claim a fix that wasn't verified to address the real
symptom. Reproducing on a real emulator/device before declaring this done is
important precisely because the original bug report is "doesn't work at all," a
strong signal, and a subtle timing bug is an easy thing to fix on paper without
confirming it was actually the cause.
## Out of scope
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).
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.