fix(street-level): review round — safer docking, mount retry, provider fallthrough

- Only a user's own collapse docks a floating window; cockpit entry,
  restores and the Cyber accordion no longer dock it or delete its saved
  place, and the accordion leaves floating windows alone.
- The viewer host records an adapter as active only after its mount
  succeeds, so a library that failed to load once is retried; concurrent
  opens share one mount, and unmounting mid-mount leaves nothing active.
- openNearest tries every active provider; one provider's error (no key,
  offline) no longer hides the others' imagery.
- The SINCE readout shows its cut-off date as soon as the committed value
  matches the thumb, even while the slider keeps focus.
- A floating window that was only dragged is bounded to the viewport and
  scrolls instead of running off-screen.
- getStats summarises coverage (summarizeCoverage) instead of building a
  full UI snapshot every second; disable() uses credits.hideAll(); smaller
  cleanups in the viewer host and sequence snapshot.
- Unit tests: panel docking and header double press, viewer-host mount
  retry/sharing/cancel, coverage summary.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Said TurkseverandClaude Opus 5.5 committed 2026-09-25 20:56:12 +01:00
1 parent 5877f24f0b
commit 6207e4144e
12 files changed
+425 -55

No files matched your search

+1 -1
View File
@@ -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 };
}
+33 -30
View File
@@ -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() {
+19 -8
View File
@@ -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<ProviderSnapshot>} 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 },
+19 -1
View File
@@ -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);
});
+41 -8
View File
@@ -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<object>}|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();
@@ -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);
});