From ac2377335286bd5fa9d5c1f700588e41ae3cfabf Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 21 Sep 2026 11:02:01 +0200 Subject: [PATCH] fix(journey): the map waits for its first view before it pans to the active entry The journey map sets its first view on a rAF after the build, and the active entry asks for its pan on a 50 ms timer. In a background tab, or on a slow frame, the timer wins. 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 entry marker queued behind it never reaches the map. The catch that was meant for this never fired, because nothing threw. The next rebuild, after any save on the page, then tore down markers that were never added and died in Leaflet's icon removal, and the error boundary took the whole journey page with it. Both pans go through whenReady now: at once on a map with a view, otherwise right after the fit lands. --- .../components/Journey/JourneyMap.test.tsx | 36 ++++++++++++++----- client/src/components/Journey/JourneyMap.tsx | 36 ++++++++++++------- 2 files changed, 51 insertions(+), 21 deletions(-) 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])