FB-07: fix Route Planner opening on Null Island
A brand-new route (or one with fewer than two pins) now falls back to the ambient GPS position for its initial map center instead of (0, 0) in the middle of the ocean, reusing FB-01's ambientPositionProvider. Routes with one or more real waypoints are unaffected. Falls back to (0, 0) as before when no ambient fix is available. Adds three widget tests covering the fixed-fix, no-fix, and real-waypoint-wins cases. Emulator verification was attempted but the Android emulator became unresponsive partway through and was abandoned per the ticket's conservative-use guidance; details in the ticket's Outcome section.
This commit is contained in:
202
docs/feedback/FB-07-route-planner-null-island.md
Normal file
202
docs/feedback/FB-07-route-planner-null-island.md
Normal file
@@ -0,0 +1,202 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user