mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-22 14:51:45 +00:00
fix(map): prevent auto zoom-out when opening/closing place inspector (issue #921)
Both Leaflet and Mapbox GL renderers now gate fitBounds strictly on fitKey increments from the parent. Selecting or dismissing a place inspector changes paddingOpts (via hasInspector) but no longer triggers a re-fit that zoomed the map out to the full trip extent when no day was selected. Also removes the zoom-12 visibility gate on Leaflet route info pills so they render at all zoom levels when a route is active.
This commit is contained in:
@@ -7,6 +7,16 @@ import { resetAllStores } from '../../../tests/helpers/store'
|
|||||||
import { buildPlace } from '../../../tests/helpers/factories'
|
import { buildPlace } from '../../../tests/helpers/factories'
|
||||||
import * as photoService from '../../services/photoService'
|
import * as photoService from '../../services/photoService'
|
||||||
|
|
||||||
|
const mapMock = vi.hoisted(() => ({
|
||||||
|
panTo: vi.fn(),
|
||||||
|
setView: vi.fn(),
|
||||||
|
fitBounds: vi.fn(),
|
||||||
|
getZoom: vi.fn().mockReturnValue(10),
|
||||||
|
on: vi.fn(),
|
||||||
|
off: vi.fn(),
|
||||||
|
panBy: vi.fn(),
|
||||||
|
}))
|
||||||
|
|
||||||
vi.mock('react-leaflet', () => ({
|
vi.mock('react-leaflet', () => ({
|
||||||
MapContainer: ({ children }: any) => <div data-testid="map-container">{children}</div>,
|
MapContainer: ({ children }: any) => <div data-testid="map-container">{children}</div>,
|
||||||
TileLayer: () => <div data-testid="tile-layer" />,
|
TileLayer: () => <div data-testid="tile-layer" />,
|
||||||
@@ -27,15 +37,7 @@ vi.mock('react-leaflet', () => ({
|
|||||||
Polyline: ({ positions }: any) => <div data-testid="polyline" data-points={JSON.stringify(positions)} />,
|
Polyline: ({ positions }: any) => <div data-testid="polyline" data-points={JSON.stringify(positions)} />,
|
||||||
CircleMarker: () => <div data-testid="circle-marker" />,
|
CircleMarker: () => <div data-testid="circle-marker" />,
|
||||||
Circle: () => <div data-testid="circle" />,
|
Circle: () => <div data-testid="circle" />,
|
||||||
useMap: () => ({
|
useMap: () => mapMock,
|
||||||
panTo: vi.fn(),
|
|
||||||
setView: vi.fn(),
|
|
||||||
fitBounds: vi.fn(),
|
|
||||||
getZoom: () => 10,
|
|
||||||
on: vi.fn(),
|
|
||||||
off: vi.fn(),
|
|
||||||
panBy: vi.fn(),
|
|
||||||
}),
|
|
||||||
useMapEvents: () => ({}),
|
useMapEvents: () => ({}),
|
||||||
}))
|
}))
|
||||||
|
|
||||||
@@ -79,6 +81,7 @@ function buildMapPlace(overrides: Record<string, any> = {}) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
vi.clearAllMocks()
|
||||||
resetAllStores()
|
resetAllStores()
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -216,4 +219,33 @@ describe('MapView', () => {
|
|||||||
render(<MapView places={places} selectedPlaceId={5} />)
|
render(<MapView places={places} selectedPlaceId={5} />)
|
||||||
expect(screen.getByTestId('marker')).toBeTruthy()
|
expect(screen.getByTestId('marker')).toBeTruthy()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('FE-COMP-MAPVIEW-018: changing selectedPlaceId/hasInspector does not refit bounds (issue #921)', () => {
|
||||||
|
const places = [
|
||||||
|
buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }),
|
||||||
|
buildMapPlace({ id: 2, lat: 48.86, lng: 2.337 }),
|
||||||
|
]
|
||||||
|
const { rerender } = render(<MapView places={places} fitKey={1} selectedPlaceId={null} hasInspector={false} />)
|
||||||
|
const initialCount = mapMock.fitBounds.mock.calls.length
|
||||||
|
|
||||||
|
// Toggle selectedPlaceId on — mimics opening place inspector (hasInspector flips,
|
||||||
|
// paddingOpts memo creates new object). fitBounds must NOT fire again.
|
||||||
|
rerender(<MapView places={places} fitKey={1} selectedPlaceId={1} hasInspector={true} />)
|
||||||
|
expect(mapMock.fitBounds).toHaveBeenCalledTimes(initialCount)
|
||||||
|
|
||||||
|
// Toggle selectedPlaceId off — mimics closing inspector via X button.
|
||||||
|
rerender(<MapView places={places} fitKey={1} selectedPlaceId={null} hasInspector={false} />)
|
||||||
|
expect(mapMock.fitBounds).toHaveBeenCalledTimes(initialCount)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('FE-COMP-MAPVIEW-019: bumping fitKey triggers a new fitBounds call', () => {
|
||||||
|
const places = [
|
||||||
|
buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }),
|
||||||
|
]
|
||||||
|
const { rerender } = render(<MapView places={places} fitKey={1} />)
|
||||||
|
const afterFirst = mapMock.fitBounds.mock.calls.length
|
||||||
|
|
||||||
|
rerender(<MapView places={places} fitKey={2} />)
|
||||||
|
expect(mapMock.fitBounds.mock.calls.length).toBeGreaterThan(afterFirst)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -186,7 +186,7 @@ function BoundsController({ places, fitKey, paddingOpts, hasDayDetail }: BoundsC
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch {}
|
} catch {}
|
||||||
}, [fitKey, places, paddingOpts, map, hasDayDetail])
|
}, [fitKey]) // eslint-disable-line react-hooks/exhaustive-deps
|
||||||
|
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
@@ -233,18 +233,7 @@ interface RouteLabelProps {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function RouteLabel({ midpoint, walkingText, drivingText }: RouteLabelProps) {
|
function RouteLabel({ midpoint, walkingText, drivingText }: RouteLabelProps) {
|
||||||
const map = useMap()
|
if (!midpoint) return null
|
||||||
const [visible, setVisible] = useState(map ? map.getZoom() >= 12 : false)
|
|
||||||
|
|
||||||
useEffect(() => {
|
|
||||||
if (!map) return
|
|
||||||
const check = () => setVisible(map.getZoom() >= 12)
|
|
||||||
check()
|
|
||||||
map.on('zoomend', check)
|
|
||||||
return () => map.off('zoomend', check)
|
|
||||||
}, [map])
|
|
||||||
|
|
||||||
if (!visible || !midpoint) return null
|
|
||||||
|
|
||||||
const icon = L.divIcon({
|
const icon = L.divIcon({
|
||||||
className: 'route-info-pill',
|
className: 'route-info-pill',
|
||||||
|
|||||||
@@ -0,0 +1,164 @@
|
|||||||
|
import React from 'react'
|
||||||
|
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'
|
||||||
|
import { render } from '../../../tests/helpers/render'
|
||||||
|
import { act } from '@testing-library/react'
|
||||||
|
import { resetAllStores } from '../../../tests/helpers/store'
|
||||||
|
import { buildPlace } from '../../../tests/helpers/factories'
|
||||||
|
import { useSettingsStore } from '../../store/settingsStore'
|
||||||
|
|
||||||
|
// Stable fake map so fitBounds call counts survive re-renders.
|
||||||
|
const glMap = vi.hoisted(() => ({
|
||||||
|
on: vi.fn(),
|
||||||
|
off: vi.fn(),
|
||||||
|
once: vi.fn(),
|
||||||
|
loaded: vi.fn().mockReturnValue(true),
|
||||||
|
fitBounds: vi.fn(),
|
||||||
|
flyTo: vi.fn(),
|
||||||
|
jumpTo: vi.fn(),
|
||||||
|
getZoom: vi.fn().mockReturnValue(10),
|
||||||
|
addControl: vi.fn(),
|
||||||
|
removeControl: vi.fn(),
|
||||||
|
remove: vi.fn(),
|
||||||
|
addSource: vi.fn(),
|
||||||
|
getSource: vi.fn().mockReturnValue(null),
|
||||||
|
addLayer: vi.fn(),
|
||||||
|
setLayoutProperty: vi.fn(),
|
||||||
|
getStyle: vi.fn().mockReturnValue({ layers: [] }),
|
||||||
|
isStyleLoaded: vi.fn().mockReturnValue(true),
|
||||||
|
getCanvasContainer: vi.fn(() => document.createElement('div')),
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('mapbox-gl', () => ({
|
||||||
|
default: {
|
||||||
|
accessToken: '',
|
||||||
|
Map: vi.fn(() => glMap),
|
||||||
|
Marker: vi.fn(() => ({
|
||||||
|
setLngLat: vi.fn().mockReturnThis(),
|
||||||
|
addTo: vi.fn().mockReturnThis(),
|
||||||
|
remove: vi.fn(),
|
||||||
|
getElement: vi.fn(() => document.createElement('div')),
|
||||||
|
})),
|
||||||
|
LngLatBounds: vi.fn(() => ({ extend: vi.fn().mockReturnThis() })),
|
||||||
|
NavigationControl: vi.fn(),
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
vi.mock('mapbox-gl/dist/mapbox-gl.css', () => ({}))
|
||||||
|
|
||||||
|
vi.mock('./mapboxSetup', () => ({
|
||||||
|
isStandardFamily: vi.fn(() => false),
|
||||||
|
supportsCustom3d: vi.fn(() => false),
|
||||||
|
wantsTerrain: vi.fn(() => false),
|
||||||
|
addCustom3dBuildings: vi.fn(),
|
||||||
|
addTerrainAndSky: vi.fn(),
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('./locationMarkerMapbox', () => ({
|
||||||
|
attachLocationMarker: vi.fn(() => ({ update: vi.fn() })),
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('./reservationsMapbox', () => ({
|
||||||
|
ReservationMapboxOverlay: vi.fn().mockImplementation(() => ({ update: vi.fn() })),
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('../../hooks/useGeolocation', () => ({
|
||||||
|
useGeolocation: vi.fn(() => ({
|
||||||
|
position: null,
|
||||||
|
mode: 'off',
|
||||||
|
error: null,
|
||||||
|
cycleMode: vi.fn(),
|
||||||
|
setMode: vi.fn(),
|
||||||
|
})),
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('../../services/photoService', () => ({
|
||||||
|
getCached: vi.fn(() => null),
|
||||||
|
isLoading: vi.fn(() => false),
|
||||||
|
fetchPhoto: vi.fn(),
|
||||||
|
onThumbReady: vi.fn(() => () => {}),
|
||||||
|
getAllThumbs: vi.fn(() => ({})),
|
||||||
|
}))
|
||||||
|
|
||||||
|
import { MapViewGL } from './MapViewGL'
|
||||||
|
|
||||||
|
function buildMapPlace(overrides: Record<string, any> = {}) {
|
||||||
|
return {
|
||||||
|
...buildPlace(),
|
||||||
|
category_name: null,
|
||||||
|
category_color: null,
|
||||||
|
category_icon: null,
|
||||||
|
...overrides,
|
||||||
|
} as any
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
useSettingsStore.setState({
|
||||||
|
settings: {
|
||||||
|
...useSettingsStore.getState().settings,
|
||||||
|
map_provider: 'mapbox-gl',
|
||||||
|
mapbox_access_token: 'pk.test_token',
|
||||||
|
mapbox_style: 'mapbox://styles/mapbox/streets-v12',
|
||||||
|
mapbox_3d_enabled: false,
|
||||||
|
},
|
||||||
|
} as any)
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.clearAllMocks()
|
||||||
|
resetAllStores()
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('MapViewGL', () => {
|
||||||
|
it('FE-COMP-MAPVIEWGL-001: opening place inspector does not refit bounds (issue #921)', async () => {
|
||||||
|
const places = [
|
||||||
|
buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }),
|
||||||
|
buildMapPlace({ id: 2, lat: 48.86, lng: 2.337 }),
|
||||||
|
]
|
||||||
|
|
||||||
|
const { rerender } = render(
|
||||||
|
<MapViewGL places={places} fitKey={1} selectedPlaceId={null} hasInspector={false} />,
|
||||||
|
)
|
||||||
|
await act(async () => {})
|
||||||
|
const after_initial = glMap.fitBounds.mock.calls.length
|
||||||
|
|
||||||
|
// Selecting a place flips hasInspector → paddingOpts memo changes.
|
||||||
|
// fitBounds must NOT fire again (this was the bug).
|
||||||
|
rerender(
|
||||||
|
<MapViewGL places={places} fitKey={1} selectedPlaceId={1} hasInspector={true} />,
|
||||||
|
)
|
||||||
|
await act(async () => {})
|
||||||
|
expect(glMap.fitBounds).toHaveBeenCalledTimes(after_initial)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('FE-COMP-MAPVIEWGL-002: closing inspector does not refit bounds (issue #921)', async () => {
|
||||||
|
const places = [
|
||||||
|
buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }),
|
||||||
|
]
|
||||||
|
|
||||||
|
const { rerender } = render(
|
||||||
|
<MapViewGL places={places} fitKey={1} selectedPlaceId={1} hasInspector={true} />,
|
||||||
|
)
|
||||||
|
await act(async () => {})
|
||||||
|
const after_initial = glMap.fitBounds.mock.calls.length
|
||||||
|
|
||||||
|
// Closing inspector (X button) clears selectedPlaceId → hasInspector=false → new paddingOpts.
|
||||||
|
rerender(
|
||||||
|
<MapViewGL places={places} fitKey={1} selectedPlaceId={null} hasInspector={false} />,
|
||||||
|
)
|
||||||
|
await act(async () => {})
|
||||||
|
expect(glMap.fitBounds).toHaveBeenCalledTimes(after_initial)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('FE-COMP-MAPVIEWGL-003: bumping fitKey triggers a new fitBounds call', async () => {
|
||||||
|
const places = [
|
||||||
|
buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }),
|
||||||
|
]
|
||||||
|
|
||||||
|
const { rerender } = render(<MapViewGL places={places} fitKey={1} />)
|
||||||
|
await act(async () => {})
|
||||||
|
const after_first = glMap.fitBounds.mock.calls.length
|
||||||
|
|
||||||
|
rerender(<MapViewGL places={places} fitKey={2} />)
|
||||||
|
await act(async () => {})
|
||||||
|
expect(glMap.fitBounds.mock.calls.length).toBeGreaterThan(after_first)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -507,13 +507,10 @@ export function MapViewGL({
|
|||||||
return { top, right: rightWidth + 40, bottom, left: leftWidth + 40 }
|
return { top, right: rightWidth + 40, bottom, left: leftWidth + 40 }
|
||||||
}, [leftWidth, rightWidth, hasInspector, hasDayDetail])
|
}, [leftWidth, rightWidth, hasInspector, hasDayDetail])
|
||||||
|
|
||||||
// Also fit when the places collection changes so the initial render
|
const prevFitKey = useRef(-1)
|
||||||
// zooms to the trip instead of the default center.
|
|
||||||
const placeBoundsKey = useMemo(
|
|
||||||
() => places.filter(p => p.lat && p.lng).map(p => `${p.id}:${p.lat}:${p.lng}`).join('|'),
|
|
||||||
[places]
|
|
||||||
)
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
if (fitKey === prevFitKey.current) return
|
||||||
|
prevFitKey.current = fitKey
|
||||||
const map = mapRef.current
|
const map = mapRef.current
|
||||||
if (!map) return
|
if (!map) return
|
||||||
const target = dayPlaces.length > 0 ? dayPlaces : places
|
const target = dayPlaces.length > 0 ? dayPlaces : places
|
||||||
@@ -533,7 +530,7 @@ export function MapViewGL({
|
|||||||
}
|
}
|
||||||
if (map.loaded()) run()
|
if (map.loaded()) run()
|
||||||
else map.once('load', run)
|
else map.once('load', run)
|
||||||
}, [fitKey, placeBoundsKey, paddingOpts, mapbox3d]) // eslint-disable-line react-hooks/exhaustive-deps
|
}, [fitKey]) // eslint-disable-line react-hooks/exhaustive-deps
|
||||||
|
|
||||||
// flyTo selected place
|
// flyTo selected place
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
|||||||
Reference in New Issue
Block a user