diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index 4d92f63f0..00911a2f1 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -5,6 +5,7 @@ import { act, fireEvent, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { resetAllStores } from '../../../tests/helpers/store' import { buildPlace, buildReservation } from '../../../tests/helpers/factories' +import { MAP_MAX_ZOOM } from '../../constants/mapDefaults' import { useAuthStore } from '../../store/authStore' import { CATEGORY_ICON_MAP } from '../shared/categoryIcons' import * as photoService from '../../services/photoService' @@ -48,9 +49,10 @@ vi.mock('../../hooks/useGeolocation', () => ({ const thumbCallbacks = vi.hoisted(() => new Map void>()) vi.mock('react-leaflet', () => ({ - // center/zoom are surfaced so tests can assert the camera the map is built with. - MapContainer: ({ children, center, zoom }: any) => ( -
{children}
+ // center/zoom are surfaced so tests can assert the camera the map is built + // with; maxZoom because a cluster refuses to attach to a map without one. + MapContainer: ({ children, center, zoom, maxZoom }: any) => ( +
{children}
), TileLayer: () =>
, Marker: ({ children, eventHandlers, position, icon, zIndexOffset }: any) => ( @@ -245,6 +247,14 @@ describe('MapView', () => { expect(screen.getByTestId('cluster-group')).toBeTruthy() }) + it('FE-COMP-MAPVIEW-010b: the map carries a zoom ceiling of its own, whatever the basemap is', () => { + // Without it, a vector basemap leaves the map with none and the cluster + // above throws on attach, taking the planner to the error boundary. The + // ceiling has to sit on the map because only a GridLayer can contribute one. + render() + expect(screen.getByTestId('map-container').getAttribute('data-maxzoom')).toBe(String(MAP_MAX_ZOOM)) + }) + it('FE-COMP-MAPVIEW-011: renders the route polyline; travel times are no longer drawn on the map', () => { const route = [[[48.0, 2.0], [49.0, 3.0]]] as unknown as [number, number][][] render() diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index fe9737e3a..c119af4a7 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -19,7 +19,7 @@ import { escapeHtml } from '@trek/shared' import type { Day, Reservation, RouteVia } from '../../types' import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories' import { resolveTrackColor, hasManualTrackColor } from './trackColors' -import { OFM_POSITRON, DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, SATELLITE_TILE_URL, SATELLITE_TILE_ATTRIBUTION, SATELLITE_TILE_MAXZOOM, attributionForTile } from '../../constants/mapDefaults' +import { OFM_POSITRON, DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, MAP_MAX_ZOOM, SATELLITE_TILE_URL, SATELLITE_TILE_ATTRIBUTION, SATELLITE_TILE_MAXZOOM, attributionForTile } from '../../constants/mapDefaults' import { resolveBasemap } from '../../utils/tileUrl' import VectorBasemap from './VectorBasemap' import { useSettingsStore } from '../../store/settingsStore' @@ -832,6 +832,12 @@ export const MapView = memo(function MapView({ center={initialView.center} zoom={initialView.zoom} zoomControl={false} + // On the map itself, not left to the base layer. Leaflet reads its zoom + // ceiling from the map options or, failing that, from a GridLayer that + // brought one; a vector basemap is neither, so a map drawn by + // VectorBasemap had no ceiling at all. MarkerClusterGroup.onAdd throws + // outright on an infinite one, which took the whole planner down. + maxZoom={MAP_MAX_ZOOM} className="w-full h-full bg-[#e5e7eb]" > {/* The basemap is a vector style by default and a raster template when the diff --git a/client/src/components/Map/mapZoomCeiling.test.ts b/client/src/components/Map/mapZoomCeiling.test.ts new file mode 100644 index 000000000..74664a0cf --- /dev/null +++ b/client/src/components/Map/mapZoomCeiling.test.ts @@ -0,0 +1,84 @@ +/** + * The zoom ceiling a Leaflet map needs before a marker cluster will attach. + * + * This is a test about Leaflet's own semantics rather than about our components, + * because that semantic is what a basemap change quietly broke: switching the + * default basemap to a vector style replaced the TileLayer with a GL canvas, the + * ceiling went with it, and MarkerClusterGroup.onAdd threw on the way in. The + * planner and the map settings preview both went to the error boundary, which + * left no screen from which to pick a different basemap. + * + * Nothing is mocked here on purpose. A mocked react-leaflet would have kept + * passing throughout. + */ +import { describe, it, expect, afterEach } from 'vitest' +import L from 'leaflet' +import 'leaflet.markercluster' +import { MAP_MAX_ZOOM } from '../../constants/mapDefaults' + +/** + * A layer that is not a GridLayer, which is what a vector basemap amounts to: + * maplibre-gl-leaflet hands back an L.Layer holding a canvas, so it has no + * beforeAdd and never reaches Leaflet's _addZoomLimit. + */ +const GlCanvasLayer = L.Layer.extend({ onAdd: () => undefined, onRemove: () => undefined }) +function glCanvasLayer(): L.Layer { + return new GlCanvasLayer() as L.Layer +} + +/** + * `leaflet.markercluster` ships no types and we do not depend on + * `@types/leaflet.markercluster`, so the one factory this file needs is named + * here rather than pulling a package in for a single call. + */ +const markerClusterGroup = (L as unknown as { markerClusterGroup: () => L.Layer }).markerClusterGroup + +const containers: HTMLElement[] = [] + +function mapOn(options: L.MapOptions): L.Map { + const el = document.createElement('div') + Object.defineProperty(el, 'clientWidth', { value: 800 }) + Object.defineProperty(el, 'clientHeight', { value: 600 }) + document.body.appendChild(el) + containers.push(el) + return L.map(el, { center: [48.85, 2.35], zoom: 12, ...options }) +} + +afterEach(() => { + for (const el of containers.splice(0)) el.remove() +}) + +describe('the zoom ceiling of a Leaflet map', () => { + it('MAPZOOM-001: a vector basemap contributes none, so the map has none', () => { + const map = mapOn({}) + glCanvasLayer().addTo(map) + // Only a GridLayer feeds Leaflet's _layersMaxZoom, via its beforeAdd hook. + expect(map.getMaxZoom()).toBe(Infinity) + }) + + it('MAPZOOM-002: a raster basemap contributes one, which is why raster maps never broke', () => { + const map = mapOn({}) + L.tileLayer('https://example.test/{z}/{x}/{y}.png', { maxZoom: 19 }).addTo(map) + expect(map.getMaxZoom()).toBe(19) + }) + + it('MAPZOOM-003: a cluster refuses to attach to a map with no ceiling', () => { + const map = mapOn({}) + glCanvasLayer().addTo(map) + // The throw is in onAdd, before any clustering, so an empty trip hits it too. + expect(() => markerClusterGroup().addTo(map)).toThrow(/maxZoom/) + }) + + it('MAPZOOM-004: the ceiling on the map itself carries every basemap kind', () => { + const map = mapOn({ maxZoom: MAP_MAX_ZOOM }) + glCanvasLayer().addTo(map) + expect(map.getMaxZoom()).toBe(MAP_MAX_ZOOM) + expect(() => markerClusterGroup().addTo(map)).not.toThrow() + }) + + it('MAPZOOM-005: it does not lower the raster and satellite layers, which already sat at 19', () => { + const map = mapOn({ maxZoom: MAP_MAX_ZOOM }) + L.tileLayer('https://example.test/{z}/{x}/{y}.png', { maxZoom: 19 }).addTo(map) + expect(map.getMaxZoom()).toBe(19) + }) +}) diff --git a/client/src/constants/mapDefaults.ts b/client/src/constants/mapDefaults.ts index fb0a64ac7..d97731181 100644 --- a/client/src/constants/mapDefaults.ts +++ b/client/src/constants/mapDefaults.ts @@ -3,6 +3,18 @@ export const DEFAULT_MAP_LNG = 0 export const DEFAULT_MAP_ZOOM = 2 export const DEFAULT_MAP_CENTER: [number, number] = [DEFAULT_MAP_LAT, DEFAULT_MAP_LNG] +/** + * Zoom ceiling for a Leaflet map, set on the map rather than on its base layer. + * + * Leaflet answers `getMaxZoom()` from the map options first and only then from a + * layer that carried one, and only a GridLayer ever contributes: a vector + * basemap is a GL canvas, so a map drawn by one has no ceiling from anywhere. + * `MarkerClusterGroup.onAdd` refuses an infinite ceiling by throwing, which is + * how a basemap choice could take down the whole planner. Matches the raster and + * satellite layers so nothing changes for the maps that already had one. + */ +export const MAP_MAX_ZOOM = 19 + // Tokenless satellite base layer (ESRI World Imagery) — works without an API key. export const SATELLITE_TILE_URL = 'https://server.arcgisonline.com/ArcGIS/rest/services/World_Imagery/MapServer/tile/{z}/{y}/{x}' diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index 88fc4b5ab..912fb22ed 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -19,7 +19,7 @@ import { createElement, useEffect, useRef } from 'react'; import { renderIconMarkup } from '../utils/iconMarkup'; import { MapContainer, Marker, Polyline, TileLayer, Tooltip, useMap } from 'react-leaflet'; import { getCategoryIcon } from '../components/shared/categoryIcons'; -import { OFM_POSITRON, DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, attributionForTile } from '../constants/mapDefaults'; +import { OFM_POSITRON, DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, MAP_MAX_ZOOM, attributionForTile } from '../constants/mapDefaults'; import VectorBasemap from '../components/Map/VectorBasemap'; import { SUPPORTED_LANGUAGES, useTranslation } from '../i18n'; import { useSettingsStore } from '../store/settingsStore'; @@ -489,6 +489,9 @@ export default function SharedTripPage() { center={initialView.center} zoom={initialView.zoom} zoomControl={false} + // Same reason as the planner map: a vector basemap contributes + // no zoom ceiling, and fitBounds below asks for one. + maxZoom={MAP_MAX_ZOOM} style={{ width: '100%', height: '100%' }} > {basemap.kind === 'vector' ? (