From 741b6f67eb3b11c5e48cf1d1033b97c83c558d64 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 3 Nov 2026 17:38:05 +0400 Subject: [PATCH] Keep the Draw control and its chosen shape across engine switches The session Draw handle keeps the shape and colour and the marks stay on the annotation board; the drawing tool (preview, click handler, Escape and Enter keys) belongs to the attached viewer, so a switch ends an open sketch and gives the pointer back. --- src/annotations/drawTool.js | 113 +++++++++++- src/annotations/drawTool.test.mjs | 15 +- src/annotations/drawToolBehaviour.test.mjs | 200 +++++++++++++++++---- src/app/tools.js | 28 +-- src/ui/geoFileImport.test.mjs | 4 +- 5 files changed, 306 insertions(+), 54 deletions(-) diff --git a/src/annotations/drawTool.js b/src/annotations/drawTool.js index a022fdcc9..fee977d67 100644 --- a/src/annotations/drawTool.js +++ b/src/annotations/drawTool.js @@ -76,6 +76,8 @@ export function initDrawTool({ navigation = null, publish = () => {}, connect = () => () => {}, + initialShape = 'area', + initialColor = 'primary', }) { // A page without the Draw controls (the Display controls view omitted) has // no draw tool. @@ -85,8 +87,8 @@ export function initDrawTool({ let active = false; let destroyed = false; let session = null; - let shape = 'area'; - let color = 'primary'; + let shape = normalizeShape(initialShape); + let color = DRAW_COLORS.includes(initialColor) ? initialColor : 'primary'; let hint = ''; let handler = null; let lease = null; @@ -494,6 +496,9 @@ export function initDrawTool({ get shape() { return shape; }, + get color() { + return color; + }, get session() { return session; }, @@ -587,6 +592,110 @@ export function initDrawTool({ return api; } +/** + * The Draw control for the page session. The marks live on the annotation + * board and the chosen shape and colour here; the drawing tool itself (its + * preview, click handler and Escape/Enter keys) belongs to the attached + * Cesium viewer. `attachScene({ viewer, navigation })` starts the tool on a + * viewer with the shape and colour last chosen; `detachScene()` ends it, an + * open shape included. Without a viewer the controls answer nothing, as on + * an engine without Draw. Same handle as initDrawTool's + * (`window.__gevDrawTool`), kept across switches. + */ +export function createSessionDrawTool({ + annotations, + navigation = null, + publish = () => {}, + connect = () => () => {}, +}) { + if (!annotations || !document.getElementById('draw-toggle')) return null; + let inner = null; + let shape = 'area'; + let color = 'primary'; + let destroyed = false; + let settled = Promise.resolve(); + const detachScene = () => { + if (!inner) return settled; + shape = inner.shape; + color = inner.color; + const previous = inner; + inner = null; + settled = previous.destroy(); + if (window.__gevDrawTool === previous) window.__gevDrawTool = wrapper; + return settled; + }; + const wrapper = { + get active() { + return inner?.active ?? false; + }, + get shape() { + return inner?.shape ?? shape; + }, + get color() { + return inner?.color ?? color; + }, + get session() { + return inner?.session ?? null; + }, + get attached() { + return Boolean(inner); + }, + setActive: (next) => inner?.setActive(next) ?? false, + setShape(next) { + shape = normalizeShape(next); + inner?.setShape(next); + }, + setColor(next) { + color = DRAW_COLORS.includes(next) ? next : 'primary'; + inner?.setColor(next); + }, + marks: () => presentDrawMarks(annotations.list?.() || []), + flyToMark: (id) => inner?.flyToMark(id) ?? false, + addVertex: (...args) => inner?.addVertex(...args) ?? false, + finish: () => inner?.finish() ?? Promise.resolve(null), + cancel: () => inner?.cancel(), + clearAll: () => (inner ? inner.clearAll() : annotations.clear?.()), + shapes: DRAW_SHAPES, + diagnostics: () => + inner?.diagnostics() ?? { + active: false, + destroyed, + sceneHandler: false, + domListeners: 0, + previewDataSources: 0, + previewEntities: 0, + pointerOwner: pointerOwner(), + }, + whenSettled: () => inner?.whenSettled() ?? settled, + attachScene({ viewer = null, navigation: next = navigation } = {}) { + if (destroyed) return; + detachScene(); + navigation = next; + if (!viewer) return; + inner = initDrawTool({ + viewer, + annotations, + navigation, + publish, + connect, + initialShape: shape, + initialColor: color, + }); + window.__gevDrawTool = wrapper; + }, + detachScene, + destroy() { + if (destroyed) return settled; + const done = detachScene(); + destroyed = true; + if (window.__gevDrawTool === wrapper) delete window.__gevDrawTool; + return done; + }, + }; + window.__gevDrawTool = wrapper; + return wrapper; +} + /** How many draw previews are attached — more than one means a leaked tool. */ function countPreviewDataSources(viewer) { const sources = viewer?.dataSources; diff --git a/src/annotations/drawTool.test.mjs b/src/annotations/drawTool.test.mjs index 309cd76f8..a1371a770 100644 --- a/src/annotations/drawTool.test.mjs +++ b/src/annotations/drawTool.test.mjs @@ -23,21 +23,22 @@ test('the application composition wires the draw tool and owns its teardown', () const tools = read('src/app/tools.js'); const engineAt = tools.indexOf('const annotations = sessionAnnotations({'); const drawAt = tools.indexOf( - 'const drawTool = initDrawTool({\n viewer,\n annotations,\n navigation: styleManager,\n publish: (view) => displayStore.publishDraw(view),', + "const drawTool = sessionTool(layerSession, 'draw', defer, () =>\n createSessionDrawTool({\n annotations,\n navigation: styleManager,\n publish: (view) => displayStore.publishDraw(view),", ); assert.ok(engineAt >= 0, 'the session annotation board is missing'); assert.ok( drawAt > engineAt, - 'initDrawTool must follow the annotation board and receive it', + 'the draw tool must follow the annotation board and receive it', ); assert.match( tools, - /import \{ initDrawTool \} from '\.\.\/annotations\/drawTool\.js';/, + /import \{ createSessionDrawTool \} from '\.\.\/annotations\/drawTool\.js';/, ); - // The tool holds a pointer claim, DOM listeners and a data source, so the - // application lifetime disposes it — not whoever last pressed the button. - const deferAt = tools.indexOf('defer(() => drawTool?.destroy());'); - assert.ok(deferAt > drawAt, 'the shell must defer the draw tool teardown'); + // The tool holds a pointer claim, DOM listeners and a data source on the + // viewer, so the viewer's attachment ends it — not whoever last pressed the + // button; the session tool ends with the page. + const detachAt = tools.indexOf('defer(() => drawTool?.detachScene());'); + assert.ok(detachAt > drawAt, 'the scene must end the draw tool'); }); test('the engine resolves manual geometry before any name resolution', () => { diff --git a/src/annotations/drawToolBehaviour.test.mjs b/src/annotations/drawToolBehaviour.test.mjs index 811142193..12526c8c5 100644 --- a/src/annotations/drawToolBehaviour.test.mjs +++ b/src/annotations/drawToolBehaviour.test.mjs @@ -5,7 +5,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import * as Cesium from 'cesium'; -import { initDrawTool } from './drawTool.js'; +import { createSessionDrawTool, initDrawTool } from './drawTool.js'; import { isPointerFree, pointerOwner, @@ -68,9 +68,17 @@ class FakeElement { } closest(selector) { // Only what the tool asks: "am I inside an activatable control?" - const activatable = ['button', 'a', 'select', '[role="button"]', '[role="radio"]']; + const activatable = [ + 'button', + 'a', + 'select', + '[role="button"]', + '[role="radio"]', + ]; if (!activatable.some((part) => selector.includes(part))) return null; - return this.tagName === 'BUTTON' || this.tagName === 'A' || this.tagName === 'SELECT' + return this.tagName === 'BUTTON' || + this.tagName === 'A' || + this.tagName === 'SELECT' ? this : null; } @@ -175,7 +183,14 @@ function fakeViewer() { } function fakeAnnotations() { - const calls = { annotate: [], clear: 0, restyle: [], setHidden: [], frame: [], remove: [] }; + const calls = { + annotate: [], + clear: 0, + restyle: [], + setHidden: [], + frame: [], + remove: [], + }; const marks = []; const listeners = new Set(); const changed = () => listeners.forEach((listener) => listener()); @@ -223,7 +238,7 @@ function fakeAnnotations() { } /** Stand up a tool against fresh doubles, with globals restored afterwards. */ -function harness({ navigation = null } = {}) { +function harness({ navigation = null, session = false } = {}) { resetPointerOwnership(); const { byId, documentDouble } = fakeDom(); const viewer = fakeViewer(); @@ -235,8 +250,7 @@ function harness({ navigation = null } = {}) { const shown = { view: null }; // The controls' actions, as the display store holds them. const controls = { actions: null }; - const tool = initDrawTool({ - viewer, + const options = { annotations, navigation, publish: (view) => (shown.view = view), @@ -244,7 +258,11 @@ function harness({ navigation = null } = {}) { controls.actions = actions; return () => (controls.actions = null); }, - }); + }; + const tool = session + ? createSessionDrawTool(options) + : initDrawTool({ viewer, ...options }); + if (session) tool.attachScene({ viewer }); return { tool, shown, @@ -299,7 +317,11 @@ test('Draw refuses to start when something else holds the pointer', async () => h.shown.view.hint, 'Directions is using the pointer — close it first.', ); - assert.equal(h.tool.diagnostics().sceneHandler, false, 'no handler was bound'); + assert.equal( + h.tool.diagnostics().sceneHandler, + false, + 'no handler was bound', + ); releasePointer(other); h.act('toggleDraw'); @@ -341,7 +363,10 @@ test('both stock viewer click actions are borrowed for the session and given bac const stockClick = () => 'viewer picks and selects'; const stockDouble = () => 'viewer tracks'; h.viewer.screenSpaceEventHandler.setInputAction(stockClick, LEFT_CLICK); - h.viewer.screenSpaceEventHandler.setInputAction(stockDouble, LEFT_DOUBLE_CLICK); + h.viewer.screenSpaceEventHandler.setInputAction( + stockDouble, + LEFT_DOUBLE_CLICK, + ); h.act('toggleDraw'); assert.equal( @@ -379,20 +404,34 @@ test('Enter finishes from the canvas and the label field, never from a focused b h.tool.addVertex(-97.74, 30.27); // Enter while the Clear button has keyboard focus means "press Clear". - h.documentDouble.emit('keydown', { key: 'Enter', target: h.element('draw-clear') }); + h.documentDouble.emit('keydown', { + key: 'Enter', + target: h.element('draw-clear'), + }); await Promise.resolve(); assert.equal( h.annotations.calls.annotate.length, 0, 'a focused button keeps its own Enter', ); - assert.equal(h.tool.session.vertices.length, 3, 'and the shape is untouched'); + assert.equal( + h.tool.session.vertices.length, + 3, + 'and the shape is untouched', + ); // Enter from the page body (the canvas has focus in practice) finishes it. - h.documentDouble.emit('keydown', { key: 'Enter', target: h.documentDouble.body }); + h.documentDouble.emit('keydown', { + key: 'Enter', + target: h.documentDouble.body, + }); await Promise.resolve(); await Promise.resolve(); - assert.equal(h.annotations.calls.annotate.length, 1, 'Enter on the map finishes'); + assert.equal( + h.annotations.calls.annotate.length, + 1, + 'Enter on the map finishes', + ); assert.equal(h.annotations.calls.annotate[0].specs[0].type, 'area'); } finally { h.restore(); @@ -421,7 +460,10 @@ test('the live preview hangs at the height the click landed on', async () => { // …and the finished shape is still flattened, deliberately: the renderer // drapes, so a height carried further would be discarded out of sight. - h.documentDouble.emit('keydown', { key: 'Enter', target: h.documentDouble.body }); + h.documentDouble.emit('keydown', { + key: 'Enter', + target: h.documentDouble.body, + }); await Promise.resolve(); await Promise.resolve(); const spec = h.annotations.calls.annotate[0].specs[0]; @@ -477,8 +519,16 @@ test('destroy releases the pointer, the handler, the listeners and the stock act stockClick, 'the viewer got its selecting click back', ); - assert.equal(h.viewer._attached.length, 0, 'the preview data source is detached'); - assert.equal(h.annotations.listeners.size, 0, 'and it no longer follows the board'); + assert.equal( + h.viewer._attached.length, + 0, + 'the preview data source is detached', + ); + assert.equal( + h.annotations.listeners.size, + 0, + 'and it no longer follows the board', + ); // Idempotent: a second destroy is harmless. await h.tool.destroy(); @@ -497,7 +547,11 @@ test('Clear wipes the board and the shape in progress', async () => { h.act('clearDraw'); assert.equal(h.annotations.calls.clear, 1, 'placed marks are removed'); - assert.equal(h.tool.session.vertices.length, 0, 'and so is the shape in progress'); + assert.equal( + h.tool.session.vertices.length, + 0, + 'and so is the shape in progress', + ); } finally { h.restore(); } @@ -511,7 +565,13 @@ test('the controls choose the shape and colour the next shape is drawn with; it h.act('setDrawColor', 'amber'); assert.deepEqual( { ...h.shown.view, hint: undefined }, - { active: true, shape: 'line', color: 'amber', hint: undefined, marks: [] }, + { + active: true, + shape: 'line', + color: 'amber', + hint: undefined, + marks: [], + }, 'the view shows what the controls chose', ); h.act('setDrawColor', 'purple'); @@ -523,7 +583,10 @@ test('the controls choose the shape and colour the next shape is drawn with; it assert.equal(h.tool.session.vertices.length, 0); h.tool.addVertex(-97.75, 30.26); h.tool.addVertex(-97.74, 30.26); - h.documentDouble.emit('keydown', { key: 'Enter', target: h.documentDouble.body }); + h.documentDouble.emit('keydown', { + key: 'Enter', + target: h.documentDouble.body, + }); await Promise.resolve(); await Promise.resolve(); const spec = h.annotations.calls.annotate[0].specs[0]; @@ -548,12 +611,31 @@ test('each drawn shape is a row whose actions reach the board, and fly-to is a n try { const board = h.annotations; board.marks.push( - { id: 'a1', type: 'route', manual: true, name: null, label: '950 m', color: 'amber', hidden: false }, - { id: 'a2', type: 'pin', manual: false, label: 'Voice pin', color: 'primary', hidden: false }, + { + id: 'a1', + type: 'route', + manual: true, + name: null, + label: '950 m', + color: 'amber', + hidden: false, + }, + { + id: 'a2', + type: 'pin', + manual: false, + label: 'Voice pin', + color: 'primary', + hidden: false, + }, ); board.changed(); assert.deepEqual( - h.shown.view.marks.map(({ id, placeholder, hidden }) => ({ id, placeholder, hidden })), + h.shown.view.marks.map(({ id, placeholder, hidden }) => ({ + id, + placeholder, + hidden, + })), [{ id: 'a1', placeholder: 'Line · 950 m', hidden: false }], 'only hand-drawn marks are listed, an unnamed line by its length', ); @@ -568,7 +650,11 @@ test('each drawn shape is a row whose actions reach the board, and fly-to is a n ]); assert.deepEqual(board.calls.setHidden, [['a1', true]]); assert.deepEqual(board.calls.frame, ['a1']); - assert.deepEqual(navigations, ['drawing'], 'fly-to goes through the navigation policy'); + assert.deepEqual( + navigations, + ['drawing'], + 'fly-to goes through the navigation policy', + ); assert.deepEqual(board.calls.remove, ['a1']); assert.deepEqual(h.shown.view.marks, [], 'the row goes with its mark'); } finally { @@ -582,30 +668,49 @@ test('Backspace on the page removes the last vertex', () => { h.act('toggleDraw'); h.tool.addVertex(-97.75, 30.26); h.tool.addVertex(-97.74, 30.26); - h.documentDouble.emit('keydown', { key: 'Backspace', target: h.documentDouble.body }); + h.documentDouble.emit('keydown', { + key: 'Backspace', + target: h.documentDouble.body, + }); assert.equal(h.tool.session.vertices.length, 1); } finally { h.restore(); } }); -test('keys on the shape list are the list\'s, not the drawing\'s', async () => { +test("keys on the shape list are the list's, not the drawing's", async () => { const h = harness(); try { const listButton = new FakeElement('BUTTON'); - listButton.closest = (selector) => (selector === '#draw-mark-list' ? {} : null); + listButton.closest = (selector) => + selector === '#draw-mark-list' ? {} : null; const elsewhere = new FakeElement('INPUT', 'location-search'); h.act('toggleDraw'); h.tool.addVertex(-97.75, 30.26); h.tool.addVertex(-97.74, 30.26); h.documentDouble.emit('keydown', { key: 'Escape', target: listButton }); - assert.equal(h.tool.session.vertices.length, 2, 'Escape in the list keeps the shape'); + assert.equal( + h.tool.session.vertices.length, + 2, + 'Escape in the list keeps the shape', + ); assert.equal(h.tool.active, true, 'and keeps Draw on'); h.documentDouble.emit('keydown', { key: 'Enter', target: elsewhere }); await Promise.resolve(); - assert.equal(h.annotations.calls.annotate.length, 0, 'another field keeps its Enter'); - h.documentDouble.emit('keydown', { key: 'Escape', target: h.documentDouble.body }); - assert.equal(h.tool.session.vertices.length, 0, 'Escape on the page drops the open shape'); + assert.equal( + h.annotations.calls.annotate.length, + 0, + 'another field keeps its Enter', + ); + h.documentDouble.emit('keydown', { + key: 'Escape', + target: h.documentDouble.body, + }); + assert.equal( + h.tool.session.vertices.length, + 0, + 'Escape on the page drops the open shape', + ); } finally { h.restore(); } @@ -632,3 +737,36 @@ test('a page without the Draw controls gets no draw tool', () => { else globalThis.document = previousDocument; } }); + +test('an engine switch ends an open sketch and keeps the chosen shape and colour', async () => { + const h = harness({ session: true }); + try { + const handle = h.tool; + assert.equal(globalThis.window.__gevDrawTool, handle); + h.act('setDrawShape', 'line'); + h.act('setDrawColor', 'amber'); + h.act('toggleDraw'); + assert.equal(pointerOwner(), 'draw'); + assert.equal(handle.active, true); + + await handle.detachScene(); + assert.equal(isPointerFree(), true, 'the sketch gave the pointer back'); + assert.equal(handle.active, false); + assert.equal(handle.attached, false); + assert.equal(h.controls.actions, null, 'no viewer answers the controls'); + assert.equal(globalThis.window.__gevDrawTool, handle); + + handle.attachScene({ viewer: fakeViewer() }); + assert.equal(handle.attached, true); + assert.equal(handle.shape, 'line'); + assert.equal(handle.color, 'amber'); + assert.equal(h.shown.view.shape, 'line'); + h.act('toggleDraw'); + assert.equal(handle.active, true); + await handle.destroy(); + assert.equal(isPointerFree(), true); + assert.equal(globalThis.window.__gevDrawTool, undefined); + } finally { + h.restore(); + } +}); diff --git a/src/app/tools.js b/src/app/tools.js index 509b82685..cb7eff417 100644 --- a/src/app/tools.js +++ b/src/app/tools.js @@ -1,6 +1,6 @@ import { SceneDirector } from '../scenes/director.js'; import { createSessionAnnotations } from '../annotations/index.js'; -import { initDrawTool } from '../annotations/drawTool.js'; +import { createSessionDrawTool } from '../annotations/drawTool.js'; import { createGeoFileImport } from '../ui/geoFileImport.js'; import { useDisplayStore } from '../ui/stores/display.js'; import { applicationPinia } from '../ui/vueRuntime.js'; @@ -161,18 +161,20 @@ export function createApplicationTools({ const token = displayStore.connect(actions); return () => displayStore.detach(token); }; - // UTILITIES ▸ Draw: the same whiteboard, drawn by hand. It claims the pointer - // while a session is open, so its teardown belongs to the application - // lifetime rather than to whoever last pressed the button. A row's fly-to - // goes through the shell's navigation policy, as Import's does. - const drawTool = initDrawTool({ - viewer, - annotations, - navigation: styleManager, - publish: (view) => displayStore.publishDraw(view), - connect: connectDisplay, - }); - defer(() => drawTool?.destroy()); + // UTILITIES ▸ Draw: the same whiteboard, drawn by hand. The marks and the + // chosen shape belong to the page session; the tool claims this viewer's + // pointer while a sketch is open, so an engine switch ends the sketch. A + // row's fly-to goes through the shell's navigation policy, as Import's does. + const drawTool = sessionTool(layerSession, 'draw', defer, () => + createSessionDrawTool({ + annotations, + navigation: styleManager, + publish: (view) => displayStore.publishDraw(view), + connect: connectDisplay, + }), + ); + drawTool?.attachScene({ viewer, navigation: styleManager }); + defer(() => drawTool?.detachScene()); // UTILITIES ▸ Import: the person's own GeoJSON/KML/KMZ/GPX files, each its // own data source. They live for the page session: an engine switch takes // them off this viewer, and the next Cesium viewer reads them again. The diff --git a/src/ui/geoFileImport.test.mjs b/src/ui/geoFileImport.test.mjs index 48b99c8eb..222db17f9 100644 --- a/src/ui/geoFileImport.test.mjs +++ b/src/ui/geoFileImport.test.mjs @@ -328,7 +328,9 @@ test('the application composition wires the import and owns its teardown', () => new URL('../app/tools.js', import.meta.url), 'utf8', ); - const drawAt = tools.indexOf('const drawTool = initDrawTool({'); + const drawAt = tools.indexOf( + "const drawTool = sessionTool(layerSession, 'draw'", + ); const importAt = tools.indexOf( "const geoFileImport = sessionTool(layerSession, 'geoImport', defer, () =>", );