From 362b1ee8d10cab279f1aa29dbe20050c292f529a Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 23 Sep 2026 22:11:51 +0200 Subject: [PATCH] fix(map): a selected stay is flown to even when no pin stands for it With the Unplanned filter on, a click on a stop in the day list moved the map while a click on the stay chip did nothing: the map only went to a selected place it showed as a pin, and the GL map also to one of the open day's stops, which a stay is neither. Both renderers now look the target up the same way, the pin first, then the day's stop, then the selected place itself, so the map goes to the stay whatever filter hides its pin. --- client/src/components/Map/MapView.test.tsx | 8 ++++++ client/src/components/Map/MapView.tsx | 12 ++++++--- client/src/components/Map/MapViewGL.tsx | 6 ++++- .../Map/selectedPlaceTarget.test.ts | 27 +++++++++++++++++++ .../src/components/Map/selectedPlaceTarget.ts | 27 +++++++++++++++++++ client/src/pages/TripPlannerPage.tsx | 1 + 6 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 client/src/components/Map/selectedPlaceTarget.test.ts create mode 100644 client/src/components/Map/selectedPlaceTarget.ts 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}