airport-editor: fix vertex drag (live geometry, handles, click suppress)

Make selected surface vertices reliably draggable with live polyline/marker
updates, single aligned handles, and post-drag map-click suppress that does
not block full refresh on dragend.
This commit is contained in:
Reese Norris
2026-07-27 14:43:11 -04:00
parent dd9f61a460
commit f2dd5a609c
4 changed files with 332 additions and 40 deletions

View File

@@ -253,17 +253,32 @@
cursor: default;
}
/* Vertex drag handles (Leaflet divIcon) */
/* Vertex drag handles (Leaflet divIcon).
* Width/height MUST match VERTEX_HANDLE_PX (map-layers.js) — keep in sync.
* Do not use negative margin; Leaflet positions via iconAnchor. */
.apted-vertex-handle {
width: 12px !important;
height: 12px !important;
margin-left: -6px !important;
margin-top: -6px !important;
position: relative; /* containing block for ::after hit pad */
overflow: visible; /* do not clip expanded hit area */
width: 16px !important;
height: 16px !important;
margin: 0 !important; /* positioning is iconAnchor only */
border: 2px solid #4a7ab0;
border-radius: 50%;
background: #fff;
box-shadow: 0 0 0 1px rgba(0, 0, 0, 0.25);
cursor: move;
cursor: grab;
box-sizing: border-box;
}
.apted-vertex-handle:active {
cursor: grabbing;
background: #e8f0fa;
}
/* Expanded hit area without growing the visible disc.
* Relies on position:relative + overflow:visible on the icon. */
.apted-vertex-handle::after {
content: '';
position: absolute;
inset: -6px; /* ~28px effective target */
}
/* Inspector form density */

View File

@@ -114,12 +114,12 @@ function main() {
},
onVertexDrag(si, vi, lat, lon) {
setVertex(doc, si, vi, { lat, lon });
// Live-update surface polyline without full refresh of handles mid-drag.
// Full refresh on dragend.
// Intentionally no refresh — OverlayController live-updates the surface layer.
// Hard invariant: never refresh/render while OverlayController._dragging.
},
onVertexDragEnd(si, vi, lat, lon) {
setVertex(doc, si, vi, { lat, lon });
afterAptMutation();
afterAptMutation({ fit: false }); // full refresh — _dragging already false
},
onAircraftDrag(index, lat, lon) {
updateAircraft(doc, index, { lat, lon });

View File

@@ -202,6 +202,111 @@ export function escapeHtml(s) {
.replace(/'/g, ''');
}
/** Keep in sync with .apted-vertex-handle width/height in airport-editor.css */
export const VERTEX_HANDLE_PX = 16;
/** Duration after dragend during which map clicks are ignored. */
export const MAP_CLICK_SUPPRESS_MS = 250;
/**
* Pure: options for the vertex L.marker (no Leaflet instance required).
* Icon is constructed at the call site with L.divIcon({...buildVertexHandleIconOptions()}).
* @param {number} vertexIndex
* @returns {object}
*/
export function buildVertexHandleOptions(vertexIndex) {
return {
draggable: true,
autoPan: false,
keyboard: false,
zIndexOffset: 2000,
bubblingMouseEvents: false,
title: `Vertex ${vertexIndex + 1}`,
};
}
/**
* Pure icon size/anchor for divIcon — symmetry asserted in unit tests.
* @returns {{ className: string, iconSize: [number, number], iconAnchor: [number, number] }}
*/
export function buildVertexHandleIconOptions() {
const px = VERTEX_HANDLE_PX;
return {
className: 'apted-vertex-handle leaflet-interactive',
iconSize: [px, px],
iconAnchor: [px / 2, px / 2],
};
}
/**
* Pure: points → Leaflet latlng tuples (finite only).
* @param {{lat:number,lon:number}[]} points
* @returns {Array<[number, number]>}
*/
export function pointsToLatLngs(points) {
const out = [];
if (!Array.isArray(points)) return out;
for (const p of points) {
if (Number.isFinite(p?.lat) && Number.isFinite(p?.lon)) {
out.push([p.lat, p.lon]);
}
}
return out;
}
/**
* Pure: apply latlngs to a surface layer duck-typed like Leaflet polyline/circleMarker.
* Prefer setLatLngs (polyline) then setLatLng (circleMarker). No-op if neither.
* @param {{ setLatLngs?: Function, setLatLng?: Function }|null|undefined} layer
* @param {Array<[number, number]>} latlngs
* @returns {'polyline'|'point'|'none'}
*/
export function applySurfaceLatLngs(layer, latlngs) {
if (!layer || !latlngs || latlngs.length === 0) return 'none';
if (typeof layer.setLatLngs === 'function') {
layer.setLatLngs(latlngs);
return 'polyline';
}
if (typeof layer.setLatLng === 'function') {
layer.setLatLng(latlngs[0]);
return 'point';
}
return 'none';
}
/**
* Pure: patch one vertex in a points array (immutable-style new array).
* @param {{lat:number,lon:number}[]} points
* @param {number} vertexIndex
* @param {number} lat
* @param {number} lon
* @returns {{lat:number,lon:number}[]}
*/
export function patchVertexPoints(points, vertexIndex, lat, lon) {
const src = Array.isArray(points) ? points : [];
return src.map((p, i) =>
i === vertexIndex ? { lat, lon } : { lat: p.lat, lon: p.lon },
);
}
/**
* Pure: whether a map click should be ignored (active drag or post-drag suppress window).
* Post-drag suppress does NOT block full render — only map click handlers use this.
* @param {{ dragging: boolean, dragEndedAt: number, suppressMs: number }} state
* @param {number} [now]
* @returns {boolean}
*/
export function shouldSuppressMapClick(state, now = Date.now()) {
if (state.dragging) return true;
if (
state.dragEndedAt > 0 &&
now - state.dragEndedAt < state.suppressMs
) {
return true;
}
return false;
}
/**
* Build a Leaflet tooltip content node with textContent only (never innerHTML).
* Leaflet 1.9 uses innerHTML for string content — always pass an Element.
@@ -381,8 +486,10 @@ export class OverlayController {
this._airport = null;
/** @type {import('./model.js').Aircraft[]} */
this._aircraft = [];
/** @type {boolean} */
/** @type {boolean} true only while a vertex/aircraft pointer drag is active */
this._dragging = false;
/** @type {number} ms timestamp of last dragend (0 = never / reset on dragstart) */
this._dragEndedAt = 0;
this._planeIcon = L.icon({
iconUrl: this.planeIconUrl,
iconSize: [16, 16],
@@ -391,7 +498,18 @@ export class OverlayController {
if (this.onMapClick) {
map.on('click', (ev) => {
if (this._dragging) return;
if (
shouldSuppressMapClick(
{
dragging: this._dragging,
dragEndedAt: this._dragEndedAt,
suppressMs: MAP_CLICK_SUPPRESS_MS,
},
Date.now(),
)
) {
return;
}
const ll = ev.latlng;
this.onMapClick(ll.lat, ll.lng, ev.originalEvent);
});
@@ -547,8 +665,21 @@ export class OverlayController {
this._layerByKey.set(key, layer);
}
/**
* Live-update the surface layer for surfaceIndex from points (no full render).
* Layer under surface:N is polyline (setLatLngs) or circleMarker (setLatLng).
* @param {number} surfaceIndex
* @param {{lat:number,lon:number}[]} points
*/
_liveSetSurfacePoints(surfaceIndex, points) {
const layer = this._layerByKey.get(`surface:${surfaceIndex}`);
applySurfaceLatLngs(layer, pointsToLatLngs(points));
}
/**
* Draggable vertex handles for selected surface.
* Single divIcon marker per vertex (no companion circleMarker).
* Normative drag state machine: paint first, clear _dragging before onVertexDragEnd.
* @param {import('./model.js').Surface} surface
* @param {number} surfaceIndex
*/
@@ -558,44 +689,36 @@ export class OverlayController {
for (let vi = 0; vi < pts.length; vi++) {
const p = pts[vi];
if (!Number.isFinite(p.lat) || !Number.isFinite(p.lon)) continue;
const marker = L.circleMarker([p.lat, p.lon], {
radius: 6,
color: '#4a7ab0',
weight: 2,
fillColor: '#fff',
fillOpacity: 1,
opacity: 1,
});
// Use drag via map events on mousedown — circleMarker is not draggable by default.
// Prefer Leaflet.Marker with divIcon for drag support when available.
const handle = L.marker([p.lat, p.lon], {
draggable: true,
zIndexOffset: 2000,
icon: L.divIcon({
className: 'apted-vertex-handle',
iconSize: [12, 12],
iconAnchor: [6, 6],
}),
title: `Vertex ${vi + 1}`,
...buildVertexHandleOptions(vi),
icon: L.divIcon(buildVertexHandleIconOptions()),
});
handle.on('dragstart', () => {
handle.on('dragstart', (ev) => {
this._dragging = true;
this._dragEndedAt = 0;
if (ev?.originalEvent) L.DomEvent.stopPropagation(ev.originalEvent);
});
handle.on('drag', (ev) => {
const ll = ev.target.getLatLng();
// 1) Overlay-local paint FIRST (order-independent of model callback).
const base = this._airport?.surfaces?.[surfaceIndex]?.points || [];
const next = patchVertexPoints(base, vi, ll.lat, ll.lng);
this._liveSetSurfacePoints(surfaceIndex, next);
// 2) Model commit path (main must not refresh).
this.onVertexDrag(surfaceIndex, vi, ll.lat, ll.lng);
});
handle.on('dragend', (ev) => {
const ll = ev.target.getLatLng();
// Final live paint (covers last frame).
const base = this._airport?.surfaces?.[surfaceIndex]?.points || [];
const next = patchVertexPoints(base, vi, ll.lat, ll.lng);
this._liveSetSurfacePoints(surfaceIndex, next);
// State machine: clear pointer-down flag BEFORE app refresh path.
this._dragEndedAt = Date.now();
this._dragging = false;
this.onVertexDragEnd(surfaceIndex, vi, ll.lat, ll.lng);
// Small delay so map click from drag end is ignored.
setTimeout(() => {
this._dragging = false;
}, 50);
});
handle.addTo(this.vertexGroup);
// Keep circle under for visibility if divIcon fails in tests.
marker.addTo(this.vertexGroup);
}
}
@@ -627,8 +750,10 @@ export class OverlayController {
this.onSelect({ type: 'aircraft', index });
});
if (this.editable) {
marker.on('dragstart', () => {
marker.on('dragstart', (ev) => {
this._dragging = true;
this._dragEndedAt = 0;
if (ev?.originalEvent) L.DomEvent.stopPropagation(ev.originalEvent);
});
marker.on('drag', (ev) => {
const ll = ev.target.getLatLng();
@@ -636,10 +761,10 @@ export class OverlayController {
});
marker.on('dragend', (ev) => {
const ll = ev.target.getLatLng();
// Same state machine as vertex: clear _dragging before callback so full refresh runs.
this._dragEndedAt = Date.now();
this._dragging = false;
this.onAircraftDragEnd(index, ll.lat, ll.lng);
setTimeout(() => {
this._dragging = false;
}, 50);
});
}
marker.addTo(this.group);

View File

@@ -1,6 +1,14 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
/**
* Pure OverlayController helpers.
*
* RC1 live path = applySurfaceLatLngs (polyline setLatLngs / point setLatLng).
* RC2 no companion circle factory in exports — single divIcon via builders.
* RC3 no CSS margin in JS positioning — iconSize/iconAnchor only.
* K5 suppress ≠ block render: dragging:false + within suppressMs still allows full render by contract.
*/
import {
surfaceStyle,
collectLatLngs,
@@ -14,6 +22,14 @@ import {
OSM_ATTRIBUTION,
ESRI_TILE_URL,
ESRI_ATTRIBUTION,
VERTEX_HANDLE_PX,
MAP_CLICK_SUPPRESS_MS,
buildVertexHandleOptions,
buildVertexHandleIconOptions,
pointsToLatLngs,
applySurfaceLatLngs,
patchVertexPoints,
shouldSuppressMapClick,
} from '../../internal/web/static/js/openfsd/airport-editor/map-layers.js';
import {
createEmptyDocument,
@@ -173,3 +189,139 @@ test('escapeHtml neutralizes markup in freeform AIR-like strings', () => {
// Surface-style label still escapes if ever routed through HTML
assert.equal(escapeHtml('G1 (park)'), 'G1 (park)');
});
test('pointsToLatLngs: empty, skips non-finite, preserves order', () => {
assert.deepEqual(pointsToLatLngs(null), []);
assert.deepEqual(pointsToLatLngs(undefined), []);
assert.deepEqual(pointsToLatLngs([]), []);
assert.deepEqual(
pointsToLatLngs([
{ lat: 1, lon: 2 },
{ lat: NaN, lon: 3 },
{ lat: 4, lon: Infinity },
{ lat: 5, lon: 6 },
]),
[
[1, 2],
[5, 6],
],
);
});
test('applySurfaceLatLngs: polyline, point, none branches', () => {
const polyCalls = [];
const poly = {
setLatLngs(ll) {
polyCalls.push(ll);
},
};
const latlngs = [
[10, 20],
[11, 21],
];
assert.equal(applySurfaceLatLngs(poly, latlngs), 'polyline');
assert.deepEqual(polyCalls, [latlngs]);
const pointCalls = [];
const point = {
setLatLng(ll) {
pointCalls.push(ll);
},
};
assert.equal(applySurfaceLatLngs(point, latlngs), 'point');
assert.deepEqual(pointCalls, [[10, 20]]);
assert.equal(applySurfaceLatLngs({}, latlngs), 'none');
assert.equal(applySurfaceLatLngs(null, latlngs), 'none');
assert.equal(applySurfaceLatLngs(poly, []), 'none');
assert.equal(applySurfaceLatLngs(poly, null), 'none');
});
test('patchVertexPoints: replaces only index vi; does not mutate input', () => {
const src = [
{ lat: 1, lon: 2 },
{ lat: 3, lon: 4 },
{ lat: 5, lon: 6 },
];
const next = patchVertexPoints(src, 1, 30, 40);
assert.deepEqual(next, [
{ lat: 1, lon: 2 },
{ lat: 30, lon: 40 },
{ lat: 5, lon: 6 },
]);
// Input not mutated
assert.deepEqual(src[1], { lat: 3, lon: 4 });
assert.deepEqual(patchVertexPoints(null, 0, 1, 2), []);
assert.deepEqual(patchVertexPoints([], 0, 1, 2), []);
});
test('shouldSuppressMapClick matrix (K5: suppress ≠ block render)', () => {
const ms = MAP_CLICK_SUPPRESS_MS;
assert.equal(ms, 250);
// dragging true → suppress (render still forbidden by caller invariant while pointer down)
assert.equal(
shouldSuppressMapClick({ dragging: true, dragEndedAt: 0, suppressMs: ms }, 1000),
true,
);
assert.equal(
shouldSuppressMapClick(
{ dragging: true, dragEndedAt: 900, suppressMs: ms },
1000,
),
true,
);
// dragging false + within suppressMs after dragEndedAt → suppress click
// (full render is still allowed by contract — separate from this helper)
assert.equal(
shouldSuppressMapClick(
{ dragging: false, dragEndedAt: 1000, suppressMs: ms },
1000 + ms - 1,
),
true,
);
// dragging false + after suppress window → do not suppress
assert.equal(
shouldSuppressMapClick(
{ dragging: false, dragEndedAt: 1000, suppressMs: ms },
1000 + ms,
),
false,
);
assert.equal(
shouldSuppressMapClick(
{ dragging: false, dragEndedAt: 1000, suppressMs: ms },
1000 + ms + 50,
),
false,
);
// dragEndedAt 0 → never suppress when not dragging
assert.equal(
shouldSuppressMapClick({ dragging: false, dragEndedAt: 0, suppressMs: ms }, 5000),
false,
);
});
test('buildVertexHandleIconOptions: iconSize/iconAnchor symmetry', () => {
const icon = buildVertexHandleIconOptions();
assert.equal(VERTEX_HANDLE_PX, 16);
assert.equal(icon.iconSize[0], VERTEX_HANDLE_PX);
assert.equal(icon.iconSize[1], VERTEX_HANDLE_PX);
assert.equal(icon.iconAnchor[0], VERTEX_HANDLE_PX / 2);
assert.equal(icon.iconAnchor[1], VERTEX_HANDLE_PX / 2);
assert.match(icon.className, /apted-vertex-handle/);
assert.match(icon.className, /leaflet-interactive/);
});
test('buildVertexHandleOptions: draggable, autoPan, bubblingMouseEvents', () => {
const opts = buildVertexHandleOptions(2);
assert.equal(opts.draggable, true);
assert.equal(opts.autoPan, false);
assert.equal(opts.keyboard, false);
assert.equal(opts.bubblingMouseEvents, false);
assert.equal(opts.zIndexOffset, 2000);
assert.equal(opts.title, 'Vertex 3');
});