mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 18:34:02 +00:00
fix(street-level): re-check an unknown key, Esc leaves full screen, say why a view is empty
- A failed or missing key status is asked again on the next switch-on, and the check gives up after 10 s instead of holding LOADING forever. - Waiting for the key status reads as LOADING while the layer is on. - Esc shrinks the photo in full screen too, before the panel's own Esc. - Destroying the panel with the iPhone popover open leaves no popover behind. - An empty view says whether the filter emptied it. - A freed upstream tile slot passes straight to the oldest waiter. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
d313b6e62c
commit
8e5e3cdd3b
10 files changed
+201
-30
No files matched your search
@@ -94,9 +94,14 @@ let upstreamActive = 0;
|
||||
/** @type {Array<() => void>} fetches waiting for a slot, oldest first */
|
||||
const upstreamWaiting = [];
|
||||
|
||||
/** Run `task` once an upstream slot is free, unless `signal` aborts first. */
|
||||
/**
|
||||
* Run `task` in an upstream slot, waiting for one unless `signal` aborts
|
||||
* first. A freed slot passes straight to the oldest waiter, still counted, so
|
||||
* a request arriving meanwhile cannot take it first.
|
||||
*/
|
||||
async function withUpstreamSlot(signal, task) {
|
||||
while (upstreamActive >= TILE_UPSTREAM_CONCURRENCY)
|
||||
if (upstreamActive < TILE_UPSTREAM_CONCURRENCY) upstreamActive++;
|
||||
else
|
||||
await new Promise((resolve, reject) => {
|
||||
const wake = () => {
|
||||
signal.removeEventListener('abort', leave);
|
||||
@@ -110,17 +115,14 @@ async function withUpstreamSlot(signal, task) {
|
||||
upstreamWaiting.push(wake);
|
||||
signal.addEventListener('abort', leave, { once: true });
|
||||
});
|
||||
if (signal.aborted) {
|
||||
// Woken, then left: hand the free slot on.
|
||||
upstreamWaiting.shift()?.();
|
||||
throw signal.reason;
|
||||
}
|
||||
upstreamActive++;
|
||||
try {
|
||||
// Handed a slot after leaving: it goes on to the next waiter.
|
||||
signal.throwIfAborted();
|
||||
return await task();
|
||||
} finally {
|
||||
upstreamActive--;
|
||||
upstreamWaiting.shift()?.();
|
||||
const next = upstreamWaiting.shift();
|
||||
if (next) next();
|
||||
else upstreamActive--;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,9 @@ import {
|
||||
|
||||
export { STREET_LEVEL_LAYER_ID } from './policy.js';
|
||||
|
||||
/** Longest wait for the server's key status; past it, the bundle's token decides. */
|
||||
const KEY_STATUS_TIMEOUT_MS = 10_000;
|
||||
|
||||
/** Coverage as the panel sees it while the `mapillary` switch is off. */
|
||||
const COVERAGE_OFF = Object.freeze({
|
||||
count: 0,
|
||||
@@ -73,15 +76,25 @@ export function createStreetLevelLayer({
|
||||
}
|
||||
state.notify = notify;
|
||||
|
||||
/** The key-status request in flight; overlapping checks share it. */
|
||||
let keyCheck = null;
|
||||
|
||||
/** Coverage waits for the key status: no tile requests without a key. */
|
||||
async function checkKey() {
|
||||
let configured;
|
||||
try {
|
||||
configured = (await source.getStatus())?.configured === true;
|
||||
} catch {
|
||||
configured = source.hasToken?.() === true;
|
||||
}
|
||||
parts.coverage.setKeyStatus(configured);
|
||||
function checkKey() {
|
||||
keyCheck ||= (async () => {
|
||||
let configured;
|
||||
try {
|
||||
// A hung server must not hold LOADING, or the next check, forever.
|
||||
const signal = AbortSignal.timeout(KEY_STATUS_TIMEOUT_MS);
|
||||
configured = (await source.getStatus({ signal }))?.configured === true;
|
||||
} catch {
|
||||
configured = source.hasToken?.() === true;
|
||||
}
|
||||
parts.coverage.setKeyStatus(configured);
|
||||
})().finally(() => {
|
||||
keyCheck = null;
|
||||
});
|
||||
return keyCheck;
|
||||
}
|
||||
|
||||
function startDrawing() {
|
||||
@@ -90,6 +103,9 @@ export function createStreetLevelLayer({
|
||||
parts.sequences.setVisible(true);
|
||||
parts.coverage.attach(viewer);
|
||||
credit.show(viewer);
|
||||
// Without a known key, a switch-on asks again: the server may be back,
|
||||
// or have a key now.
|
||||
if (['status', 'no-key'].includes(state.coverage.blocked)) checkKey();
|
||||
}
|
||||
|
||||
/** Close the photo, clear lines and cones, forget refusals. */
|
||||
|
||||
@@ -171,6 +171,64 @@ test('coverage asks for no tiles before the key status is known, and none withou
|
||||
}
|
||||
});
|
||||
|
||||
test('a key status that failed is asked again on the next switch-on', async (t) => {
|
||||
let up = false;
|
||||
let asked = 0;
|
||||
const source = fakeMapillarySource({
|
||||
hasToken: () => false,
|
||||
getStatus: async () => {
|
||||
asked++;
|
||||
if (!up) throw new TypeError('fetch failed');
|
||||
return { configured: true };
|
||||
},
|
||||
});
|
||||
const { layer, viewer } = await startLayer(t, { source, view: true });
|
||||
assert.equal(asked, 1, 'one check for init and the first switch-on');
|
||||
assert.equal(layer.getUIState().keyRequired, true);
|
||||
up = true; // the server is reachable again
|
||||
layer.disable();
|
||||
layer.enable(viewer);
|
||||
await settle();
|
||||
assert.equal(asked, 2);
|
||||
assert.equal(layer.getUIState().keyRequired, false);
|
||||
assert.ok(source.calls.tiles.length > 0, 'coverage loads');
|
||||
// A known key is not asked again.
|
||||
layer.disable();
|
||||
layer.enable(viewer);
|
||||
await settle();
|
||||
assert.equal(asked, 2);
|
||||
});
|
||||
|
||||
test('a key status that never answers gives up, so the next switch-on asks again', async (t) => {
|
||||
// The check's timeout, fired when the test says.
|
||||
const timeouts = [];
|
||||
t.mock.method(AbortSignal, 'timeout', (ms) => {
|
||||
const controller = new AbortController();
|
||||
timeouts.push({ ms, fire: () => controller.abort(new Error('timed out')) });
|
||||
return controller.signal;
|
||||
});
|
||||
let asked = 0;
|
||||
const source = fakeMapillarySource({
|
||||
hasToken: () => false,
|
||||
getStatus: ({ signal } = {}) =>
|
||||
new Promise((_, reject) => {
|
||||
asked++;
|
||||
signal?.addEventListener('abort', () => reject(signal.reason));
|
||||
}),
|
||||
});
|
||||
const { layer, viewer } = await startLayer(t, { source, view: true });
|
||||
assert.equal(asked, 1);
|
||||
assert.equal(timeouts[0].ms, 10_000, 'the check can be cut short');
|
||||
assert.equal(layer.getUIState().coverage.loading, true, 'LOADING meanwhile');
|
||||
timeouts[0].fire();
|
||||
await settle();
|
||||
assert.equal(layer.getUIState().coverage.loading, false);
|
||||
assert.equal(layer.getUIState().keyRequired, true, 'no token: KEY REQUIRED');
|
||||
layer.disable();
|
||||
layer.enable(viewer);
|
||||
assert.equal(asked, 2, 'asked again, not stuck on the first');
|
||||
});
|
||||
|
||||
test('a rejected key gates the layer as KEY REJECTED until it goes off', async (t) => {
|
||||
const rejected = Object.assign(new Error('Mapillary rejected the token'), {
|
||||
keyRejected: true,
|
||||
|
||||
@@ -407,7 +407,10 @@ export function createCoverage({ state, source }) {
|
||||
const { blocked } = state.coverage;
|
||||
return {
|
||||
count: sequenceCount(),
|
||||
loading: state.coverage.pending.size > 0,
|
||||
// Waiting for the key status is loading too, not an empty view.
|
||||
loading:
|
||||
state.coverage.pending.size > 0 ||
|
||||
(blocked === 'status' && isActive(state)),
|
||||
hint: state.coverage.hint,
|
||||
keyRequired: blocked === 'no-key' || blocked === 'rejected',
|
||||
keyRejected: blocked === 'rejected',
|
||||
|
||||
@@ -220,11 +220,16 @@ test('the key status gates coverage: nothing before it answers, nothing without
|
||||
state.coverage.blocked = 'status';
|
||||
coverage.refresh();
|
||||
assert.equal(source.calls.length, 0, 'nothing before the status answers');
|
||||
assert.equal(coverage.stats().loading, true, 'waiting reads as loading');
|
||||
state.enabled = false;
|
||||
assert.equal(coverage.stats().loading, false, 'but not while off');
|
||||
state.enabled = true;
|
||||
coverage.setKeyStatus(configured);
|
||||
if (configured)
|
||||
assert.equal(source.calls.length, 1, 'coverage loads once it is known');
|
||||
else {
|
||||
assert.equal(source.calls.length, 0, 'a key-less install never asks');
|
||||
assert.equal(coverage.stats().loading, false);
|
||||
assert.equal(coverage.stats().keyRequired, true);
|
||||
assert.equal(coverage.stats().keyRejected, false);
|
||||
coverage.unblock();
|
||||
|
||||
@@ -405,7 +405,7 @@ test('identical concurrent tile requests share one upstream fetch', async () =>
|
||||
|
||||
test('at most a few upstream fetches run at once; the rest wait their turn', async () => {
|
||||
let release;
|
||||
const gate = new Promise((resolve) => (release = resolve));
|
||||
let gate = new Promise((resolve) => (release = resolve));
|
||||
await withUpstream(
|
||||
async () => (await gate, ok()),
|
||||
async ({ calls, call }) => {
|
||||
@@ -419,6 +419,17 @@ test('at most a few upstream fetches run at once; the rest wait their turn', asy
|
||||
const done = await Promise.all(pending);
|
||||
assert.ok(done.every((res) => res.statusCode === 200));
|
||||
assert.equal(calls.length, tiles, 'the waiting ones ran too');
|
||||
// Every slot came back: a second burst runs as many at once again.
|
||||
let reopen;
|
||||
gate = new Promise((resolve) => (reopen = resolve));
|
||||
const again = Array.from({ length: tiles }, (_, i) =>
|
||||
call('/api/mapillary/tiles', `/coverage/14/${i}/4`),
|
||||
);
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
assert.equal(calls.length, tiles + TILE_UPSTREAM_CONCURRENCY);
|
||||
reopen();
|
||||
await Promise.all(again);
|
||||
assert.equal(calls.length, 2 * tiles);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -118,18 +118,18 @@ export class StreetLevelControls {
|
||||
this.listen(el.viewerExpand, 'click', () => this._toggleExpanded());
|
||||
if (this._expandMode === 'fullscreen')
|
||||
this.listen(document, 'fullscreenchange', () => this._syncExpanded());
|
||||
else {
|
||||
else
|
||||
this.listen(el.viewerWrap, 'toggle', (event) =>
|
||||
this._onPopoverToggle(event),
|
||||
);
|
||||
// Esc shrinks the photo before the panel's own Esc can collapse it.
|
||||
this.listen(el.viewerWrap, 'keydown', (event) => {
|
||||
if (event.key !== 'Escape' || !this.isViewerExpanded()) return;
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
this._exitExpanded();
|
||||
});
|
||||
}
|
||||
// Esc shrinks the photo before the panel's own Esc can collapse it, in
|
||||
// a browser that hands the key to the page while full screen too.
|
||||
this.listen(el.viewerWrap, 'keydown', (event) => {
|
||||
if (event.key !== 'Escape' || !this.isViewerExpanded()) return;
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
this._exitExpanded();
|
||||
});
|
||||
} else setProp(el.viewerExpand, 'hidden', true);
|
||||
for (const button of el.renderButtons) {
|
||||
this.listen(button, 'click', () => {
|
||||
@@ -430,6 +430,10 @@ export class StreetLevelControls {
|
||||
if (this.destroyed) return;
|
||||
this.destroyed = true;
|
||||
this._exitExpanded();
|
||||
// Its `toggle` is not heard once destroyed: a popover left on the wrap
|
||||
// would hide the viewer even inside the panel.
|
||||
if (this._expandMode === 'popover')
|
||||
this._elements.viewerWrap?.removeAttribute('popover');
|
||||
this._watchPanelHidingModes(false);
|
||||
this.listeners.abort();
|
||||
this._resizeObserver?.disconnect();
|
||||
|
||||
@@ -185,6 +185,11 @@ function panelDom({ fullscreen = true, popover = false } = {}) {
|
||||
const wrap = add(root, 'div', { id: 'sl-viewer-wrap' });
|
||||
if (fullscreen) wrap.requestFullscreen = async () => fullscreenTo(wrap);
|
||||
if (popover) {
|
||||
// `popover` reflects its attribute, as in the DOM.
|
||||
Object.defineProperty(wrap, 'popover', {
|
||||
get: () => wrap.getAttribute('popover'),
|
||||
set: (value) => wrap.setAttribute('popover', value),
|
||||
});
|
||||
// The browser fires `toggle` after the popover opens or closes.
|
||||
const toggleTo = (newState) =>
|
||||
setTimeout(() => wrap.dispatchEvent({ type: 'toggle', newState }), 0);
|
||||
@@ -559,6 +564,28 @@ test('EXPAND puts the viewer full screen in place, and the button shrinks it aga
|
||||
controls.destroy();
|
||||
}));
|
||||
|
||||
test('Esc in full screen shrinks the photo; the panel never sees it', () =>
|
||||
withDom(async (dom) => {
|
||||
const { controls } = stubPanel(dom, uiState({ open: true }));
|
||||
const expand = dom.root.querySelector('#sl-viewer-expand');
|
||||
expand.click();
|
||||
await settle();
|
||||
await settle();
|
||||
assert.equal(dom.document.fullscreenElement, dom.wrap);
|
||||
let panelEsc = 0;
|
||||
dom.root.addEventListener('keydown', () => panelEsc++);
|
||||
expand.dispatchEvent({ type: 'keydown', key: 'Escape', bubbles: true });
|
||||
await settle();
|
||||
await settle();
|
||||
assert.equal(dom.document.fullscreenElement, null);
|
||||
assert.equal(panelEsc, 0, 'the panel stays open');
|
||||
assert.deepEqual(expandLabel(dom), ['EXPAND', 'false']);
|
||||
// Not expanded, Esc is the panel's again.
|
||||
expand.dispatchEvent({ type: 'keydown', key: 'Escape', bubbles: true });
|
||||
assert.equal(panelEsc, 1);
|
||||
controls.destroy();
|
||||
}));
|
||||
|
||||
test('an image closed while full screen leaves full screen', () =>
|
||||
withDom(async (dom) => {
|
||||
const { layer, controls } = stubPanel(dom, uiState({ open: true }));
|
||||
@@ -622,6 +649,26 @@ test('on iPhone (no element fullscreen) EXPAND opens the viewer as a popover ove
|
||||
{ fullscreen: false, popover: true },
|
||||
));
|
||||
|
||||
test('destroying the panel while the iPhone popover is open leaves no popover behind', () =>
|
||||
withDom(
|
||||
async (dom) => {
|
||||
const { controls } = stubPanel(dom, uiState({ open: true }));
|
||||
dom.root.querySelector('#sl-viewer-expand').click();
|
||||
await settle();
|
||||
await settle();
|
||||
assert.equal(dom.wrap.getAttribute('popover'), 'auto');
|
||||
controls.destroy();
|
||||
await settle();
|
||||
assert.equal(dom.wrap.popoverOpen, false);
|
||||
assert.equal(
|
||||
dom.wrap.getAttribute('popover'),
|
||||
null,
|
||||
'its toggle is not heard any more',
|
||||
);
|
||||
},
|
||||
{ fullscreen: false, popover: true },
|
||||
));
|
||||
|
||||
/** A MutationObserver stand-in whose callback the test fires. */
|
||||
function fakeMutationObserver() {
|
||||
const observers = new Set();
|
||||
|
||||
@@ -103,7 +103,14 @@ function presentMeta(state) {
|
||||
: `${state.sequence.images.toLocaleString()} images in this sequence · Esc clears`;
|
||||
if (state.coverage.count > 0)
|
||||
return `${state.coverage.count.toLocaleString()} sequences in view · click a line for its photos`;
|
||||
return state.coverage.hint || '';
|
||||
if (state.coverage.hint) return state.coverage.hint;
|
||||
// Still loading, or the error line already says why.
|
||||
if (state.coverage.loading || state.keyRequired || state.coverage.error)
|
||||
return '';
|
||||
const filter = state.filter || {};
|
||||
return (filter.pano || 'all') !== 'all' || Number(filter.sinceDays) > 0
|
||||
? 'No sequences in view match the filter'
|
||||
: 'No Mapillary coverage in view';
|
||||
}
|
||||
|
||||
/** @param {{now?: number}} [options] Clock for the SINCE readout. */
|
||||
|
||||
@@ -201,6 +201,24 @@ test('the meta line never mixes the visible-sequence count with the selected seq
|
||||
assert.equal(hinted.meta, 'Point the camera at the globe');
|
||||
});
|
||||
|
||||
test('an empty view says so, and whether the filter emptied it', () => {
|
||||
const meta = (overrides) =>
|
||||
presentStreetLevelPanel(snapshot({ enabled: true, ...overrides })).meta;
|
||||
assert.equal(meta({}), 'No Mapillary coverage in view');
|
||||
assert.equal(
|
||||
meta({ filter: { pano: 'flat', sinceDays: 0 } }),
|
||||
'No sequences in view match the filter',
|
||||
);
|
||||
assert.equal(
|
||||
meta({ filter: { pano: 'all', sinceDays: 365 } }),
|
||||
'No sequences in view match the filter',
|
||||
);
|
||||
// Nothing to claim while it loads, or when the error line explains it.
|
||||
assert.equal(meta({ coverage: { loading: true } }), '');
|
||||
assert.equal(meta({ keyRequired: true }), '');
|
||||
assert.equal(meta({ coverage: { error: 'Tile HTTP 502' } }), '');
|
||||
});
|
||||
|
||||
test('viewer caption reads "Image by" left, date right, and links to Mapillary', () => {
|
||||
const view = presentStreetLevelPanel(
|
||||
snapshot({
|
||||
|
||||
Reference in new issue
Block a user