diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 95239614a..b2bc22480 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -887,5 +887,7 @@ "src/layers/streetLevel/pickRouter.test.mjs", "src/layers/streetLevel/uiState.test.mjs", "src/layers/streetLevel/providers/mapillary/provider.test.mjs", - "src/layers/streetLevel/providers/mapillary/decode.test.mjs" + "src/layers/streetLevel/providers/mapillary/decode.test.mjs", + "src/ui/panelDock.test.mjs", + "src/layers/streetLevel/viewerHost.test.mjs" ] diff --git a/src/layers/streetLevel/credits.js b/src/layers/streetLevel/credits.js index de1a9f322..b3bf7ce98 100644 --- a/src/layers/streetLevel/credits.js +++ b/src/layers/streetLevel/credits.js @@ -35,5 +35,5 @@ export function createCredits() { for (const id of [...shown.keys()]) hide(viewer, { id }); } - return { show, hide, hideAll, has: (id) => shown.has(id) }; + return { show, hide, hideAll }; } diff --git a/src/layers/streetLevel/index.js b/src/layers/streetLevel/index.js index a792cc7ab..b6849e5e0 100644 --- a/src/layers/streetLevel/index.js +++ b/src/layers/streetLevel/index.js @@ -8,7 +8,7 @@ import { createViewerHost } from './viewerHost.js'; import { requiresKeyIdFor, validateProviders } from './registry.js'; import { normalizeFilter, resolveFilter, sameFilter } from './filter.js'; import { decodeParams, encodeParams } from './params.js'; -import { composeUIState } from './uiState.js'; +import { composeUIState, summarizeCoverage } from './uiState.js'; import { viewCentre } from './view.js'; import { NEAREST_RADIUS_M, @@ -174,9 +174,10 @@ export function createStreetLevelLayer({ } function sequenceSnapshot() { + // The provider showing the open image answers first. const owner = state.providers.get(state.street.providerId); const candidates = owner - ? [owner, ...state.providers.values()] + ? [owner, ...[...state.providers.values()].filter((e) => e !== owner)] : [...state.providers.values()]; for (const entry of candidates) { const stats = entry.instance.sequenceStats?.(); @@ -239,10 +240,8 @@ export function createStreetLevelLayer({ disable() { state.enabled = false; parts.viewerHost.unmount(); - for (const entry of state.providers.values()) { - entry.instance.deactivate(); - parts.credits.hide(state.viewer, entry.def); - } + for (const entry of state.providers.values()) entry.instance.deactivate(); + parts.credits.hideAll(state.viewer); parts.selection.uninstall(); parts.marker.setVisible(false); notify(); @@ -264,18 +263,18 @@ export function createStreetLevelLayer({ }, getStats() { - const ui = getUIState(); + // The lifecycle polls this every second: summarise, don't snapshot. + const coverage = summarizeCoverage(providerSnapshots()); let loadingLabel = ''; - if (ui.keyRequired) loadingLabel = 'KEY REQUIRED'; - else if (ui.coverage.loading) loadingLabel = 'loading coverage...'; - else if (ui.coverage.hint && state.enabled) - loadingLabel = ui.coverage.hint; + if (coverage.keyRequired) loadingLabel = 'KEY REQUIRED'; + else if (coverage.loading) loadingLabel = 'loading coverage...'; + else if (coverage.hint && state.enabled) loadingLabel = coverage.hint; return { - count: ui.coverage.count, - sequences: ui.coverage.count, - loading: ui.coverage.loading, - keyRequired: ui.keyRequired, - error: ui.keyRequired ? 'KEY REQUIRED' : ui.coverage.error, + count: coverage.count, + sequences: coverage.count, + loading: coverage.loading, + keyRequired: coverage.keyRequired, + error: coverage.keyRequired ? 'KEY REQUIRED' : coverage.error, loadingLabel, }; }, @@ -325,25 +324,29 @@ export function createStreetLevelLayer({ state.street.loading = true; state.street.error = null; notify(); - try { - for (const entry of activeEntries()) { - const imageId = await entry.instance.nearestImage({ + let lastError = null; + for (const entry of activeEntries()) { + let imageId = null; + try { + imageId = await entry.instance.nearestImage({ lat: view.lat, lon: view.lon, }); - if (!imageId) continue; - await openImage(entry.def.id, imageId); - return true; + } catch (error) { + // One provider failing (no key, offline) must not hide the others. + lastError = error; + continue; } - throw new Error( - `No street-level imagery within ${NEAREST_RADIUS_M} m of the view centre`, - ); - } catch (error) { - state.street.error = error?.message || 'Nearest image unavailable'; - state.street.loading = false; - notify(); - return false; + if (!imageId) continue; + await openImage(entry.def.id, imageId); + return true; } + state.street.error = + lastError?.message || + `No street-level imagery within ${NEAREST_RADIUS_M} m of the view centre`; + state.street.loading = false; + notify(); + return false; }, /** Close the image and deselect it everywhere on the map. */ closeViewer() { diff --git a/src/layers/streetLevel/uiState.js b/src/layers/streetLevel/uiState.js index 27aab345e..d016853a3 100644 --- a/src/layers/streetLevel/uiState.js +++ b/src/layers/streetLevel/uiState.js @@ -16,6 +16,23 @@ import { COLORS } from './policy.js'; * @property {Array<{key: string, label: string, color: string}>} legend */ +/** + * What the switched-on providers add up to: summed counts, any loading, the + * first hint and error, and whether every one of them lacks its key. + * @param {Array} providers + */ +export function summarizeCoverage(providers) { + const active = providers.filter((p) => p.on); + return { + count: active.reduce((sum, p) => sum + (p.count || 0), 0), + loading: active.some((p) => p.loading), + hint: active.find((p) => p.hint)?.hint || '', + error: active.find((p) => p.error)?.error || null, + keyRequired: + active.length > 0 && active.every((p) => p.keyRequired === true), + }; +} + /** * Compose the snapshot the panel renders from the core state and one * snapshot per registered provider. Pure, so the merge rules are testable: @@ -33,8 +50,7 @@ export function composeUIState({ sequence, }) { const active = providers.filter((p) => p.on); - const keyRequired = - active.length > 0 && active.every((p) => p.keyRequired === true); + const { keyRequired, ...coverage } = summarizeCoverage(providers); const legend = []; for (const provider of active) for (const entry of provider.legend) @@ -53,12 +69,7 @@ export function composeUIState({ keyRequired, filter: { ...filter }, providers: providers.map((p) => ({ ...p, legend: [...p.legend] })), - coverage: { - loading: active.some((p) => p.loading), - count: active.reduce((sum, p) => sum + (p.count || 0), 0), - hint: active.find((p) => p.hint)?.hint || '', - error: active.find((p) => p.error)?.error || null, - }, + coverage, legend, sequence: { ...sequence }, street: { ...street }, diff --git a/src/layers/streetLevel/uiState.test.mjs b/src/layers/streetLevel/uiState.test.mjs index d540d2b4e..49999cbfa 100644 --- a/src/layers/streetLevel/uiState.test.mjs +++ b/src/layers/streetLevel/uiState.test.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { composeUIState } from './uiState.js'; +import { composeUIState, summarizeCoverage } from './uiState.js'; const provider = (overrides = {}) => ({ id: 'mapillary', @@ -102,3 +102,21 @@ test('hint and error come from the first active provider that has one', () => { assert.deepEqual(ui.filter, base.filter); assert.notEqual(ui.filter, base.filter, 'snapshot copies the filter'); }); + +test('summarizeCoverage is what getStats reports, without building a snapshot', () => { + assert.deepEqual( + summarizeCoverage([ + provider({ count: 4, hint: 'Look down' }), + provider({ id: 'b', name: 'B', count: 6, loading: true, error: 'boom' }), + provider({ id: 'c', name: 'C', count: 99, on: false, keyRequired: true }), + ]), + { + count: 10, + loading: true, + hint: 'Look down', + error: 'boom', + keyRequired: false, + }, + ); + assert.equal(summarizeCoverage([]).keyRequired, false); +}); diff --git a/src/layers/streetLevel/viewerHost.js b/src/layers/streetLevel/viewerHost.js index 64c097ca8..57c2cd46b 100644 --- a/src/layers/streetLevel/viewerHost.js +++ b/src/layers/streetLevel/viewerHost.js @@ -6,6 +6,8 @@ export function createViewerHost({ state, parts }) { /** @type {{id: string, adapter: object, unsubscribe: () => void}|null} */ let active = null; + /** @type {{id: string, promise: Promise}|null} */ + let mounting = null; let openSeq = 0; function notify() { @@ -51,20 +53,50 @@ export function createViewerHost({ state, parts }) { if (current !== sequenceId) entry.instance.selectSequence(sequenceId); } - async function mount(entry) { - if (active?.id === entry.def.id) return active.adapter; + /** + * Mount a provider's viewer adapter in the host. The adapter becomes + * `active` only once its mount succeeded, so a failed mount (a library + * that did not load) is retried on the next open; concurrent opens for the + * same provider share one mount. + */ + function mount(entry) { + if (active?.id === entry.def.id) return Promise.resolve(active.adapter); + if (mounting?.id === entry.def.id) return mounting.promise; if (active) { active.adapter.close(); active.unsubscribe(); active.adapter.unmount(); active = null; } - const { adapter } = { adapter: entry.instance.viewer }; - const unsubscribe = adapter.onPose(applyPose); - active = { id: entry.def.id, adapter, unsubscribe }; - await adapter.mount(state.street.host); - adapter.setRenderMode?.(state.street.renderMode); - return adapter; + const adapter = entry.instance.viewer; + const promise = (async () => { + const unsubscribe = adapter.onPose(applyPose); + try { + await adapter.mount(state.street.host); + } catch (error) { + unsubscribe(); + throw error; + } + if (mounting?.promise !== promise) { + // Unmounted (layer off, provider switched) while loading. + unsubscribe(); + adapter.unmount(); + throw new Error('Street-level viewer was closed'); + } + active = { id: entry.def.id, adapter, unsubscribe }; + adapter.setRenderMode?.(state.street.renderMode); + return adapter; + })(); + mounting = { id: entry.def.id, promise }; + promise.then( + () => { + if (mounting?.promise === promise) mounting = null; + }, + () => { + if (mounting?.promise === promise) mounting = null; + }, + ); + return promise; } /** @@ -172,6 +204,7 @@ export function createViewerHost({ state, parts }) { /** Tear the mounted adapter down (layer disabled or destroyed). */ function unmount() { close(); + mounting = null; if (!active) return; active.unsubscribe(); active.adapter.unmount(); diff --git a/src/layers/streetLevel/viewerHost.test.mjs b/src/layers/streetLevel/viewerHost.test.mjs new file mode 100644 index 000000000..d40d1d0be --- /dev/null +++ b/src/layers/streetLevel/viewerHost.test.mjs @@ -0,0 +1,89 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createViewerHost } from './viewerHost.js'; + +function harness(adapter) { + const state = { + enabled: true, + notify() {}, + providers: new Map(), + street: { host: {}, renderMode: 'letterbox', follow: false }, + }; + const def = { id: 'mapillary', name: 'Mapillary', label: 'MAPILLARY' }; + state.providers.set('mapillary', { def, instance: { viewer: adapter } }); + const parts = { + marker: { set() {}, clear() {} }, + follow: { followCamera() {}, lookAtPosition() {} }, + }; + return { state, host: createViewerHost({ state, parts }) }; +} + +function fakeAdapter({ failMounts = 0 } = {}) { + const calls = { mount: 0, open: [], unmount: 0, listeners: 0 }; + let emit = null; + return { + calls, + async mount() { + calls.mount++; + if (calls.mount <= failMounts) throw new Error('library failed to load'); + }, + async open(id) { + calls.open.push(id); + emit?.({ + providerId: 'mapillary', + imageId: id, + position: { lon: 1, lat: 2 }, + bearing: 90, + isPano: false, + externalUrl: `https://example.test/${id}`, + }); + }, + close() {}, + unmount() { + calls.unmount++; + }, + resize() {}, + onPose(listener) { + calls.listeners++; + emit = listener; + return () => { + calls.listeners--; + emit = null; + }; + }, + }; +} + +test('a failed mount is retried on the next open instead of being cached', async () => { + const adapter = fakeAdapter({ failMounts: 1 }); + const { state, host } = harness(adapter); + await host.open('mapillary', 'a'); + assert.equal(state.street.error, 'library failed to load'); + assert.equal(adapter.calls.listeners, 0, 'the pose listener was released'); + await host.open('mapillary', 'b'); + assert.equal(state.street.error, null); + assert.equal(adapter.calls.mount, 2); + assert.deepEqual(adapter.calls.open, ['b']); + assert.equal(state.street.imageId, 'b'); + assert.equal(state.street.externalUrl, 'https://example.test/b'); +}); + +test('concurrent opens share one mount and one pose listener', async () => { + const adapter = fakeAdapter(); + const { host } = harness(adapter); + await Promise.all([host.open('mapillary', 'a'), host.open('mapillary', 'b')]); + assert.equal(adapter.calls.mount, 1); + assert.equal(adapter.calls.listeners, 1); +}); + +test('unmounting while a mount is in flight does not leave it active', async () => { + const adapter = fakeAdapter(); + const { state, host } = harness(adapter); + const opening = host.open('mapillary', 'a'); + host.unmount(); + await opening; + assert.equal(host.activeProviderId(), null); + assert.equal(adapter.calls.listeners, 0); + assert.equal(adapter.calls.unmount, 1); + assert.equal(state.street.open, false); +}); diff --git a/src/streetLevelMarkup.test.mjs b/src/streetLevelMarkup.test.mjs index 8742533e1..320c0d55b 100644 --- a/src/streetLevelMarkup.test.mjs +++ b/src/streetLevelMarkup.test.mjs @@ -152,7 +152,7 @@ test('the viewer comes first and the panel is a portable, resizable window', () ); assert.match( read('src/ui/panelChrome.js'), - /classList\.toggle\('collapsed', nextCollapsed\);\s*this\._panelPosition\?\.onPanelCollapsed\?\.\(panelId, nextCollapsed\)/, + /classList\.toggle\('collapsed', nextCollapsed\);[\s\S]{0,200}?if \(explicit && !restore\)\s*this\._panelPosition\?\.onPanelCollapsed\?\.\(panelId, nextCollapsed\)/, ); assert.match( controls, diff --git a/src/ui/panelChrome.js b/src/ui/panelChrome.js index 82fcb9806..33ae6fa56 100644 --- a/src/ui/panelChrome.js +++ b/src/ui/panelChrome.js @@ -430,6 +430,8 @@ export class PanelChrome { if ( peer !== panelEl && peer.matches('[data-panel-id]') && + // A floating window is not part of the rail's accordion. + !peer.classList.contains('panel-floating') && !peer.hidden ) { this.setPanelCollapsed(peer.id, true, { @@ -557,7 +559,10 @@ export class PanelChrome { } } panelEl.classList.toggle('collapsed', nextCollapsed); - this._panelPosition?.onPanelCollapsed?.(panelId, nextCollapsed); + // Only a user's own collapse docks a floating window; cockpit entry, + // restores and accordion peers must not discard where they put it. + if (explicit && !restore) + this._panelPosition?.onPanelCollapsed?.(panelId, nextCollapsed); if ( nextCollapsed && this.cockpitView?.active && diff --git a/src/ui/panelDock.test.mjs b/src/ui/panelDock.test.mjs new file mode 100644 index 000000000..ea4f8f0a9 --- /dev/null +++ b/src/ui/panelDock.test.mjs @@ -0,0 +1,193 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { PanelPositionControls } from './panelPositionControls.js'; + +/** Just enough DOM for PanelPositionControls' portable-panel paths. */ +function fixture() { + const saved = Object.fromEntries( + ['document', 'window', 'localStorage', 'performance'].map((key) => [ + key, + globalThis[key], + ]), + ); + const store = new Map(); + const element = () => { + const classes = new Set(); + const style = { + removeProperty(name) { + delete this[name]; + }, + getPropertyValue: () => '', + }; + return Object.assign(new EventTarget(), { + id: '', + style, + dataset: {}, + classList: { + add: (...names) => names.forEach((name) => classes.add(name)), + remove: (...names) => names.forEach((name) => classes.delete(name)), + contains: (name) => classes.has(name), + toggle: (name, on) => (on ? classes.add(name) : classes.delete(name)), + }, + querySelector: () => null, + querySelectorAll: () => [], + closest: () => null, + removeAttribute() {}, + appendChild() {}, + getBoundingClientRect: () => ({ + left: 100, + top: 80, + width: 360, + height: 420, + right: 460, + bottom: 500, + }), + }); + }; + globalThis.document = Object.assign(new EventTarget(), { + getElementById: () => null, + querySelectorAll: () => [], + }); + globalThis.window = Object.assign(new EventTarget(), { + innerWidth: 1440, + innerHeight: 900, + }); + globalThis.localStorage = { + getItem: (key) => (store.has(key) ? store.get(key) : null), + setItem: (key, value) => store.set(key, String(value)), + removeItem: (key) => store.delete(key), + }; + const calls = { layout: 0, resized: [] }; + const owner = new PanelPositionControls({ + syncPanelCollapseButton() {}, + layoutRightPanels: () => calls.layout++, + syncCctvPanelViewport() {}, + showToast() {}, + onPanelResized: (id) => calls.resized.push(id), + }); + return { + owner, + element, + store, + calls, + restore() { + owner.destroy(); + Object.assign(globalThis, saved); + }, + }; +} + +function floatingPanel(f, id, { dockOnCollapse = false } = {}) { + const panel = f.element(); + const handle = f.element(); + panel.id = id; + panel.classList.add('panel-floating', 'panel-draggable'); + Object.assign(panel.style, { + left: '100px', + top: '80px', + width: '360px', + height: '420px', + }); + f.store.set(`godsEyeView.v8.panelPos.${id}`, '{"floating":true}'); + f.owner._portablePanels.set(id, { + panel, + min: { width: 320, height: 280 }, + dockOnCollapse, + }); + f.owner._makePanelDraggable(id, panel, handle); + return { panel, handle }; +} + +/** A left-button header press; `timeStamp` is a read-only getter on Event. */ +function press(handle, { timeStamp, clientX = 200, clientY = 90 }) { + const event = new Event('pointerdown', { cancelable: true }); + for (const [key, value] of Object.entries({ + button: 0, + clientX, + clientY, + timeStamp, + })) + Object.defineProperty(event, key, { value }); + handle.dispatchEvent(event); +} + +test('dockPanel returns a floating window to its rail and forgets its place', () => { + const f = fixture(); + try { + const { panel } = floatingPanel(f, 'street-level-panel'); + assert.equal(f.owner.dockPanel('street-level-panel'), true); + assert.equal(panel.classList.contains('panel-floating'), false); + assert.equal(panel.style.width, undefined); + assert.equal(panel.style.height, undefined); + assert.equal( + f.store.has('godsEyeView.v8.panelPos.street-level-panel'), + false, + ); + assert.deepEqual(f.calls.resized, ['street-level-panel']); + assert.equal( + f.owner.dockPanel('street-level-panel'), + false, + 'already docked', + ); + assert.equal(f.owner.dockPanel('unknown'), false); + } finally { + f.restore(); + } +}); + +test('only a dockOnCollapse panel docks when collapsed, and only on collapse', () => { + const f = fixture(); + try { + const street = floatingPanel(f, 'street-level-panel', { + dockOnCollapse: true, + }); + const cctv = floatingPanel(f, 'cctv-panel'); + f.owner.onPanelCollapsed('street-level-panel', false); + assert.equal( + street.panel.classList.contains('panel-floating'), + true, + 'expanding keeps it', + ); + f.owner.onPanelCollapsed('cctv-panel', true); + assert.equal( + cctv.panel.classList.contains('panel-floating'), + true, + 'CCTV did not opt in', + ); + f.owner.onPanelCollapsed('street-level-panel', true); + assert.equal(street.panel.classList.contains('panel-floating'), false); + } finally { + f.restore(); + } +}); + +test('two quick header presses snap a floating panel back; slow or distant ones do not', () => { + const f = fixture(); + try { + const { panel, handle } = floatingPanel(f, 'cctv-panel'); + press(handle, { timeStamp: 1000 }); + window.dispatchEvent(new Event('pointerup')); + press(handle, { timeStamp: 1600 }); + window.dispatchEvent(new Event('pointerup')); + assert.equal( + panel.classList.contains('panel-floating'), + true, + '600 ms apart is two clicks', + ); + press(handle, { timeStamp: 1700, clientX: 260 }); + window.dispatchEvent(new Event('pointerup')); + assert.equal( + panel.classList.contains('panel-floating'), + true, + '60 px apart is two clicks', + ); + press(handle, { timeStamp: 1900, clientX: 262 }); + assert.equal( + panel.classList.contains('panel-floating'), + false, + 'a double press docks', + ); + } finally { + f.restore(); + } +}); diff --git a/src/ui/streetLevelControls.js b/src/ui/streetLevelControls.js index fea68d237..9ea5e7196 100644 --- a/src/ui/streetLevelControls.js +++ b/src/ui/streetLevelControls.js @@ -341,11 +341,19 @@ export class StreetLevelControls { button.classList.toggle('is-active', active); button.setAttribute('aria-checked', String(active)); } - if (el.sinceRange && document.activeElement !== el.sinceRange) { + if (el.sinceRange) { + // Never move the thumb under the user's hand; once the committed value + // matches the thumb, show the full readout with its cut-off date. const index = String(view.since.index); - if (el.sinceRange.value !== index) el.sinceRange.value = index; - if (el.sinceLabel) el.sinceLabel.textContent = view.since.label; - el.sinceRange.setAttribute('aria-valuetext', view.since.label); + if ( + document.activeElement !== el.sinceRange && + el.sinceRange.value !== index + ) + el.sinceRange.value = index; + if (el.sinceRange.value === index) { + if (el.sinceLabel) el.sinceLabel.textContent = view.since.label; + el.sinceRange.setAttribute('aria-valuetext', view.since.label); + } } if (el.legend && el.legend.childElementCount !== view.legend.length) { el.legend.replaceChildren( diff --git a/src/ui/styles/street-level.css b/src/ui/styles/street-level.css index 99cb3d120..5888bd3c5 100644 --- a/src/ui/styles/street-level.css +++ b/src/ui/styles/street-level.css @@ -426,6 +426,14 @@ button.sl-status:focus-visible { } /* ── Floating window ─────────────────────────────────────────────────────── */ +/* A window that was only dragged keeps its natural height; bound it to the + viewport so nothing runs off-screen (the rail rule's max-height: 100% does + not bind an auto-height parent). Id-qualified to outrank that rule. */ +#right-context-rail > #street-level-panel.panel-floating > .street-level-panel-inner { + max-height: calc(100vh - 12px); + overflow-y: auto; +} + /* A resized window (PanelPositionControls writes an inline height) hands the spare height to the viewer, so the image never needs scrolling to. */ .panel-floating[style*='height'] > .street-level-panel-inner {