mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
@@ -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(<MapView places={places} selectedPlaceId={null} />)
|
||||
rerender(<MapView places={places} selectedPlaceId={11} selectedPlace={stay} />)
|
||||
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(<MapView places={places} selectedPlaceId={null} />)
|
||||
|
||||
@@ -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({
|
||||
|
||||
<MapController center={center} zoom={zoom} />
|
||||
<BoundsController places={dayPlaces.length > 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} framedOnMount={initialView.framed} focusPoints={focusPoints} fitPadding={fitPadding} />
|
||||
<SelectionController places={places} selectedPlaceId={selectedPlaceId} dayPlaces={dayPlaces} paddingOpts={paddingOpts} />
|
||||
<SelectionController places={places} selectedPlaceId={selectedPlaceId} dayPlaces={dayPlaces} selectedPlace={selectedPlace} paddingOpts={paddingOpts} />
|
||||
<MapClickHandler onClick={onMapClick} />
|
||||
<MapContextMenuHandler onContextMenu={onMapContextMenu} />
|
||||
<CameraHoverGuard movingRef={mapMovingRef} onMoveStart={clearHover} onZoom={setMapZoom} />
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
@@ -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<P extends MapTarget>(
|
||||
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)
|
||||
}
|
||||
@@ -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}
|
||||
|
||||
Reference in New Issue
Block a user