diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index 276d2c049..5f8390409 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -4,6 +4,7 @@ import { renderIconMarkup } from '../../utils/iconMarkup' import { MapContainer, TileLayer, Marker, Polyline, CircleMarker, Circle, useMap, Tooltip } from 'react-leaflet' import MarkerClusterGroup from 'react-leaflet-cluster' import { makeMarkerDraggable, makePoiDraggable, draggedPoiId } from './markerDrag' +import { CLUSTER_OPTIONS, createClusterIcon } from './markerCluster' import RoadtripViaMarkers from './RoadtripViaMarkers' import HazardLayers from './HazardLayers' import { ALT_CASING, ALT_LABEL_TEXT } from '../Roadtrip/alternativeColors' @@ -868,16 +869,6 @@ export const MapView = memo(function MapView({ } }, [placeIds, placesPhotosEnabled]) - const clusterIconCreateFunction = useCallback((cluster) => { - const count = cluster.getChildCount() - const size = count < 10 ? 36 : count < 50 ? 42 : 48 - return L.divIcon({ - html: `
${count}
`, - className: 'marker-cluster-wrapper', - iconSize: L.point(size, size), - }) - }, []) - const isTouchDevice = typeof window !== 'undefined' && navigator.maxTouchPoints > 0 // Drag a marker onto a day (#891). Pointer-driven, so it is off wherever // HTML5 drag does not exist — and the day plan is not on screen there anyway. @@ -1061,18 +1052,7 @@ export const MapView = memo(function MapView({ - + {markers} diff --git a/client/src/components/Map/markerCluster.ts b/client/src/components/Map/markerCluster.ts new file mode 100644 index 000000000..b831b8b39 --- /dev/null +++ b/client/src/components/Map/markerCluster.ts @@ -0,0 +1,44 @@ +import L from 'leaflet' + +/** + * The cluster group every place map is drawn with. + * + * The planner and the public share page draw the same trip, so they have to + * cluster it the same way: a shared link that opens on three dozen overlapping + * pins while the planner shows four tidy bubbles is one trip told two different + * ways. Both maps build their group from this block and this factory (#2343). + */ +export const CLUSTER_OPTIONS = { + chunkedLoading: true, + chunkInterval: 30, + chunkDelay: 0, + // Pixels at the current zoom, so ordinary stops still come apart by zooming in. + maxClusterRadius: 20, + disableClusteringAtZoom: 9, + spiderfyOnMaxZoom: true, + showCoverageOnHover: false, + zoomToBoundsOnClick: true, + animate: false, +} + +/** The part of a Leaflet cluster this factory reads. */ +export interface ClusterLike { + getChildCount: () => number +} + +/** + * A count bubble, sized by what it stands for. + * + * Markup rather than a component because Leaflet owns this element: + * `L.divIcon`'s html lands in the marker pane, outside React's tree — which is + * also why its styling lives in index.css (`.marker-cluster-custom`). + */ +export function createClusterIcon(cluster: ClusterLike) { + const count = cluster.getChildCount() + const size = count < 10 ? 36 : count < 50 ? 42 : 48 + return L.divIcon({ + html: `
${count}
`, + className: 'marker-cluster-wrapper', + iconSize: L.point(size, size), + }) +} diff --git a/client/src/pages/SharedTripPage.test.tsx b/client/src/pages/SharedTripPage.test.tsx index 68efc409a..30bf9db74 100644 --- a/client/src/pages/SharedTripPage.test.tsx +++ b/client/src/pages/SharedTripPage.test.tsx @@ -24,6 +24,16 @@ vi.mock('react-leaflet', () => ({ }), })); +// The markers now reach the map through a cluster group (#2343). The real wrapper +// asks React Leaflet's context for the map it belongs to, and the stubbed +// MapContainer above provides none — so it is stubbed for the same reason +// everything else here is. +vi.mock('react-leaflet-cluster', () => ({ + default: ({ children }: { children?: React.ReactNode }) => ( +
{children}
+ ), +})); + // The basemap is a MapLibre style now, and the real component reaches for // maplibre-gl through a dynamic import. The page test only cares that it is the // thing being rendered. diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index c9dadaf61..ba00b68ad 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -18,6 +18,13 @@ import { import { createElement, useEffect, useRef } from 'react'; import { renderIconMarkup } from '../utils/iconMarkup'; import { MapContainer, Marker, Polyline, TileLayer, Tooltip, useMap } from 'react-leaflet'; +import MarkerClusterGroup from 'react-leaflet-cluster'; +// leaflet.markercluster's own sheets travel with the planner's MapView, which this +// page never mounts: without them the bubbles keep the library's default styling +// and spiderfy's legs are invisible. +import 'leaflet.markercluster/dist/MarkerCluster.css'; +import 'leaflet.markercluster/dist/MarkerCluster.Default.css'; +import { CLUSTER_OPTIONS, createClusterIcon } from '../components/Map/markerCluster'; import { getCategoryIcon } from '../components/shared/categoryIcons'; import PublicLanguagePicker from '../components/shared/PublicLanguagePicker'; import { OFM_POSITRON, DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, MAP_MAX_ZOOM, attributionForTile } from '../constants/mapDefaults'; @@ -454,11 +461,17 @@ export default function SharedTripPage() { interactive={false} /> )} - {mapPlaces.map((p: any) => ( - - {p.name} - - ))} + {/* Clustered like the planner's map (#2343). A shared link draws the + whole trip at once on a 300px-tall map, where a stop a few blocks + from the next one is the same few pixels: unclustered, the pins + cover each other and the ones underneath cannot be tapped. */} + + {mapPlaces.map((p: any) => ( + + {p.name} + + ))} + diff --git a/client/tests/unit/pages/SharedTripPage.map.test.tsx b/client/tests/unit/pages/SharedTripPage.map.test.tsx new file mode 100644 index 000000000..83a213d14 --- /dev/null +++ b/client/tests/unit/pages/SharedTripPage.map.test.tsx @@ -0,0 +1,138 @@ +/** + * The share page's map is the planner's map (#2343). + * + * A shared link is the only view a guest ever gets — no account, no zoom habits, + * often a phone — and it draws the whole trip at once on a 300px-tall canvas. + * Unclustered, three dozen pins pile onto each other there and the ones + * underneath can be neither read nor tapped, while the planner shows the same + * trip as a handful of tidy bubbles. + * + * The data hooks and the Leaflet components are stubbed: what is under test is + * the page's own wiring — that every stop reaches a cluster group built from the + * shared option block — not Leaflet's rendering, which jsdom cannot do anyway. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { render, screen, within } from '../../helpers/render'; +import { CLUSTER_OPTIONS, createClusterIcon } from '../../../src/components/Map/markerCluster'; +import SharedTripPage from '../../../src/pages/SharedTripPage'; + +const mocks = vi.hoisted(() => ({ + hook: {} as Record, + clusterProps: [] as Array>, +})); + +vi.mock('../../../src/pages/sharedTrip/useSharedTrip', () => ({ + useSharedTrip: () => mocks.hook, +})); + +vi.mock('react-leaflet', () => ({ + MapContainer: ({ children }: { children?: React.ReactNode }) =>
{children}
, + TileLayer: () => null, + Polyline: () => null, + Tooltip: ({ children }: { children?: React.ReactNode }) => {children}, + Marker: ({ children }: { children?: React.ReactNode }) =>
{children}
, + useMap: () => ({ fitBounds: vi.fn(), on: vi.fn(), off: vi.fn() }), +})); + +vi.mock('react-leaflet-cluster', () => ({ + default: (props: { children?: React.ReactNode }) => { + mocks.clusterProps.push(props as unknown as Record); + return
{props.children}
; + }, +})); + +// Its effect dynamically imports maplibre-gl, which has no business in jsdom. +vi.mock('../../../src/components/Map/VectorBasemap', () => ({ default: () => null })); + +const place = (id: number, name: string, lat: number, lng: number) => ({ + id, + name, + lat, + lng, + category_color: '#6366f1', + category_icon: 'map-pin', +}); + +function payload(places: Array>) { + return { + trip: { id: 1, title: 'Autumn', start_date: '2026-09-25', end_date: '2026-09-27', currency: 'EUR' }, + days: [{ id: 5, day_number: 1, date: '2026-09-25', title: 'Day one' }], + assignments: {}, + dayNotes: {}, + places, + reservations: [], + accommodations: [], + packing: [], + budget: [], + categories: [], + permissions: { share_map: true, share_bookings: true, share_packing: true, share_budget: true }, + collab: null, + cartoApiKey: null, + }; +} + +function hookState(data: unknown) { + return { + data, + error: false, + base: 'EUR', + convert: (n: number) => n, + selectedDay: null, + setSelectedDay: vi.fn(), + activeTab: 'plan', + setActiveTab: vi.fn(), + showLangPicker: false, + setShowLangPicker: vi.fn(), + }; +} + +beforeEach(() => { + mocks.clusterProps.length = 0; +}); + +describe('the map on a public share page', () => { + it('SHAREMAP-001: every stop is handed to a cluster group, not the raw map', () => { + mocks.hook = hookState( + payload([place(11, 'The Bund', 31.24, 121.49), place(12, 'Peace Hotel', 31.2397, 121.4903)]), + ); + render(); + + const cluster = screen.getByTestId('cluster-group'); + expect(within(cluster).getAllByTestId('marker')).toHaveLength(2); + // And nothing escaped it: the map holds no bare markers of its own. + expect(within(screen.getByTestId('map')).getAllByTestId('marker')).toHaveLength(2); + }); + + it('SHAREMAP-002: the group is built from the shared options, so both maps cluster alike', () => { + mocks.hook = hookState(payload([place(11, 'The Bund', 31.24, 121.49)])); + render(); + + expect(mocks.clusterProps).toHaveLength(1); + const props = mocks.clusterProps[0]; + for (const [key, value] of Object.entries(CLUSTER_OPTIONS)) { + expect(props[key], key + ' must reach the cluster group').toBe(value); + } + expect(typeof props.iconCreateFunction).toBe('function'); + }); + + it('SHAREMAP-004: the bubble shows the count, in the size bucket that count earns', () => { + const small = createClusterIcon({ getChildCount: () => 7 }); + const medium = createClusterIcon({ getChildCount: () => 30 }); + const large = createClusterIcon({ getChildCount: () => 60 }); + + expect(small.options.className).toBe('marker-cluster-wrapper'); + expect(small.options.html).toContain('7'); + expect([small, medium, large].map((icon) => icon.options.iconSize?.x)).toEqual([36, 42, 48]); + expect(small.options.html).toContain('width:36px;height:36px'); + }); + + it('SHAREMAP-003: a place without coordinates is left out of the map, as before', () => { + mocks.hook = hookState( + payload([place(11, 'The Bund', 31.24, 121.49), { id: 12, name: 'Geocoding failed' } as never]), + ); + render(); + + expect(within(screen.getByTestId('cluster-group')).getAllByTestId('marker')).toHaveLength(1); + }); +});