diff --git a/internal/web/static/css/openfsd/airport-editor.css b/internal/web/static/css/openfsd/airport-editor.css index 5bc5ed5..5fd05ef 100644 --- a/internal/web/static/css/openfsd/airport-editor.css +++ b/internal/web/static/css/openfsd/airport-editor.css @@ -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 */ diff --git a/internal/web/static/js/openfsd/airport-editor/main.js b/internal/web/static/js/openfsd/airport-editor/main.js index 1dbbc9f..e02f17d 100644 --- a/internal/web/static/js/openfsd/airport-editor/main.js +++ b/internal/web/static/js/openfsd/airport-editor/main.js @@ -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 }); diff --git a/internal/web/static/js/openfsd/airport-editor/map-layers.js b/internal/web/static/js/openfsd/airport-editor/map-layers.js index 370a87d..f8f574e 100644 --- a/internal/web/static/js/openfsd/airport-editor/map-layers.js +++ b/internal/web/static/js/openfsd/airport-editor/map-layers.js @@ -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); diff --git a/webjs/airport-editor/map-layers.test.js b/webjs/airport-editor/map-layers.test.js index 10d12ec..cdf7b18 100644 --- a/webjs/airport-editor/map-layers.test.js +++ b/webjs/airport-editor/map-layers.test.js @@ -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'); +});