fix(roadtrip): closing offers or other roads no longer flies the phone map back to an old stop

A final pass over the feedback commits found two gaps in the phone map.

The planner keeps a point it was asked to show until its next routing
round, and once the fuel offers or the other roads for a leg close it hands
that very array back. MMapArea read it as a new focus, so closing either one
flew the camera to the stop "Show on map" had pointed at, on another day's
stage too. holdFocus now remembers the arrays it already held and only
takes a new one as a focus, with the open picker's own roads as the one
exception.

The all days view still took its pins from the planner's planned list,
which on a phone leaves out booked nights and hidden service stops, so the
drive ended at a hotel with no pin to open it from. It now reads the places
of the stored visits, like the stage does.

client/src/mobile/screens/trip/map/MMapArea.tsx

@@ -92,26 +92,49 @@ function alternativesFitPadding(insets: SafeInsets): ViewportPadding {
   }
 }
 
+type FocusPoints = readonly [number, number][]
+
 /** A camera focus the planner is holding, and the day that was on screen when it arrived. */
 interface HeldFocus {
-  points: readonly [number, number][]
+  points: FocusPoints
   dayId: number | null
   live: boolean
+  /**
+   * Every array held before this one. Weak, so the offers of a long session are not kept
+   * alive by it. Only ever added to, and adding twice changes nothing, so a render React
+   * runs twice or throws away leaves it saying the same thing.
+   */
+  shown: WeakSet<FocusPoints>
 }
 
 /**
- * Lets go of a pending focus once the day moves off the one it arrived with.
+ * Lets go of a pending focus once the day moves off the one it arrived with, and never
+ * takes one back that the planner hands over a second time.
  *
  * The planner keeps a point it was asked to show until its next routing round, whatever
  * day is picked in the meantime, and a pending focus wins over the stage's own frame. So
  * after "show on map" every stage swiped to afterwards stayed on that one point instead of
  * framing its drive. Coming back to the day does not bring the point back either: the
- * traveller has moved on since, and the camera belongs to the stage they came back to. A
- * new focus is a new array, held again from the day it arrives together with.
+ * traveller has moved on since, and the camera belongs to the stage they came back to.
+ *
+ * The same kept point is also what the planner falls back to once the fuel offers or the
+ * other roads for a leg close, as the very array it handed over before. Read as a new
+ * focus, closing either one flew the camera back to that old stop, on another day's stage
+ * too. So only an array never held before is a new focus. The one exception is the open
+ * picker's roads (`openRoads`): fuel offers asked for over an open picker take the camera
+ * from it, and once they close the picker is still asking about its leg.
  */
-function holdFocus(prev: HeldFocus, points: readonly [number, number][], dayId: number | null): HeldFocus {
-  if (prev.points !== points) return { points, dayId, live: true }
-  if (prev.live && prev.dayId !== dayId) return { points, dayId, live: false }
+function holdFocus(
+  prev: HeldFocus,
+  points: FocusPoints,
+  dayId: number | null,
+  openRoads: FocusPoints,
+): HeldFocus {
+  if (prev.points !== points) {
+    prev.shown.add(prev.points)
+    return { points, dayId, live: points === openRoads || !prev.shown.has(points), shown: prev.shown }
+  }
+  if (prev.live && prev.dayId !== dayId) return { ...prev, dayId, live: false }
   return prev
 }
 
@@ -172,8 +195,8 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) {
   )
 
   // Only what the stage carries, so a trip's other 200 pins stay off a screen that
-  // is answering one question. Without a stage (the all-days view) every planned
-  // place comes back, which is what the drive looks like end to end.
+  // is answering one question. Without a stage (the all-days view) every place a stored
+  // visit stops at comes back, which is what the drive looks like end to end.
   //
   // Out of the trip store rather than `planner.mapPlaces`, which is the plan tab's map.
   // That list drops the pins of every day its declutter has put away, and this tab moves
@@ -184,19 +207,27 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) {
   // that the chain still draws. The category filter stays off the stage too, on purpose:
   // the stage is one day's chain on a map, and a stop the chain lists with no pin under its
   // line, hidden by a control on another tab, reads as a broken map rather than a filter.
-  const stagePlaces = useMemo(
-    () => stageMap && stage
-      ? tripPlaces.filter(p => p.lat != null && p.lng != null && stageMap.placeIds.has(p.id))
-      : planner.roadtripMapPlaces,
-    [stageMap, stage, tripPlaces, planner.roadtripMapPlaces],
+  //
+  // The all-days view reads the stored visits for the same reasons, not the planner's
+  // planned list: that one is built from the day lists, which on a phone always leave out
+  // a booked night and, with the setting off, the service stops, while the drive the routing
+  // round draws still ends at that hotel and stops at that pump. Its pin is what opens the
+  // stop, so a missing one left the stop out of reach from this view.
+  const drivePlaceIds = useMemo(
+    () => new Set(Object.values(planner.storedAssignments).flat().map(visit => visit.place_id)),
+    [planner.storedAssignments],
   )
+  const stagePlaces = useMemo(() => {
+    const pinned = stageMap && stage ? stageMap.placeIds : drivePlaceIds
+    return tripPlaces.filter(p => p.lat != null && p.lng != null && pinned.has(p.id))
+  }, [stageMap, stage, tripPlaces, drivePlaceIds])
 
   // Computed during render rather than in an effect, so the frame handed over below is
   // already the right one in the render a day change happens in. See holdFocus.
   const [heldFocus, setHeldFocus] = useState<HeldFocus>(
-    () => ({ points: planner.mapFocusPoints, dayId: planner.selectedDayId, live: true }),
+    () => ({ points: planner.mapFocusPoints, dayId: planner.selectedDayId, live: true, shown: new WeakSet() }),
   )
-  const focus = holdFocus(heldFocus, planner.mapFocusPoints, planner.selectedDayId)
+  const focus = holdFocus(heldFocus, planner.mapFocusPoints, planner.selectedDayId, planner.alternativeFocusPoints)
   if (focus !== heldFocus) setHeldFocus(focus)
   const focusPending = focus.live && planner.mapFocusPoints.length > 0
 

client/tests/unit/mobile/trip/MMapArea.test.tsx

@@ -1,7 +1,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { act, render, screen } from '../../../helpers/render'
 import { buildPlanner, buildShell } from '../../../helpers/mobileTrip'
-import { buildPlace } from '../../../helpers/factories'
+import { buildAssignment, buildPlace } from '../../../helpers/factories'
 import type { AccessSpur, RoadtripDay } from '@trek/shared/roadtrip'
 import type { MTripShellApi, TripPlanner } from '../../../../src/mobile/screens/trip/MTripShell'
 import type { CompassMap } from '../../../../src/components/Map/MapCompassPill'
@@ -14,7 +14,7 @@ import type { AlternativeOverlay } from '../../../../src/components/Roadtrip/alt
 import type { LegAlternatives } from '../../../../src/components/Roadtrip/useRouteAlternatives'
 import { RT_ALT_BAR_LIFT } from '../../../../src/mobile/screens/trip/roadtrip/useMRtAlternatives'
 
-// FE-MOB-MAPAREA-001 to FE-MOB-MAPAREA-032
+// FE-MOB-MAPAREA-001 to FE-MOB-MAPAREA-035
 //
 // The stage's pins come out of the trip store rather than the planner's map list, so the
 // stage fixtures seed the store and leave `mapPlaces` to stand for what the plan tab shows.
@@ -97,8 +97,13 @@ function stagePlanner(selectedDayId: number | null): TripPlanner {
   return buildPlanner({
     selectedDayId,
     mapPlaces: [buildPlace({ id: 11 }), buildPlace({ id: 12 }), buildPlace({ id: 99 })],
-    // Every planned place, which is what the all days view draws.
-    roadtripMapPlaces: [buildPlace({ id: 11 }), buildPlace({ id: 12 })],
+    // The stored visits, which are what the all days view draws: 99 is on no day.
+    storedAssignments: {
+      '3': [
+        buildAssignment({ day_id: 3, place: buildPlace({ id: 11 }) }),
+        buildAssignment({ day_id: 3, place: buildPlace({ id: 12 }) }),
+      ],
+    },
     roadtripRoutes: {
       ...base.roadtripRoutes,
       days: [stageDay()],
@@ -386,7 +391,8 @@ describe('MMapArea', () => {
 
     const allDays = { ...planner, selectedDayId: null }
     rerender(<MMapArea planner={allDays} shell={shell} />)
-    expect(mocks.props.places).toBe(allDays.roadtripMapPlaces)
+    expect((mocks.props.places as Place[]).map(p => p.id)).toEqual([11, 12])
+    expect(mocks.props.places).toContain(renamed)
     expect(mocks.props.focusPoints).toEqual([])
 
     // Off the tab the stage hands nothing through, so the plan tab frames itself again.
@@ -641,4 +647,97 @@ describe('MMapArea', () => {
       unmount()
     }
   })
+
+  it('FE-MOB-MAPAREA-033: a point the planner hands back once offers or roads close does not take the camera again', () => {
+    const planner = drivePlanner(2)
+    const shell = stageShell()
+    const view = (dayId: number, mapFocusPoints: [number, number][], alternativeFocusPoints: [number, number][] = []) =>
+      <MMapArea planner={{ ...planner, selectedDayId: dayId, mapFocusPoints, alternativeFocusPoints }} shell={shell} />
+    const point: [number, number][] = [[50, 8]]
+    const { rerender } = render(view(2, point))
+    expect(mocks.props.focusPoints).toBe(point)
+
+    // Swiped on: the planner still keeps the point from day 2, and this stage frames itself.
+    rerender(view(3, point))
+    const stageFrame = mocks.props.focusPoints
+    expect(stageFrame).not.toBe(point)
+
+    // The fuel offers take the camera, and closing them hands the same point back. The chips
+    // still say day 3, so flying to day 2's stop would leave the map on a stage nobody picked.
+    const offers: [number, number][] = [[53.4, 9.7]]
+    rerender(view(3, offers))
+    expect(mocks.props.focusPoints).toBe(offers)
+    rerender(view(3, point))
+    expect(mocks.props.focusPoints).toBe(stageFrame)
+
+    // On its own day as well: the roads for a leg, cancelled, give the frame back to the stage.
+    const shown: [number, number][] = [[53.87, 10.69]]
+    rerender(view(3, shown))
+    expect(mocks.props.focusPoints).toBe(shown)
+    const roads: [number, number][] = [[53.55, 9.99], [53.7, 10.2], [53.87, 10.69]]
+    rerender(view(3, roads, roads))
+    expect(mocks.props.focusPoints).toBe(roads)
+    rerender(view(3, shown))
+    expect(mocks.props.focusPoints).toBe(stageFrame)
+
+    // And every time after: a second round of offers is new, the point behind it is not.
+    const again: [number, number][] = [[53.3, 9.6]]
+    rerender(view(3, again))
+    expect(mocks.props.focusPoints).toBe(again)
+    rerender(view(3, shown))
+    expect(mocks.props.focusPoints).toBe(stageFrame)
+  })
+
+  it('FE-MOB-MAPAREA-034: fuel offers asked over an open picker give the camera back to its roads when they close', () => {
+    const planner = drivePlanner(3)
+    const shell = stageShell()
+    const roads: [number, number][] = [[53.55, 9.99], [53.7, 10.2], [53.87, 10.69]]
+    const view = (mapFocusPoints: [number, number][], alternativeFocusPoints: [number, number][]) =>
+      <MMapArea planner={{ ...planner, mapFocusPoints, alternativeFocusPoints }} shell={shell} />
+    const { rerender } = render(view(roads, roads))
+    expect(mocks.props.focusPoints).toBe(roads)
+
+    const offers: [number, number][] = [[53.4, 9.7]]
+    rerender(view(offers, roads))
+    expect(mocks.props.focusPoints).toBe(offers)
+
+    // The picker is still open and still asking about its leg, so its roads are framed again.
+    rerender(view(roads, roads))
+    expect(mocks.props.focusPoints).toBe(roads)
+
+    // Closed, nothing is pending and the stage frames itself.
+    const closed: [number, number][] = []
+    rerender(view(closed, closed))
+    expect(mocks.props.focusPoints).toEqual([[53.55, 9.99], [53.87, 10.69], [53.55, 9.99]])
+  })
+
+  it('FE-MOB-MAPAREA-035: the all days view pins every stored visit, a booked night and a hidden service stop included', () => {
+    const hotel = buildPlace({ id: 20 })
+    const pump = buildPlace({ id: 21, stop_type: 'fuel' } as Partial<Place>)
+    const base = drivePlanner(null)
+    // The places browser on 'unplanned' with a category picked, which empties the plan tab's map.
+    seedStore(useTripStore, {
+      places: [buildPlace({ id: 11 }), hotel, pump, buildPlace({ id: 99 })],
+      placesFilter: 'unplanned',
+      placesCategoryFilter: new Set(['9']),
+    })
+    const planner = {
+      ...base,
+      mapPlaces: [],
+      // The planner's planned list on a phone: its day lists leave the booked night and the
+      // hidden pump out, so it is not what the drive the routing round draws stops at.
+      roadtripMapPlaces: [buildPlace({ id: 11 })],
+      storedAssignments: {
+        '3': [
+          buildAssignment({ day_id: 3, place: buildPlace({ id: 11 }) }),
+          buildAssignment({ day_id: 3, place: pump }),
+        ],
+        '4': [buildAssignment({ day_id: 4, place: hotel, accommodation_id: 5 })],
+      },
+    } as TripPlanner
+    render(<MMapArea planner={planner} shell={stageShell()} />)
+
+    expect((mocks.props.places as Place[]).map(p => p.id)).toEqual([11, 20, 21])
+    expect(mocks.props.places).not.toBe(planner.roadtripMapPlaces)
+  })
 })