From 184f694d3e2e2c747f53669a358cacb881dc46a9 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Wed, 1 Jul 2026 09:41:45 +0100 Subject: [PATCH 1/2] Min vertecies draw completion fix --- .../beta/draw-ml/src/modes/createDrawMode.js | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/plugins/beta/draw-ml/src/modes/createDrawMode.js b/plugins/beta/draw-ml/src/modes/createDrawMode.js index 069b5b00..e19863d5 100644 --- a/plugins/beta/draw-ml/src/modes/createDrawMode.js +++ b/plugins/beta/draw-ml/src/modes/createDrawMode.js @@ -1,4 +1,4 @@ -import createVertex from '../../../../../node_modules/@mapbox/mapbox-gl-draw/src/lib/create_vertex.js' +import createVertex from '../../../../../node_modules/@mapbox/mapbox-gl-draw/src/lib/create_vertex.js' // NOSONAR import { getSnapInstance, @@ -23,7 +23,7 @@ import { * @param {Function} config.validateClick - Validation function for clicks * @param {Function} config.createVertices - Function to create vertex display features */ -export const createDrawMode = (ParentMode, config) => { +export const createDrawMode = (ParentMode, config) => { // NOSONAR — factory returns a single cohesive mode object; splitting across files would obscure the event flow const { featureProp, geometryType, @@ -35,6 +35,7 @@ export const createDrawMode = (ParentMode, config) => { } = config const getFeature = (state) => state[featureProp] + const RUBBER_BAND_OFFSET = 2 // ring is [...placed, last_placed, rubber_band]; splice(-2,1) removes last_placed return { ...ParentMode, @@ -45,7 +46,7 @@ export const createDrawMode = (ParentMode, config) => { // Some parent modes (DrawLineString) interpret featureId as "continue existing" // rather than "use this ID for new feature" const parentOptions = excludeFeatureIdFromSetup - ? { ...options, featureId: undefined } + ? { ...options, featureId: null } : options const state = { @@ -122,9 +123,10 @@ export const createDrawMode = (ParentMode, config) => { } const coordsBefore = getCoords(getFeature(state)).length ParentMode.onClick.call(this, state, e) - // Push undo if a vertex was added + // Push undo and update count if a vertex was added if (getCoords(getFeature(state)).length > coordsBefore) { this.pushDrawUndo(state) + this.dispatchVertexChange(getCoords(getFeature(state))) } }, @@ -172,7 +174,7 @@ export const createDrawMode = (ParentMode, config) => { }, dispatchVertexChange (coords) { - // Subtract 1 to exclude the rubber-band vertex that mapbox-gl-draw always appends + // Both polygon ring and LineString store [v0...vN, rubber_band] during drawing — subtract 1 to get placed vertex count this.map.fire('draw.vertexchange', { numVertecies: Math.max(0, coords.length - 1) }) @@ -274,7 +276,7 @@ export const createDrawMode = (ParentMode, config) => { _removeLastVertex (state, feature, coords) { // Structure during drawing: [v1, v2, ..., vN, rubber_band] const ring = geometryType === 'Polygon' ? feature.coordinates[0] : coords - ring.splice(ring.length - 2, 1) + ring.splice(-RUBBER_BAND_OFFSET, 1) // Snap rubber band to new last vertex position const newLastVertex = ring[ring.length - 2] @@ -383,32 +385,36 @@ export const createDrawMode = (ParentMode, config) => { this.onMove(state, e) }, + _handleUndoKeydown (state, e) { + const tag = document.activeElement?.tagName + if (tag === 'INPUT' || tag === 'TEXTAREA') { + return + } + e.preventDefault() + e.stopPropagation() + const undoStack = this.map._undoStack + if (undoStack && undoStack.length > 0) { + const operation = undoStack.pop() + if (operation?.type === 'draw_vertex') { + // Set flag to prevent click interference during undo + this.map._undoInProgress = true + setTimeout(() => { this.map._undoInProgress = false }, 100) + this.undoVertex(state) + } + } + }, + onKeydown (state, e) { - // Undo with Cmd/Ctrl+Z (works without viewport focus, but not in input fields) if (e.key === 'z' && (e.metaKey || e.ctrlKey) && !e.shiftKey) { - const tag = document.activeElement?.tagName - if (tag === 'INPUT' || tag === 'TEXTAREA') { - return - } - e.preventDefault() - e.stopPropagation() - const undoStack = this.map._undoStack - if (undoStack && undoStack.length > 0) { - const operation = undoStack.pop() - if (operation?.type === 'draw_vertex') { - // Set flag to prevent click interference during undo - this.map._undoInProgress = true - setTimeout(() => { this.map._undoInProgress = false }, 100) - this.undoVertex(state) - } - } + this._handleUndoKeydown(state, e) return } if (document.activeElement !== state.container) { return } if (e.key === 'Escape') { - return e.preventDefault() + e.preventDefault() + return } if (e.key === 'Enter') { state.isActive = true From 3b06931fabf81ab3bb3b03ee80ae00378bdff487 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Wed, 1 Jul 2026 14:01:21 +0100 Subject: [PATCH 2/2] Delete vertex fix on midpoint selection --- .../src/modes/editVertex/vertexOperations.js | 9 +-- .../beta/draw-ml/src/modes/editVertexMode.js | 59 ++++++++++++------- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/plugins/beta/draw-ml/src/modes/editVertex/vertexOperations.js b/plugins/beta/draw-ml/src/modes/editVertex/vertexOperations.js index e5f15b60..634754e7 100644 --- a/plugins/beta/draw-ml/src/modes/editVertex/vertexOperations.js +++ b/plugins/beta/draw-ml/src/modes/editVertex/vertexOperations.js @@ -119,11 +119,12 @@ export const vertexOperations = { const deletedPosition = [...state.vertecies[state.selectedVertexIndex]] const deletedIndex = state.selectedVertexIndex - this._ctx.api.trash() - - // Clear DirectSelect's coordinate selection to prevent visual artifacts + // Remove the coordinate directly rather than via this._ctx.api.trash(), which routes through + // DirectSelect.trash() and calls onSetup({ featureId }) with incomplete options, crashing event registration. + const coordPath = [...result.segment.path, result.localIdx].join('.') + feature.removeCoordinate(coordPath) + this.fireUpdate() this.clearSelectedCoordinates() - // Force feature re-render to clear vertex highlights feature.changed() this._ctx.store.render() diff --git a/plugins/beta/draw-ml/src/modes/editVertexMode.js b/plugins/beta/draw-ml/src/modes/editVertexMode.js index 22a6c2af..5aa9fb7d 100755 --- a/plugins/beta/draw-ml/src/modes/editVertexMode.js +++ b/plugins/beta/draw-ml/src/modes/editVertexMode.js @@ -1,4 +1,4 @@ -import DirectSelect from '../../../../../node_modules/@mapbox/mapbox-gl-draw/src/modes/direct_select.js' +import DirectSelect from '../../../../../node_modules/@mapbox/mapbox-gl-draw/src/modes/direct_select.js' // NOSONAR import { getSnapInstance, isSnapActive, isSnapEnabled, getSnapLngLat, getSnapRadius, triggerSnapAtPoint, clearSnapIndicator, clearSnapState @@ -44,25 +44,7 @@ export const EditVertexMode = { state.midpoints = this.getMidpoints(state.featureId) this.setupEventListeners(state) - if (options.selectedVertexType === 'midpoint') { - // Clear any vertex selection when switching to midpoint - state.selectedCoordPaths = [] - this.clearSelectedCoordinates() - // Force feature re-render to clear vertex highlights - if (state.feature) { - state.feature.changed() - } - this._ctx.store.render() - this.updateMidpoint(state.midpoints[options.selectedVertexIndex - state.vertecies.length]) - } else if (options.selectedVertexIndex === -1) { - // Explicitly clear selection when re-entering with no vertex selected - state.selectedCoordPaths = [] - this.clearSelectedCoordinates() - if (state.feature) { - state.feature.changed() - } - this._ctx.store.render() - } + this.applyVertexSelection(state, options) this.map._drawEditContainer = options.container this.addTouchVertexTarget(state) @@ -122,7 +104,27 @@ export const EditVertexMode = { this.map.on('move', h.move) }, + applyVertexSelection (state, options) { + if (options.selectedVertexType === 'midpoint') { + state.selectedCoordPaths = [] + this.clearSelectedCoordinates() + if (state.feature) { state.feature.changed() } + this._ctx.store.render() + this.updateMidpoint(state.midpoints[options.selectedVertexIndex - state.vertecies.length]) + return + } + if (options.selectedVertexIndex === -1) { + state.selectedCoordPaths = [] + this.clearSelectedCoordinates() + if (state.feature) { state.feature.changed() } + this._ctx.store.render() + } + }, + onSelectionChange (state, e) { + // Refresh vertex list so numVertecies reflects the latest geometry (e.g. after midpoint insertion) + this.syncVertices(state) + const vertexCoord = e.points[e.points.length - 1]?.geometry.coordinates // Only update selectedVertexIndex from event if not keyboard mode AND event has valid vertex @@ -325,6 +327,21 @@ export const EditVertexMode = { } }, + onClick (state, e) { // NOSONAR — complexity accumulated from object-level context; single guard clause, see feedback_mgl_click_vs_mouseup.md + if (state._isInsertingVertex && state._insertedVertexIndex !== undefined) { + const insertedIndex = state._insertedVertexIndex + this.syncVertices(state) + this.pushUndo({ type: 'insert_vertex', featureId: state.featureId, vertexIndex: insertedIndex }) + state.selectedVertexIndex = insertedIndex + state.selectedVertexType = 'vertex' + state._isInsertingVertex = false + state._insertedVertexIndex = undefined + this.map.fire('draw.vertexselection', { index: insertedIndex, numVertecies: state.vertecies.length }) + return + } + DirectSelect.onClick.call(this, state, e) + }, + onMouseUp (state, e) { clearSnapState(getSnapInstance(this.map)) @@ -376,6 +393,8 @@ export const EditVertexMode = { vertexIndex: state._moveStartIndex, previousPosition: state._moveStartPosition }) + } else { + // No action } }