From fbf951f0054bd7958346f4c6caa46d15cf1b5399 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Thu, 11 Jan 2018 21:28:23 +0100 Subject: [PATCH 1/9] Stop handling events immediately when click tolerance is exceeded --- src/ol/interaction/Draw.js | 3 +++ test/spec/ol/interaction/draw.test.js | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 1422302fb3..0fb61d448e 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -423,6 +423,9 @@ Draw.prototype.handlePointerMove_ = function(event) { this.shouldHandle_ = this.freehand_ ? squaredDistance > this.squaredClickTolerance_ : squaredDistance <= this.squaredClickTolerance_; + if (!this.shouldHandle_) { + return true; + } } if (this.finishCoordinate_) { diff --git a/test/spec/ol/interaction/draw.test.js b/test/spec/ol/interaction/draw.test.js index 1fc1b912bd..cf29e89cb5 100644 --- a/test/spec/ol/interaction/draw.test.js +++ b/test/spec/ol/interaction/draw.test.js @@ -339,7 +339,6 @@ describe('ol.interaction.Draw', function() { simulateEvent('pointermove', 20, 30); // freehand - simulateEvent('pointerdown', 20, 30, true); simulateEvent('pointermove', 20, 30, true); simulateEvent('pointerdrag', 20, 30, true); simulateEvent('pointermove', 30, 40, true); From 2a64990a71020596b530a3ba0ceb39262b012a35 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 14 Jan 2018 18:41:26 +0100 Subject: [PATCH 2/9] Long press allows modification of current vertex --- examples/draw-and-modify-features.js | 5 +++++ src/ol/interaction/Draw.js | 26 +++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/examples/draw-and-modify-features.js b/examples/draw-and-modify-features.js index 4da34fced8..d28118d920 100644 --- a/examples/draw-and-modify-features.js +++ b/examples/draw-and-modify-features.js @@ -71,4 +71,9 @@ typeSelect.onchange = function() { addInteractions(); }; +// Avoid context menu for long taps when editing on mobile +map.getTargetElement().oncontextmenu = function(e) { + e.preventDefault(); +}; + addInteractions(); diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 0fb61d448e..8371f4e31a 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -56,6 +56,12 @@ const Draw = function(options) { */ this.downPx_ = null; + /** + * @type {number} + * @private + */ + this.lastDragTime_; + /** * @type {boolean} * @private @@ -255,7 +261,8 @@ const Draw = function(options) { wrapX: options.wrapX ? options.wrapX : false }), style: options.style ? options.style : - Draw.getDefaultStyleFunction() + Draw.getDefaultStyleFunction(), + updateWhileInteracting: true }); /** @@ -323,7 +330,18 @@ Draw.prototype.setMap = function(map) { */ Draw.handleEvent = function(event) { this.freehand_ = this.mode_ !== Draw.Mode_.POINT && this.freehandCondition_(event); + let move = event.type === MapBrowserEventType.POINTERMOVE; let pass = true; + if (this.lastDragTime_ && event.type === MapBrowserEventType.POINTERDRAG) { + const now = Date.now(); + if (now - this.lastDragTime_ >= 500) { + this.downPx_ = event.pixel; + this.shouldHandle_ = !this.freehand_; + move = true; + } else { + this.lastDragTime_ = undefined; + } + } if (this.freehand_ && event.type === MapBrowserEventType.POINTERDRAG && this.sketchFeature_ !== null) { @@ -332,11 +350,12 @@ Draw.handleEvent = function(event) { } else if (this.freehand_ && event.type === MapBrowserEventType.POINTERDOWN) { pass = false; - } else if (event.type === MapBrowserEventType.POINTERMOVE) { - pass = this.handlePointerMove_(event); + } else if (move) { + pass = event.type === MapBrowserEventType.POINTERMOVE && this.handlePointerMove_(event); } else if (event.type === MapBrowserEventType.DBLCLICK) { pass = false; } + return PointerInteraction.handleEvent.call(this, event) && pass; }; @@ -349,6 +368,7 @@ Draw.handleEvent = function(event) { */ Draw.handleDownEvent_ = function(event) { this.shouldHandle_ = !this.freehand_; + this.lastDragTime_ = Date.now(); if (this.freehand_) { this.downPx_ = event.pixel; From a93a811ba07ac816805f5a296c435f8e574384a7 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 14 Jan 2018 22:23:53 +0100 Subject: [PATCH 3/9] Do not update geometry when other interactions handle event --- src/ol/interaction/Draw.js | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 8371f4e31a..0496e70827 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -4,6 +4,7 @@ import {inherits} from '../index.js'; import Feature from '../Feature.js'; import MapBrowserEventType from '../MapBrowserEventType.js'; +import MapBrowserPointerEvent from '../MapBrowserPointerEvent.js'; import BaseObject from '../Object.js'; import _ol_coordinate_ from '../coordinate.js'; import _ol_events_ from '../events.js'; @@ -332,7 +333,7 @@ Draw.handleEvent = function(event) { this.freehand_ = this.mode_ !== Draw.Mode_.POINT && this.freehandCondition_(event); let move = event.type === MapBrowserEventType.POINTERMOVE; let pass = true; - if (this.lastDragTime_ && event.type === MapBrowserEventType.POINTERDRAG) { + if (!this.freehand_ && this.lastDragTime_ && event.type === MapBrowserEventType.POINTERDRAG) { const now = Date.now(); if (now - this.lastDragTime_ >= 500) { this.downPx_ = event.pixel; @@ -341,6 +342,10 @@ Draw.handleEvent = function(event) { } else { this.lastDragTime_ = undefined; } + if (this.shouldHandle_ && this.downTimeout_) { + clearTimeout(this.downTimeout_); + this.downTimeout_ = undefined; + } } if (this.freehand_ && event.type === MapBrowserEventType.POINTERDRAG && @@ -351,7 +356,12 @@ Draw.handleEvent = function(event) { event.type === MapBrowserEventType.POINTERDOWN) { pass = false; } else if (move) { - pass = event.type === MapBrowserEventType.POINTERMOVE && this.handlePointerMove_(event); + pass = event.type === MapBrowserEventType.POINTERMOVE; + if (pass && this.freehand_) { + pass = this.handlePointerMove_(event); + } else if (event.type === MapBrowserEventType.POINTERDRAG && !this.downTimeout_) { + this.handlePointerMove_(event); + } } else if (event.type === MapBrowserEventType.DBLCLICK) { pass = false; } @@ -368,7 +378,6 @@ Draw.handleEvent = function(event) { */ Draw.handleDownEvent_ = function(event) { this.shouldHandle_ = !this.freehand_; - this.lastDragTime_ = Date.now(); if (this.freehand_) { this.downPx_ = event.pixel; @@ -377,6 +386,11 @@ Draw.handleDownEvent_ = function(event) { } return true; } else if (this.condition_(event)) { + this.lastDragTime_ = Date.now(); + this.downTimeout_ = setTimeout(function() { + this.handlePointerMove_(new MapBrowserPointerEvent( + MapBrowserEventType.POINTERMOVE, event.map, event.pointerEvent, event.frameState)); + }.bind(this), 500); this.downPx_ = event.pixel; return true; } else { @@ -394,6 +408,11 @@ Draw.handleDownEvent_ = function(event) { Draw.handleUpEvent_ = function(event) { let pass = true; + if (this.downTimeout_) { + clearTimeout(this.downTimeout_); + this.downTimeout_ = undefined; + } + this.handlePointerMove_(event); const circleMode = this.mode_ === Draw.Mode_.CIRCLE; From fa75b78bf9a64c0abadf11c8efa1f897c4221029 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 14 Jan 2018 22:29:06 +0100 Subject: [PATCH 4/9] Do not draw sketch line for circles --- src/ol/interaction/Draw.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 0496e70827..58d281cc59 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -57,6 +57,12 @@ const Draw = function(options) { */ this.downPx_ = null; + /** + * @type {number} + * @private + */ + this.downTimeout_; + /** * @type {number} * @private @@ -333,7 +339,7 @@ Draw.handleEvent = function(event) { this.freehand_ = this.mode_ !== Draw.Mode_.POINT && this.freehandCondition_(event); let move = event.type === MapBrowserEventType.POINTERMOVE; let pass = true; - if (!this.freehand_ && this.lastDragTime_ && event.type === MapBrowserEventType.POINTERDRAG) { + if (this.lastDragTime_ && event.type === MapBrowserEventType.POINTERDRAG) { const now = Date.now(); if (now - this.lastDragTime_ >= 500) { this.downPx_ = event.pixel; @@ -547,9 +553,6 @@ Draw.prototype.startDrawing_ = function(event) { this.sketchLineCoords_ = this.sketchCoords_[0]; } else { this.sketchCoords_ = [start.slice(), start.slice()]; - if (this.mode_ === Draw.Mode_.CIRCLE) { - this.sketchLineCoords_ = this.sketchCoords_; - } } if (this.sketchLineCoords_) { this.sketchLine_ = new Feature( From 6bb88026ead603bffc72da4778a6f6b941218a79 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 14 Jan 2018 23:11:43 +0100 Subject: [PATCH 5/9] Avoid context menu for long tap when editing on mobile --- examples/draw-and-modify-features.js | 5 ----- src/ol/PluggableMap.js | 4 ++++ src/ol/events/EventType.js | 1 + src/ol/interaction/Draw.js | 5 +++++ 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/examples/draw-and-modify-features.js b/examples/draw-and-modify-features.js index d28118d920..4da34fced8 100644 --- a/examples/draw-and-modify-features.js +++ b/examples/draw-and-modify-features.js @@ -71,9 +71,4 @@ typeSelect.onchange = function() { addInteractions(); }; -// Avoid context menu for long taps when editing on mobile -map.getTargetElement().oncontextmenu = function(e) { - e.preventDefault(); -}; - addInteractions(); diff --git a/src/ol/PluggableMap.js b/src/ol/PluggableMap.js index 187d1b1fdf..eef8b2e74a 100644 --- a/src/ol/PluggableMap.js +++ b/src/ol/PluggableMap.js @@ -267,6 +267,8 @@ const PluggableMap = function(options) { */ this.keyHandlerKeys_ = null; + _ol_events_.listen(this.viewport_, EventType.CONTEXTMENU, + this.handleBrowserEvent, this); _ol_events_.listen(this.viewport_, EventType.WHEEL, this.handleBrowserEvent, this); _ol_events_.listen(this.viewport_, EventType.MOUSEWHEEL, @@ -491,6 +493,8 @@ PluggableMap.prototype.addOverlayInternal_ = function(overlay) { */ PluggableMap.prototype.disposeInternal = function() { this.mapBrowserEventHandler_.dispose(); + _ol_events_.unlisten(this.viewport_, EventType.CONTEXTMENU, + this.handleBrowserEvent, this); _ol_events_.unlisten(this.viewport_, EventType.WHEEL, this.handleBrowserEvent, this); _ol_events_.unlisten(this.viewport_, EventType.MOUSEWHEEL, diff --git a/src/ol/events/EventType.js b/src/ol/events/EventType.js index 78b3b6ae82..3e7689d36b 100644 --- a/src/ol/events/EventType.js +++ b/src/ol/events/EventType.js @@ -15,6 +15,7 @@ export default { CHANGE: 'change', CLEAR: 'clear', + CONTEXTMENU: 'contextmenu', CLICK: 'click', DBLCLICK: 'dblclick', DRAGENTER: 'dragenter', diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 58d281cc59..7420c9a387 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -2,6 +2,7 @@ * @module ol/interaction/Draw */ import {inherits} from '../index.js'; +import EventType from '../events/EventType.js'; import Feature from '../Feature.js'; import MapBrowserEventType from '../MapBrowserEventType.js'; import MapBrowserPointerEvent from '../MapBrowserPointerEvent.js'; @@ -336,6 +337,10 @@ Draw.prototype.setMap = function(map) { * @api */ Draw.handleEvent = function(event) { + if (event.originalEvent.type === EventType.CONTEXTMENU) { + // Avoid context menu for long taps when drawing on mobile + event.preventDefault(); + } this.freehand_ = this.mode_ !== Draw.Mode_.POINT && this.freehandCondition_(event); let move = event.type === MapBrowserEventType.POINTERMOVE; let pass = true; From 792485060148e009561bb0c1db2f9a5000e96f12 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 14 Jan 2018 23:26:01 +0100 Subject: [PATCH 6/9] Explain changes in the upgrade notes --- changelog/upgrade-notes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index 03c5b1f301..45c26c3572 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -2,6 +2,13 @@ ### Next release +#### Changed behavior of the `Draw` interaction + +For better drawing experience, especially on mobile devices, two changes were made to the behavior of the Draw interaction: + + 1. On long press, the current vertex can be moved to its desired position. + 2. When moving the mouse or panning the map, no draw cursor is shown, and the geometry being drawn is not updated. But because of 1., the draw cursor will appear on long press. + #### Changes in proj4 integration Because relying on a globally available proj4 is not practical with ES modules, we have made a change to the way we integrate proj4: From c1f8d30c28ea3f0d10b4d445c259867ea8e47409 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Mon, 15 Jan 2018 10:37:13 +0100 Subject: [PATCH 7/9] Make dragVertexTolerance configurable --- changelog/upgrade-notes.md | 2 ++ externs/olx.js | 9 +++++++++ src/ol/interaction/Draw.js | 12 +++++++++--- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index 45c26c3572..7b8beb2b27 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -9,6 +9,8 @@ For better drawing experience, especially on mobile devices, two changes were ma 1. On long press, the current vertex can be moved to its desired position. 2. When moving the mouse or panning the map, no draw cursor is shown, and the geometry being drawn is not updated. But because of 1., the draw cursor will appear on long press. +To get the old behavior, configure the `Drag` interaction with `dragVertexDelay: 0`. + #### Changes in proj4 integration Because relying on a globally available proj4 is not practical with ES modules, we have made a change to the way we integrate proj4: diff --git a/externs/olx.js b/externs/olx.js index 9ad1f7ed42..ae436811b5 100644 --- a/externs/olx.js +++ b/externs/olx.js @@ -2356,6 +2356,7 @@ olx.interaction.DragZoomOptions.prototype.out; * @typedef {{clickTolerance: (number|undefined), * features: (ol.Collection.|undefined), * source: (ol.source.Vector|undefined), + * dragVertexDelay: (number|undefined), * snapTolerance: (number|undefined), * type: (ol.geom.GeometryType|string), * stopClick: (boolean|undefined), @@ -2401,6 +2402,14 @@ olx.interaction.DrawOptions.prototype.features; olx.interaction.DrawOptions.prototype.source; +/** + * Delay in milliseconds after pointerdown before the current vertex can be + * dragged to its exact position. Default is 500 ms. + * @type {number|undefined} + */ +olx.interaction.DrawOptions.prototype.dragVertexDelay; + + /** * Pixel distance for snapping to the drawing finish. Default is `12`. * @type {number|undefined} diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 7420c9a387..bed2038584 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -205,6 +205,12 @@ const Draw = function(options) { */ this.geometryFunction_ = geometryFunction; + /** + * @type {number} + * @private + */ + this.dragVertexDelay_ = options.dragVertexDelay !== undefined ? options.dragVertexDelay : 500; + /** * Finish coordinate for the feature (first point for polygons, last point for * linestrings). @@ -346,7 +352,7 @@ Draw.handleEvent = function(event) { let pass = true; if (this.lastDragTime_ && event.type === MapBrowserEventType.POINTERDRAG) { const now = Date.now(); - if (now - this.lastDragTime_ >= 500) { + if (now - this.lastDragTime_ >= this.dragVertexDelay_) { this.downPx_ = event.pixel; this.shouldHandle_ = !this.freehand_; move = true; @@ -368,7 +374,7 @@ Draw.handleEvent = function(event) { pass = false; } else if (move) { pass = event.type === MapBrowserEventType.POINTERMOVE; - if (pass && this.freehand_) { + if (pass && this.freehand_ || this.dragVertexDelay_ === 0) { pass = this.handlePointerMove_(event); } else if (event.type === MapBrowserEventType.POINTERDRAG && !this.downTimeout_) { this.handlePointerMove_(event); @@ -401,7 +407,7 @@ Draw.handleDownEvent_ = function(event) { this.downTimeout_ = setTimeout(function() { this.handlePointerMove_(new MapBrowserPointerEvent( MapBrowserEventType.POINTERMOVE, event.map, event.pointerEvent, event.frameState)); - }.bind(this), 500); + }.bind(this), this.dragVertexDelay_); this.downPx_ = event.pixel; return true; } else { From 276194d5542c4ef0db127864e3609120f723999e Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Mon, 15 Jan 2018 13:04:26 +0100 Subject: [PATCH 8/9] Add tests for vertex drag --- test/spec/ol/interaction/draw.test.js | 41 +++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/spec/ol/interaction/draw.test.js b/test/spec/ol/interaction/draw.test.js index cf29e89cb5..693fe913bf 100644 --- a/test/spec/ol/interaction/draw.test.js +++ b/test/spec/ol/interaction/draw.test.js @@ -103,6 +103,15 @@ describe('ol.interaction.Draw', function() { expect(draw.freehandCondition_(event)).to.be(true); }); + it('accepts a dragVertexDelay option', function() { + const draw = new Draw({ + source: source, + type: 'LineString', + dragVertexDelay: 42 + }); + expect(draw.dragVertexDelay_).to.be(42); + }); + }); describe('specifying a geometryName', function() { @@ -395,6 +404,38 @@ describe('ol.interaction.Draw', function() { expect(geometry.getCoordinates()).to.eql([[10, -20], [30, -20]]); }); + it('allows dragging of the vertex after dragVertexDelay', function(done) { + // first point + simulateEvent('pointermove', 10, 20); + simulateEvent('pointerdown', 10, 20); + simulateEvent('pointerup', 10, 20); + + // second point, drag vertex + simulateEvent('pointermove', 15, 20); + simulateEvent('pointerdown', 15, 20); + setTimeout(function() { + simulateEvent('pointermove', 20, 10); + simulateEvent('pointerdrag', 20, 10); + simulateEvent('pointerup', 20, 10); + // third point + simulateEvent('pointermove', 30, 20); + simulateEvent('pointerdown', 30, 20); + simulateEvent('pointerup', 30, 20); + + // finish on third point + simulateEvent('pointerdown', 30, 20); + simulateEvent('pointerup', 30, 20); + + const features = source.getFeatures(); + expect(features).to.have.length(1); + const geometry = features[0].getGeometry(); + expect(geometry).to.be.a(LineString); + expect(geometry.getCoordinates()).to.eql([[10, -20], [20, -10], [30, -20]]); + + done(); + }, 600); + }); + it('triggers draw events', function() { const ds = sinon.spy(); const de = sinon.spy(); From be57e40e91788d1a58bb1d295a76813919d462d2 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 17 Jan 2018 01:35:17 +0100 Subject: [PATCH 9/9] Show last segment and vertex for mouse pointer move --- changelog/upgrade-notes.md | 8 +++----- src/ol/interaction/Draw.js | 6 ++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index 7b8beb2b27..4494bca99a 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -4,12 +4,10 @@ #### Changed behavior of the `Draw` interaction -For better drawing experience, especially on mobile devices, two changes were made to the behavior of the Draw interaction: +For better drawing experience, two changes were made to the behavior of the Draw interaction: - 1. On long press, the current vertex can be moved to its desired position. - 2. When moving the mouse or panning the map, no draw cursor is shown, and the geometry being drawn is not updated. But because of 1., the draw cursor will appear on long press. - -To get the old behavior, configure the `Drag` interaction with `dragVertexDelay: 0`. + 1. On long press, the current vertex can be dragged to its desired position. + 2. On touch move (e.g. when panning the map on a mobile device), no draw cursor is shown, and the geometry being drawn is not updated. But because of 1., the draw cursor will appear on long press. Mouse moves are not affected by this change. #### Changes in proj4 integration diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index bed2038584..47efd71ec5 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -19,6 +19,7 @@ import LineString from '../geom/LineString.js'; import MultiLineString from '../geom/MultiLineString.js'; import MultiPoint from '../geom/MultiPoint.js'; import MultiPolygon from '../geom/MultiPolygon.js'; +import MouseSource from '../pointer/MouseSource.js'; import Point from '../geom/Point.js'; import Polygon, {fromCircle, makeRegular} from '../geom/Polygon.js'; import DrawEventType from '../interaction/DrawEventType.js'; @@ -374,9 +375,10 @@ Draw.handleEvent = function(event) { pass = false; } else if (move) { pass = event.type === MapBrowserEventType.POINTERMOVE; - if (pass && this.freehand_ || this.dragVertexDelay_ === 0) { + if (pass && this.freehand_) { pass = this.handlePointerMove_(event); - } else if (event.type === MapBrowserEventType.POINTERDRAG && !this.downTimeout_) { + } else if (event.pointerEvent.pointerType == MouseSource.POINTER_TYPE || + (event.type === MapBrowserEventType.POINTERDRAG && !this.downTimeout_)) { this.handlePointerMove_(event); } } else if (event.type === MapBrowserEventType.DBLCLICK) {