mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 13:36:48 +00:00
fix(map): give the Leaflet map its own zoom ceiling, so a vector basemap cannot take the planner down
Opening a trip on 4.1.0 could land on the error boundary with "Map has no maxZoom specified", and so could the map settings tab, which left no screen from which to pick a different basemap. Leaflet answers getMaxZoom() from the map options first and otherwise from a layer that brought one, and only a GridLayer ever contributes via its beforeAdd hook. A vector basemap is a GL canvas on a plain L.Layer, so it contributes nothing, and MapContainer set no ceiling of its own. MarkerClusterGroup.onAdd refuses an infinite one by throwing, before any clustering, so an empty trip hit it too. That path opened when the default basemap became a MapLibre style. It reaches further than an operator choosing OpenFreeMap: resolveTileUrl sends a keyless CARTO template to the app default, which is now a vector style, so every instance still carrying the old CARTO basemap fell into it on upgrade. The ceiling belongs on the map rather than on the base layer, because it has to hold whichever of the three branches renders. 19 matches what the raster and satellite layers already carried, so nothing changes for the maps that worked. SharedTripPage gets the same treatment: it has no cluster to throw, but it draws the same vector basemap and its fitBounds asks for a ceiling. mapZoomCeiling.test.ts pins Leaflet's own semantics against the real library rather than a mock, since a mocked react-leaflet would have kept passing through all of this.
This commit is contained in:
@@ -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<string, (thumb: string) => 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) => (
|
||||
<div data-testid="map-container" data-center={JSON.stringify(center)} data-zoom={zoom}>{children}</div>
|
||||
// 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) => (
|
||||
<div data-testid="map-container" data-center={JSON.stringify(center)} data-zoom={zoom} data-maxzoom={maxZoom}>{children}</div>
|
||||
),
|
||||
TileLayer: () => <div data-testid="tile-layer" />,
|
||||
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(<MapView places={[buildMapPlace({ lat: 48.8584, lng: 2.2945 })]} />)
|
||||
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(<MapView route={route} />)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user