From 49e306ea48ee76ea165e26766b32ed5500bffbdb Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 18:23:50 +0200 Subject: [PATCH] refactor(map): report the GL viewport from a module of its own reportGlViewport says what a GL map shows, its bounds and its zoom on the raster tile scale, and has its own test for that scale. MapViewGL only wires it to moveend and idle, and its test checks that wiring. The GL map tests put the zoom back to its default before each test, as they do for every other value a test overrides, instead of each test restoring it. --- client/scripts/size-baseline.json | 2 +- client/src/components/Map/MapViewGL.test.tsx | 6 ++-- client/src/components/Map/MapViewGL.tsx | 14 +++----- client/src/components/Map/glViewport.test.ts | 35 ++++++++++++++++++++ client/src/components/Map/glViewport.ts | 21 ++++++++++++ 5 files changed, 63 insertions(+), 15 deletions(-) create mode 100644 client/src/components/Map/glViewport.test.ts create mode 100644 client/src/components/Map/glViewport.ts diff --git a/client/scripts/size-baseline.json b/client/scripts/size-baseline.json index 5bf49f75a..6ad44b8a4 100644 --- a/client/scripts/size-baseline.json +++ b/client/scripts/size-baseline.json @@ -7,7 +7,7 @@ "src/components/Budget/CostsPanel.tsx": 1816, "src/components/Map/MapView.tsx": 1401, "src/components/Map/MapViewGL.test.tsx": 2920, - "src/components/Map/MapViewGL.tsx": 2296, + "src/components/Map/MapViewGL.tsx": 2293, "src/components/Map/RouteCalculator.ts": 1195, "src/components/PDF/TripPDF.tsx": 1082, "src/components/Planner/DayDetailPanel.test.tsx": 2333, diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx index db80c2909..6a1b4a0ac 100644 --- a/client/src/components/Map/MapViewGL.test.tsx +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -257,6 +257,7 @@ beforeEach(() => { glMap.querySourceFeatures.mockReturnValue([]) glMap.getCanvas.mockImplementation(() => document.createElement('canvas')) glMap.getBearing.mockReturnValue(0) + glMap.getZoom.mockReturnValue(10) glMap.queryTerrainElevation.mockReturnValue(null) // A busy style, a refused source and a custom layer list are each set by one test // for the case it covers. Put back here, or every test after it would silently run @@ -1575,7 +1576,6 @@ describe('MapViewGL', () => { const list = calls[calls.length - 1][0] act(() => { list.querySelectorAll('button')[1].click() }) expect(onPoiClick).toHaveBeenCalledWith(other) - vi.mocked(glMap.getZoom).mockReturnValue(10) }) const pinGlyph = (Icon: LucideIcon) => renderIconMarkup(React.createElement(Icon, { size: 13, color: 'white', strokeWidth: 2.5 })) @@ -2215,14 +2215,12 @@ describe('MapViewGL', () => { expect((window as any).__trek_map).toBeUndefined() }) - it('FE-COMP-MAPVIEWGL-059: pan and zoom report the visible bbox for the POI explore pill, with the zoom on the raster tile scale', async () => { + it('FE-COMP-MAPVIEWGL-059: pan and zoom report the visible bbox and tile zoom for the POI explore pill', async () => { const onViewportChange = vi.fn() - vi.mocked(glMap.getZoom).mockReturnValue(10) render() await act(async () => {}) act(() => { mapHandler('moveend')() }) - // The GL engine counts on 512 px tiles; a raster tile's zoom is one step further in. expect(onViewportChange).toHaveBeenCalledWith({ south: 0, west: 0, north: 1, east: 1 }, 11) }) diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index a4121a45b..0acd2a492 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -8,7 +8,8 @@ import { renderIconMarkup } from '../../utils/iconMarkup' import type mapboxgl from 'mapbox-gl' import { useSettingsStore } from '../../store/settingsStore' import { useTranslation } from '../../i18n/TranslationContext' -import { applyBaseLayer, rasterTileZoom } from './glRasterLayers' +import { applyBaseLayer } from './glRasterLayers' +import { reportGlViewport } from './glViewport' import { isTourLayerHit, useTourMapLayersGL } from './mapRasterOverlay' import { MapLayerSwitcher, TourMapLayerSwitcher, MAP_LAYER_SWITCHER_INSET, type BaseLayer, type TourBaseLayer } from './MapLayerSwitcher' import { MapLockPill } from './MapLockPill' @@ -1432,15 +1433,8 @@ export function MapViewGL({ if (isTourLayerHit(map, e.point)) return // a picked via ferrata or paddle obstacle opens its card instead onClickRefs.current.map?.({ latlng: { lat: e.lngLat.lat, lng: e.lngLat.lng } }) }) - // Emit the viewport bbox (pan/zoom + once on first idle) so the POI-explore - // pill can fetch OSM places for the visible area, with the zoom on the raster tile scale. - const emitViewport = () => { - const b = map.getBounds() - onViewportChangeRef.current?.( - { south: b.getSouth(), west: b.getWest(), north: b.getNorth(), east: b.getEast() }, - rasterTileZoom(map.getZoom()), - ) - } + // Emit the viewport (pan/zoom + once on first idle) for the POI-explore pill. + const emitViewport = () => reportGlViewport(map, onViewportChangeRef.current) map.on('moveend', emitViewport) map.once('idle', emitViewport) // Clear the hover card (and the anchored POI popup) as soon as the camera diff --git a/client/src/components/Map/glViewport.test.ts b/client/src/components/Map/glViewport.test.ts new file mode 100644 index 000000000..b6294bdee --- /dev/null +++ b/client/src/components/Map/glViewport.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it, vi } from 'vitest'; +import { reportGlViewport } from './glViewport'; + +/** A GL map showing the given box at the given zoom, or no box yet. */ +function fakeMap(zoom: number, box: { south: number; west: number; north: number; east: number } | null) { + const bounds = box && { + getSouth: () => box.south, + getWest: () => box.west, + getNorth: () => box.north, + getEast: () => box.east, + }; + return { getBounds: () => bounds, getZoom: () => zoom }; +} + +describe('reportGlViewport', () => { + it('reports the visible box with the zoom on the raster tile scale', () => { + const report = vi.fn(); + reportGlViewport(fakeMap(10, { south: 47, west: 11, north: 48, east: 12 }), report); + // The GL engine counts on 512 px tiles; a raster tile's zoom is one step further in. + expect(report).toHaveBeenCalledWith({ south: 47, west: 11, north: 48, east: 12 }, 11); + }); + + it('says nothing while the map has no bounds yet', () => { + const report = vi.fn(); + reportGlViewport(fakeMap(10, null), report); + expect(report).not.toHaveBeenCalled(); + }); + + it('asks nothing of the map when no one listens', () => { + const map = fakeMap(10, { south: 47, west: 11, north: 48, east: 12 }); + const getZoom = vi.spyOn(map, 'getZoom'); + reportGlViewport(map, undefined); + expect(getZoom).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/Map/glViewport.ts b/client/src/components/Map/glViewport.ts new file mode 100644 index 000000000..f9c7464d6 --- /dev/null +++ b/client/src/components/Map/glViewport.ts @@ -0,0 +1,21 @@ +import { rasterTileZoom } from './glRasterLayers'; +import type { MapViewProps } from './mapViewContract'; + +/** Enough of a GL map to say what it shows: its bounds and its zoom. */ +interface ViewportSource { + getBounds(): { getSouth(): number; getWest(): number; getNorth(): number; getEast(): number } | null; + getZoom(): number; +} + +/** + * Tells the page what a GL map shows, so the POI explore pill can fetch the + * OSM places of the visible area: its bounds, and its zoom on the raster tile + * scale Leaflet reports too, so both maps ask for the same places at the + * same view. A map without bounds yet says nothing. + */ +export function reportGlViewport(map: ViewportSource, report: MapViewProps['onViewportChange']): void { + const bounds = map.getBounds(); + if (!report || !bounds) return; + const bbox = { south: bounds.getSouth(), west: bounds.getWest(), north: bounds.getNorth(), east: bounds.getEast() }; + report(bbox, rasterTileZoom(map.getZoom())); +}