diff --git a/client/src/components/Journey/JourneyMap.test.tsx b/client/src/components/Journey/JourneyMap.test.tsx index 08c118e6c..f0e993ae3 100644 --- a/client/src/components/Journey/JourneyMap.test.tsx +++ b/client/src/components/Journey/JourneyMap.test.tsx @@ -27,6 +27,10 @@ vi.mock('leaflet', () => { setView: vi.fn(), flyTo: vi.fn(), panTo: vi.fn(), + // Runs the callback at once on a map with a view, else on its load event. + // The default is the loaded map; the two "no view yet" tests below hold + // the callback back and release it by hand. + whenReady: vi.fn((cb: () => void) => { cb(); }), getZoom: vi.fn(() => 10), // Leaflet throws "Set map center and zoom first." out of these until the map // has a view; the loaded map is the default here, FE-COMP-JOURNEYMAP-050 @@ -334,13 +338,21 @@ describe('JourneyMap', () => { expect(mockedMap().flyTo).not.toHaveBeenCalled(); }); - it('FE-COMP-JOURNEYMAP-018: focusMarker swallows leaflet errors when the map has no view yet', () => { + it('FE-COMP-JOURNEYMAP-018: focusMarker waits for the first view when the map has none yet', () => { const ref = React.createRef(); render(); - vi.mocked(mockedMap().panTo).mockImplementationOnce(() => { throw new Error('Set map center and zoom first'); }); + // Leaflet does not refuse a pan on a viewless map, it takes it as the first + // view with the zoom undefined, and the tile layer then aborts the load event + // before the markers are added. So the pan has to wait for the fit. + let onLoad: (() => void) | null = null; + vi.mocked(mockedMap().whenReady).mockImplementationOnce((cb: () => void) => { onLoad = cb; }); - expect(() => act(() => { ref.current!.focusMarker('e1'); })).not.toThrow(); - expect(mockedMap().flyTo).not.toHaveBeenCalled(); + act(() => { ref.current!.focusMarker('e1'); }); + expect(mockedMap().panTo).not.toHaveBeenCalled(); + + act(() => { onLoad!(); }); + expect(mockedMap().panTo).toHaveBeenCalledWith({ lat: 0, lng: 0 }, { animate: true, duration: 0.5 }); + expect(mockedMap().setView).not.toHaveBeenCalledWith({ lat: 0, lng: 0 }, 12); }); it('FE-COMP-JOURNEYMAP-019: invalidateSize forwards to the leaflet map', () => { @@ -482,14 +494,22 @@ describe('JourneyMap', () => { } }); - it('FE-COMP-JOURNEYMAP-029: activeMarkerId falls back to setView while the map has no view yet', () => { + it('FE-COMP-JOURNEYMAP-029: activeMarkerId waits for the first view while the map has none yet', () => { vi.useFakeTimers(); try { render(); - vi.mocked(mockedMap().panTo).mockImplementationOnce(() => { throw new Error('Set map center and zoom first'); }); + // The 50 ms timer can beat the rAF that sets the first view, in a + // background tab most of all. A pan at that point used to become the first + // view itself, with no zoom, and left the map without its markers; the next + // rebuild then crashed the page in Leaflet's icon removal. + let onLoad: (() => void) | null = null; + vi.mocked(mockedMap().whenReady).mockImplementationOnce((cb: () => void) => { onLoad = cb; }); act(() => { vi.advanceTimersByTime(60); }); - // The catch is where a map with no view at all gets its first one. - expect(mockedMap().setView).toHaveBeenCalledWith({ lat: 0, lng: 0 }, 12); + expect(mockedMap().panTo).not.toHaveBeenCalled(); + expect(mockedMap().setView).not.toHaveBeenCalledWith({ lat: 0, lng: 0 }, 12); + + act(() => { onLoad!(); }); + expect(mockedMap().panTo).toHaveBeenCalledWith({ lat: 0, lng: 0 }, { animate: true, duration: 0.5 }); expect(mockedMap().flyTo).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); diff --git a/client/src/components/Journey/JourneyMap.tsx b/client/src/components/Journey/JourneyMap.tsx index cd340717e..4b74d93c8 100644 --- a/client/src/components/Journey/JourneyMap.tsx +++ b/client/src/components/Journey/JourneyMap.tsx @@ -161,6 +161,27 @@ function markerSvg(dayColor: string, dayLabel: number, highlighted: boolean): st ` } +/** + * Pan, don't zoom: the initial fitBounds decides how far out the reader starts, + * and a focus keeps that (discussion #2299). + * + * Deferred until the map has its first view. That view is set on a rAF after + * the build, and the active entry asks for its pan on a 50 ms timer, so a tab + * in the background or a slow frame lets the pan come first. Leaflet does not + * refuse a pan on a viewless map, it takes it as the first view, with the zoom + * still undefined: the tile layer aborts its own add on that, the load event + * stops halfway, and every marker queued behind it never makes it onto the map. + * The next rebuild then tears down markers that were never added and dies in + * Leaflet's icon removal, which took the whole journey page with it after each + * save. whenReady runs the pan at once on a map with a view and otherwise right + * after the fit lands. + */ +function panToMarker(map: L.Map, marker: L.Marker): void { + map.whenReady(() => { + map.panTo(marker.getLatLng(), { animate: true, duration: 0.5 }) + }) +} + const EMPTY_TRAIL: { lat: number; lng: number }[] = [] const EMPTY_TRACKS: JourneyTrack[] = [] /** Fallback when a track carries no colour of its own, matching the planner's default. */ @@ -254,11 +275,7 @@ function JourneyMap( const focusMarker = useCallback((id: string) => { highlightMarker(id) const marker = markersRef.current.get(id) - if (marker && mapRef.current) { - try { - mapRef.current.panTo(marker.getLatLng(), { animate: true, duration: 0.5 }) - } catch { /* map not yet initialized */ } - } + if (marker && mapRef.current) panToMarker(mapRef.current, marker) }, []) const invalidateSize = useCallback(() => { @@ -511,14 +528,7 @@ function JourneyMap( highlightMarker(activeMarkerId) const marker = markersRef.current.get(activeMarkerId) if (!marker || !mapRef.current) return - // Pan, don't zoom — see focusMarker. fitBounds may still be pending when this - // fires, and panTo on a map with no view throws "Set map center and zoom - // first", so the catch is where the map gets its first one. - try { - mapRef.current.panTo(marker.getLatLng(), { animate: true, duration: 0.5 }) - } catch { - mapRef.current.setView(marker.getLatLng(), 12) - } + panToMarker(mapRef.current, marker) }, 50) return () => clearTimeout(timer) }, [activeMarkerId])