From 288fd0b74c37477b6addfd7ec01eeedcd4623f38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sat, 19 Dec 2020 17:58:56 +0100 Subject: [PATCH 1/3] Improve Draw type to mode conversion Throw error if Draw is configured with an invalid type. --- src/ol/interaction/Draw.js | 30 +++++++++++++-------------- test/spec/ol/interaction/draw.test.js | 2 +- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index b547efa42f..0787e56212 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -1227,23 +1227,21 @@ export function createBox() { * @return {Mode} Drawing mode. */ function getMode(type) { - let mode; - if (type === GeometryType.POINT || type === GeometryType.MULTI_POINT) { - mode = Mode.POINT; - } else if ( - type === GeometryType.LINE_STRING || - type === GeometryType.MULTI_LINE_STRING - ) { - mode = Mode.LINE_STRING; - } else if ( - type === GeometryType.POLYGON || - type === GeometryType.MULTI_POLYGON - ) { - mode = Mode.POLYGON; - } else if (type === GeometryType.CIRCLE) { - mode = Mode.CIRCLE; + switch (type) { + case GeometryType.POINT: + case GeometryType.MULTI_POINT: + return Mode.POINT; + case GeometryType.LINE_STRING: + case GeometryType.MULTI_LINE_STRING: + return Mode.LINE_STRING; + case GeometryType.POLYGON: + case GeometryType.MULTI_POLYGON: + return Mode.POLYGON; + case GeometryType.CIRCLE: + return Mode.CIRCLE; + default: + throw new Error('Invalid type: ' + type); } - return /** @type {!Mode} */ (mode); } export default Draw; diff --git a/test/spec/ol/interaction/draw.test.js b/test/spec/ol/interaction/draw.test.js index 0aad9df692..9db0498682 100644 --- a/test/spec/ol/interaction/draw.test.js +++ b/test/spec/ol/interaction/draw.test.js @@ -1381,7 +1381,7 @@ describe('ol.interaction.Draw', function () { describe('#getOverlay', function () { it('returns the feature overlay layer', function () { - const draw = new Draw({}); + const draw = new Draw({type: 'Point'}); expect(draw.getOverlay()).to.eql(draw.overlay_); }); }); From 873020e1f8399a7e5408ce3d21f300be5e3375f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sat, 19 Dec 2020 19:35:18 +0100 Subject: [PATCH 2/3] Test Draw finishCondition for each type --- test/spec/ol/interaction/draw.test.js | 104 ++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 5 deletions(-) diff --git a/test/spec/ol/interaction/draw.test.js b/test/spec/ol/interaction/draw.test.js index 9db0498682..0861ff702d 100644 --- a/test/spec/ol/interaction/draw.test.js +++ b/test/spec/ol/interaction/draw.test.js @@ -4,6 +4,7 @@ import Draw, { createRegularPolygon, } from '../../../../src/ol/interaction/Draw.js'; import Feature from '../../../../src/ol/Feature.js'; +import GeometryType from '../../../../src/ol/geom/GeometryType.js'; import Interaction from '../../../../src/ol/interaction/Interaction.js'; import LineString from '../../../../src/ol/geom/LineString.js'; import Map from '../../../../src/ol/Map.js'; @@ -609,22 +610,115 @@ describe('ol.interaction.Draw', function () { }); }); + describe('finishCondition called for each type', function () { + let draw; + function createDrawInteraction(type, finishCondition) { + draw = new Draw({ + source: source, + type: type, + finishCondition: finishCondition, + }); + draw.atFinish_ = sinon.spy(Draw.prototype.atFinish_); + draw.finishDrawing = sinon.spy(Draw.prototype.finishDrawing); + map.addInteraction(draw); + } + + const testCoordinates = [ + [0, 0], + [10, 0], + [10, 10], + ]; + + function drawType(type, amount, finishCondition) { + createDrawInteraction(type, finishCondition); + + for (let i = 0; i < amount; ++i) { + const [x, y] = testCoordinates[i]; + simulateEvent('pointermove', x, y); + simulateEvent('pointerdown', x, y); + simulateEvent('pointerup', x, y); + } + if (amount > 1 && type !== GeometryType.CIRCLE) { + const [x, y] = testCoordinates[amount - 1]; + simulateEvent('pointerdown', x, y); + simulateEvent('pointerup', x, y); + } + } + + function testFinishConditionTrue(type, amount) { + const finishCondition = sinon.spy(() => true); + drawType(type, amount, finishCondition); + expect(draw.atFinish_.called).to.be(true); + expect(finishCondition.callCount).to.be(1); + expect(draw.finishDrawing.callCount).to.be(1); + expect(source.getFeatures()).to.have.length(1); + } + it('calls finishCondition:true for POINT type', function () { + testFinishConditionTrue(GeometryType.POINT, 1); + }); + it('calls finishCondition:true for MULTI_POINT type', function () { + testFinishConditionTrue(GeometryType.MULTI_POINT, 1); + }); + it('calls finishCondition:true for LINE_STRING type', function () { + testFinishConditionTrue(GeometryType.LINE_STRING, 2); + }); + it('calls finishCondition:true for MULTI_LINE_STRING type', function () { + testFinishConditionTrue(GeometryType.MULTI_LINE_STRING, 2); + }); + it('calls finishCondition:true for CIRCLE type', function () { + testFinishConditionTrue(GeometryType.CIRCLE, 2); + }); + it('calls finishCondition:true for POLYGON type', function () { + testFinishConditionTrue(GeometryType.POLYGON, 3); + }); + it('calls finishCondition:true for MULTI_POLYGON type', function () { + testFinishConditionTrue(GeometryType.MULTI_POLYGON, 3); + }); + + function testFinishConditionFalse(type, amount) { + const finishCondition = sinon.spy(() => false); + drawType(type, amount, finishCondition); + expect(draw.atFinish_.called).to.be(true); + expect(finishCondition.callCount).to.be(1); + expect(draw.finishDrawing.called).to.be(false); + expect(source.getFeatures()).to.have.length(0); + } + it('calls finishCondition:false for POINT type', function () { + testFinishConditionFalse(GeometryType.POINT, 1); + }); + it('calls finishCondition:false for MULTI_POINT type', function () { + testFinishConditionFalse(GeometryType.MULTI_POINT, 1); + }); + it('calls finishCondition:false for LINE_STRING type', function () { + testFinishConditionFalse(GeometryType.LINE_STRING, 2); + }); + it('calls finishCondition:false for MULTI_LINE_STRING type', function () { + testFinishConditionFalse(GeometryType.MULTI_LINE_STRING, 2); + }); + it('calls finishCondition:false for CIRCLE type', function () { + testFinishConditionFalse(GeometryType.CIRCLE, 2); + }); + it('calls finishCondition:false for POLYGON type', function () { + testFinishConditionFalse(GeometryType.POLYGON, 3); + }); + it('calls finishCondition:false for MULTI_POLYGON type', function () { + testFinishConditionFalse(GeometryType.MULTI_POLYGON, 3); + }); + }); + describe('drawing with a finishCondition', function () { beforeEach(function () { const draw = new Draw({ source: source, type: 'LineString', finishCondition: function (event) { - if (equals(event.coordinate, [30, -20])) { - return true; - } - return false; + return equals(event.coordinate, [30, -20]); }, }); map.addInteraction(draw); }); - it('draws a linestring failing to finish it first, the finishes it', function () { + it('draws a linestring failing to finish it first, then finishes it', function () { let features; // first point From 801dba4a38f610872d791be8ca27b4f693dd0186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sat, 19 Dec 2020 19:35:45 +0100 Subject: [PATCH 3/3] Call Draw finishCondition for every type --- src/ol/interaction/Draw.js | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/ol/interaction/Draw.js b/src/ol/interaction/Draw.js index 0787e56212..0409beb7af 100644 --- a/src/ol/interaction/Draw.js +++ b/src/ol/interaction/Draw.js @@ -625,22 +625,23 @@ class Draw extends PointerInteraction { this.handlePointerMove_(event); if (this.shouldHandle_) { - switch (true) { - case !this.finishCoordinate_: - this.startDrawing_(event.coordinate); - if (this.mode_ !== Mode.POINT) { - break; + const startingToDraw = !this.finishCoordinate_; + if (startingToDraw) { + this.startDrawing_(event.coordinate); + } + if (!startingToDraw && this.freehand_) { + this.finishDrawing(); + } else if ( + !this.freehand_ && + (!startingToDraw || this.mode_ === Mode.POINT) + ) { + if (this.atFinish_(event.pixel)) { + if (this.finishCondition_(event)) { + this.finishDrawing(); } - // eslint-disable-next-line no-fallthrough - case this.freehand_ || - (this.atFinish_(event.pixel) && this.finishCondition_(event)): - this.finishDrawing(); - break; - case !this.freehand_: + } else { this.addToDrawing_(event.coordinate); - break; - default: - break; + } } pass = false; } else if (this.freehand_) {