From 81901c72c4bd322a1745bd03d0fa511e52ea5ac5 Mon Sep 17 00:00:00 2001 From: Michael Jurkoic Date: Thu, 7 Jan 2021 22:33:08 -0800 Subject: [PATCH 1/4] Pass the renderer function to the cloned style --- src/ol/style/Style.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ol/style/Style.js b/src/ol/style/Style.js index e12cba36af..7137c5cc61 100644 --- a/src/ol/style/Style.js +++ b/src/ol/style/Style.js @@ -219,6 +219,7 @@ class Style { geometry: geometry, fill: this.getFill() ? this.getFill().clone() : undefined, image: this.getImage() ? this.getImage().clone() : undefined, + renderer: this.getRenderer() ? this.getRenderer() : undefined, stroke: this.getStroke() ? this.getStroke().clone() : undefined, text: this.getText() ? this.getText().clone() : undefined, zIndex: this.getZIndex(), From 1842170b20d8915a75b139f0b6c074152ab6839c Mon Sep 17 00:00:00 2001 From: mjjurkoic <37635304+mjjurkoic@users.noreply.github.com> Date: Fri, 8 Jan 2021 09:21:49 -0800 Subject: [PATCH 2/4] Eliminate redundant code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Stéphane Brunner --- src/ol/style/Style.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/style/Style.js b/src/ol/style/Style.js index 7137c5cc61..55d4ed6253 100644 --- a/src/ol/style/Style.js +++ b/src/ol/style/Style.js @@ -219,7 +219,7 @@ class Style { geometry: geometry, fill: this.getFill() ? this.getFill().clone() : undefined, image: this.getImage() ? this.getImage().clone() : undefined, - renderer: this.getRenderer() ? this.getRenderer() : undefined, + renderer: this.getRenderer(), stroke: this.getStroke() ? this.getStroke().clone() : undefined, text: this.getText() ? this.getText().clone() : undefined, zIndex: this.getZIndex(), From 760b5970f1058f7edecf3128d9cb7af3cb866f34 Mon Sep 17 00:00:00 2001 From: Michael Jurkoic Date: Fri, 8 Jan 2021 16:23:04 -0800 Subject: [PATCH 3/4] Added tests for renderer clone --- test/spec/ol/style/style.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/spec/ol/style/style.test.js b/test/spec/ol/style/style.test.js index ab1f3b7742..5733002e1b 100644 --- a/test/spec/ol/style/style.test.js +++ b/test/spec/ol/style/style.test.js @@ -48,6 +48,10 @@ describe('ol.style.Style', function () { image: new CircleStyle({ radius: 5, }), + renderer: function (pixelCoordinates, state) { + const geometry = state.geometry.clone(); + geometry.setCoordinates(pixelCoordinates); + }, stroke: new Stroke({ color: '#319FD3', }), @@ -64,6 +68,9 @@ describe('ol.style.Style', function () { expect(original.getImage().getRadius()).to.eql( clone.getImage().getRadius() ); + expect(original.getRenderer().toString()).to.eql( + clone.getRenderer().toString() + ); expect(original.getStroke().getColor()).to.eql( clone.getStroke().getColor() ); @@ -80,6 +87,10 @@ describe('ol.style.Style', function () { image: new CircleStyle({ radius: 5, }), + renderer: function (pixelCoordinates, state) { + const geometry = state.geometry.clone(); + geometry.setCoordinates(pixelCoordinates); + }, stroke: new Stroke({ color: '#319FD3', }), @@ -91,12 +102,16 @@ describe('ol.style.Style', function () { expect(original.getGeometry()).not.to.be(clone.getGeometry()); expect(original.getFill()).not.to.be(clone.getFill()); expect(original.getImage()).not.to.be(clone.getImage()); + expect(original.getRenderer()).not.to.be(clone.getRenderer()); expect(original.getStroke()).not.to.be(clone.getStroke()); expect(original.getText()).not.to.be(clone.getText()); clone.getGeometry().setCoordinates([1, 1, 1]); clone.getFill().setColor('#012345'); clone.getImage().setScale(2); + clone.setRenderer(function (pixelCoordinates, state) { + return; + }); clone.getStroke().setColor('#012345'); clone.getText().setText('other'); expect(original.getGeometry().getCoordinates()).not.to.eql( @@ -108,6 +123,9 @@ describe('ol.style.Style', function () { expect(original.getImage().getScale()).not.to.eql( clone.getImage().getScale() ); + expect(original.getRenderer().toString()).not.to.eql( + clone.getRenderer().toString() + ); expect(original.getStroke().getColor()).not.to.eql( clone.getStroke().getColor() ); From 94fcc049ce003a40a5f7d7e4ca14d09f9a380e07 Mon Sep 17 00:00:00 2001 From: Michael Jurkoic Date: Sun, 10 Jan 2021 10:10:09 -0800 Subject: [PATCH 4/4] Improved renderer clone tests Removed test that asserted the cloned renderer would not reference the same object as the original. Replaced tests that stringified the renderer function with tests that compare the original with the clone by reference. --- test/spec/ol/style/style.test.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/test/spec/ol/style/style.test.js b/test/spec/ol/style/style.test.js index 5733002e1b..210c0bb577 100644 --- a/test/spec/ol/style/style.test.js +++ b/test/spec/ol/style/style.test.js @@ -68,9 +68,7 @@ describe('ol.style.Style', function () { expect(original.getImage().getRadius()).to.eql( clone.getImage().getRadius() ); - expect(original.getRenderer().toString()).to.eql( - clone.getRenderer().toString() - ); + expect(original.getRenderer()).to.eql(clone.getRenderer()); expect(original.getStroke().getColor()).to.eql( clone.getStroke().getColor() ); @@ -102,7 +100,6 @@ describe('ol.style.Style', function () { expect(original.getGeometry()).not.to.be(clone.getGeometry()); expect(original.getFill()).not.to.be(clone.getFill()); expect(original.getImage()).not.to.be(clone.getImage()); - expect(original.getRenderer()).not.to.be(clone.getRenderer()); expect(original.getStroke()).not.to.be(clone.getStroke()); expect(original.getText()).not.to.be(clone.getText()); @@ -123,9 +120,7 @@ describe('ol.style.Style', function () { expect(original.getImage().getScale()).not.to.eql( clone.getImage().getScale() ); - expect(original.getRenderer().toString()).not.to.eql( - clone.getRenderer().toString() - ); + expect(original.getRenderer()).not.to.eql(clone.getRenderer()); expect(original.getStroke().getColor()).not.to.eql( clone.getStroke().getColor() );