From 0f4766e131ebb7820ef40931354d03bac973eb86 Mon Sep 17 00:00:00 2001 From: jubnl <66769052+jubnl@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:56:47 +0200 Subject: [PATCH] feat(map): open maps framed on their places (builds on #1393) (#1556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(map): fit MapLibre routes reliably * fix(map): default planner map to world view * fix(map): only await route geometry when a route is actually pending The fit armed a pending route-refit slot on every fitKey change, even with no route drawn, and only ever cleared it when a route arrived. So a route toggled on much later — after the user had panned elsewhere — was mistaken for the awaited geometry and yanked the camera back, and only on the first toggle. Arm the slot only when a route is already on screen: updateRouteForDay lays down straight lines in the same batch as the fit and upgrades them to real geometry a moment later, so an empty route at fit time means none is coming. * fix(collections): open the empty collection map on the world view A collection with no mappable places centred on Paris, the same hardcoded default this branch removes everywhere else. * feat(map): open the map framed on its places A trip in Japan opened on the world view at 0,0 and only then animated a fitBounds flight across the planet — the hardcoded default was the map's answer to a question its own places already answer. Each renderer now derives its opening camera from the places it receives, at construction: MapView for Leaflet, MapViewGL for MapLibre and Mapbox (whose zoom runs one level below Leaflet's, measuring against a 512px world tile rather than 256px). Doing it at construction is what confines it to load — the map is built once, and by then the trip's places are in hand. Nothing recomputes it afterwards, so the camera stays where the user leaves it, and the opening fit stands down rather than overruling the gentler zoom a lone place opens at. A trip with no coordinates still falls back to the world view. Collections and the public shared-trip page frame themselves the same way. * refactor(settings): drop the default map centre and zoom Nothing reads them now that every map frames itself on its own places, and a home-city default was the wrong answer for the next trip on the other side of the world. The style preview keeps a fixed location of its own: it needs a city to show label density and 3D buildings, which open ocean cannot. * fix(map): frame the map the way each renderer can actually draw Two defects the unit tests missed and running the app exposed. MapLibre and Mapbox opened on Null Island at zoom 2 regardless of the places: the effect that mirrors an external centre prop onto the camera also ran on mount, so it jumped straight to the default nobody passed and threw away the camera the map had just been built with. It now only responds to actual changes, which is what it was for. Leaflet was unaffected — its controller already guarded on the centre changing. A trip spanning Sydney, Reykjavik and Santiago lost Sydney's marker entirely. The narrowest arc containing all three crosses the antimeridian, and framing there is only sound on a renderer that repeats the world: MapLibre and Mapbox draw a marker on whichever copy is nearest the camera, Leaflet draws one world and puts the marker at its absolute position — off-screen. Leaflet now spans the long way round, as L.latLngBounds would. The test that should have caught this wrapped the x-offset in its own projection helper, quietly assuming behaviour Leaflet does not have; it now models each renderer's real wrapping. --------- Co-authored-by: Azalea --- .../components/Collections/CollectionMap.tsx | 7 +- client/src/components/Map/MapView.test.tsx | 53 +++- client/src/components/Map/MapView.tsx | 60 +++- client/src/components/Map/MapViewGL.test.tsx | 208 +++++++++++++- client/src/components/Map/MapViewGL.tsx | 93 ++++++- .../Planner/ReservationsPanel.test.tsx | 6 +- .../Settings/MapSettingsTab.test.tsx | 77 +----- .../components/Settings/MapSettingsTab.tsx | 70 ++--- client/src/constants/mapDefaults.ts | 4 + client/src/pages/DashboardPage.test.tsx | 3 - client/src/pages/SharedTripPage.tsx | 28 +- client/src/pages/TripPlannerPage.test.tsx | 51 +++- client/src/pages/TripPlannerPage.tsx | 6 +- .../src/pages/tripPlanner/useTripPlanner.ts | 10 +- client/src/store/settingsStore.ts | 3 - client/src/types.ts | 3 - client/src/utils/mapViewport.test.ts | 226 ++++++++++++++++ client/src/utils/mapViewport.ts | 256 ++++++++++++++++++ client/tests/helpers/factories.ts | 3 - .../tests/unit/stores/settingsStore.test.ts | 4 +- shared/src/i18n/ar/settings.ts | 2 - shared/src/i18n/br/settings.ts | 2 - shared/src/i18n/cs/settings.ts | 2 - shared/src/i18n/de/settings.ts | 2 - shared/src/i18n/en/settings.ts | 2 - shared/src/i18n/es/settings.ts | 2 - shared/src/i18n/fr/settings.ts | 2 - shared/src/i18n/gr/settings.ts | 2 - shared/src/i18n/hu/settings.ts | 2 - shared/src/i18n/id/settings.ts | 2 - shared/src/i18n/it/settings.ts | 2 - shared/src/i18n/ja/settings.ts | 2 - shared/src/i18n/ko/settings.ts | 2 - shared/src/i18n/nl/settings.ts | 2 - shared/src/i18n/pl/settings.ts | 2 - shared/src/i18n/ru/settings.ts | 2 - shared/src/i18n/sv/settings.ts | 2 - shared/src/i18n/tr/settings.ts | 2 - shared/src/i18n/uk/settings.ts | 2 - shared/src/i18n/vi/settings.ts | 2 - shared/src/i18n/zh-TW/settings.ts | 2 - shared/src/i18n/zh/settings.ts | 2 - 42 files changed, 987 insertions(+), 228 deletions(-) create mode 100644 client/src/constants/mapDefaults.ts create mode 100644 client/src/utils/mapViewport.test.ts create mode 100644 client/src/utils/mapViewport.ts diff --git a/client/src/components/Collections/CollectionMap.tsx b/client/src/components/Collections/CollectionMap.tsx index c0fce5fcc..0c3b56780 100644 --- a/client/src/components/Collections/CollectionMap.tsx +++ b/client/src/components/Collections/CollectionMap.tsx @@ -20,9 +20,6 @@ interface CollectionMapProps { */ export default function CollectionMap({ places, selectedPlaceId, onOpenPlace, onDeselect, dark }: CollectionMapProps): React.ReactElement { const pts = mappablePlaces(places) - const center: [number, number] = pts.length > 0 - ? [pts[0].lat as number, pts[0].lng as number] - : [48.8566, 2.3522] const tileUrl = dark ? 'https://{s}.basemaps.cartocdn.com/dark_all/{z}/{x}/{y}{r}.png' : 'https://{s}.basemaps.cartocdn.com/light_all/{z}/{x}/{y}{r}.png' @@ -35,8 +32,8 @@ export default function CollectionMap({ places, selectedPlaceId, onOpenPlace, on hoverDisabled onMarkerClick={onOpenPlace} onMapClick={onDeselect ? () => onDeselect() : undefined} - center={center} - zoom={pts.length > 0 ? 6 : 3} + // No center/zoom: the map frames itself on the collection's places at mount, and + // falls back to the world view for a collection with none. tileUrl={tileUrl} fitKey={pts.length} /> diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index b1cd71ea5..8d843e6e9 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -18,7 +18,10 @@ const mapMock = vi.hoisted(() => ({ })) vi.mock('react-leaflet', () => ({ - MapContainer: ({ children }: any) =>
{children}
, + // center/zoom are surfaced so tests can assert the camera the map is built with. + MapContainer: ({ children, center, zoom }: any) => ( +
{children}
+ ), TileLayer: () =>
, Marker: ({ children, eventHandlers, position }: any) => (
{ buildMapPlace({ id: 1, lat: 48.0, lng: 2.0 }), buildMapPlace({ id: 2, lat: 48.1, lng: 2.1 }), ] - // Day selected, route not computed yet → first fit is the two destinations. + // The map opens already framed on its places, so nothing fits on mount. const { rerender } = render() const lastBounds = () => { const c = L.latLngBounds.mock.calls; return c[c.length - 1][0] } + + // Day selected, route not computed yet → first fit is the two destinations. + L.latLngBounds.mockClear() + rerender() expect(lastBounds()).toHaveLength(2) // The day's route arrives → one-shot re-fit including the 3 route points. L.latLngBounds.mockClear() - rerender() + rerender() expect(L.latLngBounds).toHaveBeenCalled() expect(lastBounds()).toHaveLength(5) // 2 destinations + 3 route points }) + + describe('opening camera', () => { + const camera = () => { + const el = screen.getByTestId('map-container') + return { + center: JSON.parse(el.getAttribute('data-center')!) as [number, number], + zoom: Number(el.getAttribute('data-zoom')), + } + } + + it('FE-COMP-MAPVIEW-021: builds the map framed on the places', () => { + render() + + const { center, zoom } = camera() + expect(center[0]).toBeCloseTo(34.85, 1) + expect(center[1]).toBeCloseTo(135.63, 1) + expect(zoom).toBeGreaterThan(7) + expect(zoom).toBeLessThan(13) + }) + + it('FE-COMP-MAPVIEW-022: does not fit on mount when it opened already framed', async () => { + const L = ((await import('leaflet')).default) as unknown as { latLngBounds: ReturnType } + L.latLngBounds.mockClear() + + render() + + expect(L.latLngBounds).not.toHaveBeenCalled() + }) + + it('FE-COMP-MAPVIEW-023: falls back to the world view when no place has coordinates', () => { + render() + + const { center, zoom } = camera() + expect(center).toEqual([0, 0]) + expect(zoom).toBe(2) + }) + }) }) diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index bf9e13c37..862670db5 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -13,6 +13,8 @@ import { PluginMapMarkers } from './MapPluginMarkers' import { useTransportRoutes } from '../../hooks/useTransportRoutes' import type { Reservation } from '../../types' import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories' +import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults' +import { computeMapViewport, TILE_SIZE_RASTER, type ViewportPadding } from '../../utils/mapViewport' function categoryIconSvg(iconName: string | null | undefined, size: number): string { const IconComponent = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin'] @@ -241,12 +243,15 @@ interface BoundsControllerProps { 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 } -function BoundsController({ places, routeCoords, fitKey, paddingOpts, hasDayDetail }: BoundsControllerProps) { +function BoundsController({ places, routeCoords, fitKey, paddingOpts, hasDayDetail, framedOnMount = false }: BoundsControllerProps) { const map = useMap() const prevFitKey = useRef(-1) const awaitingRoute = useRef(false) + const fitRan = useRef(false) const fitTo = useCallback((coords: [number, number][]) => { if (coords.length === 0) return @@ -268,6 +273,14 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, hasDayDeta prevFitKey.current = fitKey awaitingRoute.current = false if (places.length === 0) return + // The map opened framed on these very places — re-fitting would only re-do that, and its + // maxZoom would overrule the gentler zoom a single place opens at. Later fits (picking a + // day) still run. + if (!fitRan.current && framedOnMount) { + fitRan.current = true + return + } + fitRan.current = true fitTo(places.map(p => [p.lat, p.lng] as [number, number])) awaitingRoute.current = true }, [fitKey]) // eslint-disable-line react-hooks/exhaustive-deps @@ -435,8 +448,8 @@ export const MapView = memo(function MapView({ onMarkerClick, onMapClick, onMapContextMenu = null, - center = [48.8566, 2.3522], - zoom = 10, + center = DEFAULT_MAP_CENTER, + zoom = DEFAULT_MAP_ZOOM, tileUrl = 'https://{s}.basemaps.cartocdn.com/light_all/{z}/{x}/{y}{r}.png', fitKey = 0, dayOrderMap = {}, @@ -474,16 +487,37 @@ export const MapView = memo(function MapView({ // Real road geometry for car/bus/taxi/bicycle bookings (straight line until it loads/if it fails). const transportRoutes = useTransportRoutes(visibleReservations) // Dynamic padding: account for sidebars + bottom inspector + day detail panel - const paddingOpts = useMemo((): L.FitBoundsOptions => { + // The chrome overlaying the map (side panels, day detail). Kept as a plain box so both the + // Leaflet fit options and the opening-camera maths can read the same numbers. + const paddingBox = useMemo((): ViewportPadding => { const isMobile = typeof window !== 'undefined' && window.innerWidth < 768 - if (isMobile) return { padding: [40, 20] } - const top = 60 - const bottom = hasInspector ? 320 : hasDayDetail ? 280 : 60 - const left = leftWidth + 40 - const right = rightWidth + 40 - return { paddingTopLeft: [left, top], paddingBottomRight: [right, bottom] } + if (isMobile) return { top: 20, right: 40, bottom: 20, left: 40 } + return { + top: 60, + right: rightWidth + 40, + bottom: hasInspector ? 320 : hasDayDetail ? 280 : 60, + left: leftWidth + 40, + } }, [leftWidth, rightWidth, hasInspector, hasDayDetail]) + const paddingOpts = useMemo((): L.FitBoundsOptions => ({ + paddingTopLeft: [paddingBox.left, paddingBox.top], + paddingBottomRight: [paddingBox.right, paddingBox.bottom], + }), [paddingBox]) + + // Open framed on the places rather than on the caller's default, so a trip in Japan shows + // Japan straight away instead of the world view followed by a flight across the planet. + // The initializer runs once, at mount — exactly when this should be decided; afterwards the + // camera belongs to the user. `framed` is false when no place has coordinates (a new trip), + // and then the caller's center/zoom stands. + const [initialView] = useState(() => { + const framed = computeMapViewport(dayPlaces.length > 0 ? dayPlaces : places, { + tileSize: TILE_SIZE_RASTER, + padding: paddingBox, + }) + return { center: framed?.center ?? center, zoom: framed?.zoom ?? zoom, framed: framed !== null } + }) + // Hover state for the single tooltip overlay (replaces per-marker ) const [hoveredPlace, setHoveredPlace] = useState(null) const [tooltipPos, setTooltipPos] = useState<{ x: number; y: number } | null>(null) @@ -656,8 +690,8 @@ export const MapView = memo(function MapView({
@@ -672,7 +706,7 @@ export const MapView = memo(function MapView({ /> - 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} hasDayDetail={hasDayDetail} /> + 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} hasDayDetail={hasDayDetail} framedOnMount={initialView.framed} /> diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx index f8786b106..a4a30831b 100644 --- a/client/src/components/Map/MapViewGL.test.tsx +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -5,6 +5,8 @@ import { act } from '@testing-library/react' import { resetAllStores } from '../../../tests/helpers/store' import { buildPlace } from '../../../tests/helpers/factories' import { useSettingsStore } from '../../store/settingsStore' +import maplibregl from 'maplibre-gl' +import { DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults' // Stable fake map so fitBounds call counts survive re-renders. The canvas // container is a single element so listeners registered by the component are @@ -37,6 +39,23 @@ const glMap = vi.hoisted(() => ({ easeTo: vi.fn(), })) +const glBounds = vi.hoisted(() => { + const state = { + instances: [] as Array<{ extend: ReturnType }>, + } + return { + get instances() { return state.instances }, + clear: () => { state.instances = [] }, + create: () => { + const bounds = { + extend: vi.fn(() => bounds), + } + state.instances.push(bounds) + return bounds + }, + } +}) + vi.mock('mapbox-gl', () => ({ default: { accessToken: '', @@ -52,7 +71,7 @@ vi.mock('mapbox-gl', () => ({ } }), LngLatBounds: vi.fn(function () { - return { extend: vi.fn().mockReturnThis() } + return glBounds.create() }), NavigationControl: vi.fn(), Popup: vi.fn(function () { @@ -81,7 +100,7 @@ vi.mock('maplibre-gl', () => ({ } }), LngLatBounds: vi.fn(function () { - return { extend: vi.fn().mockReturnThis() } + return glBounds.create() }), NavigationControl: vi.fn(), Popup: vi.fn(function () { @@ -148,6 +167,7 @@ beforeEach(() => { glMap.on.mockImplementation(() => glMap) glMap.off.mockImplementation(() => glMap) glMap.once.mockImplementation(() => glMap) + glMap.loaded.mockReturnValue(true) glMap.getSource.mockReturnValue(null) glMap.getLayer.mockReturnValue(null) glMap.queryRenderedFeatures.mockReturnValue([]) @@ -165,6 +185,7 @@ beforeEach(() => { afterEach(() => { vi.clearAllMocks() + glBounds.clear() resetAllStores() }) @@ -472,4 +493,187 @@ describe('MapViewGL', () => { act(() => { el.dispatchEvent(new MouseEvent('mouseenter', { clientX: 10, clientY: 10 })) }) expect(queryByTestId('tooltip')).toBeTruthy() }) + + // The map opens already framed on its places, so these exercise the fits that happen + // afterwards — picking a day bumps fitKey. + it('FE-COMP-MAPVIEWGL-014: fits bounds immediately even when MapLibre loaded() is false', async () => { + glMap.loaded.mockReturnValue(false) + const places = [ + buildMapPlace({ id: 1, lat: 35.38, lng: 136.94 }), + buildMapPlace({ id: 2, lat: 35.42, lng: 136.76 }), + ] + + const { rerender } = render( + , + ) + await act(async () => {}) + + rerender() + await act(async () => {}) + + expect(glMap.fitBounds).toHaveBeenCalled() + }) + + it('FE-COMP-MAPVIEWGL-015: fits MapLibre bounds to route geometry when it arrives after a day fit', async () => { + const dayPlaces = [ + buildMapPlace({ id: 1, lat: 35.38, lng: 136.94 }), + buildMapPlace({ id: 2, lat: 35.42, lng: 136.76 }), + ] + // The day's route is drawn as straight lines in the same batch as the fit, then + // upgraded to the real road geometry — which detours well outside the markers. + const straightLines: [number, number][][] = [[[35.38, 136.94], [35.42, 136.76]]] + const roadGeometry: [number, number][][] = [[[35.38, 136.94], [35.72, 137.51], [35.42, 136.76]]] + + const { rerender } = render( + , + ) + await act(async () => {}) + + // Pick a day: fits the markers, with only the straight-line route to go on so far. + rerender( + , + ) + await act(async () => {}) + const afterDayFit = glMap.fitBounds.mock.calls.length + expect(afterDayFit).toBeGreaterThan(0) + + // The real geometry lands a moment later and the fit widens to take it in. + rerender( + , + ) + await act(async () => {}) + + expect(glMap.fitBounds.mock.calls.length).toBeGreaterThan(afterDayFit) + const latestBounds = glBounds.instances[glBounds.instances.length - 1] + expect(latestBounds.extend).toHaveBeenCalledWith([137.51, 35.72]) + }) + + describe('opening camera', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mapOptions = () => (maplibregl.Map as any).mock.calls.at(-1)[0] + + it('FE-COMP-MAPVIEWGL-017: builds the map framed on the places, in [lng, lat] order', async () => { + const places = [ + buildMapPlace({ id: 1, lat: 35.01, lng: 135.76 }), // Kyoto + buildMapPlace({ id: 2, lat: 34.69, lng: 135.5 }), // Osaka + ] + + render() + await act(async () => {}) + + const { center, zoom } = mapOptions() + // GL takes [lng, lat] — the swap is the easiest thing to get backwards here. + expect(center[0]).toBeCloseTo(135.63, 1) + expect(center[1]).toBeCloseTo(34.85, 1) + // Framed on two cities ~30km apart: regional, not the world and not street level. + expect(zoom).toBeGreaterThan(6) + expect(zoom).toBeLessThan(12) + }) + + it('FE-COMP-MAPVIEWGL-020: does not jump to the default centre on mount, undoing the framing', async () => { + const places = [buildMapPlace({ id: 1, lat: 35.01, lng: 135.76 })] + + render() + await act(async () => {}) + + // The centre prop is the world-view default nobody passed. Jumping to it on mount would + // throw away the camera the map was just built with and land on Null Island at zoom 2. + expect(glMap.jumpTo).not.toHaveBeenCalled() + }) + + it('FE-COMP-MAPVIEWGL-018: does not fit on mount when it opened already framed', async () => { + const places = [buildMapPlace({ id: 1, lat: 35.01, lng: 135.76 })] + + const { rerender } = render() + await act(async () => {}) + + // Fitting would only re-do the framing, and its maxZoom would overrule the gentler + // zoom a lone place opens at. + expect(glMap.fitBounds).not.toHaveBeenCalled() + + // Picking a day still fits, as always. + rerender() + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalled() + }) + + it('FE-COMP-MAPVIEWGL-019: falls back to the world view when no place has coordinates', async () => { + render( + , + ) + await act(async () => {}) + + const { center, zoom } = mapOptions() + expect(center).toEqual([0, 0]) + expect(zoom).toBe(DEFAULT_MAP_ZOOM) + }) + }) + + it('FE-COMP-MAPVIEWGL-016: leaves the camera alone when a route appears long after the fit', async () => { + const dayPlaces = [ + buildMapPlace({ id: 1, lat: 35.38, lng: 136.94 }), + buildMapPlace({ id: 2, lat: 35.42, lng: 136.76 }), + ] + + const { rerender } = render( + , + ) + await act(async () => {}) + + // Pick a day with the route toggle off: no route is pending for this fit. + rerender( + , + ) + await act(async () => {}) + const afterDayFit = glMap.fitBounds.mock.calls.length + expect(afterDayFit).toBeGreaterThan(0) + + // Much later the user pans away and turns the route on. That is not the geometry this + // fit was waiting for, so the camera must stay put. + rerender( + , + ) + await act(async () => {}) + + expect(glMap.fitBounds.mock.calls.length).toBe(afterDayFit) + }) }) diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 50a84f981..0ba8fd298 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -18,6 +18,8 @@ import { useGeolocation } from '../../hooks/useGeolocation' import type { Place, Reservation } from '../../types' import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories' import { buildPoiPopupHtml } from './placePopup' +import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults' +import { computeMapViewport, TILE_SIZE_GL } from '../../utils/mapViewport' function categoryIconSvg(iconName: string | null | undefined, size: number): string { const IconComponent = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin'] @@ -43,6 +45,10 @@ function hasValidCoords(place: Place): place is PlaceWithCoords { return place.lat != null && place.lng != null && Number.isFinite(place.lat) && Number.isFinite(place.lng) } +function isValidCoordinate(coord: [number, number] | null | undefined): coord is [number, number] { + return !!coord && Number.isFinite(coord[0]) && Number.isFinite(coord[1]) +} + function buildPlaceClusterData(places: Place[]) { return { type: 'FeatureCollection' as const, @@ -191,8 +197,8 @@ export function MapViewGL({ onMarkerClick, onMapClick, onMapContextMenu = null, - center = [48.8566, 2.3522], - zoom = 10, + center = DEFAULT_MAP_CENTER, + zoom = DEFAULT_MAP_ZOOM, fitKey = 0, dayOrderMap = {}, leftWidth = 0, @@ -258,8 +264,8 @@ export function MapViewGL({ onReservationClickRef.current = onReservationClick // eslint-disable-next-line @typescript-eslint/no-explicit-any const poiMarkersRef = useRef([]) - // Single reusable hover popup (name/category/address card) shared by planned - // places and POI markers — mirrors the Leaflet map's hover tooltip. + // Single reusable hover popup for POI markers. Planned places use the + // cursor-following React tooltip below so they match the Leaflet map. // eslint-disable-next-line @typescript-eslint/no-explicit-any const popupRef = useRef(null) const onPoiClickRef = useRef(onPoiClick) @@ -275,17 +281,36 @@ export function MapViewGL({ onClickRefs.current.context = onMapContextMenu const hoverDisabledRef = useRef(hoverDisabled) hoverDisabledRef.current = hoverDisabled + const routeCoords = useMemo<[number, number][]>(() => (route || []).flat().filter(isValidCoordinate), [route]) + const routeFitKey = useMemo( + () => routeCoords.map(([lat, lng]) => `${lat.toFixed(6)},${lng.toFixed(6)}`).join('|'), + [routeCoords], + ) + // Set when the map was built already framed on its places, so the fit below knows there is + // nothing left to do on mount. + const framedOnMountRef = useRef(false) // Build/rebuild the map on provider/style/token/3d change useEffect(() => { if (!containerRef.current || (!isMapLibre && !mapboxToken)) return if (!isMapLibre) mapboxgl.accessToken = mapboxToken + // Open framed on the places rather than on the caller's default: a trip in Japan should + // show Japan straight away, not the world view followed by a flight across the planet. + // Reading them here is what makes this "on load" — the map is built once, and the trip's + // places are already loaded by then (TripPlannerPage holds a splash until they are). + const framed = computeMapViewport(dayPlaces.length > 0 ? dayPlaces : places, { + tileSize: TILE_SIZE_GL, + padding: paddingOpts, + }) + framedOnMountRef.current = framed !== null + const initial = framed ?? { center, zoom } + const mapOptions: Record = { container: containerRef.current, style: glStyle, - center: [center[1], center[0]], - zoom, + center: [initial.center[1], initial.center[0]], + zoom: initial.zoom, pitch: enableMapbox3d ? 45 : 0, attributionControl: true, antialias: mapboxQuality, @@ -940,17 +965,46 @@ export function MapViewGL({ return { top, right: rightWidth + 40, bottom, left: leftWidth + 40 } }, [leftWidth, rightWidth, hasInspector, hasDayDetail]) - const prevFitKey = useRef(-1) + const prevFitKey = useRef(-1) + const pendingRouteFitRef = useRef<{ fitKey: number | null; routeKey: string } | null>(null) + const fitRanRef = useRef(false) useEffect(() => { - if (fitKey === prevFitKey.current) return - prevFitKey.current = fitKey + const fitKeyChanged = fitKey !== prevFitKey.current + const routeArrivedForPendingFit = + !fitKeyChanged + && pendingRouteFitRef.current?.fitKey === fitKey + && !!routeFitKey + && routeFitKey !== pendingRouteFitRef.current.routeKey + if (!fitKeyChanged && !routeArrivedForPendingFit) return const map = mapRef.current if (!map) return + + // The map was built framed on these very places, so fitting now would only re-do that — + // and its maxZoom would overrule the gentler zoom a single place opens at. Adopt the + // current fitKey and stand down; every later fit (picking a day) still runs. + if (!fitRanRef.current && framedOnMountRef.current) { + fitRanRef.current = true + prevFitKey.current = fitKey + pendingRouteFitRef.current = null + return + } + fitRanRef.current = true + if (fitKeyChanged) { + prevFitKey.current = fitKey + // Only wait for better geometry when a route is already on screen: the day's + // route lands as straight lines in the same batch as the fit, then upgrades to + // the real road geometry a moment later. With no route drawn, none is coming for + // this fit — arming the slot anyway would let a route toggled on much later + // (after the user has panned somewhere else) yank the camera back. + pendingRouteFitRef.current = routeFitKey ? { fitKey, routeKey: routeFitKey } : null + } const target = dayPlaces.length > 0 ? dayPlaces : places - const valid = target.filter(p => p.lat && p.lng) - if (valid.length === 0) return + const markerPoints = target.filter(hasValidCoords).map(p => [p.lat, p.lng] as [number, number]) + const fitPoints = routeCoords.length > 0 ? [...routeCoords, ...markerPoints] : markerPoints + if (fitPoints.length === 0) return const bounds = new gl.LngLatBounds() - valid.forEach(p => bounds.extend([p.lng, p.lat])) + fitPoints.forEach(([lat, lng]) => bounds.extend([lng, lat])) + let fitted = false const run = () => { try { map.fitBounds(bounds, { @@ -959,11 +1013,13 @@ export function MapViewGL({ pitch: enableMapbox3d ? 45 : 0, duration: 400, }) + fitted = true } catch { /* noop */ } } - if (map.loaded()) run() - else map.once('load', run) - }, [fitKey]) // eslint-disable-line react-hooks/exhaustive-deps + run() + if (!fitted && typeof map.once === 'function') map.once('load', run) + if (routeArrivedForPendingFit) pendingRouteFitRef.current = null + }, [fitKey, routeFitKey]) // eslint-disable-line react-hooks/exhaustive-deps // flyTo selected place useEffect(() => { @@ -986,9 +1042,16 @@ export function MapViewGL({ }, [selectedPlaceId, enableMapbox3d]) // eslint-disable-line react-hooks/exhaustive-deps // External center/zoom prop changes — jump without animation + const jumpedToRef = useRef<[number, number] | null>(null) useEffect(() => { const map = mapRef.current if (!map) return + // Not on mount: the map was just built with its own camera, framed on the places, and + // jumping to the prop centre here would throw that away and land on the world view. + // This effect is for *changes* to the prop, which only arrive later. + const previous = jumpedToRef.current + jumpedToRef.current = [center[0], center[1]] + if (!previous || (previous[0] === center[0] && previous[1] === center[1])) return try { map.jumpTo({ center: [center[1], center[0]], zoom }) } catch { /* noop */ } }, [center[0], center[1]]) // eslint-disable-line react-hooks/exhaustive-deps diff --git a/client/src/components/Planner/ReservationsPanel.test.tsx b/client/src/components/Planner/ReservationsPanel.test.tsx index a9b931bc2..06e6a6d51 100644 --- a/client/src/components/Planner/ReservationsPanel.test.tsx +++ b/client/src/components/Planner/ReservationsPanel.test.tsx @@ -27,7 +27,7 @@ beforeEach(() => { resetAllStores(); seedStore(useAuthStore, { user: buildUser(), isAuthenticated: true }); seedStore(useTripStore, { trip: buildTrip({ id: 1 }) }); - seedStore(useSettingsStore, { settings: { time_format: '24h', blur_booking_codes: false, temperature_unit: 'celsius', language: 'en', dark_mode: false, default_currency: 'USD', default_lat: 48.8566, default_lng: 2.3522, default_zoom: 10, map_tile_url: '', show_place_description: false } }); + seedStore(useSettingsStore, { settings: { time_format: '24h', blur_booking_codes: false, temperature_unit: 'celsius', language: 'en', dark_mode: false, default_currency: 'USD', map_tile_url: '', show_place_description: false } }); }); describe('ReservationsPanel', () => { @@ -211,7 +211,7 @@ describe('ReservationsPanel', () => { }); it('FE-PLANNER-RESP-022: confirmation number is blurred when blur_booking_codes=true', () => { - seedStore(useSettingsStore, { settings: { time_format: '24h', blur_booking_codes: true, temperature_unit: 'celsius', language: 'en', dark_mode: false, default_currency: 'USD', default_lat: 48.8566, default_lng: 2.3522, default_zoom: 10, map_tile_url: '', show_place_description: false } }); + seedStore(useSettingsStore, { settings: { time_format: '24h', blur_booking_codes: true, temperature_unit: 'celsius', language: 'en', dark_mode: false, default_currency: 'USD', map_tile_url: '', show_place_description: false } }); const res = buildReservation({ confirmation_number: 'ABC123', status: 'confirmed' }); render(); const codeEl = screen.getByText('ABC123'); @@ -220,7 +220,7 @@ describe('ReservationsPanel', () => { it('FE-PLANNER-RESP-023: confirmation code revealed on hover when blurred', async () => { const user = userEvent.setup(); - seedStore(useSettingsStore, { settings: { time_format: '24h', blur_booking_codes: true, temperature_unit: 'celsius', language: 'en', dark_mode: false, default_currency: 'USD', default_lat: 48.8566, default_lng: 2.3522, default_zoom: 10, map_tile_url: '', show_place_description: false } }); + seedStore(useSettingsStore, { settings: { time_format: '24h', blur_booking_codes: true, temperature_unit: 'celsius', language: 'en', dark_mode: false, default_currency: 'USD', map_tile_url: '', show_place_description: false } }); const res = buildReservation({ confirmation_number: 'ABC123', status: 'confirmed' }); render(); const codeEl = screen.getByText('ABC123'); diff --git a/client/src/components/Settings/MapSettingsTab.test.tsx b/client/src/components/Settings/MapSettingsTab.test.tsx index 976bc97d7..8d7b8bf15 100644 --- a/client/src/components/Settings/MapSettingsTab.test.tsx +++ b/client/src/components/Settings/MapSettingsTab.test.tsx @@ -20,12 +20,7 @@ beforeEach(() => { vi.clearAllMocks(); seedStore(useAuthStore, { user: buildUser(), isAuthenticated: true }); seedStore(useSettingsStore, { - settings: buildSettings({ - map_tile_url: '', - default_lat: 48.8566, - default_lng: 2.3522, - default_zoom: 10, - }), + settings: buildSettings({ map_tile_url: '' }), updateSettings: vi.fn().mockResolvedValue(undefined), }); }); @@ -46,38 +41,10 @@ describe('MapSettingsTab', () => { expect(screen.getByText('Map Template')).toBeInTheDocument(); }); - it('FE-COMP-MAP-004: shows latitude and longitude inputs', () => { + it('FE-COMP-MAP-004: no longer offers a default map centre — each map frames its own places', () => { render(); - expect(screen.getByText('Latitude')).toBeInTheDocument(); - expect(screen.getByText('Longitude')).toBeInTheDocument(); - }); - - it('FE-COMP-MAP-005: latitude input is pre-filled from store settings', () => { - render(); - expect(screen.getByDisplayValue('48.8566')).toBeInTheDocument(); - }); - - it('FE-COMP-MAP-006: longitude input is pre-filled from store settings', () => { - render(); - expect(screen.getByDisplayValue('2.3522')).toBeInTheDocument(); - }); - - it('FE-COMP-MAP-007: typing in the latitude input updates its displayed value', async () => { - const user = userEvent.setup(); - render(); - const latInput = screen.getByDisplayValue('48.8566'); - await user.clear(latInput); - await user.type(latInput, '51.5'); - expect(screen.getByDisplayValue('51.5')).toBeInTheDocument(); - }); - - it('FE-COMP-MAP-008: typing in the longitude input updates its displayed value', async () => { - const user = userEvent.setup(); - render(); - const lngInput = screen.getByDisplayValue('2.3522'); - await user.clear(lngInput); - await user.type(lngInput, '-0.1'); - expect(screen.getByDisplayValue('-0.1')).toBeInTheDocument(); + expect(screen.queryByText('Latitude')).not.toBeInTheDocument(); + expect(screen.queryByText('Longitude')).not.toBeInTheDocument(); }); it('FE-COMP-MAP-009: tile URL text input is shown', () => { @@ -100,7 +67,7 @@ describe('MapSettingsTab', () => { const user = userEvent.setup(); const updateSettings = vi.fn().mockResolvedValue(undefined); seedStore(useSettingsStore, { - settings: buildSettings({ map_tile_url: '', default_lat: 48.8566, default_lng: 2.3522, default_zoom: 10 }), + settings: buildSettings({ map_tile_url: '' }), updateSettings, }); render(); @@ -108,27 +75,24 @@ describe('MapSettingsTab', () => { expect(updateSettings).toHaveBeenCalledTimes(1); expect(updateSettings).toHaveBeenCalledWith(expect.objectContaining({ map_tile_url: expect.any(String), - default_lat: expect.any(Number), - default_lng: expect.any(Number), - default_zoom: expect.any(Number), + map_provider: expect.any(String), })); }); - it('FE-COMP-MAP-012: Save Map parses numeric values correctly', async () => { + it('FE-COMP-MAP-012: Save Map no longer writes a default centre or zoom', async () => { const user = userEvent.setup(); const updateSettings = vi.fn().mockResolvedValue(undefined); seedStore(useSettingsStore, { - settings: buildSettings({ map_tile_url: '', default_lat: 48.8566, default_lng: 2.3522, default_zoom: 10 }), + settings: buildSettings({ map_tile_url: '' }), updateSettings, }); render(); await user.click(screen.getByText('Save Map')); - expect(updateSettings).toHaveBeenCalledWith(expect.objectContaining({ - map_tile_url: '', - default_lat: 48.8566, - default_lng: 2.3522, - default_zoom: 10, - })); + + const saved = updateSettings.mock.calls[0][0]; + expect(saved).not.toHaveProperty('default_lat'); + expect(saved).not.toHaveProperty('default_lng'); + expect(saved).not.toHaveProperty('default_zoom'); }); it('FE-COMP-MAP-013: Save Map button shows spinner while saving', async () => { @@ -156,16 +120,6 @@ describe('MapSettingsTab', () => { await screen.findByText('Save failed'); }); - it('FE-COMP-MAP-015: clicking the map updates lat/lng state', async () => { - const user = userEvent.setup(); - render(); - await user.click(screen.getByTestId('map-view')); - await waitFor(() => { - expect(screen.getByDisplayValue('51.5')).toBeInTheDocument(); - expect(screen.getByDisplayValue('-0.1')).toBeInTheDocument(); - }); - }); - it('FE-COMP-MAP-016: preset dropdown is rendered', () => { render(); expect(screen.getByText('Select template...')).toBeInTheDocument(); @@ -173,15 +127,14 @@ describe('MapSettingsTab', () => { it('FE-COMP-MAP-017: settings update from store syncs local state', async () => { const { rerender } = render(); - expect(screen.getByDisplayValue('48.8566')).toBeInTheDocument(); seedStore(useSettingsStore, { - settings: buildSettings({ default_lat: 40.0 }), + settings: buildSettings({ map_tile_url: 'https://custom.tiles/{z}/{x}/{y}.png' }), }); rerender(); await waitFor(() => { - expect(screen.getByDisplayValue('40')).toBeInTheDocument(); + expect(screen.getByDisplayValue('https://custom.tiles/{z}/{x}/{y}.png')).toBeInTheDocument(); }); }); }); diff --git a/client/src/components/Settings/MapSettingsTab.tsx b/client/src/components/Settings/MapSettingsTab.tsx index 30f6fbcfb..36601e362 100644 --- a/client/src/components/Settings/MapSettingsTab.tsx +++ b/client/src/components/Settings/MapSettingsTab.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useCallback, useMemo, useRef } from 'react' +import React, { useState, useEffect, useMemo, useRef } from 'react' import { Map, Save, Layers, Box, ChevronDown, Check, Globe2 } from 'lucide-react' import { useTranslation } from '../../i18n' import { useSettingsStore } from '../../store/settingsStore' @@ -9,7 +9,6 @@ import GlMapPreview from './MapboxPreview' import Section from './Section' import ToggleSwitch from './ToggleSwitch' import type { Place } from '../../types' -import { NumericInput } from '../shared/NumericInput' import { MAPBOX_DEFAULT_STYLE, defaultStyleForProvider, @@ -143,6 +142,14 @@ function slotStyle(provider: Provider, s: { mapbox_style?: string; maplibre_styl return provider === 'maplibre-gl' ? s.maplibre_style : s.mapbox_style } +/** + * Somewhere recognisable for the style preview to render. A city shows off label density, + * 3D buildings and satellite texture in a way open ocean cannot — it is not a user setting, + * and no map opens here: each map frames itself on its own places. + */ +const PREVIEW_CENTER: [number, number] = [48.8566, 2.3522] +const PREVIEW_ZOOM = 16 + export default function MapSettingsTab(): React.ReactElement { const { settings, updateSettings } = useSettingsStore() const { t } = useTranslation() @@ -155,9 +162,6 @@ export default function MapSettingsTab(): React.ReactElement { const [mapboxStyle, setMapboxStyle] = useState(styleForProvider(initialProvider, slotStyle(initialProvider, settings))) const [mapbox3d, setMapbox3d] = useState(settings.mapbox_3d_enabled !== false) const [mapboxQuality, setMapboxQuality] = useState(settings.mapbox_quality_mode === true) - const [defaultLat, setDefaultLat] = useState(settings.default_lat || 48.8566) - const [defaultLng, setDefaultLng] = useState(settings.default_lng || 2.3522) - const [defaultZoom, setDefaultZoom] = useState(settings.default_zoom || 10) useEffect(() => { const nextProvider = normalizeProvider(settings.map_provider) @@ -167,23 +171,15 @@ export default function MapSettingsTab(): React.ReactElement { setMapboxStyle(styleForProvider(nextProvider, slotStyle(nextProvider, settings))) setMapbox3d(settings.mapbox_3d_enabled !== false) setMapboxQuality(settings.mapbox_quality_mode === true) - setDefaultLat(settings.default_lat || 48.8566) - setDefaultLng(settings.default_lng || 2.3522) - setDefaultZoom(settings.default_zoom || 10) }, [settings]) - const handleMapClick = useCallback((mapInfo) => { - setDefaultLat(mapInfo.latlng.lat) - setDefaultLng(mapInfo.latlng.lng) - }, []) - - const mapPlaces = useMemo((): Place[] => [{ + const previewPlaces = useMemo((): Place[] => [{ id: 1, trip_id: 1, - name: 'Default map center', + name: 'Preview', description: '', - lat: defaultLat as number, - lng: defaultLng as number, + lat: PREVIEW_CENTER[0], + lng: PREVIEW_CENTER[1], address: '', category_id: 0, price: null, @@ -194,7 +190,7 @@ export default function MapSettingsTab(): React.ReactElement { place_time: null, end_time: null, created_at: Date(), - }], [defaultLat, defaultLng]) + }], []) const saveMapSettings = async (): Promise => { setSaving(true) @@ -210,9 +206,6 @@ export default function MapSettingsTab(): React.ReactElement { ...stylePatch, mapbox_3d_enabled: mapbox3d, mapbox_quality_mode: mapboxQuality, - default_lat: parseFloat(String(defaultLat)), - default_lng: parseFloat(String(defaultLng)), - default_zoom: parseInt(String(defaultZoom)), }) toast.success(t('settings.toast.mapSaved')) } catch (err: unknown) { @@ -402,28 +395,6 @@ export default function MapSettingsTab(): React.ReactElement {
)} - {/* Default map position — applies regardless of provider */} -
-
- - -
-
- - -
-
-
{provider !== 'leaflet' ? ( @@ -431,28 +402,25 @@ export default function MapSettingsTab(): React.ReactElement { provider={provider} token={mapboxToken} style={mapboxStyle} - lat={parseFloat(String(defaultLat)) || 48.8566} - lng={parseFloat(String(defaultLng)) || 2.3522} + lat={PREVIEW_CENTER[0]} + lng={PREVIEW_CENTER[1]} // Zoom in close so the style's character (3D buildings, // satellite texture, label density) is immediately visible. - zoom={Math.max(parseInt(String(defaultZoom)) || 10, 16)} + zoom={PREVIEW_ZOOM} enable3d={provider === 'mapbox-gl' && mapbox3d && supports3d} quality={provider === 'mapbox-gl' && mapboxQuality} - onClick={(ll) => { setDefaultLat(ll.lat); setDefaultLng(ll.lng) }} /> ) : ( // eslint-disable-next-line @typescript-eslint/no-explicit-any React.createElement(MapView as any, { - places: mapPlaces, + places: previewPlaces, dayPlaces: [], route: null, routeSegments: null, selectedPlaceId: null, onMarkerClick: null, - onMapClick: handleMapClick, + onMapClick: null, onMapContextMenu: null, - center: [settings.default_lat, settings.default_lng], - zoom: defaultZoom, tileUrl: mapTileUrl, fitKey: null, dayOrderMap: [], diff --git a/client/src/constants/mapDefaults.ts b/client/src/constants/mapDefaults.ts new file mode 100644 index 000000000..27aebff8c --- /dev/null +++ b/client/src/constants/mapDefaults.ts @@ -0,0 +1,4 @@ +export const DEFAULT_MAP_LAT = 0 +export const DEFAULT_MAP_LNG = 0 +export const DEFAULT_MAP_ZOOM = 2 +export const DEFAULT_MAP_CENTER: [number, number] = [DEFAULT_MAP_LAT, DEFAULT_MAP_LNG] diff --git a/client/src/pages/DashboardPage.test.tsx b/client/src/pages/DashboardPage.test.tsx index 7948856d3..d87279808 100644 --- a/client/src/pages/DashboardPage.test.tsx +++ b/client/src/pages/DashboardPage.test.tsx @@ -858,9 +858,6 @@ describe('DashboardPage', () => { seedStore(useSettingsStore, { settings: { map_tile_url: '', - default_lat: 48.8566, - default_lng: 2.3522, - default_zoom: 10, dark_mode: 'auto', default_currency: 'USD', language: 'en', diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index 14ac67762..8df436b46 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -1,5 +1,7 @@ -import { useEffect } from 'react' +import { useEffect, useRef } from 'react' import { avatarSrc } from '../utils/avatarSrc' +import { computeMapViewport, TILE_SIZE_RASTER } from '../utils/mapViewport' +import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../constants/mapDefaults' import { MapContainer, TileLayer, Marker, Tooltip, useMap } from 'react-leaflet' import L from 'leaflet' import { useTranslation, SUPPORTED_LANGUAGES } from '../i18n' @@ -30,13 +32,21 @@ function createMarkerIcon(place: any) { }) } -function FitBoundsToPlaces({ places }: { places: any[] }) { +function FitBoundsToPlaces({ places, framedOnMount }: { places: any[]; framedOnMount: boolean }) { const map = useMap() + const fitRan = useRef(false) useEffect(() => { if (places.length === 0) return + // The map already opened framed on these places; fitting again would only re-do it. + // Picking a day afterwards still refits to that day. + if (!fitRan.current && framedOnMount) { + fitRan.current = true + return + } + fitRan.current = true const bounds = L.latLngBounds(places.map(p => [p.lat, p.lng])) map.fitBounds(bounds, { padding: [40, 40], maxZoom: 14 }) - }, [places, map]) + }, [places, map]) // eslint-disable-line react-hooks/exhaustive-deps return null } @@ -70,7 +80,13 @@ export default function SharedTripPage() { ? (assignments[String(selectedDay)] || []).map((a: any) => a.place).filter((p: any) => p?.lat && p?.lng) : (places || []).filter((p: any) => p?.lat && p?.lng) - const center = mapPlaces.length > 0 ? [mapPlaces[0].lat, mapPlaces[0].lng] : [48.85, 2.35] + // Open framed on the trip's places instead of on Paris. MapContainer only reads center/zoom + // at mount, so recomputing this per render is free — and the fit below takes over from there. + const framed = computeMapViewport(mapPlaces, { + tileSize: TILE_SIZE_RASTER, + padding: { top: 40, right: 40, bottom: 40, left: 40 }, + }) + const initialView = framed ?? { center: DEFAULT_MAP_CENTER, zoom: DEFAULT_MAP_ZOOM } return (
@@ -164,9 +180,9 @@ export default function SharedTripPage() { {/* Map */} {activeTab === 'plan' && (<>
- + - + {mapPlaces.map((p: any) => ( {p.name} diff --git a/client/src/pages/TripPlannerPage.test.tsx b/client/src/pages/TripPlannerPage.test.tsx index 89dc4e8ef..cac64fe2a 100644 --- a/client/src/pages/TripPlannerPage.test.tsx +++ b/client/src/pages/TripPlannerPage.test.tsx @@ -12,8 +12,12 @@ import { server } from '../../tests/helpers/msw/server'; import { http, HttpResponse } from 'msw'; // Mock Leaflet-dependent components +const capturedMapViewProps: { current: Record } = { current: {} }; vi.mock('../components/Map/MapView', () => ({ - MapView: () => React.createElement('div', { 'data-testid': 'map-view' }), + MapView: (props: Record) => { + capturedMapViewProps.current = props; + return React.createElement('div', { 'data-testid': 'map-view' }); + }, })); vi.mock('react-leaflet', () => ({ @@ -227,6 +231,7 @@ beforeEach(() => { mockSelectAssignment.mockReset(); mockPlaceSelectionState.selectedPlaceId = null; mockPlaceSelectionState.selectedAssignmentId = null; + capturedMapViewProps.current = {}; capturedDayPlanSidebarProps.current = {}; capturedPlacesSidebarProps.current = {}; capturedReservationsPanelProps.current = {}; @@ -655,6 +660,50 @@ describe('TripPlannerPage', () => { capturedDayPlanSidebarProps.current.onSelectDay?.(day.id); }); }); + + it('bumps map fitKey even when selecting the already selected day', async () => { + vi.useFakeTimers(); + + const { day } = seedTripStore({ id: 42 }); + seedStore(useTripStore, { selectedDayId: day.id } as any); + + renderPlannerPage(42); + + act(() => { vi.runAllTimers(); }); + + vi.useRealTimers(); + + await waitFor(() => { + expect(screen.getByTestId('day-plan-sidebar')).toBeInTheDocument(); + }); + + const initialFitKey = capturedMapViewProps.current.fitKey; + + await act(async () => { + capturedDayPlanSidebarProps.current.onSelectDay?.(day.id); + }); + + await waitFor(() => { + expect(capturedMapViewProps.current.fitKey).toBe(initialFitKey + 1); + }); + }); + + it('leaves the opening camera to the map instead of passing a default centre', async () => { + vi.useFakeTimers(); + seedTripStore({ id: 42 }); + renderPlannerPage(42); + act(() => { vi.runAllTimers(); }); + vi.useRealTimers(); + + await waitFor(() => { + expect(screen.getByTestId('map-view')).toBeInTheDocument(); + }); + + // The map frames itself on the trip's places at mount; a center/zoom from settings + // would only fight that. + expect(capturedMapViewProps.current.center).toBeUndefined(); + expect(capturedMapViewProps.current.zoom).toBeUndefined(); + }); }); describe('FE-PAGE-PLANNER-020b: the transit (tram) action needs trip dates', () => { diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 7ba782f4d..0bc67673d 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -211,7 +211,7 @@ export default function TripPlannerPage(): React.ReactElement | null { handleAssignToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, handleUpdateDayTitle, handleSaveReservation, handleSaveTransport, handleDeleteReservation, selectedPlace, dayOrderMap, dayPlaces, - mapTileUrl, defaultCenter, defaultZoom, fontStyle, splashDone, + mapTileUrl, fontStyle, splashDone, } = useTripPlanner() const poi = usePoiExplore() @@ -320,8 +320,8 @@ export default function TripPlannerPage(): React.ReactElement | null { onMarkerClick={handleMarkerClick} onMapClick={handleMapClick} onMapContextMenu={handleMapContextMenu} - center={defaultCenter} - zoom={defaultZoom} + // No center/zoom: the map frames itself on the trip's places at mount, and + // falls back to the world view when the trip has none. tileUrl={mapTileUrl} fitKey={fitKey} dayOrderMap={dayOrderMap} diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 44d01957d..8d4514442 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -25,6 +25,7 @@ import { useAirtrailConnection } from '../../hooks/useAirtrailConnection' import { useIsTouch } from '../../hooks/useIsTouch' import { usePluginStore } from '../../store/pluginStore' import type { Accommodation, TripMember, Day, Place, Reservation } from '../../types' +import { DEFAULT_MAP_LAT, DEFAULT_MAP_LNG, DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults' import { resolvePoolAssignmentId } from './tripPlannerModel' /** @@ -358,12 +359,11 @@ export function useTripPlanner() { const { route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay } = useRouteCalculation({ assignments } as any, selectedDayId, routeShown, routeProfile, tripAccommodations) const handleSelectDay = useCallback((dayId: number | null, skipFit?: boolean) => { - const changed = dayId !== selectedDayId tripActions.setSelectedDay(dayId) - if (changed && !skipFit) setFitKey(k => k + 1) + if (!skipFit) setFitKey(k => k + 1) setMobileSidebarOpen(null) updateRouteForDay(dayId) - }, [updateRouteForDay, selectedDayId]) + }, [updateRouteForDay]) const handlePlaceClick = useCallback((placeId: number | null, assignmentId?: number | null) => { if (assignmentId) { @@ -877,8 +877,6 @@ export function useTripPlanner() { }, [selectedDayId, assignments]) const mapTileUrl = settings.map_tile_url || 'https://{s}.basemaps.cartocdn.com/light_all/{z}/{x}/{y}{r}.png' - const defaultCenter = [settings.default_lat || 48.8566, settings.default_lng || 2.3522] - const defaultZoom = settings.default_zoom || 10 const fontStyle = { fontFamily: "var(--font-system)" } @@ -925,6 +923,6 @@ export function useTripPlanner() { handleAssignToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, handleUpdateDayTitle, handleSaveReservation, handleSaveTransport, handleDeleteReservation, selectedPlace, dayOrderMap, dayPlaces, - mapTileUrl, defaultCenter, defaultZoom, fontStyle, splashDone, + mapTileUrl, fontStyle, splashDone, } } diff --git a/client/src/store/settingsStore.ts b/client/src/store/settingsStore.ts index 113e8ba3b..36f687e93 100644 --- a/client/src/store/settingsStore.ts +++ b/client/src/store/settingsStore.ts @@ -24,9 +24,6 @@ export const hasStoredLanguage = (): boolean => export const useSettingsStore = create((set, get) => ({ settings: { map_tile_url: '', - default_lat: 48.8566, - default_lng: 2.3522, - default_zoom: 10, dark_mode: false, // Empty = no personal display currency, so Costs falls back to the trip's own. default_currency: '', diff --git a/client/src/types.ts b/client/src/types.ts index e6a982ae1..edb95b9b6 100644 --- a/client/src/types.ts +++ b/client/src/types.ts @@ -106,9 +106,6 @@ export type DistanceUnit = 'metric' | 'imperial' export interface Settings { map_tile_url: string - default_lat: number - default_lng: number - default_zoom: number dark_mode: boolean | string /** Display currency for Costs. Empty/null = follow each trip's own currency. */ default_currency: string | null diff --git a/client/src/utils/mapViewport.test.ts b/client/src/utils/mapViewport.test.ts new file mode 100644 index 000000000..bcd8d0b36 --- /dev/null +++ b/client/src/utils/mapViewport.test.ts @@ -0,0 +1,226 @@ +import { describe, it, expect } from 'vitest' +import { + computeMapViewport, + MAX_ZOOM_GL, + MAX_ZOOM_RASTER, + SINGLE_PLACE_ZOOM_RASTER, + TILE_SIZE_GL, + TILE_SIZE_RASTER, +} from './mapViewport' + +// Pin the container size so no assertion depends on jsdom's window dimensions. +const VIEW = { width: 1000, height: 800 } as const + +const PARIS = { lat: 48.8566, lng: 2.3522 } +const LYON = { lat: 45.764, lng: 4.8357 } + +/** + * The invariant that actually matters: every place must land inside the container at the + * returned camera. Mirrors the Mercator projection the map uses. + */ +function screenPosition( + point: { lat: number; lng: number }, + viewport: { center: [number, number]; zoom: number }, + tileSize: number, + size = VIEW, + // Only MapLibre/Mapbox redraw a marker on the nearest copy of the world. Leaflet places it + // at its absolute projected position, so assuming a wrap here would hide exactly the bug + // that puts a marker off-screen in the real app. + wrapsWorld = false, +): { x: number; y: number } { + const project = (lat: number, lng: number) => { + const scale = tileSize * Math.pow(2, viewport.zoom) + const sin = Math.sin((lat * Math.PI) / 180) + return { + x: scale * (lng / 360 + 0.5), + y: scale * (0.5 - Math.log((1 + sin) / (1 - sin)) / (4 * Math.PI)), + } + } + const p = project(point.lat, point.lng) + const c = project(viewport.center[0], viewport.center[1]) + + const worldWidth = tileSize * Math.pow(2, viewport.zoom) + const raw = p.x - c.x + const dx = wrapsWorld + ? ((raw + worldWidth / 2) % worldWidth + worldWidth) % worldWidth - worldWidth / 2 + : raw + + return { x: dx + size.width / 2, y: p.y - c.y + size.height / 2 } +} + +describe('computeMapViewport', () => { + it('frames a set of places so all of them are on screen', () => { + const viewport = computeMapViewport([PARIS, LYON], { ...VIEW, tileSize: TILE_SIZE_RASTER })! + + // Latitude is the midpoint in projected space, so it sits near — but not exactly on — + // the arithmetic mean; Mercator stretches towards the poles. Longitude is linear. + expect(viewport.center[0]).toBeCloseTo((PARIS.lat + LYON.lat) / 2, 1) + expect(viewport.center[1]).toBeCloseTo((PARIS.lng + LYON.lng) / 2, 6) + + for (const place of [PARIS, LYON]) { + const { x, y } = screenPosition(place, viewport, TILE_SIZE_RASTER) + expect(x).toBeGreaterThanOrEqual(0) + expect(x).toBeLessThanOrEqual(VIEW.width) + expect(y).toBeGreaterThanOrEqual(0) + expect(y).toBeLessThanOrEqual(VIEW.height) + } + }) + + it('opens at city level for a single place', () => { + const viewport = computeMapViewport([PARIS], { ...VIEW, tileSize: TILE_SIZE_RASTER })! + + expect(viewport.center[0]).toBeCloseTo(PARIS.lat, 6) + expect(viewport.center[1]).toBeCloseTo(PARIS.lng, 6) + expect(viewport.zoom).toBe(SINGLE_PLACE_ZOOM_RASTER) + }) + + it('treats several places stacked on one spot as a single place, without dividing by zero', () => { + const viewport = computeMapViewport([PARIS, { ...PARIS }, { ...PARIS }], { + ...VIEW, + tileSize: TILE_SIZE_RASTER, + })! + + expect(Number.isFinite(viewport.zoom)).toBe(true) + expect(viewport.zoom).toBe(SINGLE_PLACE_ZOOM_RASTER) + }) + + it('frames places sharing a latitude, using the longitude span alone', () => { + const viewport = computeMapViewport( + [{ lat: 48.8566, lng: 2.0 }, { lat: 48.8566, lng: 4.0 }], + { ...VIEW, tileSize: TILE_SIZE_RASTER }, + )! + + expect(Number.isFinite(viewport.zoom)).toBe(true) + expect(viewport.zoom).toBeLessThan(MAX_ZOOM_RASTER) + expect(viewport.center[1]).toBeCloseTo(3.0, 3) + }) + + it('returns null when nothing has usable coordinates', () => { + expect(computeMapViewport([], VIEW)).toBeNull() + expect(computeMapViewport([{ lat: null, lng: null }], VIEW)).toBeNull() + expect(computeMapViewport([{ lat: 48.85, lng: null }], VIEW)).toBeNull() + expect(computeMapViewport([{ lat: NaN, lng: 2.35 }], VIEW)).toBeNull() + expect(computeMapViewport([{ lat: 200, lng: 2.35 }], VIEW)).toBeNull() + expect(computeMapViewport([{ lat: 48.85, lng: 999 }], VIEW)).toBeNull() + }) + + it('treats 0,0 as a real coordinate rather than a missing one', () => { + const viewport = computeMapViewport([{ lat: 0, lng: 0 }], { ...VIEW, tileSize: TILE_SIZE_RASTER }) + + expect(viewport).not.toBeNull() + expect(viewport!.center[0]).toBeCloseTo(0, 6) + expect(viewport!.center[1]).toBeCloseTo(0, 6) + }) + + const FIJI = { lat: -18.14, lng: 178.44 } + const SAMOA = { lat: -13.76, lng: -172.1 } + + it('takes the short way round the antimeridian on a renderer that wraps the world', () => { + // Fiji and Samoa are 10 degrees apart, not 350 — MapLibre/Mapbox draw each marker on the + // copy of the world nearest the camera, so this centre is safe there. + const viewport = computeMapViewport([FIJI, SAMOA], { ...VIEW, tileSize: TILE_SIZE_GL })! + + expect(viewport.center[1]).toBeCloseTo(-176.83, 1) + // A 350-degree span would have collapsed the zoom to the world view. + expect(viewport.zoom).toBeGreaterThan(3) + }) + + it('goes the long way round on Leaflet, which does not wrap markers', () => { + // Leaflet draws a single world and places markers at their absolute position. Centring on + // the antimeridian would leave one of these two outside it entirely — so span the 350 + // degrees instead, as L.latLngBounds would. + const viewport = computeMapViewport([FIJI, SAMOA], { ...VIEW, tileSize: TILE_SIZE_RASTER })! + + expect(viewport.center[1]).toBeCloseTo(3.17, 1) + // Nearly the whole globe: zoomed right out, not framed tight on the Pacific. + expect(viewport.zoom).toBeLessThanOrEqual(2) + + for (const place of [FIJI, SAMOA]) { + const { x } = screenPosition(place, viewport, TILE_SIZE_RASTER) + expect(x).toBeGreaterThanOrEqual(0) + expect(x).toBeLessThanOrEqual(VIEW.width) + } + }) + + it('clamps latitude at the poles instead of projecting to infinity', () => { + const viewport = computeMapViewport( + [{ lat: 90, lng: 0 }, { lat: 80, lng: 10 }], + { ...VIEW, tileSize: TILE_SIZE_RASTER }, + )! + + expect(Number.isFinite(viewport.zoom)).toBe(true) + expect(viewport.center[0]).toBeLessThanOrEqual(85.0511) + }) + + it('returns a GL zoom exactly one level below the raster zoom for the same view', () => { + const raster = computeMapViewport([PARIS, LYON], { ...VIEW, tileSize: TILE_SIZE_RASTER })! + const gl = computeMapViewport([PARIS, LYON], { ...VIEW, tileSize: TILE_SIZE_GL })! + + expect(gl.zoom).toBeCloseTo(raster.zoom - 1, 6) + expect(gl.center).toEqual(raster.center) + }) + + it('never zooms past the renderer maxZoom for two nearly identical places', () => { + const almost = { lat: PARIS.lat + 0.00001, lng: PARIS.lng + 0.00001 } + + expect(computeMapViewport([PARIS, almost], { ...VIEW, tileSize: TILE_SIZE_RASTER })!.zoom) + .toBe(MAX_ZOOM_RASTER) + expect(computeMapViewport([PARIS, almost], { ...VIEW, tileSize: TILE_SIZE_GL })!.zoom) + .toBe(MAX_ZOOM_GL) + }) + + // Sydney, Reykjavik and Santiago: the narrowest arc containing all three crosses the + // antimeridian. On Leaflet that centre put Sydney outside the one world it draws — the + // marker simply vanished. Assert against each renderer's real wrapping behaviour. + const GLOBE = [ + { lat: -33.87, lng: 151.21 }, // Sydney + { lat: 64.15, lng: -21.94 }, // Reykjavik + { lat: -33.45, lng: -70.67 }, // Santiago + ] + + it('keeps a globe-spanning trip on screen in Leaflet, which does not wrap', () => { + const viewport = computeMapViewport(GLOBE, { ...VIEW, tileSize: TILE_SIZE_RASTER })! + + expect(viewport.zoom).toBeGreaterThanOrEqual(0) + expect(viewport.zoom).toBeLessThan(5) + + for (const place of GLOBE) { + const { x, y } = screenPosition(place, viewport, TILE_SIZE_RASTER) + expect(x).toBeGreaterThanOrEqual(0) + expect(x).toBeLessThanOrEqual(VIEW.width) + expect(y).toBeGreaterThanOrEqual(0) + expect(y).toBeLessThanOrEqual(VIEW.height) + } + }) + + it('keeps a globe-spanning trip on screen in GL, which does wrap', () => { + const viewport = computeMapViewport(GLOBE, { ...VIEW, tileSize: TILE_SIZE_GL })! + + for (const place of GLOBE) { + const { x, y } = screenPosition(place, viewport, TILE_SIZE_GL, VIEW, true) + expect(x).toBeGreaterThanOrEqual(0) + expect(x).toBeLessThanOrEqual(VIEW.width) + expect(y).toBeGreaterThanOrEqual(0) + expect(y).toBeLessThanOrEqual(VIEW.height) + } + }) + + it('shifts the center so places clear a left-hand panel', () => { + const unpadded = computeMapViewport([PARIS, LYON], { ...VIEW, tileSize: TILE_SIZE_RASTER })! + const padded = computeMapViewport([PARIS, LYON], { + ...VIEW, + tileSize: TILE_SIZE_RASTER, + padding: { left: 400 }, + })! + + // The camera pulls west so the places sit right of the panel, in the visible area. + expect(padded.center[1]).toBeLessThan(unpadded.center[1]) + + // And they still fit: the panel eats 400px, so check against the visible width. + for (const place of [PARIS, LYON]) { + const { x } = screenPosition(place, padded, TILE_SIZE_RASTER) + expect(x).toBeGreaterThanOrEqual(400) + expect(x).toBeLessThanOrEqual(VIEW.width) + } + }) +}) diff --git a/client/src/utils/mapViewport.ts b/client/src/utils/mapViewport.ts new file mode 100644 index 000000000..7041e5e8c --- /dev/null +++ b/client/src/utils/mapViewport.ts @@ -0,0 +1,256 @@ +/** + * Frames a set of places: turns their coordinates into the camera (center + zoom) a map + * should open at, so a trip in Japan opens on Japan instead of on the world view followed + * by a fitBounds flight across the planet. + * + * Renderers call this once, at construction — see MapView.tsx / MapViewGL.tsx. Returns + * null when no point has usable coordinates (a brand-new trip), leaving the caller to fall + * back to DEFAULT_MAP_CENTER / DEFAULT_MAP_ZOOM. + * + * The bbox min/max here deliberately duplicates the few lines in sync/tilePrefetcher.ts + * rather than importing them: that module pulls in Dexie at import time, which has no place + * in a map's render path or in a pure-math unit test, and its padding + minimum-span rules + * are tuned for enumerating tiles, not for framing a camera. + */ + +/** Web Mercator diverges at the poles — projections clamp latitude to this. */ +const MERCATOR_MAX_LAT = 85.0511 + +/** Leaflet measures zoom against a 256px world tile; MapLibre/Mapbox against 512px. */ +export const TILE_SIZE_RASTER = 256 +export const TILE_SIZE_GL = 512 + +/** + * Match the maxZoom the fitBounds in each renderer already clamps to (MapView 16, + * MapViewGL 15), so the opening camera and any later fit agree instead of fighting. + */ +export const MAX_ZOOM_RASTER = 16 +export const MAX_ZOOM_GL = 15 + +/** + * One place (or several at the same spot) has no extent to fit, so zoom is a choice rather + * than a calculation: city level, showing the surroundings rather than just the pin. + * Expressed in the raster scheme; the GL scheme is one level lower for the same scale. + */ +export const SINGLE_PLACE_ZOOM_RASTER = 12 + +export interface MapViewport { + /** [lat, lng] — the order both renderers take as a prop (MapViewGL swaps it internally). */ + center: [number, number] + zoom: number +} + +/** Anything with coordinates: a Place, a CollectionPlace, a bare {lat, lng}. */ +export interface GeoPointish { + lat?: number | null + lng?: number | null +} + +export interface ViewportPadding { + top: number + right: number + bottom: number + left: number +} + +export interface ViewportOptions { + /** 256 for Leaflet, 512 for MapLibre/Mapbox. Drives both the zoom scale and maxZoom. */ + tileSize?: number + /** Map container size in CSS px. Defaults to the window, then to 1024x768 (SSR/jsdom). */ + width?: number + height?: number + /** Chrome overlaying the map (side panels). Shifts the center so places stay visible. */ + padding?: Partial + maxZoom?: number + singlePlaceZoom?: number + /** + * Does the renderer draw markers on the nearest copy of a repeating world? MapLibre/Mapbox + * do; Leaflet does not. Only a wrapping renderer can be framed across the antimeridian. + * Defaults to true for the GL tile scheme, false for the raster one. + */ + wrapsWorld?: boolean +} + +const NO_PADDING: ViewportPadding = { top: 0, right: 0, bottom: 0, left: 0 } + +const clampLat = (lat: number): number => + Math.max(-MERCATOR_MAX_LAT, Math.min(MERCATOR_MAX_LAT, lat)) + +/** Wrap into [-180, 180) so a center computed across the antimeridian stays a real longitude. */ +const normalizeLng = (lng: number): number => ((((lng + 180) % 360) + 360) % 360) - 180 + +/** + * Keep only points we can actually project. Note the Number.isFinite test: `lat && lng` + * would silently drop Null Island, the equator and the prime meridian — 0 is a coordinate. + */ +function usablePoints(points: readonly GeoPointish[]): Array<[number, number]> { + const out: Array<[number, number]> = [] + for (const point of points || []) { + const lat = point?.lat + const lng = point?.lng + if (typeof lat !== 'number' || typeof lng !== 'number') continue + if (!Number.isFinite(lat) || !Number.isFinite(lng)) continue + if (lat < -90 || lat > 90 || lng < -180 || lng > 180) continue + out.push([clampLat(lat), lng]) + } + return out +} + +/** Mercator y as a fraction of the world, in the [-π, π] convention the zoom math expects. */ +function latRad(lat: number): number { + const sin = Math.sin((clampLat(lat) * Math.PI) / 180) + const rad = Math.log((1 + sin) / (1 - sin)) / 2 + return Math.max(-Math.PI, Math.min(Math.PI, rad)) / 2 +} + +/** + * The narrowest arc of longitude containing every point, which may cross the antimeridian: + * a trip spanning Fiji (178) and Samoa (-172) is 10° wide, not 350°. + * + * Found by locating the widest *gap* between neighbouring longitudes — the places occupy + * everything the gap doesn't. Comparing plain min/max against its complement instead would + * be right for two points but wrong for three or more, where the complement of the min-max + * span can exclude a point sitting inside it. + * + * `east` may exceed 180 when the arc wraps; callers project it and let unproject wrap back. + * + * Only sound when the renderer wraps the world. MapLibre/Mapbox draw a marker on whichever + * copy of the world is nearest the camera; Leaflet does not, so a centre reached "the short + * way" across the antimeridian leaves markers on the far side of the single world it drew, + * off-screen entirely. For those, take the plain min..max arc — the long way round — which is + * also what L.latLngBounds would do. + */ +function lngExtent(lngs: number[], wrapsWorld: boolean): { west: number; east: number; span: number } { + const sorted = [...lngs].sort((a, b) => a - b) + const last = sorted.length - 1 + + if (!wrapsWorld) { + return { west: sorted[0], east: sorted[last], span: sorted[last] - sorted[0] } + } + + // The gap that wraps past the antimeridian, from the easternmost point back to the westernmost. + let widestGap = sorted[0] + 360 - sorted[last] + let west = sorted[0] + let east = sorted[last] + + for (let i = 0; i < last; i++) { + const gap = sorted[i + 1] - sorted[i] + if (gap > widestGap) { + widestGap = gap + // The arc resumes on the far side of the gap and runs east, across the antimeridian. + west = sorted[i + 1] + east = sorted[i] + 360 + } + } + + return { west, east, span: east - west } +} + +/** Zoom at which a span covering `fraction` of the world fills `px` pixels. */ +function zoomForFraction(px: number, tileSize: number, fraction: number, maxZoom: number): number { + // A zero fraction (identical coordinates) would divide by zero and yield Infinity. + if (!(fraction > 0)) return maxZoom + const zoom = Math.log2(px / tileSize / fraction) + return Number.isFinite(zoom) ? zoom : maxZoom +} + +function project(lat: number, lng: number, zoom: number, tileSize: number): { x: number; y: number } { + const scale = tileSize * Math.pow(2, zoom) + const sin = Math.sin((clampLat(lat) * Math.PI) / 180) + return { + x: scale * (lng / 360 + 0.5), + y: scale * (0.5 - Math.log((1 + sin) / (1 - sin)) / (4 * Math.PI)), + } +} + +function unproject(x: number, y: number, zoom: number, tileSize: number): [number, number] { + const scale = tileSize * Math.pow(2, zoom) + const lng = (x / scale - 0.5) * 360 + const n = Math.PI * (1 - 2 * (y / scale)) + const lat = (180 / Math.PI) * Math.atan(Math.sinh(n)) + return [lat, normalizeLng(lng)] +} + +/** + * The map renders its center at the middle of the container, but padding (side panels) + * means the *visible* middle sits elsewhere. Pull the center back by half the padding + * delta so the places land in the part of the map the user can actually see — the same + * thing fitBounds' padding option does. + */ +function offsetForPadding( + center: [number, number], + zoom: number, + tileSize: number, + padding: ViewportPadding, +): [number, number] { + const { top, right, bottom, left } = padding + if (!top && !right && !bottom && !left) return center + const { x, y } = project(center[0], center[1], zoom, tileSize) + return unproject(x - (left - right) / 2, y - (top - bottom) / 2, zoom, tileSize) +} + +function defaultWidth(): number { + return typeof window !== 'undefined' && window.innerWidth > 0 ? window.innerWidth : 1024 +} + +function defaultHeight(): number { + return typeof window !== 'undefined' && window.innerHeight > 0 ? window.innerHeight : 768 +} + +/** + * The camera that frames `points`, or null when none of them have coordinates. + */ +export function computeMapViewport( + points: readonly GeoPointish[], + options: ViewportOptions = {}, +): MapViewport | null { + const pts = usablePoints(points) + if (pts.length === 0) return null + + const tileSize = options.tileSize ?? TILE_SIZE_RASTER + const isGl = tileSize === TILE_SIZE_GL + const maxZoom = options.maxZoom ?? (isGl ? MAX_ZOOM_GL : MAX_ZOOM_RASTER) + const singlePlaceZoom = options.singlePlaceZoom + ?? (isGl ? SINGLE_PLACE_ZOOM_RASTER - 1 : SINGLE_PLACE_ZOOM_RASTER) + const wrapsWorld = options.wrapsWorld ?? isGl + + const padding: ViewportPadding = { ...NO_PADDING, ...options.padding } + const width = Math.max(1, (options.width ?? defaultWidth()) - padding.left - padding.right) + const height = Math.max(1, (options.height ?? defaultHeight()) - padding.top - padding.bottom) + + const lats = pts.map(p => p[0]) + const south = Math.min(...lats) + const north = Math.max(...lats) + const { west, east, span } = lngExtent(pts.map(p => p[1]), wrapsWorld) + + const latFraction = (latRad(north) - latRad(south)) / Math.PI + const lngFraction = span / 360 + + // No extent in either axis: one place, or several stacked on the same spot. + const zoom = latFraction <= 0 && lngFraction <= 0 + ? Math.min(singlePlaceZoom, maxZoom) + : Math.min( + zoomForFraction(height, tileSize, latFraction, maxZoom), + zoomForFraction(width, tileSize, lngFraction, maxZoom), + maxZoom, + ) + + // Floor rather than round: Leaflet snaps zoom to integers, and rounding up would crop + // the very places we are framing. + const safeZoom = Number.isFinite(zoom) ? Math.max(0, Math.floor(zoom * 100) / 100) : maxZoom + + // Take the midpoint in *projected* space, not the average of the latitudes: Mercator + // stretches towards the poles, so a geographic mid-latitude sits off-centre on screen and + // pushes the northernmost place out of frame. Projecting also carries the antimeridian + // handling for free — `east` may exceed 180, and unproject wraps it back. + const northWest = project(north, west, safeZoom, tileSize) + const southEast = project(south, east, safeZoom, tileSize) + const center = unproject( + (northWest.x + southEast.x) / 2, + (northWest.y + southEast.y) / 2, + safeZoom, + tileSize, + ) + + return { center: offsetForPadding(center, safeZoom, tileSize, padding), zoom: safeZoom } +} diff --git a/client/tests/helpers/factories.ts b/client/tests/helpers/factories.ts index fec8f1a42..58a825118 100644 --- a/client/tests/helpers/factories.ts +++ b/client/tests/helpers/factories.ts @@ -260,9 +260,6 @@ export function buildCategory(overrides: Partial = {}): Category { export function buildSettings(overrides: Partial = {}): Settings { return { map_tile_url: '', - default_lat: 48.8566, - default_lng: 2.3522, - default_zoom: 10, dark_mode: false, default_currency: 'USD', language: 'en', diff --git a/client/tests/unit/stores/settingsStore.test.ts b/client/tests/unit/stores/settingsStore.test.ts index d4cfa898f..eb26319a4 100644 --- a/client/tests/unit/stores/settingsStore.test.ts +++ b/client/tests/unit/stores/settingsStore.test.ts @@ -213,10 +213,10 @@ describe('settingsStore', () => { ); await expect( - useSettingsStore.getState().updateSetting('default_zoom', 15) + useSettingsStore.getState().updateSetting('default_currency', 'EUR') ).rejects.toThrow(); - expect(useSettingsStore.getState().settings.default_zoom).toBe(15); + expect(useSettingsStore.getState().settings.default_currency).toBe('EUR'); }); }); }); diff --git a/shared/src/i18n/ar/settings.ts b/shared/src/i18n/ar/settings.ts index 2d4b46d83..21a90912e 100644 --- a/shared/src/i18n/ar/settings.ts +++ b/shared/src/i18n/ar/settings.ts @@ -45,8 +45,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'نصيحة:', 'settings.mapTip': 'انقر بزر الماوس الأيمن واسحب لتدوير/إمالة الخريطة. النقر الأوسط لإضافة مكان (النقر الأيمن مخصص للتدوير).', - 'settings.latitude': 'خط العرض', - 'settings.longitude': 'خط الطول', 'settings.saveMap': 'حفظ الخريطة', 'settings.apiKeys': 'مفاتيح API', 'settings.mapsKey': 'مفتاح Google Maps API', diff --git a/shared/src/i18n/br/settings.ts b/shared/src/i18n/br/settings.ts index d70bba98a..94073e767 100644 --- a/shared/src/i18n/br/settings.ts +++ b/shared/src/i18n/br/settings.ts @@ -50,8 +50,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Dica:', 'settings.mapTip': 'Clique direito e arraste para girar/inclinar o mapa. Clique do meio para adicionar um local (o clique direito é reservado para rotação).', - 'settings.latitude': 'Latitude', - 'settings.longitude': 'Longitude', 'settings.saveMap': 'Salvar mapa', 'settings.apiKeys': 'Chaves de API', 'settings.mapsKey': 'Chave da API Google Maps', diff --git a/shared/src/i18n/cs/settings.ts b/shared/src/i18n/cs/settings.ts index dd30317fb..891b29571 100644 --- a/shared/src/i18n/cs/settings.ts +++ b/shared/src/i18n/cs/settings.ts @@ -47,8 +47,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tip:', 'settings.mapTip': 'Pravé tlačítko myši a táhněte pro rotaci/náklon mapy. Prostřední tlačítko pro přidání místa (pravé tlačítko je vyhrazeno pro rotaci).', - 'settings.latitude': 'Zeměpisná šířka', - 'settings.longitude': 'Zeměpisná délka', 'settings.saveMap': 'Uložit nastavení mapy', 'settings.apiKeys': 'API klíče', 'settings.mapsKey': 'Google Maps API klíč', diff --git a/shared/src/i18n/de/settings.ts b/shared/src/i18n/de/settings.ts index 6e8a2c110..36b94e295 100644 --- a/shared/src/i18n/de/settings.ts +++ b/shared/src/i18n/de/settings.ts @@ -49,8 +49,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tipp:', 'settings.mapTip': 'Rechtsklick und ziehen, um die Karte zu drehen/neigen. Mittelklick, um einen Ort hinzuzufügen (Rechtsklick ist für die Rotation reserviert).', - 'settings.latitude': 'Breitengrad', - 'settings.longitude': 'Längengrad', 'settings.saveMap': 'Karte speichern', 'settings.apiKeys': 'API-Schlüssel', 'settings.mapsKey': 'Google Maps API-Schlüssel', diff --git a/shared/src/i18n/en/settings.ts b/shared/src/i18n/en/settings.ts index 1c9f168a4..424ae2930 100644 --- a/shared/src/i18n/en/settings.ts +++ b/shared/src/i18n/en/settings.ts @@ -56,8 +56,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tip:', 'settings.mapTip': 'right-click and drag to rotate/pitch the map. Middle-click to add a place (right-click is reserved for rotation).', - 'settings.latitude': 'Latitude', - 'settings.longitude': 'Longitude', 'settings.saveMap': 'Save Map', 'settings.apiKeys': 'API Keys', 'settings.mapsKey': 'Google Maps API Key', diff --git a/shared/src/i18n/es/settings.ts b/shared/src/i18n/es/settings.ts index 8bf3b8cc6..0f6c064e6 100644 --- a/shared/src/i18n/es/settings.ts +++ b/shared/src/i18n/es/settings.ts @@ -50,8 +50,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Consejo:', 'settings.mapTip': 'Clic derecho y arrastrar para rotar/inclinar el mapa. Clic central para añadir un lugar (el clic derecho está reservado para la rotación).', - 'settings.latitude': 'Latitud', - 'settings.longitude': 'Longitud', 'settings.saveMap': 'Guardar mapa', 'settings.apiKeys': 'Claves API', 'settings.mapsKey': 'Clave API de Google Maps', diff --git a/shared/src/i18n/fr/settings.ts b/shared/src/i18n/fr/settings.ts index 15b46064c..dc7b0daf4 100644 --- a/shared/src/i18n/fr/settings.ts +++ b/shared/src/i18n/fr/settings.ts @@ -50,8 +50,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Astuce :', 'settings.mapTip': 'Clic droit et glisser pour pivoter/incliner la carte. Clic milieu pour ajouter un lieu (le clic droit est réservé à la rotation).', - 'settings.latitude': 'Latitude', - 'settings.longitude': 'Longitude', 'settings.saveMap': 'Enregistrer la carte', 'settings.apiKeys': 'Clés API', 'settings.mapsKey': 'Clé API Google Maps', diff --git a/shared/src/i18n/gr/settings.ts b/shared/src/i18n/gr/settings.ts index 1f75e8dd4..598f84cb8 100644 --- a/shared/src/i18n/gr/settings.ts +++ b/shared/src/i18n/gr/settings.ts @@ -52,8 +52,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Συμβουλή:', 'settings.mapTip': 'κάντε δεξί κλικ και σύρετε για περιστροφή/κλίση του χάρτη. Μεσαίο κλικ για προσθήκη τοποθεσίας (το δεξί κλικ έχει κρατηθεί για περιστροφή).', - 'settings.latitude': 'Γεωγραφικό Πλάτος', - 'settings.longitude': 'Γεωγραφικό Μήκος', 'settings.saveMap': 'Αποθήκευση Χάρτη', 'settings.apiKeys': 'Κλειδιά API', 'settings.mapsKey': 'Google Maps API Key', diff --git a/shared/src/i18n/hu/settings.ts b/shared/src/i18n/hu/settings.ts index a33d1d36c..e2520eec8 100644 --- a/shared/src/i18n/hu/settings.ts +++ b/shared/src/i18n/hu/settings.ts @@ -49,8 +49,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tipp:', 'settings.mapTip': 'Jobb klikk és húzás a térkép forgatásához/döntéséhez. Középső kattintás hely hozzáadásához (a jobb klikk a forgatáshoz van fenntartva).', - 'settings.latitude': 'Szélességi fok', - 'settings.longitude': 'Hosszúsági fok', 'settings.saveMap': 'Térkép mentése', 'settings.apiKeys': 'API kulcsok', 'settings.mapsKey': 'Google Maps API kulcs', diff --git a/shared/src/i18n/id/settings.ts b/shared/src/i18n/id/settings.ts index 59747ae5b..7594b7e45 100644 --- a/shared/src/i18n/id/settings.ts +++ b/shared/src/i18n/id/settings.ts @@ -48,8 +48,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tip:', 'settings.mapTip': 'Klik kanan dan seret untuk memutar/memiringkan peta. Klik tengah untuk menambah tempat (klik kanan untuk rotasi).', - 'settings.latitude': 'Lintang', - 'settings.longitude': 'Bujur', 'settings.saveMap': 'Simpan Peta', 'settings.apiKeys': 'API Keys', 'settings.mapsKey': 'Google Maps API Key', diff --git a/shared/src/i18n/it/settings.ts b/shared/src/i18n/it/settings.ts index a65c1ffea..9d0cc9044 100644 --- a/shared/src/i18n/it/settings.ts +++ b/shared/src/i18n/it/settings.ts @@ -50,8 +50,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Suggerimento:', 'settings.mapTip': 'Click destro e trascina per ruotare/inclinare la mappa. Click centrale per aggiungere un luogo (il click destro è riservato alla rotazione).', - 'settings.latitude': 'Latitudine', - 'settings.longitude': 'Longitudine', 'settings.saveMap': 'Salva Mappa', 'settings.apiKeys': 'Chiavi API', 'settings.mapsKey': 'Chiave API Google Maps', diff --git a/shared/src/i18n/ja/settings.ts b/shared/src/i18n/ja/settings.ts index 39e7c05eb..ebaeffa4a 100644 --- a/shared/src/i18n/ja/settings.ts +++ b/shared/src/i18n/ja/settings.ts @@ -47,8 +47,6 @@ const settings: TranslationStrings = { 'settings.mapHighQualityWarning': '低性能デバイスではパフォーマンスに影響する場合があります。', 'settings.mapTipLabel': 'ヒント:', 'settings.mapTip': '右クリック+ドラッグで回転/傾き。中クリックで場所を追加(右クリックは回転用)。', - 'settings.latitude': '緯度', - 'settings.longitude': '経度', 'settings.saveMap': '地図を保存', 'settings.apiKeys': 'APIキー', 'settings.mapsKey': 'Google Maps APIキー', diff --git a/shared/src/i18n/ko/settings.ts b/shared/src/i18n/ko/settings.ts index a3250db5f..67c75a3f7 100644 --- a/shared/src/i18n/ko/settings.ts +++ b/shared/src/i18n/ko/settings.ts @@ -47,8 +47,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': '팁:', 'settings.mapTip': '우클릭 후 드래그하여 지도를 회전/기울이세요. 가운데 클릭으로 장소를 추가할 수 있습니다 (우클릭은 회전 전용).', - 'settings.latitude': '위도', - 'settings.longitude': '경도', 'settings.saveMap': '지도 저장', 'settings.apiKeys': 'API 키', 'settings.mapsKey': 'Google Maps API 키', diff --git a/shared/src/i18n/nl/settings.ts b/shared/src/i18n/nl/settings.ts index 5d92e283e..17c683acf 100644 --- a/shared/src/i18n/nl/settings.ts +++ b/shared/src/i18n/nl/settings.ts @@ -49,8 +49,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tip:', 'settings.mapTip': 'Rechts-klik en sleep om de kaart te roteren/kantelen. Middenklik om een locatie toe te voegen (rechts-klik is voor rotatie).', - 'settings.latitude': 'Breedtegraad', - 'settings.longitude': 'Lengtegraad', 'settings.saveMap': 'Kaart opslaan', 'settings.apiKeys': 'API-sleutels', 'settings.mapsKey': 'Google Maps API-sleutel', diff --git a/shared/src/i18n/pl/settings.ts b/shared/src/i18n/pl/settings.ts index 232d9369b..6b84af0ee 100644 --- a/shared/src/i18n/pl/settings.ts +++ b/shared/src/i18n/pl/settings.ts @@ -48,8 +48,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Wskazówka:', 'settings.mapTip': 'Kliknij prawym przyciskiem i przeciągnij, aby obrócić/pochylić mapę. Środkowy przycisk dodaje miejsce (prawy jest zarezerwowany dla obrotu).', - 'settings.latitude': 'Szerokość', - 'settings.longitude': 'Długość', 'settings.saveMap': 'Zapisz mapę', 'settings.apiKeys': 'Klucze API', 'settings.mapsKey': 'Klucz Google Maps API', diff --git a/shared/src/i18n/ru/settings.ts b/shared/src/i18n/ru/settings.ts index 821696610..9d8821df1 100644 --- a/shared/src/i18n/ru/settings.ts +++ b/shared/src/i18n/ru/settings.ts @@ -47,8 +47,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Совет:', 'settings.mapTip': 'Зажмите правую кнопку мыши и перетащите, чтобы повернуть/наклонить карту. Клик средней кнопкой — добавить место (правая кнопка зарезервирована для вращения).', - 'settings.latitude': 'Широта', - 'settings.longitude': 'Долгота', 'settings.saveMap': 'Сохранить карту', 'settings.apiKeys': 'API-ключи', 'settings.mapsKey': 'API-ключ Google Maps', diff --git a/shared/src/i18n/sv/settings.ts b/shared/src/i18n/sv/settings.ts index 1ce6726a6..b06a1020c 100644 --- a/shared/src/i18n/sv/settings.ts +++ b/shared/src/i18n/sv/settings.ts @@ -49,8 +49,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Tips:', 'settings.mapTip': 'högerklicka och dra för att rotera eller luta kartan. Mittklicka för att lägga till en plats (högerklick är reserverat för rotation).', - 'settings.latitude': 'Latitud', - 'settings.longitude': 'Longitud', 'settings.saveMap': 'Spara karta', 'settings.apiKeys': 'API nycklar', 'settings.mapsKey': 'Google Maps API nyckel', diff --git a/shared/src/i18n/tr/settings.ts b/shared/src/i18n/tr/settings.ts index e2f9009eb..55715ba7e 100644 --- a/shared/src/i18n/tr/settings.ts +++ b/shared/src/i18n/tr/settings.ts @@ -48,8 +48,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'İpucu:', 'settings.mapTip': 'Haritayı döndürmek/eğmek için sağ tıklayıp sürükleyin. Yer eklemek için orta tıklama (sağ tık döndürmeye ayrılmıştır).', - 'settings.latitude': 'Enlem', - 'settings.longitude': 'Boylam', 'settings.saveMap': 'Haritayı Kaydet', 'settings.apiKeys': 'API Anahtarları', 'settings.mapsKey': 'Google Maps API Anahtarı', diff --git a/shared/src/i18n/uk/settings.ts b/shared/src/i18n/uk/settings.ts index 5c4149492..64d019e8a 100644 --- a/shared/src/i18n/uk/settings.ts +++ b/shared/src/i18n/uk/settings.ts @@ -49,8 +49,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Порада:', 'settings.mapTip': 'Затисніть праву кнопку миші та перетягніть, щоб повернути/нахилити карту. Клік середньою кнопкою — додати місце (права кнопка зарезервована для обертання).', - 'settings.latitude': 'Широта', - 'settings.longitude': 'Довгота', 'settings.saveMap': 'Зберегти карту', 'settings.apiKeys': 'API-ключі', 'settings.mapsKey': 'API-ключ Google Maps', diff --git a/shared/src/i18n/vi/settings.ts b/shared/src/i18n/vi/settings.ts index 2cd451176..327cb3913 100644 --- a/shared/src/i18n/vi/settings.ts +++ b/shared/src/i18n/vi/settings.ts @@ -50,8 +50,6 @@ const settings: TranslationStrings = { 'settings.mapTipLabel': 'Mẹo:', 'settings.mapTip': 'nhấp chuột phải và kéo để xoay/cao độ bản đồ. Nhấp chuột giữa để thêm địa điểm (nhấp chuột phải được dành riêng cho việc xoay).', - 'settings.latitude': 'Vĩ độ', - 'settings.longitude': 'Kinh độ', 'settings.saveMap': 'Lưu bản đồ', 'settings.apiKeys': 'API Key', 'settings.mapsKey': 'Google Maps API Key', diff --git a/shared/src/i18n/zh-TW/settings.ts b/shared/src/i18n/zh-TW/settings.ts index e19d755cc..2cfa481bc 100644 --- a/shared/src/i18n/zh-TW/settings.ts +++ b/shared/src/i18n/zh-TW/settings.ts @@ -46,8 +46,6 @@ const settings: TranslationStrings = { 'settings.mapHighQualityWarning': '可能影響低階裝置的效能。', 'settings.mapTipLabel': '提示:', 'settings.mapTip': '右鍵點擊並拖曳以旋轉/傾斜地圖。中鍵點擊新增地點(右鍵用於旋轉)。', - 'settings.latitude': '緯度', - 'settings.longitude': '經度', 'settings.saveMap': '儲存地圖', 'settings.apiKeys': 'API 金鑰', 'settings.mapsKey': 'Google Maps API 金鑰', diff --git a/shared/src/i18n/zh/settings.ts b/shared/src/i18n/zh/settings.ts index 64838ab32..f792bd47d 100644 --- a/shared/src/i18n/zh/settings.ts +++ b/shared/src/i18n/zh/settings.ts @@ -46,8 +46,6 @@ const settings: TranslationStrings = { 'settings.mapHighQualityWarning': '可能影响低端设备的性能。', 'settings.mapTipLabel': '提示:', 'settings.mapTip': '右键点击并拖动以旋转/倾斜地图。中键点击添加地点(右键用于旋转)。', - 'settings.latitude': '纬度', - 'settings.longitude': '经度', 'settings.saveMap': '保存地图', 'settings.apiKeys': 'API 密钥', 'settings.mapsKey': 'Google Maps API 密钥',