diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index ca8abed43..579c0e504 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -946,6 +946,14 @@ describe('MapView selection panning (#921)', () => { expect(mapMock.panTo).toHaveBeenCalledWith([48, 2], { animate: true }) }) + it('FE-COMP-MAPVIEW-083: pans to a selected stay whose pin a filter keeps off the map', () => { + const places = [buildMapPlace({ id: 9, lat: 48, lng: 2 })] + const stay = buildMapPlace({ id: 11, lat: 50, lng: 4 }) + const { rerender } = render() + rerender() + expect(mapMock.panTo).toHaveBeenCalledWith([50, 4], { animate: true }) + }) + it('FE-COMP-MAPVIEW-053: a selected place without coordinates is not panned to', () => { const places = [buildMapPlace({ id: 10, lat: null, lng: null })] const { rerender } = render() diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index 88c8bc46f..9d0276ebe 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -330,10 +330,11 @@ interface SelectionControllerProps { places: Place[] selectedPlaceId: number | null dayPlaces: Place[] + selectedPlace: Place | null paddingOpts: L.FitBoundsOptions } -function SelectionController({ places, selectedPlaceId, dayPlaces, paddingOpts }: SelectionControllerProps) { +function SelectionController({ places, selectedPlaceId, dayPlaces, selectedPlace, paddingOpts }: SelectionControllerProps) { const map = useMap() const prev = useRef(null) @@ -343,7 +344,7 @@ function SelectionController({ places, selectedPlaceId, dayPlaces, paddingOpts } // side-panel + bottom-inspector padding so the pin lands in the middle of the // *visible* map area rather than the geometric centre (where the bottom panel // would cover it). Reuses the same paddingOpts the fit-bounds path uses. - const selected = places.find(p => p.id === selectedPlaceId) + const selected = selectedPlaceTarget(selectedPlaceId, places, dayPlaces, selectedPlace) if (selected?.lat != null && selected?.lng != null) { const latlng: [number, number] = [selected.lat, selected.lng] const tl = paddingOpts.paddingTopLeft as [number, number] | undefined @@ -357,7 +358,7 @@ function SelectionController({ places, selectedPlaceId, dayPlaces, paddingOpts } } } prev.current = selectedPlaceId - }, [selectedPlaceId, places, map]) + }, [selectedPlaceId, places, dayPlaces, selectedPlace, map]) return null } @@ -553,6 +554,7 @@ import { useIsPhone } from '../../mobile/useIsPhone' // shared useGeolocation hook so the Leaflet and Mapbox variants behave // identically. Heading is shown as a rotated conic SVG when available. import type { GeoPosition, TrackingMode } from '../../hooks/useGeolocation' +import { selectedPlaceTarget } from './selectedPlaceTarget' function LeafletLocationLayer({ position, mode }: { position: GeoPosition | null; mode: TrackingMode }) { const map = useMap() @@ -678,6 +680,8 @@ export const MapView = memo(function MapView({ routeColors = null, routeSegments = [], selectedPlaceId = null, + // The selected place itself, for when no pin on this map stands for it. + selectedPlace = null, hoverDisabled = false, onMarkerClick, onMapClick, @@ -1130,7 +1134,7 @@ export const MapView = memo(function MapView({ 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} framedOnMount={initialView.framed} focusPoints={focusPoints} fitPadding={fitPadding} /> - + diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 8ff5ccecb..b122575cf 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -42,6 +42,7 @@ import { buildPoiPopupHtml } from './placePopup' import { pluginsApi, type PluginMapMarker, type PluginMapLayer } from '../../api/client' import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, SATELLITE_TILE_URL, SATELLITE_TILE_ATTRIBUTION, SATELLITE_TILE_MAXZOOM } from '../../constants/mapDefaults' import { computeMapViewport, TILE_SIZE_GL, type ViewportPadding } from '../../utils/mapViewport' +import { selectedPlaceTarget } from './selectedPlaceTarget' function categoryIconSvg(iconName: string | null | undefined, size: number): string { const IconComponent = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin'] @@ -147,6 +148,8 @@ interface Props { routeColors?: ({ line: string; casing: string } | undefined)[] | null routeSegments?: RouteSegment[] selectedPlaceId?: number | null + /** The selected place itself, for when no pin on this map stands for it. */ + selectedPlace?: Place | null onMarkerClick?: (id: number) => void hoverDisabled?: boolean onMapClick?: (info: { latlng: { lat: number; lng: number } }) => void @@ -676,6 +679,7 @@ export function MapViewGL({ routeColors = null, routeSegments = NO_ROUTE_SEGMENTS, selectedPlaceId = null, + selectedPlace = null, hoverDisabled = false, onMarkerClick, onMapClick, @@ -2308,7 +2312,7 @@ export function MapViewGL({ useEffect(() => { const map = mapRef.current if (!map || !selectedPlaceId) return - const target = places.find(p => p.id === selectedPlaceId) || dayPlaces.find(p => p.id === selectedPlaceId) + const target = selectedPlaceTarget(selectedPlaceId, places, dayPlaces, selectedPlace) if (!target?.lat || !target?.lng) return try { map.flyTo({ diff --git a/client/src/components/Map/selectedPlaceTarget.test.ts b/client/src/components/Map/selectedPlaceTarget.test.ts new file mode 100644 index 000000000..c92b71096 --- /dev/null +++ b/client/src/components/Map/selectedPlaceTarget.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' +import { selectedPlaceTarget } from './selectedPlaceTarget' + +// FE-MAP-SELTARGET-001 to FE-MAP-SELTARGET-004 + +describe('selectedPlaceTarget', () => { + const pin = { id: 1, lat: 48, lng: 2 } + const stop = { id: 2, lat: 49, lng: 3 } + const stay = { id: 3, lat: 50, lng: 4 } + + it('FE-MAP-SELTARGET-001: goes to the pin the map shows', () => { + expect(selectedPlaceTarget(1, [pin], [stop], stay)).toBe(pin) + }) + + it('FE-MAP-SELTARGET-002: goes to the open day\'s stop when a filter hides its pin', () => { + expect(selectedPlaceTarget(2, [pin], [stop], null)).toBe(stop) + }) + + it('FE-MAP-SELTARGET-003: goes to a stay that is neither a pin nor a stop of the day', () => { + expect(selectedPlaceTarget(3, [pin], [stop], stay)).toBe(stay) + }) + + it('FE-MAP-SELTARGET-004: goes nowhere without a selection, or for a place it does not know', () => { + expect(selectedPlaceTarget(null, [pin], [stop], stay)).toBeNull() + expect(selectedPlaceTarget(9, [pin], [stop], stay)).toBeNull() + }) +}) diff --git a/client/src/components/Map/selectedPlaceTarget.ts b/client/src/components/Map/selectedPlaceTarget.ts new file mode 100644 index 000000000..9f85a13d1 --- /dev/null +++ b/client/src/components/Map/selectedPlaceTarget.ts @@ -0,0 +1,27 @@ +/** A place the map can go to. */ +export interface MapTarget { + id: number + lat?: number | null + lng?: number | null +} + +/** + * Where the map goes when a place is selected, the same in both renderers. + * + * Its pin first, then the open day's stop, then the selected place itself. A filter + * that hides the pin (Unplanned, a category) keeps the pin off the map but not the + * map away from the place, and a stay shown as a chip in the day list is neither a + * pin under Unplanned nor one of the day's stops, so a click on it used to leave the + * map where it was while a click on a stop moved it (#2488). + */ +export function selectedPlaceTarget

( + selectedPlaceId: number | null | undefined, + places: P[], + dayPlaces: P[], + selectedPlace?: P | null, +): P | null { + if (!selectedPlaceId) return null + return places.find(p => p.id === selectedPlaceId) + ?? dayPlaces.find(p => p.id === selectedPlaceId) + ?? (selectedPlace?.id === selectedPlaceId ? selectedPlace : null) +} diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index f4e6a1b61..9d997a4ce 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -432,6 +432,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { selectedDayId={selectedDayId} routeSegments={roadtripActive ? roadtripRoutes.segments : overviewActive ? tripOverview.segments : routeSegments} selectedPlaceId={selectedPlaceId} + selectedPlace={selectedPlace} onMarkerClick={handleMarkerClick} onMapClick={handleMapClick} onMapContextMenu={handleMapContextMenu}