fix(share): the shared map opens framed on the whole trip, not on a corner of it

The opening camera of the public share map was computed for the browser window, while the map lives in a card a fraction of that size. Leaflet then rounded the fractional zoom up, and the first fit skipped its correction because it believed the map had already opened framed. Only picking a day and going back to the whole trip ran a real fit. The card is now measured before the map mounts and the opening frame is computed for it, on a whole zoom level, with the same padding and zoom cap the later fits use. A card without layout falls back to the first fit. Closes #2549
2026-10-18 21:16:47 +00:00 · 2026-09-29 20:55:00 +02:00
parent ec06035855
commit 5c7aaab3c6
2 changed files with 142 additions and 8 deletions
@@ -0,0 +1,103 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { render } from '@testing-library/react'
+import type { ReactNode } from 'react'
+import { computeMapViewport, TILE_SIZE_RASTER } from '../../utils/mapViewport'
+import { SharedMap, type MapPlace } from './SharedMap'
+
+// The camera the map opened with, and every fit made after it.
+const mounted: { center: [number, number]; zoom: number }[] = []
+const fitBounds = vi.fn()
+
+vi.mock('react-leaflet', () => ({
+  MapContainer: ({ center, zoom, children }: { center: [number, number]; zoom: number; children: ReactNode }) => {
+    mounted.push({ center, zoom })
+    return <div data-testid="map-container">{children}</div>
+  },
+  TileLayer: () => null,
+  Marker: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
+  Polyline: () => null,
+  Tooltip: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
+  useMap: () => ({ fitBounds }),
+}))
+vi.mock('react-leaflet-cluster', () => ({
+  default: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
+}))
+vi.mock('../../components/Map/VectorBasemap', () => ({ default: () => null }))
+
+// Synthetic points: a dense cluster plus a spread over a small region, wider than tall.
+const places: MapPlace[] = [
+  { id: 1, name: 'A', lat: 52.37, lng: 4.89 },
+  { id: 2, name: 'B', lat: 52.372, lng: 4.893 },
+  { id: 3, name: 'C', lat: 52.33, lng: 5.42 },
+  { id: 4, name: 'D', lat: 52.35, lng: 4.3 },
+  { id: 5, name: 'E', lat: 52.4, lng: 4.98 },
+]
+
+const PADDING = { top: 64, right: 48, bottom: 48, left: 48 }
+
+let cardSize = { width: 0, height: 0 }
+const realWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'clientWidth')
+const realHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'clientHeight')
+
+function renderMap(list: MapPlace[] = places) {
+  return render(
+    <SharedMap places={list} line={[]} orderByPlace={{}} days={[]} selectedDay={null} onSelectDay={() => {}} />,
+  )
+}
+
+describe('SharedMap opening frame (#2549)', () => {
+  beforeEach(() => {
+    mounted.length = 0
+    fitBounds.mockClear()
+    // A desktop window far larger than the map card, which is what exposed the bug.
+    vi.stubGlobal('innerWidth', 1920)
+    vi.stubGlobal('innerHeight', 1080)
+    Object.defineProperty(HTMLElement.prototype, 'clientWidth', { configurable: true, get: () => cardSize.width })
+    Object.defineProperty(HTMLElement.prototype, 'clientHeight', { configurable: true, get: () => cardSize.height })
+  })
+
+  afterEach(() => {
+    vi.unstubAllGlobals()
+    if (realWidth) Object.defineProperty(HTMLElement.prototype, 'clientWidth', realWidth)
+    if (realHeight) Object.defineProperty(HTMLElement.prototype, 'clientHeight', realHeight)
+  })
+
+  it('FE-SHARED-MAP-001: frames the places for the card it sits in, not for the browser window', () => {
+    cardSize = { width: 750, height: 338 }
+    renderMap()
+
+    const forCard = computeMapViewport(places, { tileSize: TILE_SIZE_RASTER, padding: PADDING, maxZoom: 14, width: 750, height: 338 })!
+    const forWindow = computeMapViewport(places, { tileSize: TILE_SIZE_RASTER, padding: PADDING, maxZoom: 14 })!
+    expect(mounted).toHaveLength(1)
+    expect(mounted[0].zoom).toBe(Math.floor(forCard.zoom))
+    expect(mounted[0].center).toEqual(forCard.center)
+    // The window-sized frame is the over-zoomed one the issue saw.
+    expect(mounted[0].zoom).toBeLessThan(Math.floor(forWindow.zoom))
+    // Opened framed, so there is nothing for the first fit to correct.
+    expect(fitBounds).not.toHaveBeenCalled()
+  })
+
+  it('FE-SHARED-MAP-002: opens on a whole zoom level, as fitBounds does', () => {
+    cardSize = { width: 750, height: 338 }
+    renderMap()
+    expect(Number.isInteger(mounted[0].zoom)).toBe(true)
+  })
+
+  it('FE-SHARED-MAP-003: a card without layout lets the first fit do the framing', () => {
+    cardSize = { width: 0, height: 0 }
+    renderMap()
+    expect(fitBounds).toHaveBeenCalledTimes(1)
+    expect(fitBounds.mock.calls[0][1]).toEqual({ paddingTopLeft: [48, 64], paddingBottomRight: [48, 48], maxZoom: 14 })
+  })
+
+  it('FE-SHARED-MAP-004: a new set of places (a picked day) still refits with the same frame', () => {
+    cardSize = { width: 750, height: 338 }
+    const { rerender } = renderMap()
+    expect(fitBounds).not.toHaveBeenCalled()
+    rerender(
+      <SharedMap places={places.slice(0, 2)} line={[]} orderByPlace={{}} days={[]} selectedDay={1} onSelectDay={() => {}} />,
+    )
+    expect(fitBounds).toHaveBeenCalledTimes(1)
+    expect(fitBounds.mock.calls[0][1]).toEqual({ paddingTopLeft: [48, 64], paddingBottomRight: [48, 48], maxZoom: 14 })
+  })
+})
@@ -1,5 +1,5 @@
 import L from 'leaflet'
-import { createElement, useEffect, useRef, type ReactNode } from 'react'
+import { createElement, useEffect, useLayoutEffect, useRef, useState, type ReactNode } from 'react'
 import { ChevronLeft, ChevronRight } from 'lucide-react'
 import { MapContainer, Marker, Polyline, TileLayer, Tooltip, useMap } from 'react-leaflet'
 import MarkerClusterGroup from 'react-leaflet-cluster'
@@ -55,6 +55,12 @@ function createMarkerIcon(place: MapPlace, orderNumbers?: number[] | null) {
  })
 }

+// One frame for both the opening camera and every later fit, so the map opens on
+// exactly the view that picking a day and going back to the whole trip produces.
+// The top edge leaves room for the day picker floating over the map.
+const FRAME_PADDING = { top: 64, right: 48, bottom: 48, left: 48 }
+const FRAME_MAX_ZOOM = 14
+
 function FitBoundsToPlaces({ places, framedOnMount }: { places: MapPlace[]; framedOnMount: boolean }) {
  const map = useMap()
  const fitRan = useRef(false)
@@ -72,7 +78,11 @@ function FitBoundsToPlaces({ places, framedOnMount }: { places: MapPlace[]; fram
    }
    fitRan.current = true
    const bounds = L.latLngBounds(places.map(p => [p.lat, p.lng]))
-    map.fitBounds(bounds, { padding: [48, 48], maxZoom: 14 })
+    map.fitBounds(bounds, {
+      paddingTopLeft: [FRAME_PADDING.left, FRAME_PADDING.top],
+      paddingBottomRight: [FRAME_PADDING.right, FRAME_PADDING.bottom],
+      maxZoom: FRAME_MAX_ZOOM,
+    })
  }, [fitKey, map]) // eslint-disable-line react-hooks/exhaustive-deps
  return null
 }
@@ -94,10 +104,31 @@ interface SharedMapProps {
 * map and the list can never disagree on which day is open.
 */
 export function SharedMap({ places, line, orderByPlace, cartoApiKey, days, selectedDay, onSelectDay }: SharedMapProps) {
+  // The card's own size, read before the map mounts. Framing for the browser window
+  // instead zoomed a card a fraction of its size far too close, and the fit below
+  // then skipped the correction because it believed the map had opened framed (#2549).
+  const cardRef = useRef<HTMLDivElement>(null)
+  const [card, setCard] = useState<{ width: number; height: number } | null>(null)
+  useLayoutEffect(() => {
+    const el = cardRef.current
+    if (el) setCard({ width: el.clientWidth, height: el.clientHeight })
+  }, [])
+  // A card with no layout yet (hidden, or a test DOM) has nothing to frame against,
+  // so the map opens near the places and the first fit does the real framing.
+  const measured = card !== null && card.width > 0 && card.height > 0
  // Open framed on the trip's places instead of on Paris. MapContainer only reads
  // center/zoom at mount, so recomputing this per render is free.
-  const framed = computeMapViewport(places, { tileSize: TILE_SIZE_RASTER, padding: { top: 64, right: 48, bottom: 48, left: 48 } })
-  const initialView = framed ?? { center: DEFAULT_MAP_CENTER, zoom: DEFAULT_MAP_ZOOM }
+  const framed = computeMapViewport(places, {
+    tileSize: TILE_SIZE_RASTER,
+    padding: FRAME_PADDING,
+    maxZoom: FRAME_MAX_ZOOM,
+    ...(measured ? { width: card.width, height: card.height } : {}),
+  })
+  // Whole levels, as fitBounds settles on: Leaflet rounds a fractional opening zoom
+  // to the nearest level, and rounding up crops the outermost places.
+  const initialView = framed
+    ? { center: framed.center, zoom: Math.floor(framed.zoom) }
+    : { center: DEFAULT_MAP_CENTER, zoom: DEFAULT_MAP_ZOOM }
  // A visitor of a share link has no settings of their own, so the basemap is the
  // app default: OpenFreeMap, a vector style that needs no key at all. The owner's
  // CARTO key still travels in the payload and is still applied, because a raster
@@ -107,8 +138,8 @@ export function SharedMap({ places, line, orderByPlace, cartoApiKey, days, selec

  return (
    // `isolate` keeps Leaflet's pane z-indexes inside the card, under the sticky tab bar.
-    <div className="relative isolate h-full w-full overflow-hidden rounded-2xl border border-edge-faint bg-surface-tertiary shadow-sm">
-      <MapContainer
+    <div ref={cardRef} className="relative isolate h-full w-full overflow-hidden rounded-2xl border border-edge-faint bg-surface-tertiary shadow-sm">
+      {card && <MapContainer
        center={initialView.center}
        zoom={initialView.zoom}
        zoomControl={false}
@@ -122,7 +153,7 @@ export function SharedMap({ places, line, orderByPlace, cartoApiKey, days, selec
        ) : (
          <TileLayer url={basemap.url} attribution={attributionForTile(basemap.url)} referrerPolicy="strict-origin-when-cross-origin" />
        )}
-        <FitBoundsToPlaces places={places} framedOnMount={framed !== null} />
+        <FitBoundsToPlaces places={places} framedOnMount={measured && framed !== null} />
        {selectedDay != null && line.length > 1 && (
          <Polyline
            positions={line.map(p => [p.lat, p.lng])}
@@ -142,7 +173,7 @@ export function SharedMap({ places, line, orderByPlace, cartoApiKey, days, selec
            </Marker>
          ))}
        </MarkerClusterGroup>
-      </MapContainer>
+      </MapContainer>}

      {days.length > 0 && <DayPicker days={days} selectedDay={selectedDay} onSelectDay={onSelectDay} />}
    </div>