Skip to content

Adapters: stop reporting a finished drawing's clicks as map clicks #352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CarsonDavis wants to merge 1 commit into
base: development
Choose a base branch
from
Open

Adapters: stop reporting a finished drawing's clicks as map clicks #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 42 additions & 20 deletions src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ import {
import { TerraDrawMapLibreGLAdapter } from 'terra-draw-maplibre-gl-adapter'
import {
committedVerticesFromChange,
DrawEndClickGuard,
drawModeKeyEvents,
DrawPointerWatch,
drawStyles,
validateDrawnLineString,
} from './DrawingHelpers'
Expand Down Expand Up @@ -276,6 +278,8 @@ export class DeckGLAdapter implements IMapEngine<Deck, Layer, PickingInfo> {
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<string, () => void>()
Expand Down Expand Up @@ -380,6 +384,9 @@ export class DeckGLAdapter implements IMapEngine<Deck, Layer, PickingInfo> {
// 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 = []
Expand Down Expand Up @@ -1115,6 +1122,7 @@ export class DeckGLAdapter implements IMapEngine<Deck, Layer, PickingInfo> {
td.clear()
td.setMode(shape)
this._drawingShape = shape
this._drawPointers.start()
this._syncLayers()
this._emitEvent('drawstart', { shape })
}
Expand All @@ -1139,6 +1147,19 @@ export class DeckGLAdapter implements IMapEngine<Deck, Layer, PickingInfo> {
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
}
Expand Down Expand Up @@ -1281,19 +1302,28 @@ export class DeckGLAdapter implements IMapEngine<Deck, Layer, PickingInfo> {
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.
Expand Down Expand Up @@ -1355,16 +1385,8 @@ export class DeckGLAdapter implements IMapEngine<Deck, Layer, PickingInfo> {
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)
Expand Down
249 changes: 249 additions & 0 deletions src/essence/Basics/MapEngines/Adapters/DrawingHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof setTimeout> | 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.
*
Expand Down
Loading
Loading