From 04c7491e2ab2df449507c5397b5a17d5dae86d02 Mon Sep 17 00:00:00 2001 From: elmocito <42518790+elmocito@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:44:43 +0200 Subject: [PATCH] fix(tours): preserve map viewport during route updates --- client/src/components/Map/MapView.test.tsx | 28 ++++++++++ client/src/components/Map/MapView.tsx | 17 +++++- client/src/components/Map/MapViewGL.test.tsx | 55 +++++++++++++++++++ client/src/components/Map/MapViewGL.tsx | 13 ++++- .../Tours/planner/useTourPlanner.test.tsx | 30 ++++++++++ .../Tours/planner/useTourPlanner.ts | 5 +- client/src/pages/TripPlannerPage.tsx | 2 + .../pages/tripPlanner/useTripPlannerPage.ts | 2 +- 8 files changed, 146 insertions(+), 6 deletions(-) diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index d63790d3a..76cc0ed0e 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -441,6 +441,34 @@ describe('MapView', () => { expect(mapMock.fitBounds).toHaveBeenCalled() }) + it('frames Tour focus once, ignores waypoint and route updates, then honors a new focus intent', () => { + const route: [number, number][][] = [[[48, 11], [48.2, 11.3]]] + const { rerender } = render( + , + ) + expect(mapMock.fitBounds).not.toHaveBeenCalled() + + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledOnce() + mapMock.fitBounds.mockClear() + + rerender( + , + ) + expect(mapMock.fitBounds).not.toHaveBeenCalled() + + rerender( + , + ) + expect(mapMock.fitBounds).toHaveBeenCalledOnce() + }) + it('keeps selection fit and the pending route-arrival refit for semantic changes', () => { const selectedPlace = buildMapPlace({ id: 901, lat: 48, lng: 11 }) const route: [number, number][][] = [[[48, 11], [48.2, 11.3]]] diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index aaaec16a6..c933e7924 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -448,6 +448,8 @@ interface BoundsControllerProps { * needs that leg on screen, which is neither the day nor the trip. */ focusPoints?: [number, number][] + /** Changes only when the caller intentionally wants a new initial frame. */ + focusKey?: number /** False while the map is locked (#2010): an arriving route no longer re-fits it. */ follow?: boolean /** @@ -470,11 +472,13 @@ function leafletPadding(box: ViewportPadding): L.FitBoundsOptions { } } -function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMount = false, focusPoints, fitPadding, follow = true }: BoundsControllerProps) { +function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMount = false, focusPoints, focusKey, fitPadding, follow = true }: BoundsControllerProps) { const map = useMap() const prevFitKey = useRef(-1) const awaitingRoute = useRef(false) const fitRan = useRef(false) + const didInitialFocus = useRef(false) + const prevFocusKey = useRef(focusKey) const fitTo = useCallback((coords: [number, number][], padding: L.FitBoundsOptions = paddingOpts) => { if (coords.length === 0) return @@ -537,10 +541,16 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo // picker leaves the map where the user left it rather than snapping back. useEffect(() => { if (!focusPoints?.length) return + if (focusKey !== undefined) { + const shouldFit = !didInitialFocus.current || focusKey !== prevFocusKey.current + prevFocusKey.current = focusKey + if (!shouldFit) return + didInitialFocus.current = true + } // A day fit that has not run yet must not overwrite this a moment later. awaitingRoute.current = false fitTo(focusPoints, fitPadding ? leafletPadding(fitPadding) : paddingOpts) - }, [focusPoints, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps + }, [focusPoints, focusKey, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps return null } @@ -786,6 +796,7 @@ export const MapView = memo(function MapView({ // the shape a caller without one gets. tileUrl = OFM_POSITRON, fitKey = 0, + focusKey, dayOrderMap = {}, leftWidth = 0, rightWidth = 0, @@ -1249,7 +1260,7 @@ export const MapView = memo(function MapView({ {hasViewBaseLayer && } - 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} framedOnMount={initialView.framed} focusPoints={focusPoints} fitPadding={fitPadding} follow={followSelection} /> + 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} framedOnMount={initialView.framed} focusPoints={focusPoints} focusKey={focusKey} fitPadding={fitPadding} follow={followSelection} /> diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx index 274c10e94..a9fc1d87c 100644 --- a/client/src/components/Map/MapViewGL.test.tsx +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -930,6 +930,61 @@ describe('MapViewGL', () => { expect(glMap.easeTo).not.toHaveBeenCalled() }) + it('frames Tour focus once, ignores waypoint and route updates, then honors a new focus intent', async () => { + loadOnAttach() + const { rerender } = render( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).not.toHaveBeenCalled() + + rerender( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledOnce() + glMap.fitBounds.mockClear() + + rerender( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).not.toHaveBeenCalled() + + rerender( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledOnce() + }) + it('keeps selection fit and the pending route-arrival refit for semantic changes', async () => { loadOnAttach() const selectedPlace = buildPlace({ id: 902, lat: 48, lng: 11 }) diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 0f2ffc39e..85229418c 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -216,6 +216,8 @@ interface Props { * needs that leg on screen, which is neither the day nor the trip. */ focusPoints?: [number, number][] + /** Changes only when the caller intentionally wants a new initial frame. */ + focusKey?: number /** * What the caller's own chrome covers while `focusPoints` is framed, in pixels per edge. * @@ -713,6 +715,7 @@ export function MapViewGL({ zoom = DEFAULT_MAP_ZOOM, fitKey = 0, focusPoints, + focusKey, fitPadding, clusterLoosely = false, hazards, @@ -2388,12 +2391,20 @@ export function MapViewGL({ // The caller's padding as a value, so a parent that builds the object inline on every // render does not move the camera each time it renders. const fitPaddingKey = fitPadding ? [fitPadding.top, fitPadding.right, fitPadding.bottom, fitPadding.left].join(' ') : '' + const didInitialFocusRef = useRef(false) + const prevFocusKeyRef = useRef(focusKey) // Frame whatever was handed over. Nothing happens when it empties, so closing the // picker leaves the map where the user left it rather than snapping back. useEffect(() => { const map = mapRef.current if (!map || !focusPoints?.length) return + if (focusKey !== undefined) { + const shouldFit = !didInitialFocusRef.current || focusKey !== prevFocusKeyRef.current + prevFocusKeyRef.current = focusKey + if (!shouldFit) return + didInitialFocusRef.current = true + } const bounds = new gl.LngLatBounds() focusPoints.forEach(([lat, lng]) => bounds.extend([lng, lat])) // A day fit still waiting on its route must not overwrite this a moment later. @@ -2406,7 +2417,7 @@ export function MapViewGL({ duration: 400, }) } catch { /* noop */ } - }, [focusPoints, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps + }, [focusPoints, focusKey, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps // flyTo selected place useEffect(() => { diff --git a/client/src/components/Tours/planner/useTourPlanner.test.tsx b/client/src/components/Tours/planner/useTourPlanner.test.tsx index f66a6a274..b349b2165 100644 --- a/client/src/components/Tours/planner/useTourPlanner.test.tsx +++ b/client/src/components/Tours/planner/useTourPlanner.test.tsx @@ -81,6 +81,36 @@ describe('useTourPlanner', () => { expect(result.current.waypoints).toHaveLength(3) }) + it('changes the map focus intent only for explicit Tour context changes', async () => { + detailTour.mockResolvedValue({ + tour: savedTour, + waypoints: [ + { lat: 48, lng: 11, role: 'start', sequence: 0 }, + { lat: 48.01, lng: 11.02, role: 'end', sequence: 1 }, + ], + }) + const { result } = renderHook(() => useTourPlanner({ tripId: 8 })) + const initialFocusKey = result.current.mapFocusKey + + act(() => { + result.current.addWaypoint(48, 11) + result.current.addWaypoint(48.01, 11.02) + }) + await act(async () => { await vi.advanceTimersByTimeAsync(450) }) + expect(result.current.mapFocusKey).toBe(initialFocusKey) + + act(() => result.current.retry()) + expect(result.current.mapFocusKey).toBe(initialFocusKey) + + await act(async () => { expect(await result.current.openTour(savedTour)).toBe(true) }) + expect(result.current.mapFocusKey).toBe(initialFocusKey + 1) + act(() => result.current.addWaypoint(48.02, 11.03)) + expect(result.current.mapFocusKey).toBe(initialFocusKey + 1) + + act(() => result.current.viewGpxTour({ ...savedTour, has_waypoints: false }, '[[48,11],[48.01,11.02]]')) + expect(result.current.mapFocusKey).toBe(initialFocusKey + 2) + }) + it('TOUR-PLANNER-002: restores a trip-scoped local draft without changing stable ids', () => { localStorage.setItem('tour-draft-8', JSON.stringify({ version: 1, diff --git a/client/src/components/Tours/planner/useTourPlanner.ts b/client/src/components/Tours/planner/useTourPlanner.ts index 2dc405f1e..f1c994fbe 100644 --- a/client/src/components/Tours/planner/useTourPlanner.ts +++ b/client/src/components/Tours/planner/useTourPlanner.ts @@ -136,6 +136,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe const [elevationProfileExpanded, setElevationProfileExpanded] = useState(true) const [routeProfileFocus, setRouteProfileFocus] = useState(null) const [newDraftGeneration, setNewDraftGeneration] = useState(0) + const [mapFocusKey, setMapFocusKey] = useState(0) const abortRef = useRef(null) const generationRef = useRef(0) const draftRevisionRef = useRef(0) @@ -334,6 +335,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe setNewTourConfirmationOpen(false) setElevationProfileExpanded(true) setNewDraftGeneration(generation => generation + 1) + setMapFocusKey(key => key + 1) }, [invalidateDetailRequest, interruptRouting, storageKey]) const startNewTour = useCallback((confirmed = false) => { @@ -429,6 +431,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe setDraftRestored(false) setEditingPlaceId(result.tour.place_id) editingPlaceIdRef.current = result.tour.place_id + setMapFocusKey(key => key + 1) setSaveOutcome(null) setReadOnlyGpxTour(null) return true @@ -613,7 +616,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe mapBaseLayer, setMapBaseLayer, mode, readOnlyGpxTour, readOnlyGpxAnalysis, viewGpxTour, closeGpxTour, startNewTour, newTourConfirmationOpen, cancelNewTour, - elevationProfileExpanded, toggleElevationProfile, newDraftGeneration, draftRestored, + elevationProfileExpanded, toggleElevationProfile, newDraftGeneration, draftRestored, mapFocusKey, editingPlaceId, openingTourId, openTour, saveOutcome, hasUnsavedChanges, canSave, save, discard, returnToNeutral, } diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 65a5066aa..729947640 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -1082,6 +1082,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { tripId={tripId} places={[]} route={tourMap.route} + followSelection={false} onMapClick={canPlaceEdit && !tourPlanner.isSaving && (tourPlanner.mode.type === 'new-draft' || tourPlanner.mode.type === 'edit-saved') ? ({ latlng }: { latlng: { lat: number; lng: number } }) => tourPlanner.addWaypoint(latlng.lat, latlng.lng) : undefined} @@ -1089,6 +1090,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { leftWidth={leftPanelPx} rightWidth={rightPanelPx} focusPoints={tourMap.focusPoints} + focusKey={tourMap.focusKey} plannerWaypoints={tourPlanner.mode.type === 'new-draft' || tourPlanner.mode.type === 'edit-saved' ? tourPlanner.waypoints : []} selectedPlannerWaypointId={tourPlanner.mode.type === 'new-draft' || tourPlanner.mode.type === 'edit-saved' ? tourPlanner.selectedWaypointId : null} onPlannerWaypointClick={tourPlanner.setSelectedWaypointId} diff --git a/client/src/pages/tripPlanner/useTripPlannerPage.ts b/client/src/pages/tripPlanner/useTripPlannerPage.ts index b83c84c35..d8a0b0568 100644 --- a/client/src/pages/tripPlanner/useTripPlannerPage.ts +++ b/client/src/pages/tripPlanner/useTripPlannerPage.ts @@ -101,6 +101,6 @@ export function useTripPlannerPage() { permissions: { canPlaceEdit, canDayEdit }, tourPlanner, tourDetails: { openerRef: tourDetailOpenerRef, onSelectTour }, - tourMap: { route, focusPoints, captureViewport }, + tourMap: { route, focusPoints, focusKey: tourPlanner.mapFocusKey, captureViewport }, } } \ No newline at end of file