mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(shared): cluster the public share page's markers
The share page drew every stop as its own pin while the planner drew the same trip as a handful of clusters, so a shared link opened on three dozen overlapping markers. On a 300px-tall map that covers a whole city, the pins underneath can be neither read nor tapped (#2343). The option block and the icon factory move to components/Map/markerCluster.ts, which both maps import, so the two cannot drift apart again. The planner's options are unchanged. leaflet.markercluster's two stylesheets were imported by MapView.tsx alone. The share page never mounts MapView, so it imports them itself: without them the bubbles keep the library's default styling and spiderfy's legs are invisible.
This commit is contained in:
@@ -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: `<div class="marker-cluster-custom" style="width:${size}px;height:${size}px;"><span>${count}</span></div>`,
|
||||
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({
|
||||
<PoiDropTarget onPoiDropOnRoute={onPoiDropOnRoute} />
|
||||
<LeafletLocationLayer position={userPosition} mode={trackingMode} />
|
||||
|
||||
<MarkerClusterGroup
|
||||
chunkedLoading
|
||||
chunkInterval={30}
|
||||
chunkDelay={0}
|
||||
maxClusterRadius={20}
|
||||
disableClusteringAtZoom={9}
|
||||
spiderfyOnMaxZoom
|
||||
showCoverageOnHover={false}
|
||||
zoomToBoundsOnClick
|
||||
animate={false}
|
||||
iconCreateFunction={clusterIconCreateFunction}
|
||||
>
|
||||
<MarkerClusterGroup {...CLUSTER_OPTIONS} iconCreateFunction={createClusterIcon}>
|
||||
{markers}
|
||||
</MarkerClusterGroup>
|
||||
|
||||
|
||||
@@ -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: `<div class="marker-cluster-custom" style="width:${size}px;height:${size}px;"><span>${count}</span></div>`,
|
||||
className: 'marker-cluster-wrapper',
|
||||
iconSize: L.point(size, size),
|
||||
})
|
||||
}
|
||||
@@ -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 }) => (
|
||||
<div data-testid="marker-cluster-group">{children}</div>
|
||||
),
|
||||
}));
|
||||
|
||||
// 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.
|
||||
|
||||
@@ -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) => (
|
||||
<Marker key={p.id} position={[p.lat, p.lng]} icon={createMarkerIcon(p, dayOrderMap[p.id] ?? null)}>
|
||||
<Tooltip>{p.name}</Tooltip>
|
||||
</Marker>
|
||||
))}
|
||||
{/* 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. */}
|
||||
<MarkerClusterGroup {...CLUSTER_OPTIONS} iconCreateFunction={createClusterIcon}>
|
||||
{mapPlaces.map((p: any) => (
|
||||
<Marker key={p.id} position={[p.lat, p.lng]} icon={createMarkerIcon(p, dayOrderMap[p.id] ?? null)}>
|
||||
<Tooltip>{p.name}</Tooltip>
|
||||
</Marker>
|
||||
))}
|
||||
</MarkerClusterGroup>
|
||||
</MapContainer>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -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<string, unknown>,
|
||||
clusterProps: [] as Array<Record<string, unknown>>,
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/pages/sharedTrip/useSharedTrip', () => ({
|
||||
useSharedTrip: () => mocks.hook,
|
||||
}));
|
||||
|
||||
vi.mock('react-leaflet', () => ({
|
||||
MapContainer: ({ children }: { children?: React.ReactNode }) => <div data-testid="map">{children}</div>,
|
||||
TileLayer: () => null,
|
||||
Polyline: () => null,
|
||||
Tooltip: ({ children }: { children?: React.ReactNode }) => <span>{children}</span>,
|
||||
Marker: ({ children }: { children?: React.ReactNode }) => <div data-testid="marker">{children}</div>,
|
||||
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<string, unknown>);
|
||||
return <div data-testid="cluster-group">{props.children}</div>;
|
||||
},
|
||||
}));
|
||||
|
||||
// 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<Record<string, unknown>>) {
|
||||
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(<SharedTripPage />);
|
||||
|
||||
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(<SharedTripPage />);
|
||||
|
||||
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('<span>7</span>');
|
||||
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(<SharedTripPage />);
|
||||
|
||||
expect(within(screen.getByTestId('cluster-group')).getAllByTestId('marker')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user