diff --git a/src/app/event-handlers.ts b/src/app/event-handlers.ts index 7c5ac0fe1a..bd17d90784 100644 --- a/src/app/event-handlers.ts +++ b/src/app/event-handlers.ts @@ -700,7 +700,27 @@ export class EventHandlerManager implements AppModule { } this.debouncedWebcamReload(); }); - this.debouncedUrlSync(); + + // Skip the immediate sync only when applyInitialUrlState() will start an + // async flyTo that makes getCenter() return stale intermediate coordinates. + // Two cases qualify: + // (a) lat+lon pair → setCenter() flyTo; both must be present since + // applyInitialUrlState only calls setCenter when both exist. + // (b) bare zoom → setZoom() animated zoom (no view preset). + // + // view is intentionally excluded: all renderers set this.state.view + // synchronously at the top of setView(), so the debounced read is always + // correct regardless of renderer. GlobeMap.onStateChanged is a no-op and + // SVG Map fires emitStateChange before the listener is installed — neither + // can rely on a later onStateChanged to drive the URL write, so they must + // use the immediate debounce path. + const { view, lat, lon, zoom } = this.ctx.initialUrlState ?? {}; + const urlHasAsyncFlyTo = + (lat !== undefined && lon !== undefined) || // setCenter → flyTo (requires both) + (!view && zoom !== undefined); // zoom-only → setZoom animated + if (!urlHasAsyncFlyTo) { + this.debouncedUrlSync(); + } } syncUrlState(): void { diff --git a/src/app/panel-layout.ts b/src/app/panel-layout.ts index 81422078a6..9a202586e2 100644 --- a/src/app/panel-layout.ts +++ b/src/app/panel-layout.ts @@ -1335,7 +1335,8 @@ export class PanelLayoutManager implements AppModule { const { view, zoom, lat, lon, timeRange, layers } = this.ctx.initialUrlState; if (view) { - this.ctx.map.setView(view); + // Pass URL zoom so the preset's default zoom doesn't overwrite it. + this.ctx.map.setView(view, zoom); } if (timeRange) { @@ -1349,9 +1350,10 @@ export class PanelLayoutManager implements AppModule { } if (lat !== undefined && lon !== undefined) { - const effectiveZoom = zoom ?? this.ctx.map.getState().zoom; - if (effectiveZoom > 2) this.ctx.map.setCenter(lat, lon, zoom); + // Always honour URL lat/lon regardless of zoom level. + this.ctx.map.setCenter(lat, lon, zoom); } else if (!view && zoom !== undefined) { + // zoom-only without a view preset: apply directly. this.ctx.map.setZoom(zoom); } diff --git a/src/components/DeckGLMap.ts b/src/components/DeckGLMap.ts index fed5260cd3..78d1536352 100644 --- a/src/components/DeckGLMap.ts +++ b/src/components/DeckGLMap.ts @@ -483,6 +483,10 @@ export class DeckGLMap { private handleThemeChange: () => void; private handleMapThemeChange: () => void; private moveTimeoutId: ReturnType | null = null; + /** Target center set eagerly by setView() so getCenter() returns the correct + * destination before moveend fires, preventing stale intermediate coords + * from being written to the URL during flyTo. Cleared on moveend. */ + private pendingCenter: { lat: number; lon: number } | null = null; private lastAircraftFetchCenter: [number, number] | null = null; private lastAircraftFetchZoom = -1; private aircraftFetchSeq = 0; @@ -833,6 +837,7 @@ export class DeckGLMap { }); this.maplibreMap.on('moveend', () => { + this.pendingCenter = null; this.lastSCZoom = -1; this.rafUpdateLayers(); this.debouncedFetchBases(); @@ -4619,15 +4624,21 @@ export class DeckGLMap { } } - public setView(view: DeckMapView): void { + public setView(view: DeckMapView, zoom?: number): void { const preset = VIEW_PRESETS[view]; if (!preset) return; this.state.view = view; + // Eagerly write target zoom+center so getState()/getCenter() return the + // correct destination before moveend fires. Without this a 250ms URL sync + // reads the old cached zoom or an intermediate animated center and + // overwrites URL params (e.g. ?view=mena&zoom=4 → wrong coords). + this.state.zoom = zoom ?? preset.zoom; + this.pendingCenter = { lat: preset.latitude, lon: preset.longitude }; if (this.maplibreMap) { this.maplibreMap.flyTo({ center: [preset.longitude, preset.latitude], - zoom: preset.zoom, + zoom: this.state.zoom, duration: 1000, }); } @@ -4667,6 +4678,7 @@ export class DeckGLMap { } public getCenter(): { lat: number; lon: number } | null { + if (this.pendingCenter) return this.pendingCenter; if (this.maplibreMap) { const center = this.maplibreMap.getCenter(); return { lat: center.lat, lon: center.lng }; diff --git a/src/components/GlobeMap.ts b/src/components/GlobeMap.ts index 99702142b3..61ee990cb3 100644 --- a/src/components/GlobeMap.ts +++ b/src/components/GlobeMap.ts @@ -2551,12 +2551,21 @@ export class GlobeMap { oceania: { lat: -25, lng: 140, altitude: 1.5 }, }; - public setView(view: MapView): void { + public setView(view: MapView, zoom?: number): void { this.currentView = view; if (!this.globe) return; this.wakeGlobe(); - const pov = GlobeMap.VIEW_POVS[view] ?? GlobeMap.VIEW_POVS.global; - this.globe.pointOfView(pov, 1200); + const preset = GlobeMap.VIEW_POVS[view] ?? GlobeMap.VIEW_POVS.global; + let altitude = preset.altitude; + if (zoom !== undefined) { + if (zoom >= 7) altitude = 0.08; + else if (zoom >= 6) altitude = 0.15; + else if (zoom >= 5) altitude = 0.3; + else if (zoom >= 4) altitude = 0.5; + else if (zoom >= 3) altitude = 0.8; + else altitude = 1.5; + } + this.globe.pointOfView({ lat: preset.lat, lng: preset.lng, altitude }, 1200); } public setCenter(lat: number, lon: number, zoom?: number): void { diff --git a/src/components/Map.ts b/src/components/Map.ts index 3346a2e3f8..5ab606a562 100644 --- a/src/components/Map.ts +++ b/src/components/Map.ts @@ -3331,7 +3331,7 @@ export class MapComponent { return getHotspotEscalation(hotspotId); } - public setView(view: MapView): void { + public setView(view: MapView, zoom?: number): void { this.state.view = view; // Region-specific zoom and pan settings @@ -3348,7 +3348,7 @@ export class MapComponent { }; const settings = viewSettings[view]; - this.state.zoom = settings.zoom; + this.state.zoom = zoom ?? settings.zoom; this.state.pan = settings.pan; this.applyTransform(); this.render(); diff --git a/src/components/MapContainer.ts b/src/components/MapContainer.ts index 66c6c7bb34..cbdd40f790 100644 --- a/src/components/MapContainer.ts +++ b/src/components/MapContainer.ts @@ -349,9 +349,9 @@ export class MapContainer { if (this.useDeckGL) { this.deckGLMap?.setIsResizing(isResizing); } else { this.svgMap?.setIsResizing(isResizing); } } - public setView(view: MapView): void { - if (this.useGlobe) { this.globeMap?.setView(view); return; } - if (this.useDeckGL) { this.deckGLMap?.setView(view as DeckMapView); } else { this.svgMap?.setView(view); } + public setView(view: MapView, zoom?: number): void { + if (this.useGlobe) { this.globeMap?.setView(view, zoom); return; } + if (this.useDeckGL) { this.deckGLMap?.setView(view as DeckMapView, zoom); } else { this.svgMap?.setView(view, zoom); } } public setZoom(zoom: number): void { diff --git a/tests/url-sync-initial.test.mts b/tests/url-sync-initial.test.mts new file mode 100644 index 0000000000..c2063c379e --- /dev/null +++ b/tests/url-sync-initial.test.mts @@ -0,0 +1,211 @@ +/** + * Regression tests for initial URL-sync suppression and DeckGLMap.pendingCenter. + * + * These cover the bugs fixed in the fix/url-params-overwrite series: + * - urlHasAsyncFlyTo guard (event-handlers.ts setupUrlStateSync) + * - DeckGLMap.pendingCenter eager cache (prevents stale center during flyTo) + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +// --------------------------------------------------------------------------- +// Inline the pure urlHasAsyncFlyTo logic so these tests are zero-dependency +// (no DOM, no maplibre). This mirrors the exact condition in setupUrlStateSync. +// --------------------------------------------------------------------------- +function urlHasAsyncFlyTo( + initialUrlState: { view?: string; lat?: number; lon?: number; zoom?: number } | undefined, +): boolean { + const { view, lat, lon, zoom } = initialUrlState ?? {}; + return ( + (lat !== undefined && lon !== undefined) || // setCenter → flyTo (both required) + (!view && zoom !== undefined) // zoom-only → setZoom animated + ); +} + +describe('urlHasAsyncFlyTo — suppression guard', () => { + it('returns false when initialUrlState is undefined (cold load)', () => { + assert.equal(urlHasAsyncFlyTo(undefined), false); + }); + + it('returns false for bare ?view=mena (no lat/lon, no zoom)', () => { + assert.equal(urlHasAsyncFlyTo({ view: 'mena' }), false); + }); + + it('returns false for lone ?lat=41 without lon', () => { + // Partial params must NOT suppress the immediate sync — only a full lat+lon + // pair triggers an async flyTo via setCenter(). + assert.equal(urlHasAsyncFlyTo({ lat: 41 }), false); + }); + + it('returns false for lone ?lon=29 without lat', () => { + assert.equal(urlHasAsyncFlyTo({ lon: 29 }), false); + }); + + it('returns true for full ?lat=41&lon=29 pair', () => { + // setCenter() is only called when both coords are present → async flyTo. + assert.equal(urlHasAsyncFlyTo({ lat: 41, lon: 29 }), true); + }); + + it('returns true for full lat+lon+zoom triplet', () => { + assert.equal(urlHasAsyncFlyTo({ lat: 41, lon: 29, zoom: 6 }), true); + }); + + it('returns true for bare ?zoom without view (animated setZoom)', () => { + // No view preset means setZoom() is called, which animates the transition. + assert.equal(urlHasAsyncFlyTo({ zoom: 5 }), true); + }); + + it('returns false for ?view=mena&zoom=4 (view+zoom uses setView, synchronous)', () => { + // When a view is present, setView() is used (not bare setZoom), so DeckGLMap + // writes state.zoom eagerly — no suppression needed. + assert.equal(urlHasAsyncFlyTo({ view: 'mena', zoom: 4 }), false); + }); + + it('returns false for ?view=eu with lat+lon absent', () => { + assert.equal(urlHasAsyncFlyTo({ view: 'eu' }), false); + }); + + it('returns true for ?view=eu&lat=50&lon=15 (setCenter overrides view)', () => { + // When lat+lon are present applyInitialUrlState calls setCenter regardless + // of view — async flyTo path. + assert.equal(urlHasAsyncFlyTo({ view: 'eu', lat: 50, lon: 15 }), true); + }); +}); + +// --------------------------------------------------------------------------- +// DeckGLMap.pendingCenter behaviour — tested via a minimal in-process stub +// that replicates the exact field logic without requiring maplibre or a DOM. +// --------------------------------------------------------------------------- + +/** Minimal stub that mirrors only the pendingCenter + getCenter + setView logic. */ +class DeckGLMapStub { + public state = { view: 'global', zoom: 1.5 }; + private pendingCenter: { lat: number; lon: number } | null = null; + + private readonly VIEW_PRESETS: Record = { + global: { longitude: 0, latitude: 20, zoom: 1.5 }, + mena: { longitude: 45, latitude: 28, zoom: 3.5 }, + eu: { longitude: 15, latitude: 50, zoom: 3.5 }, + america:{ longitude: -95, latitude: 38, zoom: 3 }, + }; + + setView(view: string, zoom?: number): void { + const preset = this.VIEW_PRESETS[view]; + if (!preset) return; + this.state.view = view; + this.state.zoom = zoom ?? preset.zoom; + this.pendingCenter = { lat: preset.latitude, lon: preset.longitude }; + // (maplibreMap.flyTo would be called here in the real impl) + } + + /** Called by the real moveend listener. */ + simulateMoveEnd(finalLat: number, finalLon: number, finalZoom: number): void { + this.pendingCenter = null; + this.state.zoom = finalZoom; + // (onStateChange?.(this.getState()) would fire here) + } + + getCenter(): { lat: number; lon: number } | null { + if (this.pendingCenter) return this.pendingCenter; + return null; // maplibreMap absent in stub + } + + getState() { + return { view: this.state.view, zoom: this.state.zoom }; + } +} + +describe('DeckGLMap.pendingCenter — eager center cache', () => { + it('setView sets pendingCenter to preset coords', () => { + const m = new DeckGLMapStub(); + m.setView('mena'); + const c = m.getCenter(); + assert.ok(c, 'getCenter() must return non-null after setView'); + assert.equal(c.lat, 28); + assert.equal(c.lon, 45); + }); + + it('setView eagerly updates state.zoom to preset default', () => { + const m = new DeckGLMapStub(); + m.setView('mena'); + assert.equal(m.getState().zoom, 3.5); + }); + + it('setView with explicit zoom overrides preset zoom', () => { + const m = new DeckGLMapStub(); + m.setView('mena', 4); + assert.equal(m.getState().zoom, 4); + // center must still be the preset's lat/lon + const c = m.getCenter(); + assert.ok(c); + assert.equal(c.lat, 28); + assert.equal(c.lon, 45); + }); + + it('getCenter returns pendingCenter before moveend fires', () => { + const m = new DeckGLMapStub(); + m.setView('eu'); + const c = m.getCenter(); + assert.ok(c, 'must return pending center during flyTo animation'); + assert.equal(c.lat, 50); + assert.equal(c.lon, 15); + }); + + it('moveend clears pendingCenter', () => { + const m = new DeckGLMapStub(); + m.setView('mena'); + m.simulateMoveEnd(28, 45, 3.5); + // After moveend, pendingCenter is null — getCenter() falls through to + // maplibreMap (absent in stub → null). Real impl would use maplibreMap.getCenter(). + assert.equal(m.getCenter(), null); + }); + + it('moveend updates state.zoom to actual final zoom', () => { + const m = new DeckGLMapStub(); + m.setView('mena', 4); + // flyTo might settle at a slightly different zoom + m.simulateMoveEnd(28, 45, 4.02); + assert.equal(m.getState().zoom, 4.02); + }); + + it('consecutive setView calls reset pendingCenter to new preset', () => { + const m = new DeckGLMapStub(); + m.setView('mena'); + m.setView('eu'); + const c = m.getCenter(); + assert.ok(c); + assert.equal(c.lat, 50); + assert.equal(c.lon, 15); + }); + + it('setView updates state.view synchronously', () => { + const m = new DeckGLMapStub(); + m.setView('america'); + assert.equal(m.getState().view, 'america'); + }); +}); + +// --------------------------------------------------------------------------- +// Integration: urlHasAsyncFlyTo + pendingCenter interaction +// Regression for: "?view=mena URL gained wrong lat/lon after initial sync" +// --------------------------------------------------------------------------- + +describe('regression: ?view=mena initial sync writes correct coords', () => { + it('view-only URL does NOT suppress sync (urlHasAsyncFlyTo=false)', () => { + // The listener must fire the immediate debounce so the URL is updated. + assert.equal(urlHasAsyncFlyTo({ view: 'mena' }), false); + }); + + it('pendingCenter holds preset coords during flyTo so buildMapUrl gets correct lat/lon', () => { + const m = new DeckGLMapStub(); + // applyInitialUrlState calls setView('mena') → pendingCenter is set + m.setView('mena'); + // When debouncedUrlSync fires (250ms) it calls map.getCenter() + const center = m.getCenter(); + assert.ok(center, 'center must be available for URL builder'); + assert.equal(center.lat, 28, 'lat must be mena preset, not 0/20 global default'); + assert.equal(center.lon, 45, 'lon must be mena preset'); + assert.equal(m.getState().zoom, 3.5, 'zoom must be mena preset'); + }); +});