mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-11-14 00:01:03 +00:00
fix(collections): the collection map opens on all of its places
The map framed its first view for the browser window instead of its own box and then skipped the fit, so a list of cafes across Europe opened on northern France. It now fits the places in its own box with room for the label bar and the controls, fits again when the set of places changes, keeps a view the user moved, opens a single place at town level and keeps a list across the date line on its short arc.
This commit is contained in:
@@ -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(<MapView places={[]} focusKey="list:17" focusPoints={cafes} />)
|
||||
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(<MapView places={[]} focusKey="list:20" focusPoints={viewpoints} />)
|
||||
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]]]
|
||||
|
||||
@@ -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<MapViewProps, 'focusPoints' | 'focusKey' | 'fitPadding'> {
|
||||
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
|
||||
|
||||
@@ -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(
|
||||
<MapViewGL
|
||||
places={[]}
|
||||
followSelection={false}
|
||||
focusKey="1,2,3"
|
||||
focusPoints={[
|
||||
[-18.14, 178.44],
|
||||
[-21.14, -175.2],
|
||||
[-13.83, -171.76],
|
||||
]}
|
||||
glProvider="maplibre-gl"
|
||||
/>
|
||||
);
|
||||
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) => {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user