From 8fc9b23ab8224dffe23e918e82109a6268c0bbd5 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sun, 30 Mar 2014 14:10:21 +0200 Subject: [PATCH 1/5] Do not return as handled unless a geometry was modified With this change, it is more straightforward to determine whether an event is considered as handled, which results in the removal of the modifiable_ and lastVertexCoordinate_ states. Instead, we only need to know whether we're on a real vertex or a virtual one. For that a new snappedToVertex_ flag is introduced. To stop a click after vertex deletion from triggering a feature selection, vertex deletion is now triggered by a configurable event condition, which defaults to singleclick (same as in the select interaction) with no modifier keys. --- src/objectliterals.jsdoc | 3 ++ src/ol/interaction/modifyinteraction.js | 70 ++++++++++++++----------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/objectliterals.jsdoc b/src/objectliterals.jsdoc index f3b1a22684..f75877ba26 100644 --- a/src/objectliterals.jsdoc +++ b/src/objectliterals.jsdoc @@ -487,6 +487,9 @@ /** * @typedef {Object} olx.interaction.ModifyOptions + * @property {ol.events.ConditionType} deleteCondition Condition that determines + * which event results in a vertex deletion. Default is a `singleclick` + * event with no modifier keys. * @property {number|undefined} pixelTolerance Pixel tolerance for considering * the pointer close enough to a vertex for editing. Default is 10 pixels. * @property {ol.style.Style|Array.|ol.feature.StyleFunction|undefined} style FeatureOverlay style. diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index d133822d22..8ef2a6ea76 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -10,6 +10,7 @@ goog.require('ol.FeatureOverlay'); goog.require('ol.MapBrowserEvent.EventType'); goog.require('ol.ViewHint'); goog.require('ol.coordinate'); +goog.require('ol.events.condition'); goog.require('ol.extent'); goog.require('ol.feature'); goog.require('ol.geom.GeometryType'); @@ -44,6 +45,14 @@ ol.interaction.Modify = function(options) { goog.base(this); + /** + * @type {ol.events.ConditionType} + * @private + */ + this.deleteCondition_ = goog.isDef(options.deleteCondition) ? + options.pixelTolerance : goog.functions.and( + ol.events.condition.noModifierKeys, ol.events.condition.singleClick); + /** * Editing vertex. * @type {ol.Feature} @@ -58,18 +67,6 @@ ol.interaction.Modify = function(options) { */ this.vertexSegments_ = null; - /** - * @type {boolean} - * @private - */ - this.modifiable_ = false; - - /** - * @type {ol.Coordinate} - * @private - */ - this.lastVertexCoordinate_; - /** * @type {ol.Pixel} * @private @@ -90,6 +87,12 @@ ol.interaction.Modify = function(options) { this.pixelTolerance_ = goog.isDef(options.pixelTolerance) ? options.pixelTolerance : 10; + /** + * @type {boolean} + * @private + */ + this.snappedToVertex_; + /** * @type {Array} * @private @@ -404,9 +407,8 @@ ol.interaction.Modify.prototype.handlePointerDown = function(evt) { for (i = insertVertices.length - 1; i >= 0; --i) { this.insertVertex_.apply(this, insertVertices[i]); } - this.lastVertexCoordinate_ = goog.array.clone(vertex); } - return this.modifiable_; + return !goog.isNull(this.vertexFeature_); }; @@ -461,20 +463,12 @@ ol.interaction.Modify.prototype.handlePointerDrag = function(evt) { * @inheritDoc */ ol.interaction.Modify.prototype.handlePointerUp = function(evt) { - var geometry = this.vertexFeature_.getGeometry(); - goog.asserts.assertInstanceof(geometry, ol.geom.Point); - if (goog.array.equals(this.lastVertexCoordinate_, - geometry.getCoordinates())) { - this.removeVertex_(); - } else { - var segmentData; - for (var i = this.dragSegments_.length - 1; i >= 0; --i) { - segmentData = this.dragSegments_[i][0]; - this.rBush_.update(ol.extent.boundingExtent(segmentData.segment), - segmentData); - } + var segmentData; + for (var i = this.dragSegments_.length - 1; i >= 0; --i) { + segmentData = this.dragSegments_[i][0]; + this.rBush_.update(ol.extent.boundingExtent(segmentData.segment), + segmentData); } - return false; }; @@ -483,12 +477,18 @@ ol.interaction.Modify.prototype.handlePointerUp = function(evt) { */ ol.interaction.Modify.prototype.handleMapBrowserEvent = function(mapBrowserEvent) { + var handled; if (!mapBrowserEvent.map.getView().getHints()[ol.ViewHint.INTERACTING] && mapBrowserEvent.type == ol.MapBrowserEvent.EventType.POINTERMOVE) { this.handlePointerMove_(mapBrowserEvent); } - return (goog.base(this, 'handleMapBrowserEvent', mapBrowserEvent) && - !this.modifiable_); + if (!goog.isNull(this.vertexFeature_) && this.snappedToVertex_ && + this.deleteCondition_(mapBrowserEvent)) { + var geometry = this.vertexFeature_.getGeometry(); + goog.asserts.assertInstanceof(geometry, ol.geom.Point); + handled = this.removeVertex_(); + } + return goog.base(this, 'handleMapBrowserEvent', mapBrowserEvent) && !handled; }; @@ -520,7 +520,6 @@ ol.interaction.Modify.prototype.handlePointerAtPixel_ = function(pixel, map) { [pixel[0] + this.pixelTolerance_, pixel[1] - this.pixelTolerance_]); var box = ol.extent.boundingExtent([lowerLeft, upperRight]); - this.modifiable_ = false; var rBush = this.rBush_; var nodes = rBush.getInExtent(box); if (nodes.length > 0) { @@ -557,7 +556,6 @@ ol.interaction.Modify.prototype.handlePointerAtPixel_ = function(pixel, map) { } } this.vertexSegments_ = vertexSegments; - this.modifiable_ = true; return; } } @@ -638,6 +636,7 @@ ol.interaction.Modify.prototype.insertVertex_ = function(segmentData, vertex) { /** * Removes a vertex from all matching features. + * @return {boolean} True when a vertex was removed. * @private */ ol.interaction.Modify.prototype.removeVertex_ = function() { @@ -724,6 +723,15 @@ ol.interaction.Modify.prototype.removeVertex_ = function() { } } } + return deleted; +}; + + +/** + * @inheritDoc + */ +ol.interaction.Modify.prototype.shouldStopEvent = function(handled) { + return handled; }; From f2acbd332c23c469a4fce37062f7f49871ba5533 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sun, 30 Mar 2014 14:11:31 +0200 Subject: [PATCH 2/5] Use the pixelTolerance to determine whether to snap to a vertex --- src/objectliterals.jsdoc | 3 ++- src/ol/interaction/modifyinteraction.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/objectliterals.jsdoc b/src/objectliterals.jsdoc index f75877ba26..d8b12b7385 100644 --- a/src/objectliterals.jsdoc +++ b/src/objectliterals.jsdoc @@ -491,7 +491,8 @@ * which event results in a vertex deletion. Default is a `singleclick` * event with no modifier keys. * @property {number|undefined} pixelTolerance Pixel tolerance for considering - * the pointer close enough to a vertex for editing. Default is 10 pixels. + * the pointer close enough to a segment or vertex for editing. Default is + * 10 pixels. * @property {ol.style.Style|Array.|ol.feature.StyleFunction|undefined} style FeatureOverlay style. * @property {ol.Collection} features The features the interaction works on. */ diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index 8ef2a6ea76..41024993d8 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -536,7 +536,8 @@ ol.interaction.Modify.prototype.handlePointerAtPixel_ = function(pixel, map) { var squaredDist1 = ol.coordinate.squaredDistance(vertexPixel, pixel1); var squaredDist2 = ol.coordinate.squaredDistance(vertexPixel, pixel2); var dist = Math.sqrt(Math.min(squaredDist1, squaredDist2)); - if (dist <= 10) { + this.snappedToVertex_ = dist <= this.pixelTolerance_; + if (this.snappedToVertex_) { vertex = squaredDist1 > squaredDist2 ? closestSegment[1] : closestSegment[0]; } From 2cb045b0b37ef5d86de6f54e30874fdd5f4d90df Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sun, 30 Mar 2014 14:11:46 +0200 Subject: [PATCH 3/5] Better documentation for the singleclick condition --- src/ol/events/condition.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/events/condition.js b/src/ol/events/condition.js index f04add9eaa..285298b486 100644 --- a/src/ol/events/condition.js +++ b/src/ol/events/condition.js @@ -66,7 +66,7 @@ ol.events.condition.never = goog.functions.FALSE; /** * @param {ol.MapBrowserEvent} mapBrowserEvent Map browser event. - * @return {boolean} True if the event is a click event. + * @return {boolean} True if the event is a `singleclick` event. * @todo stability experimental */ ol.events.condition.singleClick = function(mapBrowserEvent) { From 523b51d69afac675a3c5bce48a406e705604afbe Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sun, 30 Mar 2014 16:59:22 +0200 Subject: [PATCH 4/5] Add missing goog.requires; fix types and a typo --- src/objectliterals.jsdoc | 6 +++--- src/ol/interaction/modifyinteraction.js | 10 +++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/objectliterals.jsdoc b/src/objectliterals.jsdoc index d8b12b7385..717416a1ba 100644 --- a/src/objectliterals.jsdoc +++ b/src/objectliterals.jsdoc @@ -487,9 +487,9 @@ /** * @typedef {Object} olx.interaction.ModifyOptions - * @property {ol.events.ConditionType} deleteCondition Condition that determines - * which event results in a vertex deletion. Default is a `singleclick` - * event with no modifier keys. + * @property {ol.events.ConditionType|undefined} deleteCondition Condition that + * determines which event results in a vertex deletion. Default is a + * `singleclick` event with no modifier keys. * @property {number|undefined} pixelTolerance Pixel tolerance for considering * the pointer close enough to a segment or vertex for editing. Default is * 10 pixels. diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index 41024993d8..787905ef2a 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -3,6 +3,7 @@ goog.provide('ol.interaction.Modify'); goog.require('goog.array'); goog.require('goog.asserts'); goog.require('goog.events'); +goog.require('goog.functions'); goog.require('ol.Collection'); goog.require('ol.CollectionEventType'); goog.require('ol.Feature'); @@ -50,8 +51,10 @@ ol.interaction.Modify = function(options) { * @private */ this.deleteCondition_ = goog.isDef(options.deleteCondition) ? - options.pixelTolerance : goog.functions.and( - ol.events.condition.noModifierKeys, ol.events.condition.singleClick); + options.deleteCondition : + /** @type {ol.events.ConditionType} */ (goog.functions.and( + ol.events.condition.noModifierKeys, + ol.events.condition.singleClick)); /** * Editing vertex. @@ -643,7 +646,8 @@ ol.interaction.Modify.prototype.insertVertex_ = function(segmentData, vertex) { ol.interaction.Modify.prototype.removeVertex_ = function() { var dragSegments = this.dragSegments_; var segmentsByFeature = {}; - var component, coordinates, deleted, dragSegment, geometry, i, index, left; + var deleted = false; + var component, coordinates, dragSegment, geometry, i, index, left; var newIndex, newSegment, right, segmentData, uid; for (i = dragSegments.length - 1; i >= 0; --i) { dragSegment = dragSegments[i]; From bf06129256772e824e8951d14a1d175191c62e5c Mon Sep 17 00:00:00 2001 From: ahocevar Date: Fri, 4 Apr 2014 14:11:02 +0200 Subject: [PATCH 5/5] Initialize snappedToVertex_ --- src/ol/interaction/modifyinteraction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index 787905ef2a..75979b419e 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -94,7 +94,7 @@ ol.interaction.Modify = function(options) { * @type {boolean} * @private */ - this.snappedToVertex_; + this.snappedToVertex_ = false; /** * @type {Array}