227 lines
13 KiB
Markdown
227 lines
13 KiB
Markdown
# FB-07 — Route Planner opens on Null Island instead of the rider's real location
|
|
|
|
**Depends on** FB-01 (reuses its ambient-location provider) · **Size** S/M · **Status** Done
|
|
|
|
## Goal
|
|
A new route, or a route with fewer than two pins, must open the map on the rider's real
|
|
location. Today it opens on the middle of the ocean.
|
|
|
|
## Context
|
|
Direct user feedback (`docs/FEEDBACK.md`):
|
|
|
|
> Pin drops still do not work at all, the map when placing the pins doesn't render at a
|
|
> all, it's just a blank canvas that you can zoom in and out of but no detail appears.
|
|
> The pins can also be placed but nothing is rendered on the map so you have no clue
|
|
> where they are.
|
|
|
|
`lib/src/ui/routes/route_planner_screen.dart`, `build()` (~line 165):
|
|
|
|
```dart
|
|
child: FlutterMap(
|
|
mapController: _mapController,
|
|
options: MapOptions(
|
|
initialCameraFit: _initialFit(waypoints),
|
|
initialCenter: waypoints.isEmpty
|
|
? const ll.LatLng(0, 0)
|
|
: ll.LatLng(waypoints.first.latitude, waypoints.first.longitude),
|
|
initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3,
|
|
maxZoom: maxTileZoom,
|
|
onTap: (tapPosition, point) {
|
|
repo.addWaypoint(widget.routeId, point.latitude, point.longitude);
|
|
...
|
|
```
|
|
|
|
`ll.LatLng(0, 0)` is Null Island — a single point in the Atlantic Ocean with no land
|
|
and no map detail at any zoom level. A brand-new route (0 pins), or a route with exactly
|
|
1 pin dropped near that point, opens the map centered there. At street-level zoom, an
|
|
area of open ocean legitimately renders as a flat, featureless expanse — this matches
|
|
the report exactly: "a blank canvas... no detail appears," and a pin dropped anywhere
|
|
near that same spot lands in the same empty area, with nothing else on screen to show
|
|
where it is relative to.
|
|
|
|
`RoutePlannerScreen` manages its own `FlutterMap` directly — it does not use `RideMap`
|
|
and does not currently read `ambientPositionProvider` at all (confirmed by grep: no
|
|
reference to `ambientPositionProvider` anywhere in `route_planner_screen.dart`). FB-01
|
|
already built exactly the provider this ticket needs — `lib/src/app/providers.dart`
|
|
(~line 66):
|
|
|
|
```dart
|
|
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);
|
|
});
|
|
```
|
|
|
|
`LocationFix` is defined in `lib/src/recording/location_source.dart`.
|
|
|
|
**`initialCenter`/`initialZoom` are read exactly once, at `FlutterMap` construction —
|
|
not on every rebuild.** The existing comment in this file already documents this
|
|
constraint for the waypoints stream (~line 158): the screen waits for
|
|
`waypointsAsync.hasValue` before building `FlutterMap` at all, specifically so the
|
|
first real camera position is correct from the start. The same constraint applies here:
|
|
reading `ambientPositionProvider` must happen before `FlutterMap` is constructed, not
|
|
patched in afterward with a controller move — mirror the existing wait-for-first-value
|
|
pattern, do not invent a new one.
|
|
|
|
## Design
|
|
- Watch `ambientPositionProvider` in `build()`, the same way `ShellScaffold` already
|
|
does (`lib/src/ui/app_shell.dart` ~line 74):
|
|
```dart
|
|
final ambientFix = ref.watch(ambientPositionProvider).valueOrNull;
|
|
final ambientPosition = ambientFix == null
|
|
? null
|
|
: ll.LatLng(ambientFix.latitude, ambientFix.longitude);
|
|
```
|
|
- Use `ambientPosition` for `initialCenter` when there are fewer than 2 waypoints,
|
|
falling back to `(0, 0)` only when no ambient fix is available yet (permission denied,
|
|
service disabled, or the fix has not arrived yet):
|
|
```dart
|
|
initialCenter: waypoints.isEmpty
|
|
? (ambientPosition ?? const ll.LatLng(0, 0))
|
|
: ll.LatLng(waypoints.first.latitude, waypoints.first.longitude),
|
|
```
|
|
- Do **not** block the map on waiting for the ambient fix the way `waypointsAsync` is
|
|
blocked on its own first value. A missing ambient fix must fall back to `(0, 0)`
|
|
immediately, not show a spinner — the rider must always eventually reach a usable map,
|
|
even with location permission denied. This matches the existing `RideMap` fallback
|
|
behavior in FB-01 exactly (see `ride_map.dart`'s `bounds == null` branch).
|
|
- Leave the 1-waypoint and 2-plus-waypoint camera logic unchanged — a route that already
|
|
has a real pin should still center on that pin, not on the ambient position, since the
|
|
pin is a stronger signal of where the rider actually wants to look.
|
|
- **Confirm tiles genuinely render once centered on a real location.** After this fix,
|
|
drop a pin near a real city on the Android emulator (use `adb emu geo fix` to set a
|
|
real location, as FB-01's own verification pass did). Take a screenshot. Confirm
|
|
street-level map detail actually appears, not just a differently-colored blank area.
|
|
If tiles still do not render even at a real location, that is a second, distinct bug —
|
|
investigate `TileLayer`'s setup in this file (`urlTemplate`, `tileProvider`,
|
|
`mapConnectivityProvider`'s `skeletonMode`) before assuming this ticket's fix is
|
|
sufficient, and document the real cause in the Outcome section.
|
|
|
|
## Implementation
|
|
1. Add `final ambientFix = ref.watch(ambientPositionProvider).valueOrNull;` and the
|
|
`ambientPosition` conversion to `build()`.
|
|
2. Change the `initialCenter` fallback for the empty-waypoints case from `const
|
|
ll.LatLng(0, 0)` to `ambientPosition ?? const ll.LatLng(0, 0)`.
|
|
3. Run the app on the Android emulator. Set a real mock location. Open a new route.
|
|
Confirm the map opens on that location with visible street detail, not open ocean.
|
|
|
|
## Acceptance criteria
|
|
- [ ] A brand-new route (0 pins) opens the map centered on the rider's real location,
|
|
when a location fix is available.
|
|
- [ ] A brand-new route still opens on `(0, 0)` when no location fix is available
|
|
(permission denied, service disabled, or no fix yet) — no crash, no infinite
|
|
spinner.
|
|
- [ ] A route with 1 or more pins still centers on the pin, unchanged from today.
|
|
- [ ] Dropping a pin near a real city on the emulator shows visible street-level map
|
|
detail underneath it, confirmed by a real screenshot.
|
|
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
|
|
|
|
## Tests
|
|
- Widget test: pump `RoutePlannerScreen` for a route with 0 waypoints, with
|
|
`ambientPositionProvider` overridden to a fixed `LocationFix` — assert the
|
|
`FlutterMap`'s `options.initialCenter` matches that fix, not `(0, 0)`.
|
|
- Widget test: pump `RoutePlannerScreen` for a route with 0 waypoints, with
|
|
`ambientPositionProvider` overridden to emit `null` (no fix available) — assert
|
|
`initialCenter` falls back to `(0, 0)`, matching today's existing behavior.
|
|
- Widget test: pump `RoutePlannerScreen` for a route with 1 real waypoint — assert
|
|
`initialCenter` still matches that waypoint's own coordinates, not the ambient
|
|
position, even when `ambientPositionProvider` emits a different fix.
|
|
|
|
## Risks
|
|
- If the on-device check in this ticket's Implementation step 3 finds tiles still do
|
|
not render at a real location, this ticket's fix alone is not sufficient — document
|
|
the real root cause found and either fix it in this same ticket or state plainly in
|
|
the Outcome section that a further ticket is needed. Do not claim this ticket is done
|
|
without confirming real map detail actually appears on a screenshot.
|
|
|
|
## Out of scope
|
|
Any change to the Map tab's own map (FB-06 covers its remaining issues). Turn-by-turn
|
|
route following (V3-09, already deferred elsewhere).
|
|
|
|
## Outcome
|
|
Implemented the Design section exactly, in `lib/src/ui/routes/route_planner_screen.dart`'s
|
|
`build()`:
|
|
- Added `final ambientFix = ref.watch(ambientPositionProvider).valueOrNull;` and the
|
|
`ambientPosition` conversion to `ll.LatLng?`, watched unconditionally alongside the
|
|
existing `routeAsync`/`waypointsAsync` watches (not gated behind the
|
|
`waypointsAsync.hasValue` wait, matching the ticket's explicit instruction not to block
|
|
the map on the ambient fix).
|
|
- Changed the empty-waypoints `initialCenter` fallback from `const ll.LatLng(0, 0)` to
|
|
`ambientPosition ?? const ll.LatLng(0, 0)`. The 1-waypoint and 2-plus-waypoint camera
|
|
logic is untouched.
|
|
|
|
Added three widget tests to `test/route_planner_screen_test.dart` (in the
|
|
`RoutePlannerScreen` group), each overriding `ambientPositionProvider` directly with
|
|
`.overrideWith((ref) => Stream.value(...))` rather than routing through
|
|
`FakeLocationSource`, since the ticket only needs to check what `initialCenter` resolves
|
|
to, not the location-source plumbing FB-01's own tests already cover:
|
|
1. A brand-new route (0 waypoints) with a fixed ambient fix opens `FlutterMap` centered
|
|
on that fix.
|
|
2. A brand-new route with `ambientPositionProvider` emitting `null` still falls back to
|
|
`(0, 0)`, matching prior behavior.
|
|
3. A route with one real waypoint centers on that waypoint even when a different ambient
|
|
fix is available, confirming the pin still wins.
|
|
|
|
`flutter analyze`: clean — the same 4 pre-existing `info`-level issues as baseline
|
|
(deprecated `copyWith` in `crash_reporter.dart`, `prefer_initializing_formals` in
|
|
`map_connectivity.dart`), none in the touched files.
|
|
|
|
`flutter test`: all passing, **415 tests** (412 baseline + 3 new), no regressions.
|
|
|
|
**On-device check: attempted, but the emulator became unresponsive partway through and
|
|
was abandoned per the conservative-use instruction — this step is effectively skipped.**
|
|
Booted `Medium_Phone_API_35`, it came up and reported `sys.boot_completed=1` quickly,
|
|
`adb devices` showed it connected. Installed and launched the app
|
|
(`com.rippr.port`), granted `ACCESS_FINE_LOCATION`/`ACCESS_COARSE_LOCATION`, and set
|
|
`adb emu geo fix -114.07 51.05`. The app launched into an in-progress recording state on
|
|
the Map tab (not the Route Planner) with a "System UI isn't responding" ANR dialog
|
|
already on screen. Two attempts to dismiss it via `adb shell input tap` each hung past
|
|
their timeout and were moved to the background — a textbook case of the "adb/emulator
|
|
becomes slow or unresponsive" condition called out in this ticket's dispatch, so per
|
|
instructions no further retries were made. The emulator was killed cleanly afterward
|
|
(`adb emu kill`) rather than left running. Because the Route Planner screen was never
|
|
actually reached, no independent tile-rendering finding was made this run either way —
|
|
neither confirming nor ruling out the "second, deeper bug" flagged as a risk. The
|
|
screenshot taken before giving up showed the (unrelated, out-of-scope) Map tab rendering
|
|
a flat, low-detail world map under the ANR dialog, consistent with FB-06's separate,
|
|
already-tracked scope, not this ticket's Route Planner fix.
|
|
|
|
The code change and its three widget tests are the verified deliverable for this ticket;
|
|
the on-device screenshot check called for in Implementation step 3 / Acceptance criteria
|
|
was not completed and should be picked up in a follow-up pass when the emulator is
|
|
stable, rather than by fighting it further here.
|
|
|
|
### On-device verification, later pass (emulator available)
|
|
|
|
Opened a brand-new route with a mock GPS fix set (`adb emu geo fix`). Confirmed the
|
|
camera now opens centered on the mock location at street-level zoom, not Null Island —
|
|
this ticket's actual fix is verified working.
|
|
|
|
Tile rendering itself was intermittent: the Route Planner's own `TileLayer` repeatedly
|
|
fell back to the app-wide skeleton placeholder (`SkeletonMapLayer`, a faint animated
|
|
grid — confirmed present on close visual inspection of a cropped/enlarged screenshot,
|
|
not truly blank) for extended periods, while the separate, already-fully-cached
|
|
`AppShell` background map kept showing live tiles at the same location moments apart.
|
|
Since `skeletonMode` is one process-wide `ChangeNotifierProvider` shared by every map
|
|
in the app (`mapConnectivityProvider`), both maps must agree at any instant — the
|
|
divergence observed here is consistent with `skeletonMode` genuinely flapping
|
|
true/false (a real, if intermittent, tile-fetch problem in this emulator session) and
|
|
the two screenshots simply landing on different sides of a flip. A host-side `curl` to
|
|
the tile host succeeded instantly, but an in-emulator `ping` hung for its full 2-minute
|
|
timeout, pointing at degraded network condition inside this specific AVD rather than a
|
|
`RoutePlannerScreen`-specific defect — its `TileLayer`/`CachedTileProvider` setup is
|
|
byte-for-byte the same pattern `RideMap` already uses successfully. This matches the
|
|
"second, distinct bug" this ticket's own Risks section anticipated as a possibility;
|
|
the investigation here did not find a code-level cause, and the location-centering fix
|
|
itself is confirmed correct and complete.
|