diff --git a/client/scripts/rtl-disable-baseline.json b/client/scripts/rtl-disable-baseline.json index 92a17010c..253de175d 100644 --- a/client/scripts/rtl-disable-baseline.json +++ b/client/scripts/rtl-disable-baseline.json @@ -1,4 +1,5 @@ { + "components/Collections/CollectionMap.tsx": 1, "components/Studio/LineHandles.tsx": 1, "components/Studio/StudioSelectionChrome.tsx": 2, "components/Studio/inspector/FocalPad.tsx": 1, diff --git a/client/scripts/size-baseline.json b/client/scripts/size-baseline.json index b2fc1b963..839be15bd 100644 --- a/client/scripts/size-baseline.json +++ b/client/scripts/size-baseline.json @@ -5,7 +5,7 @@ "src/components/Admin/AdminPluginsPanel.tsx": 1667, "src/components/Budget/CostsPanel.test.tsx": 2736, "src/components/Budget/CostsPanel.tsx": 1798, - "src/components/Map/MapView.tsx": 1399, + "src/components/Map/MapView.tsx": 1385, "src/components/Map/MapViewGL.test.tsx": 2919, "src/components/Map/MapViewGL.tsx": 2290, "src/components/Map/RouteCalculator.ts": 1076, diff --git a/client/src/components/Collections/CollectionMap.test.tsx b/client/src/components/Collections/CollectionMap.test.tsx new file mode 100644 index 000000000..bfc230529 --- /dev/null +++ b/client/src/components/Collections/CollectionMap.test.tsx @@ -0,0 +1,260 @@ +/** + * The collection map frames every place of the list in the box it is given (FE-COMP-COLMAP-*). + * + * Real Leaflet on purpose, as in MapView.leaflet.test.tsx. The places went missing between + * two sizes: the renderer opened on a camera framed for the browser window, while this map + * is a box a fraction of that, and a mocked map has neither. The window here is a desktop + * of 1600 x 1000 and the map the 567 x 724 split it gets beside the list, where the demo's + * cafes opened on northern France with one pin in sight. + */ +import type { CollectionPlace } from '@trek/shared'; +import L from 'leaflet'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, render, waitFor } from '../../../tests/helpers/render'; +import { resetAllStores } from '../../../tests/helpers/store'; +import CollectionMap from './CollectionMap'; + +vi.mock('../../hooks/useGeolocation', () => ({ + useGeolocation: () => ({ + position: null, + mode: 'off', + error: null, + errorCode: null, + cycleMode: vi.fn(), + setMode: vi.fn(), + }), +})); + +vi.mock('../Ferratas/FerrataLayerLeaflet', () => ({ FerrataMapLayerLeaflet: () => null })); + +// A raster template keeps the basemap a plain TileLayer: the vector default needs WebGL. +vi.mock('../../hooks/useTileUrl', () => ({ useTileUrl: () => 'https://tiles.example.test/{z}/{x}/{y}.png' })); + +vi.mock('../../services/photoService', () => ({ + getCached: vi.fn(() => null), + isLoading: vi.fn(() => false), + fetchPhoto: vi.fn(), + onThumbReady: vi.fn(() => () => {}), + getAllThumbs: vi.fn(() => ({})), +})); + +// Every map this file builds, newest last. +const maps: L.Map[] = []; +L.Map.addInitHook(function (this: L.Map) { + maps.push(this); +}); + +/** Half a selected pin (44 px): a point this far inside an edge keeps its whole pin on the map. */ +const PIN = 22; +/** The desktop panel's control bar ends 52 px below the map's top edge. */ +const BAR = 52; +/** The satellite and locate buttons fill the bottom 54 px of the map's corners. */ +const CONTROLS = 54; + +// Leaflet sizes the map from its container, which jsdom lays out at 0 x 0. +const box = { width: 567, height: 724 }; +const sized = ['clientWidth', 'clientHeight'] as const; +const originals = new Map(); +const startWindow = { width: window.innerWidth, height: window.innerHeight }; + +function setWindow(width: number, height: number) { + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: width }); + Object.defineProperty(window, 'innerHeight', { configurable: true, writable: true, value: height }); +} + +beforeAll(() => { + for (const key of sized) { + originals.set(key, Object.getOwnPropertyDescriptor(HTMLElement.prototype, key)); + Object.defineProperty(HTMLElement.prototype, key, { + configurable: true, + get: () => (key === 'clientWidth' ? box.width : box.height), + }); + } +}); +afterAll(() => { + for (const key of sized) { + const original = originals.get(key); + if (original) Object.defineProperty(HTMLElement.prototype, key, original); + else delete (HTMLElement.prototype as unknown as Record)[key]; + } +}); + +beforeEach(() => { + // The desktop split beside the list, unless a test says otherwise. + setWindow(1600, 1000); + Object.assign(box, { width: 567, height: 724 }); +}); +afterEach(() => { + setWindow(startWindow.width, startWindow.height); + resetAllStores(); +}); + +function place(id: number, name: string, lat: number, lng: number): CollectionPlace { + return { id, collection_id: 1, name, lat, lng, status: 'idea' }; +} + +// The demo's cafes: Vienna twice, Trieste, Rome, Paris, Lisbon, Porto and Copenhagen. +const CAFES = [ + place(1, 'Cafe Central', 48.2105, 16.366), + place(2, 'Cafe Sperl', 48.1999, 16.3634), + place(3, 'Caffe San Marco', 45.6513, 13.777), + place(4, "Sant'Eustachio il Caffe", 41.8984, 12.4753), + place(5, 'Cafe de Flore', 48.8542, 2.3331), + place(6, 'A Brasileira', 38.7102, -9.1417), + place(7, 'Cafe Majestic', 41.1472, -8.6064), + place(8, 'The Coffee Collective', 55.696, 12.5509), +]; + +// The demo's viewpoints, from the Norwegian fjords to the Cinque Terre. +const VIEWPOINTS = [ + place(11, 'Seceda', 46.6046, 11.722), + place(12, 'Preikestolen', 58.9863, 6.1904), + place(13, 'Trolltunga', 60.1241, 6.74), + place(14, 'Rigi Kulm', 47.056, 8.485), + place(15, 'Zugspitze', 47.4211, 10.9852), + place(16, 'Eibsee loop', 47.4575, 10.9918), + place(17, 'Hallstatt overlook', 47.5622, 13.6493), + place(18, 'Monterosso al Mare', 44.1456, 9.654), +]; + +// Temples and shrines across Japan: eight as well, and far outside the cafes' frame. +const TEMPLES = [ + place(31, 'Fushimi Inari', 34.9671, 135.7727), + place(32, 'Kiyomizu-dera', 34.9949, 135.785), + place(33, 'Senso-ji', 35.7148, 139.7967), + place(34, 'Chureito Pagoda', 35.5014, 138.8016), + place(35, 'Itsukushima', 34.2959, 132.3199), + place(36, 'Himeji Castle', 34.8394, 134.6939), + place(37, 'Shirakawa-go', 36.2578, 136.9066), + place(38, 'Todai-ji', 34.689, 135.8398), +]; + +// "All saved" across the globe, from Patagonia to Taiwan. +const WORLD = [ + place(21, 'Torres del Paine', -50.942, -73.4068), + place(22, 'Pico do Arieiro', 32.7356, -16.9281), + place(23, 'Landmannalaugar', 63.9926, -19.0607), + place(24, 'Taroko Gorge', 24.1594, 121.6211), + place(25, 'Lycian Way', 36.5497, 29.1153), + ...VIEWPOINTS.slice(0, 3), +]; + +function mapOf(places: CollectionPlace[], underBar: boolean) { + return ( + + ); +} + +async function renderMap(places: CollectionPlace[], underBar = true) { + const view = render(mapOf(places, underBar)); + await waitFor(() => expect(view.container.querySelector('.leaflet-container')).not.toBeNull()); + return { view, map: maps[maps.length - 1] }; +} + +/** Where each place sits on the map, in pixels from its top left corner. */ +function pointsOf(map: L.Map, places: CollectionPlace[]): L.Point[] { + return places.map((p) => map.latLngToContainerPoint([p.lat ?? 0, p.lng ?? 0])); +} + +/** Every pin whole on the map, below the bar when there is one, and above the bottom controls. */ +function expectEveryPinClear(map: L.Map, places: CollectionPlace[], top: number) { + for (const { x, y } of pointsOf(map, places)) { + expect(x).toBeGreaterThanOrEqual(PIN); + expect(x).toBeLessThanOrEqual(box.width - PIN); + expect(y).toBeGreaterThanOrEqual(top + PIN); + expect(y).toBeLessThanOrEqual(box.height - CONTROLS - PIN); + } +} + +/** Whether an element around the map carries the class. */ +function wrappedIn(map: L.Map, className: string): boolean { + for (let el = map.getContainer().parentElement; el; el = el.parentElement) { + if (el.classList.contains(className)) return true; + } + return false; +} + +/** The places fill a good part of the free box along one axis: framed, not shown from orbit. */ +function expectFramedClosely(map: L.Map, places: CollectionPlace[], top: number) { + const points = pointsOf(map, places); + const spanX = Math.max(...points.map((p) => p.x)) - Math.min(...points.map((p) => p.x)); + const spanY = Math.max(...points.map((p) => p.y)) - Math.min(...points.map((p) => p.y)); + const freeX = box.width - 2 * PIN; + const freeY = box.height - top - CONTROLS - 2 * PIN; + expect(Math.max(spanX / freeX, spanY / freeY)).toBeGreaterThan(0.4); +} + +describe('CollectionMap framing', () => { + it('FE-COMP-COLMAP-001: the desktop split opens with every cafe on screen, clear of the edges and the bar', async () => { + const { map } = await renderMap(CAFES); + + expectEveryPinClear(map, CAFES, BAR); + expectFramedClosely(map, CAFES, BAR); + }); + + it('FE-COMP-COLMAP-002: another list of the same length frames again', async () => { + const { view, map } = await renderMap(CAFES); + + // Both lists hold eight places, so a fit keyed on the count never noticed the switch. + // The temples lie far east of the cafes' frame, so their pins come clear only by a new fit. + expect(pointsOf(map, TEMPLES).every((p) => p.x > box.width)).toBe(true); + view.rerender(mapOf(TEMPLES, true)); + + await waitFor(() => expectEveryPinClear(map, TEMPLES, BAR)); + expectFramedClosely(map, TEMPLES, BAR); + }); + + it('FE-COMP-COLMAP-003: a map the user moved stays put through a reload, a new order or an edited place', async () => { + const { view, map } = await renderMap(CAFES); + act(() => { + map.setView([50, 10], 7, { animate: false }); + }); + + const reloaded = [...CAFES].reverse().map((p) => ({ ...p, status: 'visited' as const })); + view.rerender(mapOf(reloaded, true)); + + expect(map.getZoom()).toBe(7); + expect(map.getCenter().lat).toBeCloseTo(50, 5); + expect(map.getCenter().lng).toBeCloseTo(10, 5); + }); + + it('FE-COMP-COLMAP-004: a filter that hides every place leaves the map where it was', async () => { + const { view, map } = await renderMap(CAFES); + const zoom = map.getZoom(); + const center = map.getCenter(); + + view.rerender(mapOf([], true)); + + expect(map.getZoom()).toBe(zoom); + expect(map.getCenter().lat).toBeCloseTo(center.lat, 5); + expect(map.getCenter().lng).toBeCloseTo(center.lng, 5); + }); + + it('FE-COMP-COLMAP-005: a list of one opens on its town, not on its street', async () => { + const { map } = await renderMap([CAFES[0]]); + + expectEveryPinClear(map, [CAFES[0]], BAR); + expect(map.getZoom()).toBeGreaterThanOrEqual(10); + expect(map.getZoom()).toBeLessThanOrEqual(13); + }); + + it('FE-COMP-COLMAP-006: "All saved" across the globe keeps Patagonia and Taiwan on screen', async () => { + const { map } = await renderMap(WORLD); + + expectEveryPinClear(map, WORLD, BAR); + }); + + it("FE-COMP-COLMAP-007: the phone's card frames every cafe clear of its edges and its bottom controls", async () => { + setWindow(390, 844); + Object.assign(box, { width: 356, height: 414 }); + + const { map } = await renderMap(CAFES, false); + + expectEveryPinClear(map, CAFES, 0); + expectFramedClosely(map, CAFES, 0); + // The locate and satellite buttons stand on --bottom-nav-h, which lifts them over the tab + // bar under a full-screen map. The card ends above the tab bar, so its wrapper zeroes it + // and the buttons sit in the bottom 54 px the frame keeps free. + expect(wrappedIn(map, '[--bottom-nav-h:0px]')).toBe(true); + }); +}); diff --git a/client/src/components/Collections/CollectionMap.tsx b/client/src/components/Collections/CollectionMap.tsx index 3dd3cc742..ad9ece60f 100644 --- a/client/src/components/Collections/CollectionMap.tsx +++ b/client/src/components/Collections/CollectionMap.tsx @@ -1,9 +1,10 @@ -import React from 'react' +import React, { useMemo } from 'react' import { MapViewAuto } from '../Map/MapViewAuto' import type { CollectionPlace } from '@trek/shared' import { mappablePlaces } from '../../pages/collections/collectionsModel' import { useTileUrl } from '../../hooks/useTileUrl' import { OFM_DARK, OFM_POSITRON } from '../../constants/mapDefaults' +import type { ViewportPadding } from '../../utils/mapViewport' interface CollectionMapProps { places: CollectionPlace[] @@ -12,20 +13,75 @@ interface CollectionMapProps { /** Clicking the map background clears the selection. */ onDeselect?: () => void dark: boolean + /** A control bar floats over the top of the map (the desktop panel's), so the frame starts below it. */ + underBar?: boolean +} + +/** How far a pin reaches from its point: half a selected pin (44 px) and the rating badge on its corner. */ +const PIN_REACH = 26 +/** What every edge keeps clear: a pin's reach and a little air, so no pin touches the frame. */ +const EDGE = PIN_REACH + 14 +/** The desktop panel's control bar: 12 px off the top edge and at most 40 px tall. */ +const TOP_BAR = 52 +/** The round controls in the bottom corners (satellite, locate): 12 px off the edge and 42 px tall. */ +const BOTTOM_CONTROLS = 54 +/** + * Half the side of the square framed around a lone place, in degrees: about 11 km + * from north to south, so a list of one opens on its town rather than on the street + * the closest fit would zoom to. + */ +const LONE_PLACE_SPAN = 0.05 + +/** What the frame keeps clear of pins, in map pixels per edge. */ +function framePadding(underBar: boolean): ViewportPadding { + const top = (underBar ? TOP_BAR : 0) + EDGE + return { top, right: EDGE, bottom: BOTTOM_CONTROLS + PIN_REACH, left: EDGE } // rtl-lint-disable: pixels of the map, not sides of the text } /** - * Map view — reuses the trip map stack (MapViewAuto → Leaflet / GL with marker + * The coordinates the map frames. A lone place, or several on one spot, has no extent + * to fit, so a small square around it stands in. + */ +function framePoints(places: CollectionPlace[]): [number, number][] { + const points = places.flatMap(p => ( + typeof p.lat === 'number' && typeof p.lng === 'number' ? [[p.lat, p.lng] as [number, number]] : [] + )) + if (points.length === 0) return points + const [lat, lng] = points[0] + if (points.some(([a, b]) => a !== lat || b !== lng)) return points + return [ + [Math.max(-90, lat - LONE_PLACE_SPAN), lng - LONE_PLACE_SPAN], + [Math.min(90, lat + LONE_PLACE_SPAN), lng + LONE_PLACE_SPAN], + ] +} + +/** + * Map view: reuses the trip map stack (MapViewAuto → Leaflet / GL with marker * clustering). One of the three list views; clicking a marker selects the place. * The parent `.col-mapwrap` supplies the rounded, bordered box + height, so this * just fills it. + * + * The map frames every place itself, through the renderers' focus props. Their + * opening camera is framed for the browser window, and this map is a box a fraction + * of that size (the desktop split, the phone's card), so it opened zoomed in on the + * middle of the list with most places outside; and the count it used to key the fit + * on never changed between two lists of eight. The frame is fitted in the map's own + * box instead, clear of the edges and of the controls floating over it, and lands + * while the map is built rather than flying in. */ -export default function CollectionMap({ places, selectedPlaceId, onOpenPlace, onDeselect, dark }: CollectionMapProps): React.ReactElement { - const pts = mappablePlaces(places) +export default function CollectionMap({ places, selectedPlaceId, onOpenPlace, onDeselect, dark, underBar = false }: CollectionMapProps): React.ReactElement { + const pts = useMemo(() => mappablePlaces(places), [places]) const tileUrl = useTileUrl(dark ? OFM_DARK : OFM_POSITRON) + const focusPoints = useMemo(() => framePoints(pts), [pts]) + // The places as a set: another list, a filter or a new place frames again, while a + // reload, another sort order or an edited note leaves a map the user moved alone. + const focusKey = useMemo(() => pts.map(p => p.id).sort((a, b) => a - b).join(','), [pts]) return ( -
+ // The map's own controls stand on --bottom-nav-h to clear the phone's tab bar on a + // full-screen map. This one ends above the tab bar, so they belong on its bottom edge, + // where the frame's bottom padding keeps the pins out from under them. +
onDeselect() : undefined} - // No center/zoom: the map frames itself on the collection's places at mount, and - // falls back to the world view for a collection with none. tileUrl={tileUrl} - fitKey={pts.length} + focusPoints={focusPoints} + focusKey={focusKey} + fitPadding={framePadding(underBar)} />
) diff --git a/client/src/components/Collections/CollectionMapPanel.test.tsx b/client/src/components/Collections/CollectionMapPanel.test.tsx new file mode 100644 index 000000000..f2779fc01 --- /dev/null +++ b/client/src/components/Collections/CollectionMapPanel.test.tsx @@ -0,0 +1,51 @@ +/** + * The collections page's map panel tells the map whether its bar floats over it (FE-COMP-COLMAPPANEL-*). + * + * The frame itself is CollectionMap.test.tsx's. The page tests stand the panel in with a double, + * so this is where the bar's height reaches the frame: without it the top pins land under the bar. + */ +import { describe, expect, it, vi } from 'vitest'; +import { render } from '../../../tests/helpers/render'; +import CollectionMapPanel from './CollectionMapPanel'; + +const handedToMap = vi.hoisted(() => ({ underBar: undefined as boolean | undefined })); +vi.mock('./CollectionMap', () => ({ + default: (props: { underBar?: boolean }) => { + handedToMap.underBar = props.underBar; + return null; + }, +})); + +function panel(overlay: boolean) { + return ( + key} + /> + ); +} + +describe('CollectionMapPanel', () => { + it('FE-COMP-COLMAPPANEL-001: the desktop split floats its bar over the map, which frames its places below it', () => { + const { container } = render(panel(true)); + + expect(container.querySelector('.col-map-topbar')).not.toBeNull(); + expect(handedToMap.underBar).toBe(true); + }); + + it('FE-COMP-COLMAPPANEL-002: a map without the bar frames its places from its own top edge', () => { + const { container } = render(panel(false)); + + expect(container.querySelector('.col-map-topbar')).toBeNull(); + expect(handedToMap.underBar).toBe(false); + }); +}); diff --git a/client/src/components/Collections/CollectionMapPanel.tsx b/client/src/components/Collections/CollectionMapPanel.tsx index 4658da62a..a3ec7214a 100644 --- a/client/src/components/Collections/CollectionMapPanel.tsx +++ b/client/src/components/Collections/CollectionMapPanel.tsx @@ -50,6 +50,7 @@ export default function CollectionMapPanel({ onOpenPlace={onSelect} onDeselect={onDeselect} dark={dark} + underBar={overlay} /> {overlay && (
diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index 5f8d6b6b8..9d172f497 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -534,6 +534,21 @@ describe('MapView', () => { expect(mapMock.fitBounds).toHaveBeenCalledOnce() }) + it('FE-COMP-MAPVIEW-089: a frame handed over while the map is built lands at once, a later one flies', () => { + // The map opens on a guess framed for the browser window. A map in a smaller box, the + // collection's beside its list, hands its own frame over at once, and must open there + // rather than show the guess and then fly to the frame. + const cafes: [number, number][] = [[48.21, 16.37], [38.71, -9.14], [55.7, 12.55]] + const { rerender } = render() + expect(mapMock.fitBounds.mock.calls).toEqual([[expect.anything(), expect.objectContaining({ animate: false, maxZoom: 16 })]]) + mapMock.fitBounds.mockClear() + + // Another list on the same map is a change the user asked for, and it flies as before. + const viewpoints: [number, number][] = [[60.12, 6.74], [44.15, 9.65], [47.56, 13.65]] + rerender() + expect(mapMock.fitBounds.mock.calls).toEqual([[expect.anything(), expect.objectContaining({ animate: true, maxZoom: 16 })]]) + }) + it('keeps selection fit and the pending route-arrival refit for semantic changes', () => { const selectedPlace = buildMapPlace({ id: 901, lat: 48, lng: 11 }) const route: [number, number][][] = [[[48, 11], [48.2, 11.3]]] diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index 2327d5358..1c8f1d755 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -427,35 +427,16 @@ function MapController({ center, zoom }: MapControllerProps) { // fit to that day's destinations immediately, then — once the day's route has // finished computing asynchronously — re-fit once more to include the full route // polyline, so a route that bulges past its stops stays in view (#1128). -interface BoundsControllerProps { +// The focus props are the contract's, documented there once rather than copied here. +interface BoundsControllerProps extends Pick { places: Place[] routeCoords: [number, number][] fitKey: number paddingOpts: L.FitBoundsOptions /** The map was built already framed on these places, so the opening fit has nothing to do. */ framedOnMount?: boolean - /** - * An explicit stretch of map to frame, independent of the day being shown. - * - * `fitKey` cannot express this: it carries no coordinates, and each renderer decides - * for itself that it means "the selected day". Weighing the ways of driving one leg - * needs that leg on screen, which is neither the day nor the trip. - */ - focusPoints?: [number, number][] - /** Changes only when the caller intentionally wants a new initial frame. */ - focusKey?: number | string /** False while the map is locked (#2010): an arriving route no longer re-fits it. */ follow?: boolean - /** - * What the caller's own chrome covers while `focusPoints` is framed, in pixels per edge. - * - * `paddingOpts` knows the planner's panels and nothing else, and on a phone it is a flat - * margin. A shell that lays its own bars over the map passes what they cover, so the - * frame lands in the part still visible. Only the fit on `focusPoints` reads it. - * Compared by value: the same numbers in a new object do not refit, while new numbers - * refit the points already handed over, because the chrome they must clear has moved. - */ - fitPadding?: ViewportPadding } /** A padding box as Leaflet's fit options spell it, corner pairs rather than edges. */ @@ -473,6 +454,7 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo const fitRan = useRef(false) const didInitialFocus = useRef(false) const prevFocusKey = useRef(focusKey) + const built = useRef(false) const fitTo = useCallback((coords: [number, number][], padding: L.FitBoundsOptions = paddingOpts) => { if (coords.length === 0) return @@ -493,7 +475,7 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo * (#1982). MapViewGL never had the nudge, so this also brings the two * renderers back into agreement. */ - map.fitBounds(bounds, { ...padding, maxZoom: 16, animate: true }) + map.fitBounds(bounds, { maxZoom: 16, animate: true, ...padding }) } } catch {} }, [map, paddingOpts]) @@ -534,6 +516,10 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo // Frame whatever was handed over. Nothing happens when it empties, so closing the // picker leaves the map where the user left it rather than snapping back. useEffect(() => { + // The first run is the map being built. A frame handed over by then is where the map + // opens, so it lands at once instead of flying in from the window-sized opening guess. + const opening = !built.current + built.current = true if (!focusPoints?.length) return if (focusKey !== undefined) { const shouldFit = !didInitialFocus.current || focusKey !== prevFocusKey.current @@ -543,7 +529,7 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo } // A day fit that has not run yet must not overwrite this a moment later. awaitingRoute.current = false - fitTo(focusPoints, fitPadding ? leafletPadding(fitPadding) : paddingOpts) + fitTo(focusPoints, { ...(fitPadding ? leafletPadding(fitPadding) : paddingOpts), animate: !opening }) }, [focusPoints, focusKey, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps return null diff --git a/client/src/components/Map/MapViewGL.plannerWaypoints.test.tsx b/client/src/components/Map/MapViewGL.plannerWaypoints.test.tsx index 0b183cd10..7bfdef967 100644 --- a/client/src/components/Map/MapViewGL.plannerWaypoints.test.tsx +++ b/client/src/components/Map/MapViewGL.plannerWaypoints.test.tsx @@ -7,8 +7,8 @@ import { resetAllStores } from '../../../tests/helpers/store'; import type { GeoPosition, TrackingMode } from '../../hooks/useGeolocation'; import { useSettingsStore } from '../../store/settingsStore'; -// The planner-waypoint and location-search camera cases of MapViewGL, and the tour -// map's via ferrata layer (RC-10), with the same map doubles as MapViewGL.test.tsx. +// The planner-waypoint, location-search and antimeridian camera cases of MapViewGL, and +// the tour map's via ferrata layer (RC-10), with the same map doubles as MapViewGL.test.tsx. // Stable fake map so fitBounds call counts survive re-renders. The canvas // container is a single element so listeners registered by the component are @@ -356,6 +356,31 @@ describe('MapViewGL', () => { expect(glMap.fitBounds).toHaveBeenCalledOnce(); }); + it('frames handed-over points across the antimeridian the short way, as the opening camera does', async () => { + loadOnAttach(); + // Fiji, Tonga and Samoa, a collection's list: a box from plain min and max would run + // from -175 to 178, round the far side of the world, with each pin on an edge. + render( + + ); + await act(async () => {}); + + expect(glMap.fitBounds).toHaveBeenCalledOnce(); + const framed = glBounds.instances.find((bounds) => bounds === glMap.fitBounds.mock.calls[0][0]); + const lngs = framed?.extend.mock.calls.map(([lngLat]) => Math.round(lngLat[0] * 100) / 100); + expect(lngs).toEqual([178.44, 184.8, 188.24]); + }); + it.each(['maplibre-gl', 'mapbox-gl'] as const)( 'RC-08: %s previews waypoint drags locally and commits only on pointer up without changing the camera', async (provider) => { diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx index ff0f01519..a5550a010 100644 --- a/client/src/components/Map/MapViewGL.test.tsx +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -944,7 +944,7 @@ describe('MapViewGL', () => { />, ) await act(async () => {}) - expect(glMap.fitBounds).toHaveBeenCalledOnce() + expect(glMap.fitBounds.mock.calls).toEqual([[expect.anything(), expect.objectContaining({ duration: 0 })]]) glMap.fitBounds.mockClear() const mapConstructions = (maplibregl.Map as any).mock.calls.length const retainedCenter = { lng: 135.8, lat: 35.2 } @@ -979,7 +979,7 @@ describe('MapViewGL', () => { />, ) await act(async () => {}) - expect(glMap.fitBounds).toHaveBeenCalledOnce() + expect(glMap.fitBounds.mock.calls).toEqual([[expect.anything(), expect.objectContaining({ duration: 400 })]]) }) it('keeps selection fit and the pending route-arrival refit for semantic changes', async () => { diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 1acc0675e..fa809c80d 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -48,7 +48,7 @@ import { addGpxTrackLayers, GPX_HIT_LAYER_ID, hasGpxTrackLayers, selectGpxTrack, import { buildPoiPopupHtml } from './placePopup' import { pluginsApi, type PluginMapMarker, type PluginMapLayer } from '../../api/client' import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults' -import { computeMapViewport, TILE_SIZE_GL } from '../../utils/mapViewport' +import { alongShortestArc, computeMapViewport, TILE_SIZE_GL } from '../../utils/mapViewport' import { selectedPlaceTarget } from './selectedPlaceTarget' import { PLACE_CLUSTER_CIRCLE_LAYER_ID, PLACE_CLUSTER_COUNT_LAYER_ID } from './placeClusterLayers' @@ -2091,7 +2091,7 @@ export function MapViewGL({ didInitialFocusRef.current = true } const bounds = new gl.LngLatBounds() - focusPoints.forEach(([lat, lng]) => bounds.extend([lng, lat])) + alongShortestArc(focusPoints).forEach(([lat, lng]) => bounds.extend([lng, lat])) // A day fit still waiting on its route must not overwrite this a moment later. pendingRouteFitRef.current = null try { @@ -2099,7 +2099,7 @@ export function MapViewGL({ padding: fitPadding ?? paddingOpts, maxZoom: 15, pitch: enableMapbox3d ? 45 : 0, - duration: 400, + duration: mapReady ? 400 : 0, // handed over while the map is built: it opens there, no flight in }) } catch { /* noop */ } }, [focusPoints, focusKey, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps diff --git a/client/src/utils/mapViewport.test.ts b/client/src/utils/mapViewport.test.ts index fab9a2cbb..8fefc9760 100644 --- a/client/src/utils/mapViewport.test.ts +++ b/client/src/utils/mapViewport.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest' import { + alongShortestArc, computeMapViewport, MAX_ZOOM_GL, MAX_ZOOM_RASTER, @@ -263,3 +264,26 @@ describe('computeMapViewport', () => { expect(withPadding.zoom).toBe(equivalent.zoom) }) }) + +describe('alongShortestArc', () => { + const FIJI: [number, number] = [-18.14, 178.44] + const TONGA: [number, number] = [-21.14, -175.2] + const SAMOA: [number, number] = [-13.83, -171.76] + + it('carries the places past the antimeridian on beyond 180, so a min-max box between them is the short one', () => { + const lngs = alongShortestArc([FIJI, TONGA, SAMOA]).map(([, lng]) => Math.round(lng * 100) / 100) + + // From Fiji east across the date line: under ten degrees wide, not the 350 the other way round. + expect(lngs).toEqual([178.44, 184.8, 188.24]) + }) + + it('leaves places that do not cross the antimeridian as they are', () => { + const cafes: [number, number][] = [[48.21, 16.37], [38.71, -9.14], [55.7, 12.55]] + + expect(alongShortestArc(cafes)).toEqual(cafes) + }) + + it('has nothing to lay out for no places', () => { + expect(alongShortestArc([])).toEqual([]) + }) +}) diff --git a/client/src/utils/mapViewport.ts b/client/src/utils/mapViewport.ts index b0dce0835..5e869886b 100644 --- a/client/src/utils/mapViewport.ts +++ b/client/src/utils/mapViewport.ts @@ -146,6 +146,18 @@ function lngExtent(lngs: number[], wrapsWorld: boolean): { west: number; east: n return { west, east, span: east - west } } +/** + * `[lat, lng]` points with their longitudes laid along the narrowest arc that holds them all + * (lngExtent), for a renderer that wraps the world: a point past the antimeridian goes on + * beyond 180 instead of starting over at -180. A bounding box built from them by plain min and max (MapLibre's and Mapbox's + * extend) then spans the few degrees from Fiji to Samoa, not the rest of the world. + */ +export function alongShortestArc(points: readonly [number, number][]): [number, number][] { + if (points.length === 0) return [] + const { west } = lngExtent(points.map(([, lng]) => lng), true) + return points.map(([lat, lng]) => [lat, lng < west ? lng + 360 : lng]) +} + /** Zoom at which a span covering `fraction` of the world fills `px` pixels. */ function zoomForFraction(px: number, tileSize: number, fraction: number, maxZoom: number): number { // A zero fraction (identical coordinates) would divide by zero and yield Infinity.