From 71a8b24f8dae9dd1c77a873da82c3bc6d646454a Mon Sep 17 00:00:00 2001 From: pipipiper Date: Mon, 28 Sep 2026 11:24:02 +0800 Subject: [PATCH] fix(map): keep the overview frame when the trip is edited --- .../Map/useTripRouteOverview.test.ts | 55 +++++++++++++++++-- .../components/Map/useTripRouteOverview.ts | 33 +++++++++-- 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/client/src/components/Map/useTripRouteOverview.test.ts b/client/src/components/Map/useTripRouteOverview.test.ts index c51051c4c..e236e7a75 100644 --- a/client/src/components/Map/useTripRouteOverview.test.ts +++ b/client/src/components/Map/useTripRouteOverview.test.ts @@ -32,9 +32,9 @@ const ASSIGNMENTS: AssignmentsMap = { const render = (enabled = true, assignments: AssignmentsMap = ASSIGNMENTS, days = DAYS) => renderHook( - ({ assignments: a }: { assignments: AssignmentsMap }) => - useTripRouteOverview(7, days, a, [], [], 'driving', enabled), - { initialProps: { assignments } }, + ({ enabled: on, assignments: a }: { enabled: boolean; assignments: AssignmentsMap }) => + useTripRouteOverview(7, days, a, [], [], 'driving', on), + { initialProps: { enabled, assignments } }, ) beforeEach(() => { @@ -129,6 +129,7 @@ describe('useTripRouteOverview', () => { // A new object identity with the same geometry: the plan memo re-runs, the // routing round must not. rerender({ + enabled: true, assignments: { ...ASSIGNMENTS, '1': ASSIGNMENTS['1'].map(a => ({ ...a, place: { ...a.place, name: 'Renamed' } })), @@ -183,7 +184,8 @@ describe('useTripRouteOverview', () => { it('FE-MAP-TRO-011: keeps the frame it set on the straight lines until the round is over', async () => { // A fresh `focusPoints` array is what tells the map to fit the camera. One per // answering leg would take the map back from wherever the reader has panned to, so - // the frame changes exactly twice: on the straight lines, and on the finished roads. + // within a round the frame changes exactly once — on the straight lines — and the + // finished roads are framed only on an activation round (see TRO-012/013). const answer: RouteAnswer = { coordinates: [[48.86, 2.35], [48.87, 2.355], [48.88, 2.36]], distance: 12000, duration: 900, @@ -206,8 +208,51 @@ describe('useTripRouteOverview', () => { await act(async () => { answers[1](answer) }) await waitFor(() => expect(result.current.loading).toBe(false)) // The finished result frames the roads themselves, which have more points than the - // straight lines had. + // straight lines had — the second of the two fits an activation makes. expect(result.current.focusPoints).not.toBe(frame) expect(result.current.focusPoints.length).toBeGreaterThan(frame.length) }) + + it('FE-MAP-TRO-012: editing the trip does not reframe the map', async () => { + // Dropping a place onto a day re-plans the route. Publishing a fresh frame for + // that recompute yanked the camera mid-edit — the map zoomed out to the whole + // trip every time a place was dragged onto the plan. + const { result, rerender } = render() + await waitFor(() => expect(result.current.loading).toBe(false)) + const frame = result.current.focusPoints + const totalBefore = result.current.totalDistance + + rerender({ + enabled: true, + assignments: { + ...ASSIGNMENTS, + // The walking override on the middle stop makes the new leg a routing + // request of its own: neighbouring legs in one mode travel as a single + // chunk, which a third driving stop would have joined instead. + '2': [ + ASSIGNMENTS['2'][0], + { ...ASSIGNMENTS['2'][1], leg_transport_mode: 'walking' }, + at(45.8, 4.9, 2, 2), + ], + }, + }) + + await waitFor(() => expect(result.current.loading).toBe(false)) + // The recompute really happened — a third stop means a second leg on day 2. + expect(result.current.totalDistance).toBe(totalBefore + 12000) + // …and the camera frame is the one the activation published, unchanged. + expect(result.current.focusPoints).toBe(frame) + }) + + it('FE-MAP-TRO-013: switching the overview back on publishes a fresh frame', async () => { + const { result, rerender } = render() + await waitFor(() => expect(result.current.loading).toBe(false)) + const frame = result.current.focusPoints + + rerender({ enabled: false, assignments: ASSIGNMENTS }) + rerender({ enabled: true, assignments: ASSIGNMENTS }) + + await waitFor(() => expect(result.current.loading).toBe(false)) + expect(result.current.focusPoints).not.toBe(frame) + }) }) diff --git a/client/src/components/Map/useTripRouteOverview.ts b/client/src/components/Map/useTripRouteOverview.ts index 81f8689ac..99b7771fc 100644 --- a/client/src/components/Map/useTripRouteOverview.ts +++ b/client/src/components/Map/useTripRouteOverview.ts @@ -40,6 +40,14 @@ export function useTripRouteOverview( const distanceUnit = useSettingsStore(s => s.settings.distance_unit) const [result, setResult] = useState(EMPTY) const abortRef = useRef(null) + // The map refits its camera whenever a NEW focusPoints array arrives, so the + // array's identity is a camera command. A content edit — a place dropped onto + // a day — re-plans the route, and if that published a fresh frame the map + // would yank itself out from under the edit every time. A new frame is only + // published when the overview is (re)activated or another trip loads; edits + // while it is on reuse the reference and the camera stays where the user put it. + const frameRef = useRef<{ tripId: number | null; points: [number, number][] } | null>(null) + const wasEnabled = useRef(false) const plan = useMemo( () => (enabled @@ -59,7 +67,12 @@ export function useTripRouteOverview( useEffect(() => { abortRef.current?.abort() - if (!plan.length) { setResult(EMPTY); return } + if (!plan.length) { + setResult(EMPTY) + // Off (or nothing to draw): the next activation publishes a fresh frame. + wasEnabled.current = false + return + } const controller = new AbortController() abortRef.current = controller @@ -67,7 +80,13 @@ export function useTripRouteOverview( // Straight lines first so the shape of the trip is on screen immediately, then the // real roads replace them — the same two-step the single-day route draws with. const first = summariseTripRoute(assembleTripRoute(plan, emptyAnswers(plan))) - setResult({ ...first, loading: true }) + // Reframe only on activation or a trip switch, never on a content edit (see + // frameRef above). + const reframe = frameRef.current === null || frameRef.current.tripId !== tripId || !wasEnabled.current + if (reframe) frameRef.current = { tripId, points: first.focusPoints } + wasEnabled.current = true + const frame = frameRef.current!.points + setResult({ ...first, focusPoints: frame, loading: true }) // Published leg by leg: the legs go to the router one at a time with a pause between // them, so a cold trip takes a second per leg, and roads that fill in as they answer @@ -75,13 +94,15 @@ export function useTripRouteOverview( // // The frame is the exception. A fresh `focusPoints` array is what tells the map to // fit the camera, and a fit every second would take the map back from wherever the - // reader has panned to. The frame set on the straight lines therefore holds until - // the round is over, and only the finished result brings a new one, exactly the two - // fits the overview made when every answer landed at once. + // reader has panned to. The frame set on the straight lines therefore holds while + // the round answers; a reframe round ends on the routed roads (the two fits the + // overview has always made on activation), an edit round ends on the frame it + // started with and the camera never moves. const publish = (routed: TripRouteAnswers, loading: boolean): void => { if (controller.signal.aborted) return const next = summariseTripRoute(assembleTripRoute(plan, routed)) - setResult({ ...next, focusPoints: loading ? first.focusPoints : next.focusPoints, loading }) + if (!loading && reframe) frameRef.current = { tripId, points: next.focusPoints } + setResult({ ...next, focusPoints: loading ? frame : frameRef.current!.points, loading }) } routeTripLegs(plan, { tripId, signal: controller.signal, onAnswer: routed => publish(routed, true) }) .then(routed => publish(routed, false))