From 71620eb061732516c836d48e5d4fb6be90779de5 Mon Sep 17 00:00:00 2001 From: Carson Davis Date: Thu, 27 Aug 2026 11:52:21 -0500 Subject: [PATCH] [351] Stop reporting a finished drawing's clicks as map clicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit terra-draw commits a shape on `pointerup`, but the click of that same gesture reaches the engine only afterwards — Leaflet on the native `click` that follows, deck.gl a tap interval later, since its `click` recognizer waits to see whether a double-click is coming. Both land with the session already over, so the adapters' "am I drawing?" check no longer covers them and every consumer is handed a map click the user never made: one that arrives after `drawcomplete` and clears the selection or dismisses whatever a plugin just opened. A time-based guard closes that gap. `DrawPointerWatch` tracks the pointer a session ends on, and `DrawEndClickGuard` holds back the clicks that pointer can still produce — through both taps of a double-click finish, holding terra-draw's double-click-zoom re-enable back with them, and closing early the moment the user opens a gesture of their own. Leaflet's click subscribers now fan out from one adapter-owned map listener, so the session is checked once where clicks are reported, the shape DeckGLAdapter's pointer path already had. --- .../MapEngines/Adapters/DeckGLAdapter.ts | 62 +- .../MapEngines/Adapters/DrawingHelpers.ts | 249 ++++++++ .../MapEngines/Adapters/LeafletAdapter.ts | 106 +++- src/essence/Basics/Map_/Map_.js | 12 +- tests/unit/LeafletAdapter.spec.js | 301 ++++++++++ tests/unit/deckGLAdapter.spec.js | 535 ++++++++++++++++-- 6 files changed, 1184 insertions(+), 81 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 456192158..4ec55aa49 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -73,7 +73,9 @@ import { import { TerraDrawMapLibreGLAdapter } from 'terra-draw-maplibre-gl-adapter' import { committedVerticesFromChange, + DrawEndClickGuard, drawModeKeyEvents, + DrawPointerWatch, drawStyles, validateDrawnLineString, } from './DrawingHelpers' @@ -276,6 +278,8 @@ export class DeckGLAdapter implements IMapEngine { private _drawingShape: DrawShape | null = null private _terraDraw: TerraDraw | null = null private _terraDrawListeners: Array<() => void> = [] + private _drawEndClick = new DrawEndClickGuard() + private _drawPointers = new DrawPointerWatch() /** Registry of anchored HTML overlays (id -> teardown function). */ private _overlays = new Map void>() @@ -380,6 +384,9 @@ export class DeckGLAdapter implements IMapEngine { // session that is about to have no engine. this.disableDrawing() + this._drawEndClick.dispose() + this._drawPointers.stop() + if (this._terraDraw) { this._terraDrawListeners.forEach((off) => { try { off() } catch { /* ignore */ } }) this._terraDrawListeners = [] @@ -1115,6 +1122,7 @@ export class DeckGLAdapter implements IMapEngine { td.clear() td.setMode(shape) this._drawingShape = shape + this._drawPointers.start() this._syncLayers() this._emitEvent('drawstart', { shape }) } @@ -1139,6 +1147,19 @@ export class DeckGLAdapter implements IMapEngine { try { this._terraDraw.clear() } catch { /* mid-vertex */ } try { this._terraDraw.stop() } catch { /* idempotent */ } } + // The drawing's clicks may still be on their way here: deck's + // recognizers hold each one back a tap interval to see whether a + // double-click is coming, so both the click that ended the session and + // the one that placed its last vertex can arrive after this. The watch + // is what knows which of those is still owed. Armed after terra-draw + // has stopped, because stopping is what turns double-click zoom back + // on for the guard to hold back again. + this._drawEndClick.arm( + this._drawPointers.pendingClickFrom, + this._drawEventElement(), + (this._basemap as any)?.doubleClickZoom + ) + this._drawPointers.stop() this._syncLayers() return shape } @@ -1281,19 +1302,28 @@ export class DeckGLAdapter implements IMapEngine { this._deckSetProps({ viewState: clamped }) this._emitEvent('moveend', clamped) }, - onClick: (info: PickingInfo) => { - if (this._drawingShape) return - this._featureClickHandler?.(pickInfoToResult(info)) - this._emitClick(info) - }, - onHover: (info: PickingInfo) => { - if (this._drawingShape) return - this._featureHoverHandler?.(pickInfoToResult(info)) - this._emitMouseMove(info) - }, + onClick: this._onPointerClick, + onHover: this._onPointerHover, } as any) } + /** + * Report a click deck picked, unless the drawing session owns it: the ones + * terra-draw is taking as vertices, and the ones deck was still holding as + * the session ended (see {@link DrawEndClickGuard}). + */ + private _onPointerClick = (info: PickingInfo): void => { + if (this._drawingShape || this._drawEndClick.pending) return + this._featureClickHandler?.(pickInfoToResult(info)) + this._emitClick(info) + } + + private _onPointerHover = (info: PickingInfo): void => { + if (this._drawingShape) return + this._featureHoverHandler?.(pickInfoToResult(info)) + this._emitMouseMove(info) + } + /** * Initialise Mapbox overlay mode. Loads `mapbox-gl` via a dynamic import so * the library is only bundled when this code path is actually reached. @@ -1355,16 +1385,8 @@ export class DeckGLAdapter implements IMapEngine { this._overlay = new MapboxOverlay({ interleaved: true, layers: [], - onClick: (info: PickingInfo) => { - if (this._drawingShape) return - this._featureClickHandler?.(pickInfoToResult(info)) - this._emitClick(info) - }, - onHover: (info: PickingInfo) => { - if (this._drawingShape) return - this._featureHoverHandler?.(pickInfoToResult(info)) - this._emitMouseMove(info) - }, + onClick: this._onPointerClick, + onHover: this._onPointerHover, }) this._basemap.addControl(this._overlay as unknown as object) diff --git a/src/essence/Basics/MapEngines/Adapters/DrawingHelpers.ts b/src/essence/Basics/MapEngines/Adapters/DrawingHelpers.ts index 7297af4db..23ed662ef 100644 --- a/src/essence/Basics/MapEngines/Adapters/DrawingHelpers.ts +++ b/src/essence/Basics/MapEngines/Adapters/DrawingHelpers.ts @@ -136,6 +136,255 @@ export function validateDrawnLineString( return { valid: true } } +/** + * hammer's tap interval, in milliseconds — the one number every click deck + * delivers is timed against. + * + * deck builds its `click` and `dblclick` on hammer `Tap` recognizers and leaves + * their `interval: 300` and `posThreshold: 10` defaults alone (mjolnir.js + * `hammerjs/recognizers/tap`, `@deck.gl/core` `RECOGNIZERS`). Two taps are one + * double-click when their pointerups fall inside that interval and within those + * ten pixels. Only `click` is wired to require the other's failure, and that is + * what makes it the late one: with a failure to wait on, it holds its tap for a + * further interval after the pointerup before emitting, where `dblclick`, which + * waits on nothing, emits on the second pointerup itself. + */ +const TAP_INTERVAL_MS = 300 + +/** + * How long the guard keeps absorbing after the last pointer of the gesture that + * ended a drawing, in milliseconds. + * + * The latest click that gesture can produce is deck's, one + * {@link TAP_INTERVAL_MS} after its final pointerup; doubling the interval + * leaves that much margin again for a busy main thread running the recognizer's + * timer late. The margin is cheap: the user's own next gesture opens with a + * pointerdown, and that closes the window outright rather than waiting for it. + */ +const CLICK_SETTLE_MS = TAP_INTERVAL_MS * 2 + +/** + * The `doubleClickZoom` handler a map exposes, in either engine's spelling — + * Leaflet's `L.Handler` and MapLibre's `DoubleClickZoomHandler` agree on + * `enable`/`disable` and differ only on how the state is read back. + */ +export interface DoubleClickZoomHandler { + enable(): void + disable(): void + /** Leaflet. */ + enabled?(): boolean + /** MapLibre. */ + isEnabled?(): boolean +} + +/** + * Where a drawing session's pointer is, for the sake of the clicks the engine + * has yet to deliver from it. + * + * terra-draw drives its modes from `pointerdown`, `pointerup` and `keyup` + * listeners on the map element and emits `finish` synchronously from inside + * them, so a session that the user clicked or double-clicked to an end ends + * inside a pointer event, and one they pressed Enter for does not. Two things + * make that dependable. The capture phase runs from the root down, so a + * listener on `window` sees a pointer before terra-draw's own listeners on the + * map element do, whatever order those went on in. And an event's `eventPhase` + * is `NONE` exactly while it is not being dispatched, so the end of a gesture + * needs no bookkeeping of its own. + * + * The gesture being over is not the end of the story, though, which is why the + * last pointerup is timed as well as watched — see {@link pendingClickFrom}. + */ +export class DrawPointerWatch { + private _event: Event | null = null + private _lastPointerUpAt = 0 + + private readonly _onPointer = (event: Event): void => { + this._event = event + if (event.type === 'pointerup') this._lastPointerUpAt = Date.now() + } + + /** Watch for as long as a drawing session is live. Starting twice is safe. */ + start(): void { + if (typeof window === 'undefined') return + window.addEventListener('pointerdown', this._onPointer, true) + window.addEventListener('pointerup', this._onPointer, true) + } + + /** Stop watching, and let go of the last gesture. */ + stop(): void { + this._event = null + this._lastPointerUpAt = 0 + if (typeof window === 'undefined') return + window.removeEventListener('pointerdown', this._onPointer, true) + window.removeEventListener('pointerup', this._onPointer, true) + } + + /** Whether a pointer of a gesture on the map is being dispatched now. */ + private get _inGesture(): boolean { + return this._event !== null && this._event.eventPhase !== Event.NONE + } + + /** + * The pointer a click the engine still owes this session would be timed + * from, or null when it owes none. + * + * A pointer being dispatched right now is one: terra-draw commits from + * inside it, and the engine turns the same gesture into a click only + * afterwards. So is a pointerup {@link TAP_INTERVAL_MS} ago or less, even + * though the session is ending on something else. deck holds every click + * for that interval to see whether a double-click is coming, so the click + * that placed the final vertex is still in its hands when Enter — or a + * plugin that deferred past the pointerup — ends the session under it, and + * lands once the drawing is over. + * + * A pointerup any older has had its click delivered already, on either + * engine. A session ended with the pointer that idle — Enter pressed while + * reading the shape back, a plugin's own button — leaves nothing on the + * way, and covering it would only swallow the next click the user makes. + */ + get pendingClickFrom(): number | null { + if (this._inGesture) return Date.now() + const at = this._lastPointerUpAt + return at > 0 && Date.now() - at <= TAP_INTERVAL_MS ? at : null + } +} + +/** + * Tells an engine that the clicks still to arrive belong to the drawing that + * just ended, so they are not reported as map clicks. + * + * terra-draw commits a shape on `pointerup` and the engine hears about the same + * gesture's clicks only afterwards: Leaflet on the native `click` that follows, + * deck.gl a tap interval later, since its `click` waits to see whether a + * double-click is coming. Both land after the session has ended, so an engine's + * "am I drawing?" check no longer covers them, and reporting them hands every + * consumer a map click the user never made — one that arrives after + * `drawcomplete` and so dismisses whatever a plugin opened in response to it. + * + * That gesture is not always a single click. Finishing on a double-click is + * trained behaviour, and terra-draw commits on the first of the two taps: the + * second is still part of the same gesture, and it reaches the engine either as + * a second Leaflet `click` — Leaflet has no double-click disambiguation — or as + * the `onClick` deck maps its `dblclick` recognizer onto. So the guard absorbs + * by time rather than by count: + * + * - A pointerdown within {@link TAP_INTERVAL_MS} of the pointer the window is + * timed from may still be that second tap, so the window stays open. + * - A pointerdown any later cannot be, which makes it the user starting a + * gesture of their own, and the window closes there and then. + * - Every pointerup inside the window re-opens it for {@link CLICK_SETTLE_MS}, + * which is how long the engine may take to turn that pointer into a click. + * + * The finish is not always what the window is timed from. Enter, or a plugin + * that deferred past the pointerup, can end a session while deck is still + * holding the click that placed the final vertex, and that click lands with the + * drawing over exactly as a finishing one would. Timing from the pointer rather + * than from the finish is what covers it without stretching the window: the + * click is no later for the session having ended above it. + * + * A session that ends with no pointer that recent has no click of its own on + * the way, so it opens no window at all: waiting there would swallow the first + * click the user makes afterwards. The window a gesture does open closes on its + * own too, so a finishing click the engine never delivers cannot leave the + * guard absorbing either. + */ +export class DrawEndClickGuard { + private _armedAt = 0 + private _openUntil = 0 + private _element: HTMLElement | null = null + private _closeTimer: ReturnType | null = null + private _zoom: DoubleClickZoomHandler | null = null + private _zoomWasEnabled = false + + private readonly _onPointerDown = (): void => { + if (this.pending && Date.now() - this._armedAt > TAP_INTERVAL_MS) { + this._close() + } + } + + private readonly _onPointerUp = (): void => { + if (this.pending) this._openFor(CLICK_SETTLE_MS) + } + + /** + * Cover the clicks the engine may still deliver from the gesture a drawing + * session leaves behind. + * + * @param pointerAt The pointer the clicks belong to, as + * {@link DrawPointerWatch.pendingClickFrom} timed it, or null when the + * session leaves no click behind — then no window opens and whatever the + * user does next is theirs. + * @param element The element the engine's pointer events reach. Without one + * the guard stays open: an engine with no map cannot be delivering clicks. + * @param doubleClickZoom The map's double-click zoom handler, when it has + * one. terra-draw re-enables it the moment the mode stops, which would let + * a double-click finish zoom the map as well; the guard holds the re-enable + * back until its window closes. Pass it after terra-draw has stopped, so + * the state captured to restore is the one terra-draw left behind. + */ + arm( + pointerAt: number | null, + element: HTMLElement | null, + doubleClickZoom?: DoubleClickZoomHandler | null + ): void { + if (pointerAt === null) return + if (!element) return + if (element !== this._element) { + this.dispose() + this._element = element + element.addEventListener('pointerdown', this._onPointerDown, true) + element.addEventListener('pointerup', this._onPointerUp, true) + } + this._armedAt = pointerAt + this._openFor(pointerAt + CLICK_SETTLE_MS - Date.now()) + this._suspendDoubleClickZoom(doubleClickZoom) + } + + /** Whether a click reaching the engine now can only be the drawing's. */ + get pending(): boolean { + return Date.now() < this._openUntil + } + + /** Stop watching for the next gesture. Arming again resumes it. */ + dispose(): void { + this._close() + this._element?.removeEventListener('pointerdown', this._onPointerDown, true) + this._element?.removeEventListener('pointerup', this._onPointerUp, true) + this._element = null + } + + private _openFor(ms: number): void { + this._openUntil = Date.now() + ms + if (this._closeTimer) clearTimeout(this._closeTimer) + this._closeTimer = setTimeout(() => this._close(), ms) + } + + private _close(): void { + this._openUntil = 0 + if (this._closeTimer) { + clearTimeout(this._closeTimer) + this._closeTimer = null + } + const zoom = this._zoom + this._zoom = null + // The map is torn down under the guard on a mission swap. + if (zoom && this._zoomWasEnabled) { + try { zoom.enable() } catch { /* map already gone */ } + } + } + + private _suspendDoubleClickZoom(handler?: DoubleClickZoomHandler | null): void { + if (!handler) return + // Re-arming inside an open window must not read the state back off a + // handler this guard is the one holding disabled. + if (!this._zoom) { + this._zoomWasEnabled = handler.enabled?.() ?? handler.isEnabled?.() ?? true + } + this._zoom = handler + try { handler.disable() } catch { /* map already gone */ } + } +} + /** * Reads a design token off :root, resolved to a concrete value. * diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index ab3a033d2..bff9d0979 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -48,7 +48,9 @@ import { import { TerraDrawLeafletAdapter } from 'terra-draw-leaflet-adapter' import { committedVerticesFromChange, + DrawEndClickGuard, drawModeKeyEvents, + DrawPointerWatch, drawStyles, validateDrawnLineString, } from './DrawingHelpers' @@ -102,6 +104,18 @@ export default class LeafletAdapter implements IMapEngine, IMapEn */ private _eventHandlers: Map = new Map() + /** + * Click subscribers registered through {@link on}. They hang off the + * adapter's own map listener rather than off Leaflet directly, so that + * whether a drawing session owns a click is answered once, where clicks + * are reported, instead of at every subscription — the same shape + * DeckGLAdapter's pointer-click path has. + */ + private _clickListeners: Set<(e: any) => void> = new Set() + + /** Whether {@link _onMapClick} is currently on the map. */ + private _mapClickAttached = false + /** * Stored initialization options */ @@ -111,10 +125,11 @@ export default class LeafletAdapter implements IMapEngine, IMapEn private _basemapAccessToken: string | undefined /** - * Wrapped map listeners installed by onFeatureClick / onFeatureHover. - * Stored so replacing the handler (or destroying the adapter) cleanly - * detaches the prior listener — without this Leaflet's `.on()` would - * accumulate one extra click listener per call. + * The listeners onFeatureClick / onFeatureHover installed. The click one + * is called by {@link _onMapClick}, the hover ones sit on the map. Stored + * so replacing the handler (or destroying the adapter) cleanly detaches + * the prior listener — without this Leaflet's `.on()` would accumulate one + * extra listener per call. */ private _featureClickListener: ((e: any) => void) | null = null private _featureHoverMoveListener: ((e: any) => void) | null = null @@ -123,6 +138,8 @@ export default class LeafletAdapter implements IMapEngine, IMapEn private _terraDraw: TerraDraw | null = null private _drawingShape: DrawShape | null = null private _terraDrawListeners: Array<() => void> = [] + private _drawEndClick = new DrawEndClickGuard() + private _drawPointers = new DrawPointerWatch() /** * Initialize the Leaflet map instance @@ -317,6 +334,9 @@ export default class LeafletAdapter implements IMapEngine, IMapEn }) this._overlays.clear() + this._drawEndClick.dispose() + this._drawPointers.stop() + if (this._terraDraw) { this._terraDrawListeners.forEach((off) => { try { off() } catch { /* ignore */ } }) this._terraDrawListeners = [] @@ -324,6 +344,8 @@ export default class LeafletAdapter implements IMapEngine, IMapEn this._terraDraw = null } + this._clickListeners.clear() + this._detachMapClickListener() this._detachFeatureClickListener() this._detachFeatureHoverListeners() @@ -793,15 +815,56 @@ export default class LeafletAdapter implements IMapEngine, IMapEn handler(normalizedEvent) } - this._map.on(eventName, wrappedHandler) + // Clicks reach their subscribers through the adapter's own map + // listener, the one place a drawing session is checked for all of them + // (see {@link _onMapClick}). Every other event goes straight to + // Leaflet. + if (eventName === 'click') { + this._clickListeners.add(wrappedHandler) + this._attachMapClickListener() + } else { + this._map.on(eventName, wrappedHandler) + } const key = `${eventName}_${handler.toString()}` this._eventHandlers.set(key, wrappedHandler) } + /** + * Report a click Leaflet delivered, unless the drawing session owns it: + * the ones terra-draw is taking as vertices, and the ones that arrive once + * the session has ended (see {@link DrawEndClickGuard}). Nothing in + * Leaflet holds a vertex click back on its own — terra-draw's adapter + * never stops click propagation — so the session is checked here, at the + * single point every click the adapter reports passes through, as + * DeckGLAdapter's own click path does. + */ + private _onMapClick = (e: any): void => { + if (this._drawingShape || this._drawEndClick.pending) return + this._featureClickListener?.(e) + this._clickListeners.forEach((listener) => listener(e)) + } + + /** Put {@link _onMapClick} on the map, once, for its first subscriber. */ + private _attachMapClickListener(): void { + if (this._mapClickAttached || !this._map) return + this._map.on('click', this._onMapClick) + this._mapClickAttached = true + } + + private _detachMapClickListener(): void { + if (!this._mapClickAttached) return + this._map?.off('click', this._onMapClick) + this._mapClickAttached = false + } + off(eventName: string, handler?: MapEventHandler): void { if (!handler) { - this._map.off(eventName) + if (eventName === 'click') { + this._clickListeners.clear() + } else { + this._map.off(eventName) + } this._eventHandlers.forEach((wrappedHandler, key) => { if (key.startsWith(eventName + '_')) { this._eventHandlers.delete(key) @@ -811,7 +874,11 @@ export default class LeafletAdapter implements IMapEngine, IMapEn const key = `${eventName}_${handler.toString()}` const wrappedHandler = this._eventHandlers.get(key) if (wrappedHandler) { - this._map.off(eventName, wrappedHandler) + if (eventName === 'click') { + this._clickListeners.delete(wrappedHandler as (e: any) => void) + } else { + this._map.off(eventName, wrappedHandler) + } this._eventHandlers.delete(key) } } @@ -859,10 +926,10 @@ export default class LeafletAdapter implements IMapEngine, IMapEn /** * Register a handler called when the user clicks a rendered feature. - * Attaches a map-level click listener; on each click iterates registered - * vector layers to find the topmost feature under the cursor. Returns an - * unsubscribe function. Replace semantics: calling again with a new - * handler detaches the prior listener first. + * Hangs off the adapter's map click listener; on each click iterates + * registered vector layers to find the topmost feature under the cursor. + * Returns an unsubscribe function. Replace semantics: calling again with a + * new handler detaches the prior listener first. */ onFeatureClick(handler: FeatureInteractionHandler): () => void { this._detachFeatureClickListener() @@ -878,7 +945,7 @@ export default class LeafletAdapter implements IMapEngine, IMapEn }) } this._featureClickListener = listener - this._map.on('click', listener) + this._attachMapClickListener() return () => { if (this._featureClickListener === listener) { this._detachFeatureClickListener() @@ -887,9 +954,6 @@ export default class LeafletAdapter implements IMapEngine, IMapEn } private _detachFeatureClickListener(): void { - if (this._featureClickListener && this._map) { - this._map.off('click', this._featureClickListener) - } this._featureClickListener = null } @@ -1013,6 +1077,7 @@ export default class LeafletAdapter implements IMapEngine, IMapEn td.clear() td.setMode(shape) this._drawingShape = shape + this._drawPointers.start() this.emit('drawstart', { shape }) } @@ -1036,6 +1101,17 @@ export default class LeafletAdapter implements IMapEngine, IMapEn try { this._terraDraw.clear() } catch { /* terra-draw mid-vertex */ } try { this._terraDraw.stop() } catch { /* idempotent */ } } + // The drawing's clicks reach Leaflet after this, on the native clicks + // that follow its pointerups; the watch is what knows whether one is + // still owed. Armed after terra-draw has stopped, because stopping is + // what turns double-click zoom back on for the guard to hold back + // again. + this._drawEndClick.arm( + this._drawPointers.pendingClickFrom, + this._drawEventElement(), + (this._map as any)?.doubleClickZoom + ) + this._drawPointers.stop() return shape } diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index 96b9cc059..24d6e1edd 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -440,7 +440,11 @@ let Map_ = { }) } - Map_.map.addEventListener('click', clearOnMapClick) + // Through the engine rather than the native map, so this inherits + // the adapter's guard against reporting the click a drawing ended + // on. Subscribing on the L.Map directly would let a finished + // drawing deselect the user's active feature. + this.engine.on('click', clearOnMapClick) } else { this.engine.on('moveend', function () { L_.enforceVisibilityCutoffs() @@ -2602,9 +2606,13 @@ function clearOnMapClick(event) { } else if ('getBounds' in layer) { // Use the pixel bounds because longitude/latitude conversions for bounds // may be odd in the case of polar projections + // L.Bounds only accepts an L.Point or an [x, y] pair; the + // engine reports the click's layer point as plain + // {x, y}, which it would take for a bounds and throw on. if ( layer._pxBounds && - layer._pxBounds.contains(event.layerPoint) + event.layerPoint && + layer._pxBounds.contains(L.point(event.layerPoint)) ) { return true } diff --git a/tests/unit/LeafletAdapter.spec.js b/tests/unit/LeafletAdapter.spec.js index 95018467b..0e7c8244b 100644 --- a/tests/unit/LeafletAdapter.spec.js +++ b/tests/unit/LeafletAdapter.spec.js @@ -791,6 +791,307 @@ test.describe('LeafletAdapter - onFeatureClick', () => { }) }) +// ─── on / off ───────────────────────────────────────────────────────────────── + +test.describe('LeafletAdapter - on / off', () => { + + // Click subscribers hang off the adapter's own map listener rather than + // off Leaflet, so unsubscribing has to take them off that fan-out — handing + // the handler back to Leaflet cannot remove a listener Leaflet never had. + test('off() stops a click subscriber the adapter fans out to', () => { + const { mockMap } = setupWithLayerMocks() + const adapter = new LeafletAdapter() + adapter.init({ containerId: 'map' }) + + let mapClick = null + mockMap.on = (event, cb) => { if (event === 'click') mapClick = cb } + + const clicks = [] + const handler = (e) => clicks.push(e.latlng) + adapter.on('click', handler) + mapClick({ latlng: { lat: 1, lng: 2 } }) + + adapter.off('click', handler) + mapClick({ latlng: { lat: 3, lng: 4 } }) + + expect(clicks).toEqual([{ lat: 1, lng: 2 }]) + }) +}) + +// ─── the click a drawing ended on ───────────────────────────────────────────── + +test.describe('LeafletAdapter - the click a drawing ended on', () => { + + /** + * Stands in for the map container terra-draw and the guard listen on. + * Removal matches on the capture flag the way the DOM does, so a listener + * taken off with the other one stays attached. + */ + function makeEventTarget() { + const listeners = [] + return { + addEventListener: (type, fn, capture = false) => { + listeners.push({ type, fn, capture: !!capture }) + }, + removeEventListener: (type, fn, capture = false) => { + const i = listeners.findIndex( + (l) => l.type === type && l.fn === fn && l.capture === !!capture + ) + if (i !== -1) listeners.splice(i, 1) + }, + fire: (type) => { + listeners + .filter((l) => l.type === type) + .forEach((l) => l.fn()) + }, + listenerCount: () => listeners.length, + } + } + + /** The map's double-click zoom handler, reporting the state it is left in. */ + function makeDoubleClickZoom() { + let enabled = false + return { + enabled: () => enabled, + enable: () => { enabled = true }, + disable: () => { enabled = false }, + } + } + + /** + * Stands in for terra-draw, which turns double-click zoom back on as it + * stops the mode — and throws rather than stopping a mode twice, leaving + * whatever it stopped the first time in place. + */ + function makeTerraDraw(doubleClickZoom, { started = true } = {}) { + return { + clear: () => { }, + stop: () => { + if (!started) throw new Error('Mode must be started to be stopped') + started = false + doubleClickZoom.enable() + }, + } + } + + /** + * End a drawing session the way a click on the map does. terra-draw commits + * from inside the pointerup, so the adapter's pointer watch is looking at + * that very event as the session ends — which is what tells the guard a + * click of the drawing's is still to come. + */ + function stopOnPointer(adapter) { + adapter._drawPointers.start() + const stop = () => adapter._stopDrawing() + window.addEventListener('pointerup', stop) + window.dispatchEvent(new Event('pointerup')) + window.removeEventListener('pointerup', stop) + } + + /** + * An adapter mid-rectangle, with the map's click subscribers captured so a + * spec can deliver the click itself. + */ + function setupDrawing() { + const { mockMap } = setupWithLayerMocks() + const container = makeEventTarget() + mockMap.getContainer = () => container + const adapter = new LeafletAdapter() + adapter.init({ containerId: 'map' }) + + const subscribers = new Map() + mockMap.on = (event, cb) => { + if (!subscribers.has(event)) subscribers.set(event, []) + subscribers.get(event).push(cb) + } + mockMap.fire = (event, data) => { + subscribers.get(event)?.forEach((cb) => cb({ ...data, type: event })) + } + + const clicks = [] + const picks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + adapter.onFeatureClick((result) => picks.push(result)) + adapter._drawingShape = 'rectangle' + + return { + adapter, + mockMap, + container, + clicks, + picks, + click: () => + mockMap.fire('click', { + latlng: { lat: 40, lng: -120 }, + containerPoint: { x: 12, y: 34 }, + }), + } + } + + // Nothing in Leaflet holds back the click that places a vertex: it fires a + // map `click` for every native one, and terra-draw's Leaflet adapter never + // stops click propagation. Reported, those clicks would dismiss whatever a + // plugin has open and clear its selection halfway through a drawing — on + // the 2D engine only, since DeckGLAdapter has always checked the session. + test('a click placing a vertex mid-session is not reported', () => { + const { clicks, picks, click } = setupDrawing() + + click() + + expect(clicks).toEqual([]) + expect(picks).toEqual([]) + }) + + // The session check must not reach past clicks: the drawing's own events + // are emitted through this same wrapper while the session is live. + test('a session does not hold back the drawing events themselves', () => { + const { adapter } = setupDrawing() + const vertices = [] + adapter.on('drawvertex', (e) => vertices.push(e.shape)) + + adapter.emit('drawvertex', { shape: 'rectangle' }) + + expect(vertices).toEqual(['rectangle']) + }) + + // terra-draw commits a shape on `pointerup`, and the native `click` that + // finished it reaches Leaflet right after — by which time the session is + // over. Reporting it hands every consumer a map click the user never made, + // one that would dismiss the popup a plugin opened from the `drawcomplete` + // that came first. + test('is not reported as a map click', () => { + const { adapter, clicks, picks, click } = setupDrawing() + + stopOnPointer(adapter) + click() + + expect(clicks).toEqual([]) + expect(picks).toEqual([]) + }) + + // Finishing on a double-click is trained behaviour, and terra-draw commits + // on the first of the two clicks. Leaflet has no double-click + // disambiguation — `_fireDOMEvent` fires a map `click` for every native + // click — so the second one arrives as an ordinary click, from a gesture + // the user made to finish the drawing rather than to click the map. + test('swallows both clicks of a double-click finish', () => { + vi.useFakeTimers() + try { + const { adapter, container, clicks, picks, click } = setupDrawing() + + // Tap 1: terra-draw commits on its pointerup, and the native click + // that follows is the one the guard was first written for. + stopOnPointer(adapter) + click() + + // Tap 2, inside the 300ms tap interval that makes the pair a + // double-click. + vi.advanceTimersByTime(150) + container.fire('pointerdown') + vi.advanceTimersByTime(50) + container.fire('pointerup') + click() + + expect(clicks).toEqual([]) + expect(picks).toEqual([]) + } finally { + vi.useRealTimers() + } + }) + + // The guard gives double-click zoom back, it does not hand it out: a map + // that had it off — the session having ended with terra-draw already + // stopped, so nothing turned it on — must still have it off once the + // window closes. Enabling regardless would give every deployment that + // configures double-click zoom off the behaviour it turned down, from the + // user's first drawing to the end of the session. + test('leaves double-click zoom off when it was off as the session ended', () => { + vi.useFakeTimers() + try { + const { adapter, mockMap } = setupDrawing() + const zoom = makeDoubleClickZoom() + mockMap.doubleClickZoom = zoom + adapter._terraDraw = makeTerraDraw(zoom, { started: false }) + + stopOnPointer(adapter) + vi.advanceTimersByTime(600) + + expect(zoom.enabled()).toBe(false) + } finally { + vi.useRealTimers() + } + }) + + // Two sessions can end inside one window — a drawing cancelled, another + // started and cancelled straight after — and the second arming reads the + // handler again to know what to give back. By then the guard is the one + // holding double-click zoom down, so what it reads must be the state from + // before that: taking its own disable for the map's setting would leave + // double-click zoom off for the rest of the session. + test('gives back the double-click zoom state from before it held it down', () => { + vi.useFakeTimers() + try { + const { adapter, mockMap } = setupDrawing() + const zoom = makeDoubleClickZoom() + mockMap.doubleClickZoom = zoom + adapter._terraDraw = makeTerraDraw(zoom) + + stopOnPointer(adapter) + expect(zoom.enabled()).toBe(false) + + // A second session, ending while the first window is still open. + vi.advanceTimersByTime(100) + adapter._drawingShape = 'polygon' + stopOnPointer(adapter) + + vi.advanceTimersByTime(600) + expect(zoom.enabled()).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + // A mission swap tears the map down with a window still open. The guard + // has to let go of the container it is watching — a removal that does not + // match how it subscribed takes nothing off — and give double-click zoom + // back on the way out, since nothing else will now that terra-draw's mode + // is already stopped. + test('lets go of the container and double-click zoom when the adapter is destroyed', () => { + vi.useFakeTimers() + try { + const { adapter, mockMap, container } = setupDrawing() + const zoom = makeDoubleClickZoom() + mockMap.doubleClickZoom = zoom + adapter._terraDraw = makeTerraDraw(zoom) + + stopOnPointer(adapter) + expect(container.listenerCount()).toBe(2) + expect(zoom.enabled()).toBe(false) + + adapter.destroy() + + expect(container.listenerCount()).toBe(0) + expect(zoom.enabled()).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + // A plugin ending the drawing from its own panel — a Finish button, a + // mode switch, the Escape it handles itself — never touched the map, so + // the click the user makes next is theirs from the first one. The same + // arm-with-no-recent-pointer path covers a session ended on Enter. + test('a session a plugin ended does not swallow the click that follows', () => { + const { adapter, clicks, picks, click } = setupDrawing() + + adapter.disableDrawing() + click() + + expect(clicks).toEqual([{ lat: 40, lng: -120 }]) + expect(picks).toHaveLength(1) + }) +}) + // ─── onFeatureHover ─────────────────────────────────────────────────────────── test.describe('LeafletAdapter - onFeatureHover', () => { diff --git a/tests/unit/deckGLAdapter.spec.js b/tests/unit/deckGLAdapter.spec.js index 8f4135b3c..8abb6effa 100644 --- a/tests/unit/deckGLAdapter.spec.js +++ b/tests/unit/deckGLAdapter.spec.js @@ -6,6 +6,57 @@ import { DeckGLAdapter } from '../../src/essence/Basics/MapEngines/Adapters/Deck import { MAP_ENGINE } from '../../src/essence/Basics/MapEngines/types/engine.ts' import { drawModeKeyEvents } from '../../src/essence/Basics/MapEngines/Adapters/DrawingHelpers.ts' +// Both init() branches build their engine through a real constructor, so the +// props they wire the adapter's handlers into can only be read back off that +// constructor. jsdom has no WebGL, so the constructors are replaced — and only +// they are: everything else the adapter imports alongside them stays real. +const constructed = vi.hoisted(() => ({ deck: [], overlay: [] })) + +vi.mock('@deck.gl/core', async (importOriginal) => { + const actual = await importOriginal() + class MockDeck { + constructor(props) { + constructed.deck.push(props) + } + setProps() {} + redraw() {} + finalize() {} + } + return { ...actual, Deck: MockDeck } +}) + +vi.mock('@deck.gl/mapbox', async (importOriginal) => { + const actual = await importOriginal() + class MockMapboxOverlay { + constructor(props) { + constructed.overlay.push(props) + } + setProps() {} + finalize() {} + } + return { ...actual, MapboxOverlay: MockMapboxOverlay } +}) + +vi.mock('maplibre-gl', async (importOriginal) => { + const actual = await importOriginal() + class MockMap { + constructor() { + this._canvas = document.createElement('canvas') + } + addControl() {} + removeControl() {} + on() {} + off() {} + once() {} + setMaxBounds() {} + remove() {} + getCanvas() { + return this._canvas + } + } + return { ...actual, Map: MockMap } +}) + function makeAdapter({ longitude = -120, latitude = 40, zoom = 5 } = {}) { const adapter = new DeckGLAdapter() adapter._viewState = { longitude, latitude, zoom, bearing: 0, pitch: 0 } @@ -16,6 +67,68 @@ function makeLayer(id, props = {}) { return { id, ...props, clone: (overrides = {}) => makeLayer(id, { ...props, ...overrides }) } } +// Just enough of the maplibre Map API for TerraDrawMapLibreGLAdapter to +// construct, register its layers, place a pointer on the globe, and tear itself +// down. Registered ids are tracked so getLayer() answers the way a real style +// would. The map is given a size and put in the document because terra-draw +// drops a pointer that falls outside the map's own bounds, and because an +// event only reaches a window listener from a node that is in the page. +function makeDrawingBasemap() { + const canvas = document.createElement('canvas') + const container = document.createElement('div') + container.appendChild(canvas) + document.body.appendChild(container) + const bounds = { + x: 0, y: 0, left: 0, top: 0, right: 800, bottom: 600, width: 800, height: 600, + } + container.getBoundingClientRect = () => bounds + canvas.getBoundingClientRect = () => bounds + const styleLayers = new Set() + return { + getContainer: () => container, + getCanvas: () => canvas, + project: ({ lng, lat }) => ({ x: lng, y: lat }), + unproject: ({ x, y }) => ({ lng: x, lat: y }), + dragRotate: { isEnabled: () => true, enable: () => {}, disable: () => {} }, + dragPan: { isEnabled: () => true, enable: () => {}, disable: () => {} }, + doubleClickZoom: { enable: () => {}, disable: () => {} }, + addSource: vi.fn(), + addLayer: vi.fn((layer) => styleLayers.add(layer.id)), + removeLayer: vi.fn((id) => styleLayers.delete(id)), + removeSource: vi.fn(), + getLayer: (id) => (styleLayers.has(id) ? { id } : undefined), + getSource: () => ({ setData: () => {} }), + setStyle: vi.fn(() => styleLayers.clear()), + off: vi.fn(), + removeControl: vi.fn(), + remove: vi.fn(), + version: '5.8.0', + } +} + +/** An overlay-mode adapter a real terra-draw session can be started on. */ +function makeOverlayDrawingAdapter() { + const adapter = makeAdapter() + adapter._isOverlayMode = true + adapter._basemap = makeDrawingBasemap() + adapter._overlay = { setProps: vi.fn(), finalize: vi.fn() } + return adapter +} + +/** + * End a drawing session the way a click on the map does. terra-draw commits + * from inside the pointerup, so the adapter's pointer watch is looking at that + * very event as the session ends — which is what tells the guard a click of + * the drawing's is still to come. + */ +function stopOnPointer(adapter) { + adapter._drawPointers.start() + const stop = () => adapter._stopDrawing() + window.addEventListener('pointerup', stop) + window.dispatchEvent(new Event('pointerup')) + window.removeEventListener('pointerup', stop) +} + test.describe('DeckGLAdapter', () => { test.describe('engineType', () => { test('is deckgl', () => { @@ -354,48 +467,13 @@ test.describe('DeckGLAdapter', () => { test.describe('drawing overlay stacking', () => { const ANCHOR_ID = 'td-polygon' - // Just enough of the maplibre Map API for TerraDrawMapLibreGLAdapter - // to construct, register its layers, and tear them down. Registered ids - // are tracked so getLayer() answers the way a real style would. - function makeDrawingBasemap() { - const canvas = document.createElement('canvas') - const container = document.createElement('div') - const styleLayers = new Set() - return { - getContainer: () => container, - getCanvas: () => canvas, - dragRotate: { isEnabled: () => true, enable: () => {}, disable: () => {} }, - dragPan: { isEnabled: () => true, enable: () => {}, disable: () => {} }, - doubleClickZoom: { enable: () => {}, disable: () => {} }, - addSource: vi.fn(), - addLayer: vi.fn((layer) => styleLayers.add(layer.id)), - removeLayer: vi.fn((id) => styleLayers.delete(id)), - removeSource: vi.fn(), - getLayer: (id) => (styleLayers.has(id) ? { id } : undefined), - getSource: () => ({ setData: () => {} }), - setStyle: vi.fn(() => styleLayers.clear()), - off: vi.fn(), - removeControl: vi.fn(), - remove: vi.fn(), - version: '5.8.0', - } - } - - function makeDrawingAdapter() { - const adapter = makeAdapter() - adapter._isOverlayMode = true - adapter._basemap = makeDrawingBasemap() - adapter._overlay = { setProps: vi.fn(), finalize: vi.fn() } - return adapter - } - function lastSyncedLayers(adapter) { const calls = adapter._overlay.setProps.mock.calls return calls[calls.length - 1][0].layers } test('enableDrawing anchors every deck layer below the terra-draw stack', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() adapter.addLayer(makeLayer('raster')) adapter.addLayer(makeLayer('vector')) adapter.enableDrawing('polygon') @@ -406,20 +484,20 @@ test.describe('DeckGLAdapter', () => { }) test('the anchor id matches the bottom-most layer terra-draw registers', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() adapter.enableDrawing('polygon') expect(adapter._basemap.addLayer.mock.calls[0][0].id).toBe('td-polygon') }) test('layers added mid-draw are anchored too', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() adapter.enableDrawing('rectangle') adapter.addLayer(makeLayer('added-mid-draw')) expect(lastSyncedLayers(adapter).map((l) => l.beforeId)).toEqual([ANCHOR_ID]) }) test('no anchor is stamped while the terra-draw layers are out of the style', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() adapter.addLayer(makeLayer('raster')) adapter.enableDrawing('polygon') adapter._basemap.getLayer = () => undefined @@ -431,7 +509,7 @@ test.describe('DeckGLAdapter', () => { }) test('setBasemapStyle drops the anchor before the swap wipes the terra-draw layers', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() adapter.addLayer(makeLayer('raster')) adapter.enableDrawing('polygon') adapter.setBasemapStyle('https://example.com/style.json') @@ -442,7 +520,7 @@ test.describe('DeckGLAdapter', () => { }) test('setBasemapStyle cancels the live drawing session', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() const cancels = [] adapter.on('drawcancel', (e) => cancels.push(e)) adapter.enableDrawing('polygon') @@ -452,7 +530,7 @@ test.describe('DeckGLAdapter', () => { }) test('destroy cancels the live drawing session', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() const cancels = [] adapter.on('drawcancel', (e) => cancels.push(e)) adapter.enableDrawing('polygon') @@ -461,7 +539,7 @@ test.describe('DeckGLAdapter', () => { }) test('disableDrawing drops the anchor', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() adapter.addLayer(makeLayer('raster')) adapter.enableDrawing('polygon') adapter.disableDrawing() @@ -469,7 +547,7 @@ test.describe('DeckGLAdapter', () => { }) test('the layer registry keeps the original un-anchored instances', () => { - const adapter = makeDrawingAdapter() + const adapter = makeOverlayDrawingAdapter() const original = makeLayer('raster') adapter.addLayer(original) adapter.enableDrawing('polygon') @@ -690,6 +768,190 @@ test.describe('DeckGLAdapter', () => { expect(keys).toEqual(['Enter']) }) + // A deck pick, whose `coordinate` is in [lng, lat] order. + const pickAt = (lng, lat) => ({ coordinate: [lng, lat], x: 12, y: 34 }) + + // deck reports a click through its `click` recognizer, which waits for + // a double-click to fail before it fires, so the click terra-draw + // committed the shape on arrives long after the session it ended — with + // `_drawingShape` already null, so that guard is no longer looking. + // Reporting it hands every consumer a map click the user never made, + // one that would dismiss the popup a plugin opened from the + // `drawcomplete` that came first. + test('the click a drawing ended on is not reported as a map click', () => { + const { adapter } = makeSessionAdapter('rectangle') + const clicks = [] + const picks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + adapter.onFeatureClick((result) => picks.push(result)) + + // terra-draw commits on pointerup, and the adapter ends the session + // there and then — this is what deck calls afterwards. + stopOnPointer(adapter) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([]) + expect(picks).toEqual([]) + }) + + // A pointer that goes down more than a tap interval after the finish is + // too late to be the second tap of a double-click that ended the + // drawing, so it is the user's own next gesture. deck reports its click + // a further interval later, once the double-click that would have + // outranked it has failed. + test('the click that starts the next gesture is still reported', () => { + vi.useFakeTimers() + try { + const { adapter, canvas } = makeSessionAdapter('rectangle') + const clicks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + + stopOnPointer(adapter) + vi.advanceTimersByTime(400) + canvas.dispatchEvent(new Event('pointerdown')) + vi.advanceTimersByTime(50) + canvas.dispatchEvent(new Event('pointerup')) + vi.advanceTimersByTime(300) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([{ lat: 40, lng: -120 }]) + } finally { + vi.useRealTimers() + } + }) + + // Finishing on a double-click is trained behaviour, and terra-draw + // commits on the first of the two taps. deck maps `dblclick` onto + // `onClick` alongside `click` (@deck.gl/core EVENT_HANDLERS), and + // because it wires the two recognizers to require each other's failure, + // the winner emits a tap interval after the second tap's pointerup — + // half a second after the drawing was committed, and after the + // pointerdown that used to be taken as proof of a new gesture. + test('the double-click a drawing ended on is not reported as a map click', () => { + vi.useFakeTimers() + try { + const { adapter, canvas } = makeSessionAdapter('rectangle') + const clicks = [] + const picks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + adapter.onFeatureClick((result) => picks.push(result)) + + // Tap 1's pointerup is where terra-draw commits; the guard's + // own listeners only go on from here, so that pointerup is not + // one of the events it sees. + stopOnPointer(adapter) + + // Tap 2, inside the 300ms interval that makes the pair a + // double-click. + vi.advanceTimersByTime(150) + canvas.dispatchEvent(new Event('pointerdown')) + vi.advanceTimersByTime(50) + canvas.dispatchEvent(new Event('pointerup')) + + // The recognizer's own wait, and then the click. + vi.advanceTimersByTime(300) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([]) + expect(picks).toEqual([]) + } finally { + vi.useRealTimers() + } + }) + + // The widest the finishing gesture can be: the second tap released at + // the very edge of the interval that still makes it a double-click, + // and deck's recognizer waiting a full interval on top of that. + test('a double-click finish is covered to the edge of the interval', () => { + vi.useFakeTimers() + try { + const { adapter, canvas } = makeSessionAdapter('rectangle') + const clicks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + + stopOnPointer(adapter) + vi.advanceTimersByTime(250) + canvas.dispatchEvent(new Event('pointerdown')) + vi.advanceTimersByTime(49) + canvas.dispatchEvent(new Event('pointerup')) + vi.advanceTimersByTime(300) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([]) + } finally { + vi.useRealTimers() + } + }) + + // The click a finishing gesture was covered for does not always come: + // a pointerup deck reads as the end of a drag produces none. The window + // closes on its own so the guard cannot sit there absorbing the user's + // clicks indefinitely. + test('the guard stops absorbing once the finish window has passed', () => { + vi.useFakeTimers() + try { + const { adapter } = makeSessionAdapter('rectangle') + const clicks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + + stopOnPointer(adapter) + vi.advanceTimersByTime(600) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([{ lat: 40, lng: -120 }]) + } finally { + vi.useRealTimers() + } + }) + + // terra-draw turns double-click zoom back on the moment the mode stops, + // so the second click of a double-click finish would zoom the map on + // top of everything else it does. Hold that re-enable back for as long + // as the guard is still absorbing the same gesture's clicks. + test('double-click zoom stays off until the finish window closes', () => { + vi.useFakeTimers() + try { + const { adapter, canvas } = makeSessionAdapter('rectangle') + let enabled = false + adapter._basemap = { + getCanvas: () => canvas, + doubleClickZoom: { + isEnabled: () => enabled, + enable: () => { enabled = true }, + disable: () => { enabled = false }, + }, + } + // Stopping the mode is what turns it back on, inside + // _stopDrawing. + adapter._terraDraw = { + clear: () => { }, + stop: () => { enabled = true }, + } + + stopOnPointer(adapter) + expect(enabled).toBe(false) + + vi.advanceTimersByTime(600) + expect(enabled).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + // A plugin ending the drawing from its own panel — a Finish button, a + // mode switch, the Escape it handles itself — never touched the map, so + // the click the user makes next is theirs from the first one. + test('a session a plugin ended does not swallow the click that follows', () => { + const { adapter } = makeSessionAdapter('polygon') + const clicks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + + adapter.disableDrawing() + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([{ lat: 40, lng: -120 }]) + }) + test('disableDrawing emits drawcancel once', () => { const { adapter } = makeDrawingAdapter({ finishes: false }) const shapes = [] @@ -700,6 +962,191 @@ test.describe('DeckGLAdapter', () => { }) }) + // Which clicks a finish leaves behind is not something terra-draw says: it + // emits `finish` from inside whatever event it is reacting to, and the + // pointer that placed the last vertex may be a separate event again. These + // drive real terra-draw sessions, so the answer comes from the events the + // way it does on a real map. + test.describe('drawing - the clicks a finish leaves behind', () => { + const pointer = (canvas, type, x, y) => + canvas.dispatchEvent( + new PointerEvent(type, { + clientX: x, + clientY: y, + bubbles: true, + isPrimary: true, + }) + ) + + /** A two-vertex linestring, one click short of a finish. */ + const drawLine = (adapter, canvas) => { + adapter.enableDrawing('linestring') + pointer(canvas, 'pointerdown', 10, 10) + pointer(canvas, 'pointerup', 10, 10) + pointer(canvas, 'pointermove', 50, 50) + pointer(canvas, 'pointerdown', 50, 50) + pointer(canvas, 'pointerup', 50, 50) + } + + // A deck pick, whose `coordinate` is in [lng, lat] order. + const pickAt = (lng, lat) => ({ coordinate: [lng, lat], x: 12, y: 34 }) + + // Point mode commits on the pointerup of the click that places it, and + // deck reports that click a tap interval later — with the session over. + test('a shape finished on a click leaves the guard covering it', () => { + const adapter = makeOverlayDrawingAdapter() + const canvas = adapter._basemap.getCanvas() + const finished = [] + adapter.on('drawcomplete', (e) => finished.push(e)) + + adapter.enableDrawing('point') + pointer(canvas, 'pointerdown', 10, 10) + pointer(canvas, 'pointerup', 10, 10) + + expect(finished).toHaveLength(1) + expect(adapter._drawEndClick.pending).toBe(true) + }) + + // Enter finishes from a keyup, but deck is still holding the click that + // placed the last vertex: it holds every one a tap interval to see + // whether a double-click is coming. That click lands with the session + // over, and reporting it would dismiss whatever a plugin opened from + // the `drawcomplete` the key produced a moment earlier. + test('the last vertex click deck still holds when Enter finishes is covered', () => { + vi.useFakeTimers() + try { + const adapter = makeOverlayDrawingAdapter() + const canvas = adapter._basemap.getCanvas() + const finished = [] + const clicks = [] + adapter.on('drawcomplete', (e) => finished.push(e)) + adapter.on('click', (e) => clicks.push(e.latlng)) + + drawLine(adapter, canvas) + vi.advanceTimersByTime(100) + canvas.dispatchEvent( + new KeyboardEvent('keyup', { key: 'Enter', bubbles: true }) + ) + + // deck's recognizer, a tap interval after that last pointerup. + vi.advanceTimersByTime(200) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(finished).toHaveLength(1) + expect(clicks).toEqual([]) + } finally { + vi.useRealTimers() + } + }) + + // Enter used the way it usually is — the shape read back, then the key + // — comes with the last vertex click long delivered, so there is + // nothing left to cover and the user's next click is theirs. + test('a shape finished on Enter with the pointer idle leaves the guard out of the way', () => { + vi.useFakeTimers() + try { + const adapter = makeOverlayDrawingAdapter() + const canvas = adapter._basemap.getCanvas() + const finished = [] + const clicks = [] + adapter.on('drawcomplete', (e) => finished.push(e)) + adapter.on('click', (e) => clicks.push(e.latlng)) + + drawLine(adapter, canvas) + vi.advanceTimersByTime(400) + canvas.dispatchEvent( + new KeyboardEvent('keyup', { key: 'Enter', bubbles: true }) + ) + + expect(finished).toHaveLength(1) + expect(adapter._drawEndClick.pending).toBe(false) + + // The user's own next click, deck reporting it an interval + // after the gesture as always. + canvas.dispatchEvent(new Event('pointerdown')) + canvas.dispatchEvent(new Event('pointerup')) + vi.advanceTimersByTime(300) + adapter._onPointerClick(pickAt(-120, 40)) + + expect(clicks).toEqual([{ lat: 40, lng: -120 }]) + } finally { + vi.useRealTimers() + } + }) + }) + + // The props the engine is constructed with are where the adapter's handlers + // are connected to deck's input. Calling those handlers directly, as the + // tests above do, says nothing about which function deck ends up calling, + // and each mode wires its own copy. + test.describe('constructed engine props', () => { + const CONTAINER_ID = 'deckgl-init' + const MAPLIBRE_BASEMAP = { + provider: 'maplibre', + style: 'https://example.com/style.json', + } + + // A deck pick, whose `coordinate` is in [lng, lat] order. + const pickAt = (lng, lat) => ({ coordinate: [lng, lat], x: 12, y: 34 }) + + // Run the real init() path and hand back the props of whichever engine + // it built: deck's own in standalone mode, the MapboxOverlay's in + // overlay mode. + function initAdapter(basemap) { + constructed.deck.length = 0 + constructed.overlay.length = 0 + let container = document.getElementById(CONTAINER_ID) + if (!container) { + container = document.createElement('div') + container.id = CONTAINER_ID + document.body.appendChild(container) + } + const adapter = new DeckGLAdapter() + adapter.init({ + containerId: CONTAINER_ID, + center: { lat: 40, lng: -120 }, + zoom: 5, + ...(basemap ? { basemap } : {}), + }) + return { + adapter, + props: basemap ? constructed.overlay[0] : constructed.deck[0], + } + } + + for (const [mode, basemap] of [ + ['standalone', null], + ['overlay', MAPLIBRE_BASEMAP], + ]) { + test(`${mode} mode reports the clicks deck picks`, () => { + const { adapter, props } = initAdapter(basemap) + const clicks = [] + const picks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + adapter.onFeatureClick((result) => picks.push(result)) + + props.onClick(pickAt(-120, 40)) + + expect(clicks).toEqual([{ lat: 40, lng: -120 }]) + expect(picks).toHaveLength(1) + }) + + test(`${mode} mode holds back the clicks a drawing takes as vertices`, () => { + const { adapter, props } = initAdapter(basemap) + const clicks = [] + const picks = [] + adapter.on('click', (e) => clicks.push(e.latlng)) + adapter.onFeatureClick((result) => picks.push(result)) + adapter._drawingShape = 'polygon' + + props.onClick(pickAt(-120, 40)) + + expect(clicks).toEqual([]) + expect(picks).toEqual([]) + }) + } + }) + test.describe('destroy', () => { test('clears all layers', () => { const adapter = makeAdapter()