Merge pull request #2346 from WenyinWei/fix/share-map-clustering

fix(shared): cluster the public share page's markers

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'
@@ -876,16 +877,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.
@@ -1077,18 +1068,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>
 

client/src/components/Map/markerCluster.ts

@@ -0,0 +1,34 @@
+import L from 'leaflet'
+
+/**
+ * The cluster group the planner and the public share page both draw places with,
+ * so a shared link clusters a trip the way the planner does (#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 its count, styled by `.marker-cluster-custom` in index.css. */
+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),
+  })
+}

client/src/pages/SharedTripPage.test.tsx

@@ -24,6 +24,13 @@ vi.mock('react-leaflet', () => ({
   }),
 }));
 
+// The real cluster group needs the map context the stubbed MapContainer lacks (#2343).
+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.

client/src/pages/SharedTripPage.tsx

@@ -18,6 +18,11 @@ 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';
+// MapView brings these sheets for the planner, and this page never mounts it.
+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';
@@ -457,11 +462,14 @@ 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, so nearby stops stay tappable (#2343). */}
+                <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>
 

client/tests/unit/pages/SharedTripPage.map.test.tsx

@@ -0,0 +1,130 @@
+/**
+ * The share page clusters its markers like the planner (#2343). The data hook and
+ * Leaflet are stubbed: under test is the page's wiring, not Leaflet's rendering.
+ */
+import React from 'react';
+import L from 'leaflet';
+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('FE-PAGE-SHARED-MAP-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('FE-PAGE-SHARED-MAP-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('FE-PAGE-SHARED-MAP-003: 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)).toEqual([L.point(36, 36), L.point(42, 42), L.point(48, 48)]);
+    expect(small.options.html).toContain('width:36px;height:36px');
+  });
+
+  it('FE-PAGE-SHARED-MAP-004: 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);
+  });
+});