fix(street-level): viewer lifecycle, hover cost, keyboard focus, panel lift

From the IC8 review of #768 (P2s), each with a test that fails before:

- Switching the layer off and on during the first photo download no longer
  leaves the viewer stuck on "not mounted": an outdated mount only unmounts
  an adapter nobody else holds and subscribes poses only once current, and
  the MapillaryJS adapter never builds a viewer after it was unmounted.
- Hover picks at most every 120 ms (the CCTV convention), not while the
  camera moves or a button is held, and not after the layer went off.
- Esc shrinks the expanded viewer wherever focus is (document capture), so
  the map keeps its selection, and Tab from outside returns into it.
  Closing a photo moves focus to the panel instead of dropping it on body.
- Portable panels lift only after a mouse or pen moves past the drag
  threshold: a click on a docked panel's edge or a touch swipe on its
  header no longer turns it into a floating window, and pointercancel
  saves nothing and puts the panel back.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

src/layers/streetLevel/providers/mapillary/viewer.js

@@ -14,6 +14,8 @@ export function createMapillaryViewer({ source, render } = {}) {
   let prewarming = false;
   /** In-flight viewer construction, so pre-warm and open never build two. */
   let creating = null;
+  /** Bumped by `unmount`, so a construction it overtook builds nothing. */
+  let generation = 0;
   let renderMode = 'letterbox';
   /** Metadata of the image on screen; pov/position events reuse it. */
   let current = null;
@@ -106,8 +108,12 @@ export function createMapillaryViewer({ source, render } = {}) {
   }
 
   async function createViewer(host) {
+    const built = generation;
     destroyViewer();
     const { Viewer } = await ensureLibrary();
+    // Unmounted (layer off) while the library loaded: a viewer built now
+    // would hold a WebGL context nobody releases.
+    if (built !== generation) throw new Error('Mapillary viewer was unmounted');
     viewer = new Viewer({
       accessToken: source.token,
       container: host,
@@ -165,6 +171,9 @@ export function createMapillaryViewer({ source, render } = {}) {
 
     unmount() {
       pendingOpen = null;
+      // The next mount builds afresh rather than joining a stale build.
+      generation++;
+      creating = null;
       destroyViewer();
     },
 

src/layers/streetLevel/selection.js

@@ -1,6 +1,12 @@
 import * as Cesium from 'cesium';
 import { STREET_LEVEL_LAYER_ID } from './policy.js';
 
+/**
+ * Minimum time between hover picks: ~8 `scene.pick` calls a second while the
+ * pointer moves, as the CCTV layer's hover does, never one per frame.
+ */
+const HOVER_PICK_INTERVAL_MS = 120;
+
 /** One click/hover handler for every provider's coverage and image cones. */
 export function createSelection({ state, parts }) {
   const { picking, input } = state.services;
@@ -43,33 +49,100 @@ export function createSelection({ state, parts }) {
 
   let hoverQueued = false;
   let hoverCursor = false;
-  /** Pointer cursor over anything this layer owns, so lines read as clickable. */
+  let hoverPosition = null;
+  let hoverLastPickAt = -Infinity;
+  let hoverTimer = null;
+  let cameraMoving = false;
+  let pointerButtons = 0;
+  let removeHoverWatchers = null;
+
+  function setHoverCursor(canvas, hit) {
+    if (hit && !hoverCursor) {
+      canvas.style.cursor = 'pointer';
+      hoverCursor = true;
+    } else if (!hit && hoverCursor) {
+      canvas.style.cursor = '';
+      hoverCursor = false;
+    }
+  }
+
+  function pickHover() {
+    hoverQueued = false;
+    const viewer = state.viewer;
+    // Switched off (or torn down) since the frame was queued.
+    if (!viewer || !state.enabled || !state.clickHandler) return;
+    // Orbiting, or a drag in progress: the pointer is the camera's.
+    if (cameraMoving || pointerButtons !== 0) return;
+    const canvas = viewer.scene?.canvas;
+    if (!canvas || viewer.isDestroyed?.()) return;
+    hoverLastPickAt = Date.now();
+    let hit = false;
+    try {
+      const picked = viewer.scene.pick(hoverPosition);
+      const id = picking?.resolvePickId
+        ? picking.resolvePickId(picked)
+        : picked?.id;
+      hit = ownsPick(id);
+    } catch {
+      hit = false;
+    }
+    setHoverCursor(canvas, hit);
+  }
+
+  /**
+   * Pointer cursor over anything this layer owns, so lines read as clickable.
+   * Picks the latest position at most every HOVER_PICK_INTERVAL_MS; a move
+   * inside the interval is picked when it ends, so the cursor never sticks.
+   */
   function onMove(movement) {
     const viewer = state.viewer;
-    if (!viewer || !state.enabled || hoverQueued) return;
+    if (!viewer || !state.enabled) return;
+    hoverPosition = movement.endPosition;
+    if (hoverQueued) return;
     hoverQueued = true;
-    requestAnimationFrame(() => {
-      hoverQueued = false;
-      const canvas = viewer.scene?.canvas;
-      if (!canvas || viewer.isDestroyed?.()) return;
-      let hit = false;
-      try {
-        const picked = viewer.scene.pick(movement.endPosition);
-        const id = picking?.resolvePickId
-          ? picking.resolvePickId(picked)
-          : picked?.id;
-        hit = ownsPick(id);
-      } catch {
-        hit = false;
-      }
-      if (hit && !hoverCursor) {
-        canvas.style.cursor = 'pointer';
-        hoverCursor = true;
-      } else if (!hit && hoverCursor) {
-        canvas.style.cursor = '';
-        hoverCursor = false;
-      }
+    const wait = hoverLastPickAt + HOVER_PICK_INTERVAL_MS - Date.now();
+    if (wait <= 0) {
+      requestAnimationFrame(pickHover);
+      return;
+    }
+    hoverTimer = setTimeout(() => {
+      hoverTimer = null;
+      requestAnimationFrame(pickHover);
+    }, wait);
+  }
+
+  /** Track what makes a hover pick pointless: camera motion, held buttons. */
+  function watchHover(viewer) {
+    const canvas = viewer.scene.canvas;
+    const camera = viewer.camera;
+    const onButtons = (event) => {
+      pointerButtons = event.buttons ?? 0;
+    };
+    for (const type of ['pointerdown', 'pointermove', 'pointerup'])
+      canvas.addEventListener?.(type, onButtons);
+    const removeStart = camera?.moveStart?.addEventListener(() => {
+      cameraMoving = true;
     });
+    const removeEnd = camera?.moveEnd?.addEventListener(() => {
+      cameraMoving = false;
+    });
+    removeHoverWatchers = () => {
+      for (const type of ['pointerdown', 'pointermove', 'pointerup'])
+        canvas.removeEventListener?.(type, onButtons);
+      removeStart?.();
+      removeEnd?.();
+    };
+  }
+
+  function unwatchHover() {
+    removeHoverWatchers?.();
+    removeHoverWatchers = null;
+    clearTimeout(hoverTimer);
+    hoverTimer = null;
+    hoverQueued = false;
+    hoverLastPickAt = -Infinity;
+    cameraMoving = false;
+    pointerButtons = 0;
   }
 
   function install(viewer) {
@@ -85,6 +158,7 @@ export function createSelection({ state, parts }) {
       onMove,
       Cesium.ScreenSpaceEventType.MOUSE_MOVE,
     );
+    watchHover(viewer);
     document.addEventListener('keydown', onKeyDown);
     picking?.registerPickOwner?.(STREET_LEVEL_LAYER_ID, ownsPick);
   }
@@ -92,6 +166,7 @@ export function createSelection({ state, parts }) {
   function uninstall() {
     // Nothing to undo for a layer that was never enabled.
     if (!state.clickHandler) return;
+    unwatchHover();
     if (hoverCursor && state.viewer?.scene?.canvas) {
       state.viewer.scene.canvas.style.cursor = '';
       hoverCursor = false;

src/layers/streetLevel/selection.test.mjs

@@ -0,0 +1,140 @@
+import assert from 'node:assert/strict';
+import test, { mock } from 'node:test';
+import * as Cesium from 'cesium';
+import { createSelection } from './selection.js';
+
+/**
+ * The selection installed on a stand-in viewer. Animation frames queue until
+ * `frame()` runs them; timers and Date.now() are mocked so a burst of moves
+ * has a known timeline.
+ */
+function harness() {
+  const saved = {
+    document: globalThis.document,
+    requestAnimationFrame: globalThis.requestAnimationFrame,
+  };
+  mock.timers.enable({ apis: ['setTimeout', 'Date'] });
+  const frames = [];
+  globalThis.requestAnimationFrame = (task) => frames.push(task);
+  globalThis.document = new EventTarget();
+  const picks = [];
+  const canvas = Object.assign(new EventTarget(), {
+    style: {},
+    // Keep Cesium's handler on the canvas; there is no real document here.
+    disableRootEvents: true,
+    onwheel: null,
+  });
+  const camera = { moveStart: new Cesium.Event(), moveEnd: new Cesium.Event() };
+  const viewer = {
+    scene: {
+      canvas,
+      pick(position) {
+        picks.push(position);
+        return null;
+      },
+    },
+    camera,
+    isDestroyed: () => false,
+  };
+  const state = {
+    viewer,
+    enabled: true,
+    clickHandler: null,
+    services: { picking: null, input: null },
+  };
+  const selection = createSelection({
+    state,
+    parts: {
+      router: { ownsPick: () => false, resolve: () => null },
+      hasSelectedSequence: () => false,
+      clearSequences() {},
+    },
+  });
+  selection.install(viewer);
+  const onMove = state.clickHandler.getInputAction(
+    Cesium.ScreenSpaceEventType.MOUSE_MOVE,
+  );
+  return {
+    state,
+    canvas,
+    camera,
+    picks,
+    move: (x) => onMove({ endPosition: { x, y: 10 } }),
+    frame() {
+      for (const task of frames.splice(0)) task();
+    },
+    /** Let `ms` pass, running the frames that come due on the way. */
+    wait(ms) {
+      this.frame();
+      mock.timers.tick(ms);
+      this.frame();
+    },
+    buttons(type, buttons) {
+      const event = new Event(type);
+      Object.defineProperty(event, 'buttons', { value: buttons });
+      canvas.dispatchEvent(event);
+    },
+    restore() {
+      selection.uninstall();
+      mock.timers.reset();
+      Object.assign(globalThis, saved);
+    },
+  };
+}
+
+test('hover picks at most every 120 ms through a burst of moves, ending on the last one', () => {
+  const h = harness();
+  try {
+    // One second of a pointer moving at 60 Hz.
+    for (let i = 0; i < 60; i++) {
+      h.move(i);
+      h.wait(16);
+    }
+    assert.ok(h.picks.length <= 9, `${h.picks.length} picks in ~1 s`);
+    assert.ok(h.picks.length >= 7, `${h.picks.length} picks in ~1 s`);
+    h.wait(200);
+    assert.deepEqual(h.picks.at(-1), { x: 59, y: 10 }, 'the resting position');
+    const settled = h.picks.length;
+    h.wait(1000);
+    assert.equal(h.picks.length, settled, 'a still pointer costs nothing');
+  } finally {
+    h.restore();
+  }
+});
+
+test('hover does not pick while the camera moves or a mouse button is held', () => {
+  const h = harness();
+  try {
+    h.camera.moveStart.raiseEvent();
+    h.move(1);
+    h.wait(200);
+    assert.equal(h.picks.length, 0, 'camera in motion');
+    h.camera.moveEnd.raiseEvent();
+    h.move(2);
+    h.wait(200);
+    assert.equal(h.picks.length, 1, 'picks again once the camera rests');
+
+    h.buttons('pointerdown', 1);
+    h.move(3);
+    h.wait(200);
+    assert.equal(h.picks.length, 1, 'a drag in progress');
+    h.buttons('pointerup', 0);
+    h.move(4);
+    h.wait(200);
+    assert.equal(h.picks.length, 2);
+  } finally {
+    h.restore();
+  }
+});
+
+test('a hover frame queued before the layer went off does not pick', () => {
+  const h = harness();
+  try {
+    h.move(1);
+    h.state.enabled = false;
+    h.wait(200);
+    assert.equal(h.picks.length, 0);
+  } finally {
+    h.restore();
+  }
+});

src/layers/streetLevel/viewerHost.js

@@ -76,19 +76,19 @@ export function createViewerHost({ state, parts }) {
     }
     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;
-      }
+      await adapter.mount(state.street.host);
       if (mounting?.promise !== promise) {
-        // Unmounted (layer off, provider switched) while loading.
-        unsubscribe();
-        adapter.unmount();
+        // Unmounted (layer off, provider switched) while loading. A newer
+        // mount of this provider shares the adapter and its viewer: tearing
+        // it down here would leave that mount active with no viewer.
+        const reused =
+          active?.id === entry.def.id || mounting?.id === entry.def.id;
+        if (!reused) adapter.unmount();
         throw new Error('Street-level viewer was closed');
       }
+      // Listen only once current: an outdated mount releasing the same
+      // `applyPose` would otherwise unhook the mount that replaced it.
+      const unsubscribe = adapter.onPose(applyPose);
       active = { id: entry.def.id, adapter, unsubscribe };
       warmed.delete(entry.def.id);
       adapter.setRenderMode?.(state.street.renderMode);

src/layers/streetLevel/viewerHost.test.mjs

@@ -1,7 +1,35 @@
 import assert from 'node:assert/strict';
+import { registerHooks } from 'node:module';
 import test from 'node:test';
 import { createViewerHost } from './viewerHost.js';
 
+// MapillaryJS needs a browser; the adapter tests swap in a stand-in library.
+// Every import gets a fresh copy that waits for the current test's gate, so
+// each test controls when its "download" finishes.
+let libraryImports = 0;
+registerHooks({
+  resolve(specifier, context, next) {
+    if (!specifier.startsWith('mapillary-js')) return next(specifier, context);
+    const kind = specifier === 'mapillary-js' ? 'library' : 'asset';
+    return {
+      url: `gev-test-stub:mapillary-js/${kind}?${++libraryImports}`,
+      shortCircuit: true,
+    };
+  },
+  load(url, context, next) {
+    if (!url.startsWith('gev-test-stub:mapillary-js/'))
+      return next(url, context);
+    const source = url.includes('/library?')
+      ? 'const lib = await globalThis.__gevMapillaryLibrary();\n' +
+        'export const Viewer = lib.Viewer;\n' +
+        'export const RenderMode = lib.RenderMode;\n'
+      : 'export default {};\n';
+    return { format: 'module', shortCircuit: true, source };
+  },
+});
+const { createMapillaryViewer } =
+  await import('./providers/mapillary/viewer.js');
+
 function harness(adapter) {
   const state = {
     enabled: true,
@@ -18,14 +46,23 @@ function harness(adapter) {
   return { state, host: createViewerHost({ state, parts }) };
 }
 
-function fakeAdapter({ failMounts = 0 } = {}) {
+/**
+ * A stand-in adapter. Like the real one, concurrent mounts share one
+ * construction (`gate` holds it open) and `unmount` destroys the viewer that
+ * every later `open` needs.
+ */
+function fakeAdapter({ failMounts = 0, gate = null } = {}) {
   const calls = { mount: 0, open: [], unmount: 0, listeners: 0 };
   let emit = null;
+  let mounted = false;
   return {
     calls,
     async mount() {
       calls.mount++;
-      if (calls.mount <= failMounts) throw new Error('library failed to load');
+      const attempt = calls.mount;
+      await gate?.promise;
+      if (attempt <= failMounts) throw new Error('library failed to load');
+      mounted = true;
     },
     /** A pose event from the library, as MapillaryJS fires them. */
     emitPose(id) {
@@ -40,12 +77,14 @@ function fakeAdapter({ failMounts = 0 } = {}) {
       });
     },
     async open(id) {
+      if (!mounted) throw new Error('viewer is not mounted');
       calls.open.push(id);
       this.emitPose(id);
     },
     close() {},
     unmount() {
       calls.unmount++;
+      mounted = false;
     },
     resize() {},
     onPose(listener) {
@@ -146,3 +185,99 @@ test('a pose that arrives after the photo was closed does not bring it back', as
   assert.equal(state.street.sequenceId, null);
   assert.deepEqual(state.street, before);
 });
+
+function deferred() {
+  let resolve;
+  const promise = new Promise((done) => {
+    resolve = done;
+  });
+  return { promise, resolve };
+}
+
+test('switching the layer off and on during a cold first open keeps the shared viewer', async () => {
+  const gate = deferred();
+  const adapter = fakeAdapter({ gate });
+  const { state, host } = harness(adapter);
+  const first = host.open('mapillary', 'a'); // the library is still loading
+  host.unmount(); // layer off …
+  const second = host.open('mapillary', 'b'); // … on again, and a new click
+  gate.resolve();
+  await Promise.all([first, second]);
+  assert.equal(adapter.calls.unmount, 0, 'the outdated mount left it alone');
+  assert.equal(adapter.calls.listeners, 1);
+  assert.equal(state.street.error, null);
+  assert.equal(state.street.imageId, 'b');
+  await host.open('mapillary', 'c');
+  assert.equal(state.street.error, null, 'later opens are not stuck');
+  assert.equal(state.street.imageId, 'c');
+});
+
+/** MapillaryJS, counting live viewers (each holds a WebGL context). */
+function fakeLibrary() {
+  const gate = deferred();
+  const viewers = { created: 0, live: 0 };
+  class Viewer {
+    constructor() {
+      viewers.created++;
+      viewers.live++;
+      this.removed = false;
+    }
+    on() {}
+    async getPosition() {
+      return { lng: 1, lat: 2 };
+    }
+    async getPointOfView() {
+      return { bearing: 10, tilt: 0 };
+    }
+    async moveTo(id) {
+      return { id, cameraType: 'perspective', sequenceId: 's' };
+    }
+    remove() {
+      if (this.removed) return;
+      this.removed = true;
+      viewers.live--;
+    }
+    setRenderMode() {}
+    resize() {}
+  }
+  globalThis.__gevMapillaryLibrary = async () => {
+    await gate.promise;
+    return { Viewer, RenderMode: {} };
+  };
+  return { gate, viewers };
+}
+
+test('the Mapillary viewer survives the layer going off and on during its first download', async () => {
+  const { gate, viewers } = fakeLibrary();
+  const adapter = createMapillaryViewer({ source: { token: 't' } });
+  const { state, host } = harness(adapter);
+  const first = host.open('mapillary', 'img1');
+  await Promise.resolve();
+  host.unmount();
+  const second = host.open('mapillary', 'img2');
+  await Promise.resolve();
+  gate.resolve();
+  await Promise.all([first, second]);
+  assert.equal(state.street.error, null);
+  assert.equal(state.street.imageId, 'img2');
+  await host.open('mapillary', 'img3');
+  assert.equal(state.street.error, null, 'later opens are not stuck');
+  assert.equal(state.street.imageId, 'img3');
+  assert.equal(viewers.live, 1);
+});
+
+test('a Mapillary viewer unmounted mid-download is never built; the next mount builds afresh', async () => {
+  const { gate, viewers } = fakeLibrary();
+  const adapter = createMapillaryViewer({ source: { token: 't' } });
+  const element = {};
+  const stale = adapter.mount(element);
+  adapter.unmount();
+  const fresh = adapter.mount(element);
+  gate.resolve();
+  await assert.rejects(stale, /unmounted/);
+  await fresh;
+  assert.equal(viewers.created, 1, 'only the fresh mount built one');
+  await adapter.open('x');
+  adapter.unmount();
+  assert.equal(viewers.live, 0, 'nothing left holding a WebGL context');
+});