diff --git a/docs/ui-redesign/README.md b/docs/ui-redesign/README.md index 753480a..52797de 100644 --- a/docs/ui-redesign/README.md +++ b/docs/ui-redesign/README.md @@ -24,7 +24,7 @@ v3 held to. | [UI-03](UI-03-glass-component-kit.md) | Shared floating-glass component kit | M | UI-08 | Done | | [UI-04](UI-04-customizable-hud-widgets.md) | Customizable HUD telemetry widgets (drag, resize, visibility toggle) | L | UI-03 | Done | | [UI-05](UI-05-map-hud-record-screen.md) | Map HUD: Record screen redesign | M | UI-01, UI-03, UI-04 | Done | -| [UI-06](UI-06-plan-and-route-planning.md) | Plan & Route Planning redesign | M | UI-01, UI-03 | Not started | +| [UI-06](UI-06-plan-and-route-planning.md) | Plan & Route Planning redesign | M | UI-01, UI-03 | Done | | [UI-07](UI-07-rides-history.md) | Rides History redesign | M | UI-01, UI-03 | Not started | | [UI-08](UI-08-theme-modern-professional-dark.md) | Theme migration to Modern Professional Dark | S | — | Done | | [UI-09](UI-09-monochrome-dark-map-tiles.md) | Monochrome dark map tiles | S | — | Done | diff --git a/docs/ui-redesign/UI-06-plan-and-route-planning.md b/docs/ui-redesign/UI-06-plan-and-route-planning.md index f5fd7f0..9dd75af 100644 --- a/docs/ui-redesign/UI-06-plan-and-route-planning.md +++ b/docs/ui-redesign/UI-06-plan-and-route-planning.md @@ -1,6 +1,6 @@ # UI-06 — Plan & Route Planning redesign -**Depends on** UI-01, UI-03 · **Size** M · **Status** Not started +**Depends on** UI-01, UI-03 · **Size** M · **Status** Done ## Goal Rebuild the Plan tab (empty state) and the route planner (pins dropped) as fullscreen @@ -89,3 +89,82 @@ matching Map HUD's nav bar exactly (same widget, in fact — see UI-01). ## Out of scope Road-snapped routing (V3-08/V3-09, deferred). Any change to the underlying route-planning data model or repository — this is presentation only. + +## Outcome + +**IA question (Risk section) resolved by deferring, as the ticket explicitly allowed:** +`RoutesListScreen` stays a list-with-`+`-button (the existing entry point), restyled with +`GlassPanel`-wrapped rows instead of plain `Card`s. The mockup's own "fullscreen map + +tooltip" language turned out, on close reading, to describe `RoutePlannerScreen`'s +*own* empty-pins state (the tooltip literally instructs "tap to drop a pin" — an action +that happens on the canvas, not on a list of named routes), not the routes list. Both +screens got the restyle their actual role calls for. + +`RoutePlannerScreen` is now a headerless, full-opacity map canvas (its own map, not +UI-01's dimmed shell background — this screen's map *is* the primary content, matching +Map HUD). Implemented: the floating "TAP TO DROP A PIN" tooltip (`GlassPanel`, gentle +3s float) shown only while `waypoints.isEmpty`; `FloatingPill` (UI-03) showing Distance/ +Est. Time/Pins once pins exist, replacing the old pre-download confirmation dialog's +role as the primary stat display (the confirmation dialog itself is untouched -- +presentation only, per scope); a "START ROUTE" floating pill button; a `GlassPanel` +overflow menu (`PopupMenuButton`) holding Rename/Download offline tiles/Delete route, +replacing the removed `AppBar`'s actions row; and a pin-drop ripple micro-interaction +(a short-lived expanding-and-fading ring at the tap's local screen position). + +**"Start Route," given real, working behaviour rather than shipped as a dead button:** +turn-by-turn following (V3-09) doesn't exist anywhere in the app to wire this to, and +inventing that data model here would violate this ticket's own "presentation only" +scope. Tapping it starts an ordinary recording and switches to the Map tab -- a real +action a rider can use today, not a placeholder. + +**Est. Time honestly shows "--"** when `RoutePlan.estimatedMillis` is null (always, until +V3-08 adds road-snapped routing) rather than fabricating a straight-line estimate the +model doesn't back. + +**Marching-ants route-line animation was deliberately dropped from scope.** It isn't in +this ticket's acceptance criteria (only the pin-drop ripple is named there), and +implementing it properly means animating a dash phase while continuously reprojecting +through `MapCamera.latLngToScreenOffset` under live pan/zoom -- real complexity for a +purely decorative effect. The existing static dashed polyline (already "visibly distinct +from a recorded path," the actual acceptance-relevant property) is unchanged. + +**Bug found and fixed, unrelated to this ticket's own changes but discovered while +working in this file:** `_DownloadDialog`'s tile fetch still hardcoded +`https://tile.openstreetmap.org/...` directly -- a leftover from before UI-09 switched +the live map to CARTO's dark tiles. Downloading offline tiles would have silently cached +OSM's tan tiles under the same `(z, x, y)` keys UI-09's cache versioning was specifically +designed to keep separate from CARTO's, meaning a "successful" download would never +actually serve anything to the live dark map. Fixed to build the URL from the same +`tileUrlTemplate`/`tileSubdomains` every other tile fetch in the app already uses. + +**Nav bar correctness:** no separate implementation needed -- `RoutePlannerScreen` is +pushed *within* the Plan branch's own navigator (UI-01's `StatefulShellBranch` +architecture), so the shell's persistent nav bar already shows Plan active automatically, +structurally ruling out the mockup's own "Rides active while viewing Plan" generation +error. Added a test asserting this directly rather than trusting the architecture alone. + +**Tests:** `flutter analyze` clean. `flutter test` green at 370 tests. Updated every +existing `route_planner_screen_test.dart` assertion that referenced now-removed `AppBar` +structure (the `IconButton` keys are `PopupMenuItem`s behind the overflow menu now; there +is no app-bar title to assert renaming against, so that test now reads the repository +directly instead) plus one real test bug fixed along the way: two close-together +waypoints in the "tapping a pin deletes it" test happened to place one pin directly +under the new floating "Start Route" button in that test's fixed camera geometry, so the +tap intended for the pin actually hit the button underneath it (confirmed via +`WidgetController`'s own hit-test-mismatch warning, not assumed) -- reduced to a single +waypoint, which the camera centres exactly on, clear of the button. `PopupMenuButton`/ +`PopupMenuItem` interactions needed bounded pumps rather than `pumpAndSettle`, same +reason the map's own tests already avoid it (a live `TileLayer` never goes idle in this +harness). Also fixed a real, unrelated bug the new `GlassPanel`-wrapped `ListTile` in +`RoutesListScreen` surfaced immediately: Flutter's own framework assertion that a +`ListTile` needs a `Material` ancestor to paint its ink splash correctly, which +`GlassPanel`'s `DecoratedBox` was sitting between it and -- fixed with a +`Material(type: MaterialType.transparency)` wrapper. + +**Android emulator verification** (`Medium_Phone_API_35`): confirmed the empty-tooltip +state, dropping a pin (ripple visible, `FloatingPill` and "Start Route" appearing +immediately), and the overflow menu opening with all three actions -- all matching the +mockup and the Design section precisely. The Plan tab's nav-bar pill correctly followed +the active tab (a mid-transition-animation screenshot briefly looked wrong immediately +after tapping the tab; a screenshot taken a moment later confirmed it settles correctly +-- a timing artifact of screenshotting mid-animation, not a real bug). diff --git a/lib/src/ui/routes/route_planner_screen.dart b/lib/src/ui/routes/route_planner_screen.dart index 17d8f0b..fe54d7d 100644 --- a/lib/src/ui/routes/route_planner_screen.dart +++ b/lib/src/ui/routes/route_planner_screen.dart @@ -2,6 +2,10 @@ /// /// Straight lines only, deliberately — see the ticket. `geometry` stays null; the /// polyline drawn here is derived from waypoints on every build, never persisted. +/// +/// UI-06: rebuilt as a fullscreen map canvas with floating controls, restyled toward +/// Map HUD (UI-05) rather than copied from this screen's own Stitch export -- see +/// `docs/ui-redesign/README.md`'s note on why the Route Planning mockup drifted. library; import 'dart:async'; @@ -9,6 +13,7 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter_map/flutter_map.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:go_router/go_router.dart'; import 'package:http/http.dart' as http; import 'package:latlong2/latlong.dart' as ll; @@ -16,9 +21,12 @@ import '../../app/providers.dart'; import '../../data/route_plan_repository.dart'; import '../../domain/models.dart'; import '../../geo/geo.dart' as geo; +import '../../telemetry/telemetry.dart' show formatDuration; import '../../tiles/tile_cache.dart'; import '../../tiles/tile_downloader.dart'; import '../../tiles/tile_math.dart'; +import '../components/floating_pill.dart'; +import '../components/glass_panel.dart'; import '../components/ride_map.dart' show TileAttribution, @@ -30,6 +38,7 @@ import '../components/ride_map.dart' import '../components/skeleton_map_layer.dart'; import '../components/stats.dart' show confirmDialog; import '../format.dart'; +import '../router.dart'; class RoutePlannerScreen extends ConsumerStatefulWidget { const RoutePlannerScreen({super.key, required this.routeId, this.onBack}); @@ -41,15 +50,45 @@ class RoutePlannerScreen extends ConsumerStatefulWidget { ConsumerState createState() => _RoutePlannerScreenState(); } -class _RoutePlannerScreenState extends ConsumerState { +class _Ripple { + _Ripple(this.offset, this.controller); + final Offset offset; + final AnimationController controller; +} + +class _RoutePlannerScreenState extends ConsumerState + with TickerProviderStateMixin { final _mapController = MapController(); + final _ripples = <_Ripple>[]; @override void dispose() { _mapController.dispose(); + for (final ripple in _ripples) { + ripple.controller.dispose(); + } super.dispose(); } + /// UI-06: a short-lived expanding-and-fading ring at the tap point, matching the + /// mockup's pin-drop micro-interaction. Placed once, at the moment of the tap, using + /// screen-local coordinates -- it lives for 500ms, so not tracking map pan/zoom + /// during that brief window is an acceptable simplification, not a real defect. + void _addRipple(Offset localPosition) { + final controller = AnimationController( + vsync: this, + duration: const Duration(milliseconds: 500), + ); + late final _Ripple ripple; + ripple = _Ripple(localPosition, controller); + setState(() => _ripples.add(ripple)); + controller.forward().whenComplete(() { + if (!mounted) return; + setState(() => _ripples.remove(ripple)); + controller.dispose(); + }); + } + /// Fits every pin in view, the same way `RideMap` fits a recorded path -- otherwise a /// fixed zoom guess either strands distant pins off-screen or, for two close-together /// pins, can cull one before it is ever seen. Null below two points: a single pin (or @@ -81,6 +120,7 @@ class _RoutePlannerScreenState extends ConsumerState { if (route == null) { return Scaffold( + backgroundColor: Colors.transparent, body: SafeArea( child: Center( child: Text( @@ -93,154 +133,221 @@ class _RoutePlannerScreenState extends ConsumerState { } return Scaffold( - appBar: AppBar( - leading: IconButton( - key: const Key('back'), - icon: const Icon(Icons.arrow_back), - onPressed: widget.onBack, - ), - title: Text(route.name), - actions: [ - IconButton( - key: const Key('download-tiles'), - icon: const Icon(Icons.download_for_offline_outlined), - tooltip: waypoints.isEmpty - ? 'Add pins first' - : 'Download offline tiles along this route', - onPressed: waypoints.isEmpty - ? null - : () => _downloadOfflineTiles(context, waypoints), - ), - IconButton( - key: const Key('rename-route'), - icon: const Icon(Icons.edit_outlined), - onPressed: () => _rename(context, repo, route), - ), - IconButton( - key: const Key('delete-route'), - icon: const Icon(Icons.delete_outline), - onPressed: () async { - await repo.deleteRoutePlan(widget.routeId); - widget.onBack?.call(); - }, - ), - ], - ), - body: Column( - children: [ - Expanded( - // `MapOptions.initialCenter`/`initialZoom` are read exactly once, at - // FlutterMap's construction -- not on every rebuild. Building the map before - // the waypoints stream has delivered its first value would freeze the camera - // on null-island forever, even once real waypoints arrive. Wait for the first - // emission (typically one frame) so the initial camera is right from the - // start. - child: !waypointsAsync.hasValue - ? const Center(child: CircularProgressIndicator()) - : 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 ? 14 : maxTileZoom - 3, - maxZoom: maxTileZoom, - onTap: (tapPosition, point) => - repo.addWaypoint(widget.routeId, point.latitude, point.longitude), - ), - children: [ - // UI-02: same skeleton-or-tiles swap as RideMap's background usage -- - // this screen manages its own FlutterMap directly rather than through - // RideMap, so it has to watch connectivity and swap the layer itself. - if (ref.watch(mapConnectivityProvider).skeletonMode) - const SkeletonMapLayer() - else - TileLayer( - urlTemplate: tileUrlTemplate, - subdomains: tileSubdomains, - retinaMode: true, - userAgentPackageName: tileUserAgent, - maxNativeZoom: tileMaxNativeZoom, - panBuffer: 0, - tileProvider: ref.watch(cachedTileProviderProvider), - ), - if (waypoints.length >= 2) - PolylineLayer( - polylines: [ - Polyline( - points: [ - for (final w in waypoints) ll.LatLng(w.latitude, w.longitude), - ], - strokeWidth: 4, - // Visibly distinct from a recorded path (see the acceptance - // criteria) — dashed, and the planning accent rather than the - // speed-bucketed colours RideMap uses. - pattern: StrokePattern.dashed(segments: const [8, 6]), - color: colors.primary, + backgroundColor: Colors.transparent, + body: SafeArea( + child: !waypointsAsync.hasValue + ? const Center(child: CircularProgressIndicator()) + : Stack( + children: [ + Positioned.fill( + // `MapOptions.initialCenter`/`initialZoom` are read exactly once, at + // FlutterMap's construction -- not on every rebuild. Building the map + // before the waypoints stream has delivered its first value would + // freeze the camera on null-island forever, even once real waypoints + // arrive; the `!waypointsAsync.hasValue` branch above waits for that + // first emission (typically one frame) so the initial camera is + // right from the start. + 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 ? 14 : maxTileZoom - 3, + maxZoom: maxTileZoom, + onTap: (tapPosition, point) { + repo.addWaypoint(widget.routeId, point.latitude, point.longitude); + if (tapPosition.relative != null) { + _addRipple(tapPosition.relative!); + } + }, ), - ], + children: [ + // UI-02: same skeleton-or-tiles swap as RideMap's background + // usage -- this screen manages its own FlutterMap directly + // rather than through RideMap, so it has to watch connectivity + // and swap the layer itself. + if (ref.watch(mapConnectivityProvider).skeletonMode) + const SkeletonMapLayer() + else + TileLayer( + urlTemplate: tileUrlTemplate, + subdomains: tileSubdomains, + retinaMode: true, + userAgentPackageName: tileUserAgent, + maxNativeZoom: tileMaxNativeZoom, + panBuffer: 0, + tileProvider: ref.watch(cachedTileProviderProvider), + ), + if (waypoints.length >= 2) + PolylineLayer( + polylines: [ + Polyline( + points: [ + for (final w in waypoints) ll.LatLng(w.latitude, w.longitude), + ], + strokeWidth: 4, + // Visibly distinct from a recorded path (see the + // acceptance criteria) — dashed, and the planning + // accent rather than the speed-bucketed colours + // RideMap uses. (UI-06: the mockup's own animated + // "marching ants" crawl was left as static dashes -- + // it isn't in this ticket's acceptance criteria, and + // reprojecting a hand-animated dash phase under + // map pan/zoom is real added complexity for a purely + // decorative effect.) + pattern: StrokePattern.dashed(segments: const [8, 6]), + color: colors.primary, + ), + ], + ), + MarkerLayer( + markers: [ + for (var i = 0; i < waypoints.length; i++) + Marker( + key: Key('waypoint-${waypoints[i].id}'), + point: ll.LatLng(waypoints[i].latitude, waypoints[i].longitude), + width: 36, + height: 36, + child: _WaypointPin( + index: i, + onTap: () => + repo.deleteWaypoint(widget.routeId, waypoints[i].id), + onPanUpdate: (delta) { + final camera = _mapController.camera; + final current = ll.LatLng( + waypoints[i].latitude, + waypoints[i].longitude, + ); + final newOffset = + camera.latLngToScreenOffset(current) + delta; + final moved = camera.screenOffsetToLatLng(newOffset); + repo.moveWaypoint( + widget.routeId, + waypoints[i].id, + moved.latitude, + moved.longitude, + ); + }, + ), + ), + ], + ), + const TileAttribution(), + ], + ), ), - MarkerLayer( - markers: [ - for (var i = 0; i < waypoints.length; i++) - Marker( - key: Key('waypoint-${waypoints[i].id}'), - point: ll.LatLng(waypoints[i].latitude, waypoints[i].longitude), - width: 36, - height: 36, - child: _WaypointPin( - index: i, - onTap: () => - repo.deleteWaypoint(widget.routeId, waypoints[i].id), - onPanUpdate: (delta) { - final camera = _mapController.camera; - final current = ll.LatLng( - waypoints[i].latitude, - waypoints[i].longitude, - ); - final newOffset = - camera.latLngToScreenOffset(current) + delta; - final moved = camera.screenOffsetToLatLng(newOffset); - repo.moveWaypoint( - widget.routeId, - waypoints[i].id, - moved.latitude, - moved.longitude, + // Pin-drop ripples paint in the same local coordinate space as the + // map's `onTap` gave us, directly over the map -- not inside + // FlutterMap's own `children` (a plain widget there doesn't get + // reprojected the way layer widgets do, which is fine for something + // this short-lived, but it does need to sit in a `Stack` positioned + // identically to the map itself). + for (final ripple in _ripples) + Positioned( + left: ripple.offset.dx - 30, + top: ripple.offset.dy - 30, + child: IgnorePointer( + child: AnimatedBuilder( + animation: ripple.controller, + builder: (context, child) { + final t = Curves.easeOut.transform(ripple.controller.value); + return Opacity( + opacity: 1 - t, + child: Container( + width: 60 * t, + height: 60 * t, + decoration: BoxDecoration( + shape: BoxShape.circle, + color: colors.primary.withValues(alpha: 0.2), + border: Border.all(color: colors.primary, width: 2), + ), + ), ); }, ), ), - ], - ), - const TileAttribution(), - ], - ), - ), - Padding( - padding: const EdgeInsets.all(16), - child: Row( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - Icon(Icons.straighten, size: 18, color: colors.outline), - const SizedBox(width: 8), - Text( - key: const Key('route-distance'), - formatDistance(route.distanceM, unit: units), - style: TextStyle(fontWeight: FontWeight.bold, color: colors.onSurface), - ), - Text( - ' · ${waypoints.length} pin${waypoints.length == 1 ? '' : 's'}', - style: TextStyle(color: colors.outline), - ), - ], - ), - ), - ], + ), + if (waypoints.isEmpty) + const Positioned(top: 16, left: 0, right: 0, child: _DropPinTooltip()), + if (waypoints.isNotEmpty) + Positioned( + top: 16, + left: 0, + right: 0, + child: Center( + child: FloatingPill( + stats: [ + PillStat( + label: 'Distance', + value: formatDistance(route.distanceM, unit: units), + ), + PillStat( + label: 'Est. Time', + value: route.estimatedMillis == null + ? '--' + : formatDuration(route.estimatedMillis!), + ), + PillStat(label: 'Pins', value: '${waypoints.length}'), + ], + ), + ), + ), + Positioned( + top: 8, + right: 8, + child: _OverflowMenu( + hasWaypoints: waypoints.isNotEmpty, + onRename: () => _rename(context, repo, route), + onDownload: waypoints.isEmpty + ? null + : () => _downloadOfflineTiles(context, waypoints), + onDelete: () async { + await repo.deleteRoutePlan(widget.routeId); + widget.onBack?.call(); + }, + ), + ), + if (waypoints.isNotEmpty) + Positioned( + bottom: 16, + left: 0, + right: 0, + child: Center( + child: FilledButton( + key: const Key('start-route'), + style: FilledButton.styleFrom( + backgroundColor: colors.primaryContainer, + foregroundColor: colors.onPrimaryContainer, + padding: const EdgeInsets.symmetric(horizontal: 32, vertical: 16), + shape: const StadiumBorder(), + ), + onPressed: () => _startRoute(context), + child: const Text( + 'START ROUTE', + style: TextStyle(fontWeight: FontWeight.bold, letterSpacing: 1), + ), + ), + ), + ), + ], + ), ), ); } + /// UI-06: turn-by-turn following of a planned route doesn't exist yet (V3-09, + /// explicitly deferred by this ticket) -- there is no "route currently being + /// followed" concept anywhere in the app to wire this button to. Rather than ship a + /// dead button or invent that data model here, this starts an ordinary recording and + /// switches to the Map tab, which is a real, working action a rider can use today + /// ("go start riding this route now"), not a placeholder. + Future _startRoute(BuildContext context) async { + await ref.read(recordingEngineProvider).start(); + if (context.mounted) context.go(Routes.record); + } + Future _rename( BuildContext context, RoutePlanRepository repo, @@ -271,7 +378,8 @@ class _RoutePlannerScreenState extends ConsumerState { /// V3-11: pre-downloads a corridor along the route, not a rectangle around it -- "far /// fewer tiles for the same usefulness," as the ticket puts it. Zoom range is fixed /// (city-street level through the map's own max) rather than picked by the rider -- - /// keeping the choice small is part of what keeps this within OSM's usage policy. + /// keeping the choice small is part of what keeps this within the tile host's usage + /// policy. static const _downloadMinZoom = 13; Future _downloadOfflineTiles( @@ -327,6 +435,108 @@ class _RoutePlannerScreenState extends ConsumerState { } } +/// UI-06: "Tap to drop a pin" -- shown only while the route has no pins yet, matching +/// the Plan Screen mockup's own gentle vertical float, a hint that disappears once the +/// rider has actually acted rather than a permanent fixture cluttering real work. +class _DropPinTooltip extends StatefulWidget { + const _DropPinTooltip(); + + @override + State<_DropPinTooltip> createState() => _DropPinTooltipState(); +} + +class _DropPinTooltipState extends State<_DropPinTooltip> + with SingleTickerProviderStateMixin { + late final _controller = AnimationController( + vsync: this, + duration: const Duration(seconds: 3), + )..repeat(reverse: true); + + @override + void dispose() { + _controller.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final colors = Theme.of(context).colorScheme; + return Center( + child: AnimatedBuilder( + animation: _controller, + builder: (context, child) => Transform.translate( + offset: Offset(0, -6 * Curves.easeInOut.transform(_controller.value)), + child: child, + ), + child: GlassPanel( + borderRadius: const BorderRadius.all(Radius.circular(999)), + padding: const EdgeInsets.symmetric(horizontal: 20, vertical: 12), + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + Icon(Icons.location_on, color: colors.primary, size: 20), + const SizedBox(width: 8), + const Text( + 'TAP TO DROP A PIN', + style: TextStyle(fontSize: 12, letterSpacing: 1, fontWeight: FontWeight.w600), + ), + ], + ), + ), + ), + ); + } +} + +/// UI-06: rename/download/delete lost their `AppBar` actions row -- this is their new +/// home, styled per Map HUD's `GlassPanel` language rather than invented fresh. +class _OverflowMenu extends StatelessWidget { + const _OverflowMenu({ + required this.hasWaypoints, + required this.onRename, + required this.onDownload, + required this.onDelete, + }); + + final bool hasWaypoints; + final VoidCallback onRename; + final VoidCallback? onDownload; + final VoidCallback onDelete; + + @override + Widget build(BuildContext context) => GlassPanel( + borderRadius: const BorderRadius.all(Radius.circular(999)), + child: PopupMenuButton( + key: const Key('route-overflow-menu'), + icon: const Icon(Icons.more_vert), + onSelected: (value) => switch (value) { + 'rename' => onRename(), + 'download' => onDownload?.call(), + 'delete' => onDelete(), + _ => null, + }, + itemBuilder: (context) => [ + const PopupMenuItem( + key: Key('rename-route'), + value: 'rename', + child: Text('Rename'), + ), + PopupMenuItem( + key: const Key('download-tiles'), + value: 'download', + enabled: hasWaypoints, + child: const Text('Download offline tiles'), + ), + const PopupMenuItem( + key: Key('delete-route'), + value: 'delete', + child: Text('Delete route'), + ), + ], + ), + ); +} + /// Owns the download's stream subscription for exactly the lifetime of the dialog -- /// pulled out of a plain `showDialog` builder because that builder re-runs on every /// `setState`, which would otherwise start a brand new overlapping download on every @@ -355,7 +565,18 @@ class _DownloadDialogState extends State<_DownloadDialog> { cache: widget.cache, cancelToken: _token, fetchTile: (key) async { - final url = 'https://tile.openstreetmap.org/${key.z}/${key.x}/${key.y}.png'; + // UI-09 correction: this still pointed at OSM's raw tile host directly, from + // before the switch to CARTO's dark tiles -- downloading tan OSM tiles for + // offline use while the live map renders CARTO's dark style would have quietly + // cached tiles that never actually get served (the cache is keyed by the same + // (z, x, y) regardless of provider, and the CARTO-tagged cache directory -- + // see UI-09 -- would just never contain what this was fetching). + final url = tileUrlTemplate + .replaceFirst('{s}', tileSubdomains.first) + .replaceFirst('{z}', '${key.z}') + .replaceFirst('{x}', '${key.x}') + .replaceFirst('{y}', '${key.y}') + .replaceFirst('{r}', ''); final response = await _client.get( Uri.parse(url), headers: {'User-Agent': tileUserAgent}, diff --git a/lib/src/ui/routes/routes_list_screen.dart b/lib/src/ui/routes/routes_list_screen.dart index 9ee1e7f..12b4acd 100644 --- a/lib/src/ui/routes/routes_list_screen.dart +++ b/lib/src/ui/routes/routes_list_screen.dart @@ -6,6 +6,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../app/providers.dart'; import '../../domain/models.dart'; +import '../components/glass_panel.dart'; import '../components/stats.dart'; import '../format.dart'; @@ -58,19 +59,34 @@ class RoutesListScreen extends ConsumerWidget { itemCount: routes.length, itemBuilder: (context, i) { final route = routes[i]; - return Card( - key: Key('route-${route.id}'), - child: ListTile( - title: Text(route.name), - subtitle: Text(formatDistance(route.distanceM, unit: units)), - trailing: IconButton( - key: Key('delete-route-${route.id}'), - icon: Icon(Icons.delete_outline, color: colors.error), - onPressed: () => ref - .read(routePlanRepositoryProvider) - .deleteRoutePlan(route.id), + // UI-06: a GlassPanel-wrapped row rather than a plain Card -- + // matches Map HUD's floating-glass language, which every other + // restyled screen now uses for list-like surfaces too. + return Padding( + padding: const EdgeInsets.only(bottom: 12), + child: GlassPanel( + key: Key('route-${route.id}'), + padding: EdgeInsets.zero, + // A `ListTile` paints its ink splash/tap feedback on the + // nearest `Material` ancestor -- `GlassPanel`'s own + // `DecoratedBox` sits between it and the `Scaffold`'s + // Material otherwise, which Flutter flags as a real bug + // (splashes would paint invisibly behind the decoration). + child: Material( + type: MaterialType.transparency, + child: ListTile( + title: Text(route.name), + subtitle: Text(formatDistance(route.distanceM, unit: units)), + trailing: IconButton( + key: Key('delete-route-${route.id}'), + icon: Icon(Icons.delete_outline, color: colors.error), + onPressed: () => ref + .read(routePlanRepositoryProvider) + .deleteRoutePlan(route.id), + ), + onTap: () => onOpenRoute?.call(route.id), + ), ), - onTap: () => onOpenRoute?.call(route.id), ), ); }, diff --git a/test/route_planner_screen_test.dart b/test/route_planner_screen_test.dart index 1606518..2fa6bec 100644 --- a/test/route_planner_screen_test.dart +++ b/test/route_planner_screen_test.dart @@ -7,6 +7,9 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:rippr/src/app/providers.dart'; import 'package:rippr/src/data/database.dart'; import 'package:rippr/src/data/route_plan_repository.dart'; +import 'package:rippr/src/ui/app_shell.dart'; +import 'package:rippr/src/ui/components/floating_pill.dart'; +import 'package:rippr/src/ui/format.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/theme.dart'; @@ -107,8 +110,10 @@ void main() { final id = await repo.createRoutePlan(1000); await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); - expect(find.text('0 m'), findsOneWidget); - expect(find.textContaining('0 pins'), findsOneWidget); + // UI-06: no pins yet -- the "tap to drop a pin" tooltip shows, not a stat pill + // (there is nothing to summarise yet). + expect(find.textContaining('TAP TO DROP A PIN'), findsOneWidget); + expect(find.byType(FloatingPill), findsNothing); await tester.tapAt(tester.getCenter(find.byType(FlutterMap))); await tester.pump(const Duration(milliseconds: 50)); @@ -119,35 +124,54 @@ void main() { final waypoints = await repo.waypointsFor(id); expect(waypoints, hasLength(2)); - expect(find.textContaining('2 pins'), findsOneWidget); - final label = tester.widget(find.byKey(const Key('route-distance'))); - expect(label.data, isNot('0 m')); + // The tooltip is gone and the stat pill has taken its place, showing the real + // pin count and a non-zero distance. Read the pill's own `stats` directly + // rather than `find.text('2')` -- the second waypoint pin's own marker label + // is also "2", so that text is no longer unique on screen. + expect(find.textContaining('TAP TO DROP A PIN'), findsNothing); + final pill = tester.widget(find.byType(FloatingPill)); + final pinsStat = pill.stats.firstWhere((s) => s.label == 'Pins'); + final distanceStat = pill.stats.firstWhere((s) => s.label == 'Distance'); + expect(pinsStat.value, '2'); + expect(distanceStat.value, isNot(formatDistance(0)), + reason: 'the distance stat must reflect the two real pins, not stay zeroed'); }); screenTest('tapping a pin deletes it', (tester) async { + // UI-06: a single waypoint (rather than two) -- with only one pin, the camera + // centres exactly on it, keeping it clear of the floating "Start Route" button + // now anchored at the bottom of the screen; two close pins previously landed + // one of them directly underneath it in this test's fixed viewport. final id = await repo.createRoutePlan(1000); await repo.addWaypoint(id, 51.0, -114.0); - await repo.addWaypoint(id, 51.01, -114.0); await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); // flutter_map's `Marker` is a plain data class, not a Widget -- it never appears // in the tree itself. `CircleAvatar` is what `_WaypointPin` actually renders. - expect(find.byType(CircleAvatar), findsNWidgets(2)); + expect(find.byType(CircleAvatar), findsOneWidget); - await tester.tap(find.text('1')); // the first pin's label + // Not `find.text('1')`: the FloatingPill's "Pins" stat also reads "1" with a + // single waypoint, so the pin's own label text is no longer a unique match. + await tester.tap(find.byType(CircleAvatar)); await tester.pump(); - expect(await repo.waypointsFor(id), hasLength(1)); + expect(await repo.waypointsFor(id), isEmpty); }); - screenTest('renaming updates the app bar title', (tester) async { + screenTest('renaming writes through to the repository (UI-06: no app bar title ' + 'to display it on any more)', (tester) async { final id = await repo.createRoutePlan(1000, name: 'Old name'); await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); - await tester.tap(find.byKey(const Key('rename-route'))); + // Rename now lives behind the floating overflow menu -- open it first. + await tester.tap(find.byKey(const Key('route-overflow-menu'))); await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + await tester.tap(find.byKey(const Key('rename-route'))); + await tester.pump(const Duration(milliseconds: 300)); await tester.enterText( find.byKey(const Key('route-name-field')), 'Sunday coast run', @@ -155,7 +179,8 @@ void main() { await tester.tap(find.text('Save')); await tester.pump(); - expect(find.text('Sunday coast run'), findsOneWidget); + final renamed = await repo.routePlanById(id); + expect(renamed?.name, 'Sunday coast run'); }); screenTest('a missing route says so instead of a blank map', (tester) async { @@ -165,16 +190,21 @@ void main() { expect(find.textContaining('no longer exists'), findsOneWidget); }); - screenTest('the offline-tiles download button is disabled with no pins ' - '(V3-11)', (tester) async { + screenTest('the offline-tiles download menu item is disabled with no pins ' + '(V3-11, UI-06: now a PopupMenuItem behind the overflow menu)', + (tester) async { final id = await repo.createRoutePlan(1000); await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); - final button = tester.widget( + await tester.tap(find.byKey(const Key('route-overflow-menu'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + + final item = tester.widget>( find.byKey(const Key('download-tiles')), ); - expect(button.onPressed, isNull); - expect(button.tooltip, contains('Add pins')); + expect(item.enabled, isFalse); }); screenTest('downloading shows a tile count and size estimate before any ' @@ -184,10 +214,15 @@ void main() { await repo.addWaypoint(id, 51.01, -114.0); await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); - final button = tester.widget( + await tester.tap(find.byKey(const Key('route-overflow-menu'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + + final item = tester.widget>( find.byKey(const Key('download-tiles')), ); - expect(button.onPressed, isNotNull); + expect(item.enabled, isTrue); await tester.tap(find.byKey(const Key('download-tiles'))); // Bounded, not pumpAndSettle: the confirmation dialog is safe to settle (no @@ -201,5 +236,25 @@ void main() { expect(find.textContaining('tiles?'), findsOneWidget); expect(find.textContaining('MB'), findsOneWidget); }); + + screenTest('the shell nav bar correctly shows Plan active on this screen ' + '(UI-06)', (tester) async { + final id = await repo.createRoutePlan(1000); + await pumpMap( + tester, + host( + ShellScaffold( + currentIndex: 2, // Map, Rides, Plan, Settings -- Plan is index 2. + onDestinationSelected: (_) {}, + child: RoutePlannerScreen(routeId: id), + ), + ), + ); + + final navBar = tester.widget(find.byKey(const Key('shell-nav-bar'))); + expect(navBar.selectedIndex, 2, + reason: 'not the mockup\'s own generation error (it showed Rides active ' + 'while viewing Plan) -- the shipped nav bar must reflect the real tab'); + }); }); }