From 22ca08179d2d1fdc25c0d9a61d79ea95b31bacd1 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 20:20:06 +0200 Subject: [PATCH 1/8] interaction/modify: Use expect().to.be() assertion ... instead of expect(a === b).to.be.ok() --- test/spec/ol/interaction/modifyinteraction.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/ol/interaction/modifyinteraction.test.js b/test/spec/ol/interaction/modifyinteraction.test.js index 7ef55b719b..4ae43e091c 100644 --- a/test/spec/ol/interaction/modifyinteraction.test.js +++ b/test/spec/ol/interaction/modifyinteraction.test.js @@ -89,7 +89,7 @@ describe('ol.interaction.Modify', function() { }); var rbushEntries = modify.rBush_.getAll(); expect(rbushEntries.length).to.be(1); - expect(rbushEntries[0].feature === feature).to.be.ok(); + expect(rbushEntries[0].feature).to.be(feature); }); }); From e3ead5df06d2209346b30b4f9a3b1307e63d913e Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 20:46:21 +0200 Subject: [PATCH 2/8] events/condition: Add doubleClick condition --- src/ol/events/condition.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/ol/events/condition.js b/src/ol/events/condition.js index de8c3d20fe..c2ead5189f 100644 --- a/src/ol/events/condition.js +++ b/src/ol/events/condition.js @@ -111,6 +111,18 @@ ol.events.condition.singleClick = function(mapBrowserEvent) { }; +/** + * Return `true` if the event is a map `dblclick` event, `false` otherwise. + * + * @param {ol.MapBrowserEvent} mapBrowserEvent Map browser event. + * @return {boolean} True if the event is a map `dblclick` event. + * @api stable + */ +ol.events.condition.doubleClick = function(mapBrowserEvent) { + return mapBrowserEvent.type == ol.MapBrowserEvent.EventType.DBLCLICK; +}; + + /** * Return `true` if no modifier key (alt-, shift- or platform-modifier-key) is * pressed. From bafd8548d1b34b91c484ce7b9b9c5187ea52f047 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 20:52:31 +0200 Subject: [PATCH 3/8] interaction/modify: Add tests for deleteCondition option --- .../ol/interaction/modifyinteraction.test.js | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/spec/ol/interaction/modifyinteraction.test.js b/test/spec/ol/interaction/modifyinteraction.test.js index 4ae43e091c..206d0af72f 100644 --- a/test/spec/ol/interaction/modifyinteraction.test.js +++ b/test/spec/ol/interaction/modifyinteraction.test.js @@ -158,6 +158,53 @@ describe('ol.interaction.Modify', function() { }); }); + describe('double click deleteCondition', function() { + + it('should delete vertex on double click', function() { + var modify = new ol.interaction.Modify({ + features: new ol.Collection(features), + deleteCondition: ol.events.condition.doubleClick + }); + map.addInteraction(modify); + + var feature = features[0]; + + expect(feature.getGeometry().getRevision()).to.equal(1); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); + + simulateEvent('pointerdown', 10, -20, false, 0); + simulateEvent('pointerup', 10, -20, false, 0); + simulateEvent('click', 10, -20, false, 0); + simulateEvent('pointerdown', 10, -20, false, 0); + simulateEvent('pointerup', 10, -20, false, 0); + simulateEvent('click', 10, -20, false, 0); + simulateEvent('dblclick', 10, -20, false, 0); + + expect(feature.getGeometry().getRevision()).to.equal(2); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(4); + }); + + it('should do nothing on single click', function() { + var modify = new ol.interaction.Modify({ + features: new ol.Collection(features), + deleteCondition: ol.events.condition.doubleClick + }); + map.addInteraction(modify); + + var feature = features[0]; + + expect(feature.getGeometry().getRevision()).to.equal(1); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); + + simulateEvent('pointerdown', 10, -20, false, 0); + simulateEvent('pointerup', 10, -20, false, 0); + simulateEvent('click', 10, -20, false, 0); + simulateEvent('singleclick', 10, -20, false, 0); + + expect(feature.getGeometry().getRevision()).to.equal(1); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); + }); + }); }); goog.require('goog.dispose'); From bf9156cbeb444bd7bf23df290f0db5d2fbe3a71e Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 21:03:18 +0200 Subject: [PATCH 4/8] interaction/modify: Adjust simulated events This is exactly matching the event sent by Chrome now --- test/spec/ol/interaction/modifyinteraction.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/spec/ol/interaction/modifyinteraction.test.js b/test/spec/ol/interaction/modifyinteraction.test.js index 206d0af72f..b85c2d15f7 100644 --- a/test/spec/ol/interaction/modifyinteraction.test.js +++ b/test/spec/ol/interaction/modifyinteraction.test.js @@ -106,7 +106,6 @@ describe('ol.interaction.Modify', function() { expect(feature.getGeometry().getRevision()).to.equal(1); expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); - simulateEvent('pointermove', 10, -20, false, 0); simulateEvent('pointerdown', 10, -20, false, 0); simulateEvent('pointerup', 10, -20, false, 0); simulateEvent('click', 10, -20, false, 0); @@ -127,7 +126,6 @@ describe('ol.interaction.Modify', function() { expect(feature.getGeometry().getRevision()).to.equal(1); expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); - simulateEvent('pointermove', 40, -20, false, 0); simulateEvent('pointerdown', 40, -20, false, 0); simulateEvent('pointerup', 40, -20, false, 0); simulateEvent('click', 40, -20, false, 0); @@ -150,6 +148,7 @@ describe('ol.interaction.Modify', function() { simulateEvent('pointermove', 40, -20, false, 0); simulateEvent('pointerdown', 40, -20, false, 0); + simulateEvent('pointermove', 30, -20, false, 0); simulateEvent('pointerdrag', 30, -20, false, 0); simulateEvent('pointerup', 30, -20, false, 0); From b001a4856789b32c7fd41647e829f8c97ef7022a Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 21:12:59 +0200 Subject: [PATCH 5/8] interaction/modify: Simplify lastNewVertexPixel condition --- src/ol/interaction/modifyinteraction.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index e206af9053..cc1797db20 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -626,8 +626,8 @@ ol.interaction.Modify.handleEvent = function(mapBrowserEvent) { } if (!goog.isNull(this.vertexFeature_) && this.deleteCondition_(mapBrowserEvent)) { - if (!(this.lastNewVertexPixel_[0] === this.lastPixel_[0] && - this.lastNewVertexPixel_[1] === this.lastPixel_[1])) { + if (this.lastNewVertexPixel_[0] !== this.lastPixel_[0] || + this.lastNewVertexPixel_[1] !== this.lastPixel_[1]) { var geometry = this.vertexFeature_.getGeometry(); goog.asserts.assertInstanceof(geometry, ol.geom.Point, 'geometry should be an ol.geom.Point'); From 968c8aa34e29eb87c117961c996a018560ffdf40 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 21:53:25 +0200 Subject: [PATCH 6/8] interaction/modify: Replace lastNewVertexPixel with ignoreNextSingleClick The previous approach did not work on mobile devices where no `pointermove` event is sent except from dragging. Logic now is: Upon vertex creation due to `pointerdown` we will ignore the next `singleclick` event unless there is a `pointerdrag` event, which will not lead to a `singleclick` event following the vertex creation. Resolves #3935 --- src/ol/interaction/modifyinteraction.js | 21 +++++++++----- .../ol/interaction/modifyinteraction.test.js | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index cc1797db20..75818f52f5 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -142,12 +142,12 @@ ol.interaction.Modify = function(options) { this.lastPixel_ = [0, 0]; /** - * Keep track of the last inserted pixel location to avoid - * unintentional deletion. - * @type {ol.Pixel} + * Tracks if the next `singleclick` event should be ignored to prevent + * accidental deletion right after vertex creation. + * @type {boolean} * @private */ - this.lastNewVertexPixel_ = [NaN, NaN]; + this.ignoreNextSingleClick_ = false; /** * Segment RTree for each layer @@ -543,6 +543,8 @@ ol.interaction.Modify.handleDownEvent_ = function(evt) { * @private */ ol.interaction.Modify.handleDragEvent_ = function(evt) { + this.ignoreNextSingleClick_ = false; + var vertex = evt.coordinate; for (var i = 0, ii = this.dragSegments_.length; i < ii; ++i) { var dragSegment = this.dragSegments_[i]; @@ -626,8 +628,8 @@ ol.interaction.Modify.handleEvent = function(mapBrowserEvent) { } if (!goog.isNull(this.vertexFeature_) && this.deleteCondition_(mapBrowserEvent)) { - if (this.lastNewVertexPixel_[0] !== this.lastPixel_[0] || - this.lastNewVertexPixel_[1] !== this.lastPixel_[1]) { + if (mapBrowserEvent.type != ol.MapBrowserEvent.EventType.SINGLECLICK || + !this.ignoreNextSingleClick_) { var geometry = this.vertexFeature_.getGeometry(); goog.asserts.assertInstanceof(geometry, ol.geom.Point, 'geometry should be an ol.geom.Point'); @@ -636,6 +638,11 @@ ol.interaction.Modify.handleEvent = function(mapBrowserEvent) { handled = true; } } + + if (mapBrowserEvent.type == ol.MapBrowserEvent.EventType.SINGLECLICK) { + this.ignoreNextSingleClick_ = false; + } + return ol.interaction.Pointer.handleEvent.call(this, mapBrowserEvent) && !handled; }; @@ -789,7 +796,7 @@ ol.interaction.Modify.prototype.insertVertex_ = function(segmentData, vertex) { rTree.insert(ol.extent.boundingExtent(newSegmentData2.segment), newSegmentData2); this.dragSegments_.push([newSegmentData2, 0]); - this.lastNewVertexPixel_ = this.lastPixel_; + this.ignoreNextSingleClick_ = true; }; diff --git a/test/spec/ol/interaction/modifyinteraction.test.js b/test/spec/ol/interaction/modifyinteraction.test.js index b85c2d15f7..7931ee336c 100644 --- a/test/spec/ol/interaction/modifyinteraction.test.js +++ b/test/spec/ol/interaction/modifyinteraction.test.js @@ -135,6 +135,34 @@ describe('ol.interaction.Modify', function() { expect(feature.getGeometry().getCoordinates()[0]).to.have.length(6); }); + it('single clicking on created vertex should delete it again', function() { + var modify = new ol.interaction.Modify({ + features: new ol.Collection(features) + }); + map.addInteraction(modify); + + var feature = features[0]; + + expect(feature.getGeometry().getRevision()).to.equal(1); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); + + simulateEvent('pointerdown', 40, -20, false, 0); + simulateEvent('pointerup', 40, -20, false, 0); + simulateEvent('click', 40, -20, false, 0); + simulateEvent('singleclick', 40, -20, false, 0); + + expect(feature.getGeometry().getRevision()).to.equal(2); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(6); + + simulateEvent('pointerdown', 40, -20, false, 0); + simulateEvent('pointerup', 40, -20, false, 0); + simulateEvent('click', 40, -20, false, 0); + simulateEvent('singleclick', 40, -20, false, 0); + + expect(feature.getGeometry().getRevision()).to.equal(3); + expect(feature.getGeometry().getCoordinates()[0]).to.have.length(5); + }); + it('clicking with drag should add vertex and +r3', function() { var modify = new ol.interaction.Modify({ features: new ol.Collection(features) From c3f51c676a41255ef939ddc58036e787a67955a4 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 22:05:44 +0200 Subject: [PATCH 7/8] interaction/modify: Fix identation issue --- 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 75818f52f5..91009a09a3 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -629,7 +629,7 @@ ol.interaction.Modify.handleEvent = function(mapBrowserEvent) { if (!goog.isNull(this.vertexFeature_) && this.deleteCondition_(mapBrowserEvent)) { if (mapBrowserEvent.type != ol.MapBrowserEvent.EventType.SINGLECLICK || - !this.ignoreNextSingleClick_) { + !this.ignoreNextSingleClick_) { var geometry = this.vertexFeature_.getGeometry(); goog.asserts.assertInstanceof(geometry, ol.geom.Point, 'geometry should be an ol.geom.Point'); From 7a34d22b3759c050243150f2d9c5324952f28f8e Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 1 Aug 2015 22:09:32 +0200 Subject: [PATCH 8/8] interaction/modify: Add missing goog.require() to test --- test/spec/ol/interaction/modifyinteraction.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/spec/ol/interaction/modifyinteraction.test.js b/test/spec/ol/interaction/modifyinteraction.test.js index 7931ee336c..48a64022a7 100644 --- a/test/spec/ol/interaction/modifyinteraction.test.js +++ b/test/spec/ol/interaction/modifyinteraction.test.js @@ -243,6 +243,7 @@ goog.require('ol.Feature'); goog.require('ol.Map'); goog.require('ol.MapBrowserPointerEvent'); goog.require('ol.View'); +goog.require('ol.events.condition'); goog.require('ol.geom.Point'); goog.require('ol.geom.Polygon'); goog.require('ol.interaction.Modify');