Merge FB-07: fix Route Planner opening on Null Island
# Conflicts: # docs/feedback/FB-07-route-planner-null-island.md
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# FB-07 — Route Planner opens on Null Island instead of the rider's real location
|
# 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** Not started
|
**Depends on** FB-01 (reuses its ambient-location provider) · **Size** S/M · **Status** Done
|
||||||
|
|
||||||
## Goal
|
## Goal
|
||||||
A new route, or a route with fewer than two pins, must open the map on the rider's real
|
A new route, or a route with fewer than two pins, must open the map on the rider's real
|
||||||
@@ -147,3 +147,56 @@ pattern, do not invent a new one.
|
|||||||
## Out of scope
|
## Out of scope
|
||||||
Any change to the Map tab's own map (FB-06 covers its remaining issues). Turn-by-turn
|
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).
|
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.
|
||||||
|
|||||||
@@ -119,6 +119,16 @@ class _RoutePlannerScreenState extends ConsumerState<RoutePlannerScreen>
|
|||||||
final units = ref.watch(unitSystemProvider);
|
final units = ref.watch(unitSystemProvider);
|
||||||
final repo = ref.read(routePlanRepositoryProvider);
|
final repo = ref.read(routePlanRepositoryProvider);
|
||||||
|
|
||||||
|
// FB-07: a brand-new route (or one with <2 pins) should open on the rider's real
|
||||||
|
// location rather than Null Island (0, 0). This mirrors ShellScaffold's own use of
|
||||||
|
// ambientPositionProvider (lib/src/ui/app_shell.dart). A missing fix (permission
|
||||||
|
// denied, service disabled, or not arrived yet) falls back to (0, 0) immediately --
|
||||||
|
// it must not block the map the way waypointsAsync is blocked above.
|
||||||
|
final ambientFix = ref.watch(ambientPositionProvider).valueOrNull;
|
||||||
|
final ambientPosition = ambientFix == null
|
||||||
|
? null
|
||||||
|
: ll.LatLng(ambientFix.latitude, ambientFix.longitude);
|
||||||
|
|
||||||
// Mirrors waypointsAsync's own guard below: `.valueOrNull` alone can't tell "still
|
// 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
|
// loading" apart from "genuinely doesn't exist" -- both collapse to null. On a
|
||||||
// brand-new route (fresh navigation right after `repo.createRoutePlan`), this
|
// brand-new route (fresh navigation right after `repo.createRoutePlan`), this
|
||||||
@@ -168,7 +178,7 @@ class _RoutePlannerScreenState extends ConsumerState<RoutePlannerScreen>
|
|||||||
options: MapOptions(
|
options: MapOptions(
|
||||||
initialCameraFit: _initialFit(waypoints),
|
initialCameraFit: _initialFit(waypoints),
|
||||||
initialCenter: waypoints.isEmpty
|
initialCenter: waypoints.isEmpty
|
||||||
? const ll.LatLng(0, 0)
|
? (ambientPosition ?? 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 ? ambientZoom : maxTileZoom - 3,
|
initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3,
|
||||||
maxZoom: maxTileZoom,
|
maxZoom: maxTileZoom,
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import 'package:flutter_test/flutter_test.dart';
|
|||||||
import 'package:rippr/src/app/providers.dart';
|
import 'package:rippr/src/app/providers.dart';
|
||||||
import 'package:rippr/src/data/database.dart';
|
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/recording/location_source.dart' show LocationFix;
|
||||||
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/components/ride_map.dart' show ambientZoom;
|
||||||
@@ -30,8 +31,8 @@ void main() {
|
|||||||
|
|
||||||
tearDown(() async => db.close());
|
tearDown(() async => db.close());
|
||||||
|
|
||||||
Widget host(Widget child) => ProviderScope(
|
Widget host(Widget child, {List<Override> overrides = const []}) => ProviderScope(
|
||||||
overrides: [databaseProvider.overrideWithValue(db)],
|
overrides: [databaseProvider.overrideWithValue(db), ...overrides],
|
||||||
child: MaterialApp(theme: ripprTheme(), home: child),
|
child: MaterialApp(theme: ripprTheme(), home: child),
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -227,6 +228,79 @@ void main() {
|
|||||||
expect(map.options.initialZoom, ambientZoom);
|
expect(map.options.initialZoom, ambientZoom);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
screenTest('a brand-new route opens centered on the rider\'s real location, '
|
||||||
|
'not Null Island (FB-07)', (tester) async {
|
||||||
|
const fix = LocationFix(
|
||||||
|
timestamp: 1000,
|
||||||
|
latitude: 51.05,
|
||||||
|
longitude: -114.07,
|
||||||
|
speedMps: 0,
|
||||||
|
altitudeM: 0,
|
||||||
|
accuracyM: 5,
|
||||||
|
bearingDeg: 0,
|
||||||
|
);
|
||||||
|
final id = await repo.createRoutePlan(1000);
|
||||||
|
await pumpMap(
|
||||||
|
tester,
|
||||||
|
host(
|
||||||
|
RoutePlannerScreen(routeId: id),
|
||||||
|
overrides: [
|
||||||
|
ambientPositionProvider.overrideWith((ref) => Stream.value(fix)),
|
||||||
|
],
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
|
||||||
|
expect(map.options.initialCenter.latitude, closeTo(51.05, 1e-9));
|
||||||
|
expect(map.options.initialCenter.longitude, closeTo(-114.07, 1e-9));
|
||||||
|
});
|
||||||
|
|
||||||
|
screenTest('a brand-new route falls back to (0, 0) when no ambient fix is '
|
||||||
|
'available (FB-07)', (tester) async {
|
||||||
|
final id = await repo.createRoutePlan(1000);
|
||||||
|
await pumpMap(
|
||||||
|
tester,
|
||||||
|
host(
|
||||||
|
RoutePlannerScreen(routeId: id),
|
||||||
|
overrides: [
|
||||||
|
ambientPositionProvider.overrideWith((ref) => Stream.value(null)),
|
||||||
|
],
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
|
||||||
|
expect(map.options.initialCenter.latitude, 0);
|
||||||
|
expect(map.options.initialCenter.longitude, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
screenTest('a route with a real waypoint still centers on the pin, not the '
|
||||||
|
'ambient position (FB-07)', (tester) async {
|
||||||
|
const fix = LocationFix(
|
||||||
|
timestamp: 1000,
|
||||||
|
latitude: 51.05,
|
||||||
|
longitude: -114.07,
|
||||||
|
speedMps: 0,
|
||||||
|
altitudeM: 0,
|
||||||
|
accuracyM: 5,
|
||||||
|
bearingDeg: 0,
|
||||||
|
);
|
||||||
|
final id = await repo.createRoutePlan(1000);
|
||||||
|
await repo.addWaypoint(id, 51.0, -114.0);
|
||||||
|
await pumpMap(
|
||||||
|
tester,
|
||||||
|
host(
|
||||||
|
RoutePlannerScreen(routeId: id),
|
||||||
|
overrides: [
|
||||||
|
ambientPositionProvider.overrideWith((ref) => Stream.value(fix)),
|
||||||
|
],
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
final map = tester.widget<FlutterMap>(find.byType(FlutterMap));
|
||||||
|
expect(map.options.initialCenter.latitude, closeTo(51.0, 1e-9));
|
||||||
|
expect(map.options.initialCenter.longitude, closeTo(-114.0, 1e-9));
|
||||||
|
});
|
||||||
|
|
||||||
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 {
|
||||||
|
|||||||
Reference in New Issue
Block a user