diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json index be157c389..a326ebf69 100644 --- a/client/scripts/eslint-baseline.json +++ b/client/scripts/eslint-baseline.json @@ -30,7 +30,7 @@ "no-useless-escape": 1 }, "suppressed": { - "@typescript-eslint/no-explicit-any": 40, + "@typescript-eslint/no-explicit-any": 38, "react-hooks/exhaustive-deps": 46 }, "tests": { diff --git a/client/scripts/size-baseline.json b/client/scripts/size-baseline.json index e2a571f46..0ae42de61 100644 --- a/client/scripts/size-baseline.json +++ b/client/scripts/size-baseline.json @@ -7,7 +7,7 @@ "src/components/Budget/CostsPanel.tsx": 1798, "src/components/Map/MapView.tsx": 1385, "src/components/Map/MapViewGL.test.tsx": 2919, - "src/components/Map/MapViewGL.tsx": 2290, + "src/components/Map/MapViewGL.tsx": 2218, "src/components/Map/RouteCalculator.ts": 1063, "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 381f9906d..a5550a010 100644 --- a/client/src/components/Map/MapViewGL.test.tsx +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -1768,35 +1768,6 @@ describe('MapViewGL', () => { expect(drawn).not.toContainEqual([2.3, 48.3]) }) - // The bubbles are DOM so they share the Leaflet map's font and hover; the canvas - // layers under them stay only as the hit area. - it('FE-COMP-MAPVIEWGL-106: every cluster draws one HTML count bubble that zooms in on a click', async () => { - loadOnAttach() - projectAtDayZoom() - const clusterSource = { ...geoSource(), getClusterExpansionZoom: vi.fn(() => 11) } - glMap.getSource.mockImplementation((id: string) => (id === 'trip-place-clusters' ? clusterSource : null)) - const cluster = { properties: { cluster_id: 5, point_count: 12 }, geometry: { coordinates: [139.7, 35.6] } } - glMap.querySourceFeatures.mockImplementation((_id: string, query?: { filter?: unknown[] }) => - query?.filter?.[0] === 'has' - // The same cluster crossing a tile edge, and a feature without a count. - ? [cluster, cluster, { properties: { cluster_id: 6 }, geometry: { coordinates: [1, 1] } }] - : []) - - render() - await flushFrames() - - const bubbles = glMarkers.created.filter(m => m.element.querySelector('.marker-cluster-custom')) - expect(bubbles).toHaveLength(1) - expect(bubbles[0].lngLat).toEqual([139.7, 35.6]) - expect(bubbles[0].element.textContent).toBe('12') - - act(() => { bubbles[0].element.dispatchEvent(new MouseEvent('click', { bubbles: true })) }) - expect(clusterSource.getClusterExpansionZoom).toHaveBeenCalledWith(5, expect.any(Function)) - expect(glMap.easeTo).toHaveBeenCalledWith({ center: [139.7, 35.6], zoom: 11, duration: 350 }) - glMap.querySourceFeatures.mockReset() - glMap.querySourceFeatures.mockReturnValue([]) - }) - // One building modelled as several stops — drop the bags, check in, the museum inside // it — puts every pin on the same spot, and all but the top one are unreachable. There // is no spiderfy on a GL map, so the stack draws as the one pin worth seeing (#2344). diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 88f942a8c..817c40d75 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -50,16 +50,10 @@ import { pluginsApi, type PluginMapMarker, type PluginMapLayer } from '../../api import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults' import { alongShortestArc, computeMapViewport, TILE_SIZE_GL } from '../../utils/mapViewport' import { selectedPlaceTarget } from './selectedPlaceTarget' -import { PLACE_CLUSTER_CIRCLE_LAYER_ID, PLACE_CLUSTER_COUNT_LAYER_ID, createClusterBubble } from './placeClusterLayers' - -// Marker grouping for the GL map (#1385): MapLibre/Mapbox can't show the rich -// HTML photo markers *and* cluster them natively, so we feed the place points -// into a clustered GeoJSON source. The cluster bubbles render as GL circles + -// a count label; the individual rich HTML markers are then only drawn for the -// points the source reports as currently unclustered. Grouping is always on, -// matching the Leaflet map's MarkerClusterGroup. -const PLACE_CLUSTER_SOURCE_ID = 'trip-place-clusters' -const PLACE_UNCLUSTERED_LAYER_ID = 'trip-place-unclustered-hit' +import { + PLACE_CLUSTER_CIRCLE_LAYER_ID, PLACE_CLUSTER_COUNT_LAYER_ID, PLACE_CLUSTER_SOURCE_ID, + addPlaceClusterLayers, drawClusterBubbles, expandCluster, +} from './placeClusterLayers' type PlaceWithCoords = Place & { lat: number; lng: number } @@ -112,93 +106,6 @@ interface Props extends MapViewProps { gl: any } -/** - * How eagerly place markers merge into a cluster. - * - * The overview band the Leaflet map clusters in as well (`CLUSTER_UNTIL_ZOOM` there), - * kept as plain numbers rather than shared constants so the GL bundle does not have to - * carry Leaflet for them. Above the cutoff supercluster hands every point back on its - * own, and the stops that share a coordinate are folded together at the pin level - * instead — see the reconcile below. - */ -const CLUSTER_RADIUS = 20 -const CLUSTER_MAX_ZOOM = 8 - -/** The cluster source and the three layers over it. Extracted so the rebuild on a mode - * change and the initial build cannot drift apart. */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function addPlaceClusterLayers(map: any): void { - map.addSource(PLACE_CLUSTER_SOURCE_ID, { - type: 'geojson', - data: { type: 'FeatureCollection', features: [] }, - cluster: true, - clusterRadius: CLUSTER_RADIUS, - clusterMaxZoom: CLUSTER_MAX_ZOOM, - }) - map.addLayer({ - id: PLACE_CLUSTER_CIRCLE_LAYER_ID, - type: 'circle', - source: PLACE_CLUSTER_SOURCE_ID, - filter: ['has', 'point_count'], - // Invisible: the bubble the user sees is DOM (createClusterBubble), these two - // layers stay as the hit area the track and route layers check a click against. - paint: { - 'circle-opacity': 0, - 'circle-stroke-opacity': 0, - 'circle-radius': ['step', ['get', 'point_count'], 18, 10, 21, 50, 24], - }, - }) - map.addLayer({ - id: PLACE_CLUSTER_COUNT_LAYER_ID, - type: 'symbol', - source: PLACE_CLUSTER_SOURCE_ID, - filter: ['has', 'point_count'], - layout: { - 'text-field': ['get', 'point_count_abbreviated'], - 'text-size': 12, - 'text-allow-overlap': true, - }, - paint: { 'text-opacity': 0 }, - }) - map.addLayer({ - id: PLACE_UNCLUSTERED_LAYER_ID, - type: 'circle', - source: PLACE_CLUSTER_SOURCE_ID, - filter: ['!', ['has', 'point_count']], - paint: { - 'circle-radius': 24, - 'circle-opacity': 0, - 'circle-stroke-opacity': 0, - }, - }) -} - -/** Eases the map in to the zoom a cluster falls apart at. Both SDKs answer that - * question, one through a callback and one through a promise. */ -function expandCluster(map: ClusterMap, clusterId: unknown, coordinates: unknown): void { - if (clusterId == null || !Array.isArray(coordinates)) return - const source = map.getSource(PLACE_CLUSTER_SOURCE_ID) as ClusterSource | undefined - const easeToZoom = (nextZoom: number) => { - try { map.easeTo({ center: coordinates, zoom: nextZoom, duration: 350 }) } catch { /* noop */ } - } - try { - const maybeZoom = source?.getClusterExpansionZoom?.(clusterId, (err, nextZoom) => { - if (!err && typeof nextZoom === 'number') easeToZoom(nextZoom) - }) - if (typeof maybeZoom === 'number') easeToZoom(maybeZoom) - else if (maybeZoom instanceof Promise) maybeZoom.then(easeToZoom).catch(() => {}) - } catch { /* noop */ } -} - -/** The two map calls `expandCluster` makes, the same on both SDKs. */ -interface ClusterMap { - getSource: (id: string) => unknown - easeTo: (options: { center: unknown[]; zoom: number; duration: number }) => void -} -interface ClusterSource { - getClusterExpansionZoom?: (id: unknown, done: (err: Error | null, zoom: number) => void) => unknown -} - function createMarkerElement(place: Place & PlaceMarkerFlags & { category_color?: string; category_icon?: string }, photoUrl: string | null, orderNumbers: number[] | null, selected: boolean): HTMLDivElement { // A stop that interrupts the drive gets its own small disc, decided before the photo // branch: the brand logo a fuel search comes back with is exactly what this replaces. @@ -1718,19 +1625,9 @@ export function MapViewGL({ clusterPinsRef.current.forEach(pin => pin.remove()) clusterPinsRef.current = [] } - // One bubble per cluster id: a cluster crossing a tile edge comes back once per tile. const reconcileClusterPins = () => { dropClusterPins() - const seenClusters = new Set() - for (const feature of map.querySourceFeatures(PLACE_CLUSTER_SOURCE_ID, { filter: ['has', 'point_count'] }) || []) { - const { cluster_id: clusterId, point_count: count } = feature?.properties ?? {} - const coordinates = feature?.geometry?.coordinates - if (typeof count !== 'number' || !Array.isArray(coordinates) || seenClusters.has(clusterId)) continue - seenClusters.add(clusterId) - const el = createClusterBubble(count) - el.addEventListener('click', (ev) => { ev.stopPropagation(); expandCluster(map, clusterId, coordinates) }) - clusterPinsRef.current.push(attachPin(map, gl, pinLayerRef.current, el, coordinates[0], coordinates[1])) - } + clusterPinsRef.current = drawClusterBubbles(map, (el, lng, lat) => attachPin(map, gl, pinLayerRef.current, el, lng, lat)) } const runReconcile = () => { raf = null diff --git a/client/src/components/Map/placeClusterLayers.test.ts b/client/src/components/Map/placeClusterLayers.test.ts index 887c53406..1108de5de 100644 --- a/client/src/components/Map/placeClusterLayers.test.ts +++ b/client/src/components/Map/placeClusterLayers.test.ts @@ -1,5 +1,27 @@ -import { describe, it, expect } from 'vitest'; -import { clusterBubbleSize, createClusterBubble } from './placeClusterLayers'; +import { describe, expect, it, vi } from 'vitest'; + +import { + type ClusterMap, + PLACE_CLUSTER_SOURCE_ID, + addPlaceClusterLayers, + clusterBubbleSize, + createClusterBubble, + drawClusterBubbles, +} from './placeClusterLayers'; + +function fakeMap(features: unknown[], expansionZoom: unknown = 11) { + const source = { getClusterExpansionZoom: vi.fn(() => expansionZoom) }; + const map = { + addSource: vi.fn(), + addLayer: vi.fn(), + getSource: vi.fn((id: string) => (id === PLACE_CLUSTER_SOURCE_ID ? source : undefined)), + easeTo: vi.fn(), + querySourceFeatures: vi.fn((_id: string, query: { filter: unknown[] }) => + query.filter[0] === 'has' ? features : [] + ), + }; + return { map: map as unknown as ClusterMap & typeof map, source }; +} describe('cluster bubbles', () => { it('FE-COMP-PLACECLUSTER-001: a bubble grows in three steps with its count', () => { @@ -15,4 +37,50 @@ describe('cluster bubbles', () => { expect(bubble.style.height).toBe('42px'); expect(bubble.textContent).toBe('38'); }); + + it('FE-COMP-PLACECLUSTER-003: the canvas circle and count stay as an invisible hit area', () => { + const { map } = fakeMap([]); + addPlaceClusterLayers(map); + expect(map.addSource).toHaveBeenCalledWith(PLACE_CLUSTER_SOURCE_ID, expect.objectContaining({ cluster: true })); + const layers = map.addLayer.mock.calls.map(([layer]) => layer as { id: string; paint: Record }); + expect(layers.find((l) => l.id === 'trip-place-clusters-circle')?.paint['circle-opacity']).toBe(0); + expect(layers.find((l) => l.id === 'trip-place-clusters-count')?.paint['text-opacity']).toBe(0); + }); + + it('FE-COMP-PLACECLUSTER-004: one bubble per cluster, placed at its centre, that zooms in on a click', () => { + const cluster = { properties: { cluster_id: 5, point_count: 12 }, geometry: { coordinates: [139.7, 35.6] } }; + const { map, source } = fakeMap([ + cluster, + cluster, // the same cluster crossing a tile edge + { properties: { cluster_id: 6 }, geometry: { coordinates: [1, 1] } }, // no count + { properties: { cluster_id: 7, point_count: 3 } }, // no geometry + ]); + const placed: { el: HTMLElement; lng: number; lat: number }[] = []; + const pins = drawClusterBubbles(map, (el, lng, lat) => { + placed.push({ el, lng, lat }); + return lng; + }); + + expect(pins).toEqual([139.7]); + expect(placed[0].el.textContent).toBe('12'); + expect(placed[0].lat).toBe(35.6); + + const onMapClick = vi.fn(); + document.body.appendChild(placed[0].el); + document.body.addEventListener('click', onMapClick); + placed[0].el.dispatchEvent(new MouseEvent('click', { bubbles: true })); + document.body.removeEventListener('click', onMapClick); + placed[0].el.remove(); + + // The click is the bubble's: the map underneath never sees it. + expect(onMapClick).not.toHaveBeenCalled(); + expect(source.getClusterExpansionZoom).toHaveBeenCalledWith(5, expect.any(Function)); + expect(map.easeTo).toHaveBeenCalledWith({ center: [139.7, 35.6], zoom: 11, duration: 350 }); + }); + + it('FE-COMP-PLACECLUSTER-005: a source that reports nothing draws nothing', () => { + const { map } = fakeMap([]); + map.querySourceFeatures.mockReturnValue(null as never); + expect(drawClusterBubbles(map, () => 1)).toEqual([]); + }); }); diff --git a/client/src/components/Map/placeClusterLayers.ts b/client/src/components/Map/placeClusterLayers.ts index cf659c8c0..a3d86382c 100644 --- a/client/src/components/Map/placeClusterLayers.ts +++ b/client/src/components/Map/placeClusterLayers.ts @@ -6,6 +6,144 @@ export const PLACE_CLUSTER_CIRCLE_LAYER_ID = 'trip-place-clusters-circle'; export const PLACE_CLUSTER_COUNT_LAYER_ID = 'trip-place-clusters-count'; +// Marker grouping for the GL map (#1385): MapLibre/Mapbox can't show the rich +// HTML photo markers *and* cluster them natively, so we feed the place points +// into a clustered GeoJSON source. The source decides what is clustered; the +// bubbles are drawn as DOM (`drawClusterBubbles`) and the individual rich HTML +// markers only for the points the source reports as currently unclustered. +// Grouping is always on, matching the Leaflet map's MarkerClusterGroup. +export const PLACE_CLUSTER_SOURCE_ID = 'trip-place-clusters'; +const PLACE_UNCLUSTERED_LAYER_ID = 'trip-place-unclustered-hit'; + +/** + * How eagerly place markers merge into a cluster. + * + * The overview band the Leaflet map clusters in as well (`CLUSTER_UNTIL_ZOOM` there), + * kept as plain numbers rather than shared constants so the GL bundle does not have to + * carry Leaflet for them. Above the cutoff supercluster hands every point back on its + * own, and the stops that share a coordinate are folded together at the pin level + * instead — see the reconcile in MapViewGL. + */ +const CLUSTER_RADIUS = 20; +const CLUSTER_MAX_ZOOM = 8; + +/** The part of either SDK's map the cluster code calls. */ +export interface ClusterMap { + addSource: (id: string, source: object) => void; + addLayer: (layer: object) => void; + getSource: (id: string) => unknown; + easeTo: (options: { center: unknown[]; zoom: number; duration: number }) => void; + querySourceFeatures: (id: string, options: { filter: unknown[] }) => ClusterFeature[] | null | undefined; +} + +interface ClusterFeature { + properties?: { cluster_id?: unknown; point_count?: unknown } | null; + geometry?: { coordinates?: unknown }; +} + +interface ClusterSource { + getClusterExpansionZoom?: (id: unknown, done: (err: Error | null, zoom: number) => void) => unknown; +} + +/** The cluster source and the three layers over it. Extracted so the rebuild on a mode + * change and the initial build cannot drift apart. */ +export function addPlaceClusterLayers(map: Pick): void { + map.addSource(PLACE_CLUSTER_SOURCE_ID, { + type: 'geojson', + data: { type: 'FeatureCollection', features: [] }, + cluster: true, + clusterRadius: CLUSTER_RADIUS, + clusterMaxZoom: CLUSTER_MAX_ZOOM, + }); + // Invisible: the bubble the user sees is DOM (createClusterBubble), these two + // layers stay as the hit area the track and route layers check a click against. + map.addLayer({ + id: PLACE_CLUSTER_CIRCLE_LAYER_ID, + type: 'circle', + source: PLACE_CLUSTER_SOURCE_ID, + filter: ['has', 'point_count'], + paint: { + 'circle-opacity': 0, + 'circle-stroke-opacity': 0, + 'circle-radius': ['step', ['get', 'point_count'], 18, 10, 21, 50, 24], + }, + }); + map.addLayer({ + id: PLACE_CLUSTER_COUNT_LAYER_ID, + type: 'symbol', + source: PLACE_CLUSTER_SOURCE_ID, + filter: ['has', 'point_count'], + layout: { + 'text-field': ['get', 'point_count_abbreviated'], + 'text-size': 12, + 'text-allow-overlap': true, + }, + paint: { 'text-opacity': 0 }, + }); + map.addLayer({ + id: PLACE_UNCLUSTERED_LAYER_ID, + type: 'circle', + source: PLACE_CLUSTER_SOURCE_ID, + filter: ['!', ['has', 'point_count']], + paint: { + 'circle-radius': 24, + 'circle-opacity': 0, + 'circle-stroke-opacity': 0, + }, + }); +} + +/** Eases the map in to the zoom a cluster falls apart at. Both SDKs answer that + * question, one through a callback and one through a promise. */ +export function expandCluster( + map: Pick, + clusterId: unknown, + coordinates: unknown +): void { + if (clusterId == null || !Array.isArray(coordinates)) return; + const source = map.getSource(PLACE_CLUSTER_SOURCE_ID) as ClusterSource | undefined; + const easeToZoom = (nextZoom: number) => { + try { + map.easeTo({ center: coordinates, zoom: nextZoom, duration: 350 }); + } catch { + /* noop */ + } + }; + try { + const maybeZoom = source?.getClusterExpansionZoom?.(clusterId, (err, nextZoom) => { + if (!err && typeof nextZoom === 'number') easeToZoom(nextZoom); + }); + if (typeof maybeZoom === 'number') easeToZoom(maybeZoom); + else if (maybeZoom instanceof Promise) maybeZoom.then(easeToZoom).catch(() => {}); + } catch { + /* noop */ + } +} + +/** + * Puts one count bubble on the map per cluster the source currently reports, through + * `place` (the caller's pin factory), and returns what `place` handed back. A cluster + * crossing a tile edge comes back once per tile, so bubbles are keyed by cluster id. + */ +export function drawClusterBubbles(map: ClusterMap, place: (el: HTMLElement, lng: number, lat: number) => T): T[] { + const pins: T[] = []; + const seen = new Set(); + for (const feature of map.querySourceFeatures(PLACE_CLUSTER_SOURCE_ID, { filter: ['has', 'point_count'] }) ?? []) { + const clusterId = feature?.properties?.cluster_id; + const count = feature?.properties?.point_count; + const coordinates = feature?.geometry?.coordinates; + if (typeof count !== 'number' || !Array.isArray(coordinates) || seen.has(clusterId)) continue; + seen.add(clusterId); + const el = createClusterBubble(count); + el.addEventListener('click', (ev) => { + ev.stopPropagation(); + expandCluster(map, clusterId, coordinates); + }); + pins.push(place(el, coordinates[0], coordinates[1])); + } + return pins; +} + /** A count bubble's diameter in pixels: both renderers size their bubbles by it. */ export function clusterBubbleSize(count: number): number { return count < 10 ? 36 : count < 50 ? 42 : 48;