From 01b3f9a97badb84afd3df7f932857f3903dba513 Mon Sep 17 00:00:00 2001 From: Maximilian Kroeg Date: Tue, 25 Feb 2020 10:33:02 +0100 Subject: [PATCH 1/4] Allow cluster source to unlisten from its source This adds a setSource method to change or remove the cluster source's source. --- src/ol/source/Cluster.js | 43 ++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/ol/source/Cluster.js b/src/ol/source/Cluster.js index e9f1fd35b9..1f62b6e51b 100644 --- a/src/ol/source/Cluster.js +++ b/src/ol/source/Cluster.js @@ -29,7 +29,7 @@ import VectorSource from './Vector.js'; * ``` * See {@link module:ol/geom/Polygon~Polygon#getInteriorPoint} for a way to get a cluster * calculation point for polygons. - * @property {VectorSource} source Source. + * @property {VectorSource} [source=null] Source. * @property {boolean} [wrapX=true] Whether to wrap the world horizontally. */ @@ -39,6 +39,10 @@ import VectorSource from './Vector.js'; * Layer source to cluster vector data. Works out of the box with point * geometries. For other geometry types, or if not all geometries should be * considered for clustering, a custom `geometryFunction` can be defined. + * + * If the instance is disposed without also disposing the underlying + * source `setSource(null)` has to be called to remove the listener reference + * from the wrapped source. * @api */ class Cluster extends VectorSource { @@ -81,13 +85,17 @@ class Cluster extends VectorSource { return geometry; }; - /** - * @type {VectorSource} - * @protected - */ - this.source = options.source; + this.boundRefresh_ = this.refresh.bind(this); - this.source.addEventListener(EventType.CHANGE, this.refresh.bind(this)); + this.setSource(options.source || null); + } + + /** + * @override + */ + clear(opt_fast) { + this.features.length = 0; + super.clear(opt_fast); } /** @@ -132,7 +140,23 @@ class Cluster extends VectorSource { } /** - * handle the source changing + * Replace the wrapped source. + * @param {VectorSource|null} source The new source for this instance. + * @api + */ + setSource(source) { + if (this.source) { + this.source.removeEventListener(EventType.CHANGE, this.boundRefresh_); + } + this.source = source; + if (source) { + this.sourceListenKey_ = source.addEventListener(EventType.CHANGE, this.boundRefresh_); + } + this.refresh(); + } + + /** + * Handle the source changing. * @override */ refresh() { @@ -145,10 +169,9 @@ class Cluster extends VectorSource { * @protected */ cluster() { - if (this.resolution === undefined) { + if (this.resolution === undefined || !this.source) { return; } - this.features.length = 0; const extent = createEmpty(); const mapDistance = this.distance * this.resolution; const features = this.source.getFeatures(); From b25fc6741ee52ea8d7bf90f6e5f6607ba2825d30 Mon Sep 17 00:00:00 2001 From: Maximilian Kroeg Date: Tue, 25 Feb 2020 11:15:56 +0100 Subject: [PATCH 2/4] Add tests for ol/source/Cluster~Cluster#setSource --- test/spec/ol/source/cluster.test.js | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/spec/ol/source/cluster.test.js b/test/spec/ol/source/cluster.test.js index c72a45eac9..9738788ab5 100644 --- a/test/spec/ol/source/cluster.test.js +++ b/test/spec/ol/source/cluster.test.js @@ -6,6 +6,7 @@ import {get as getProjection} from '../../../../src/ol/proj.js'; import Cluster from '../../../../src/ol/source/Cluster.js'; import Source from '../../../../src/ol/source/Source.js'; import VectorSource from '../../../../src/ol/source/Vector.js'; +import EventType from '../../../../src/ol/events/EventType.js'; describe('ol.source.Cluster', function() { @@ -76,3 +77,33 @@ describe('ol.source.Cluster', function() { }); }); + +describe('#setSource', function() { + it('removes the change listener from the old source', function() { + const source = new VectorSource(); + const clusterSource = new Cluster({ + source: source + }); + expect(source.hasListener(EventType.CHANGE)).to.be(true); + clusterSource.setSource(null); + expect(source.hasListener(EventType.CHANGE)).to.be(false); + }); + + it('properly removes the previous features', function() { + const source = new Cluster({ + source: new VectorSource({ + features: [new Feature(new Point([0, 0]))] + }) + }); + + const projection = getProjection('EPSG:3857'); + const extent = [-1, -1, 1, 1]; + source.loadFeatures(extent, 1, projection); + + expect(source.features.length).to.be(1); + source.setSource(null); + expect(source.features.length).to.be(0); + }); + +}); + From f18b78d2da3816dfaf7235a9f3385d05896316c0 Mon Sep 17 00:00:00 2001 From: Maximilian Kroeg Date: Thu, 27 Feb 2020 10:00:41 +0100 Subject: [PATCH 3/4] Do requested changes 2/3 to cluster source --- src/ol/source/Cluster.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ol/source/Cluster.js b/src/ol/source/Cluster.js index 1f62b6e51b..35b02010ae 100644 --- a/src/ol/source/Cluster.js +++ b/src/ol/source/Cluster.js @@ -29,7 +29,7 @@ import VectorSource from './Vector.js'; * ``` * See {@link module:ol/geom/Polygon~Polygon#getInteriorPoint} for a way to get a cluster * calculation point for polygons. - * @property {VectorSource} [source=null] Source. + * @property {VectorSource} [source] Source. * @property {boolean} [wrapX=true] Whether to wrap the world horizontally. */ @@ -150,7 +150,7 @@ class Cluster extends VectorSource { } this.source = source; if (source) { - this.sourceListenKey_ = source.addEventListener(EventType.CHANGE, this.boundRefresh_); + source.addEventListener(EventType.CHANGE, this.boundRefresh_); } this.refresh(); } From ba84cfad6196b527a92a196ed7013dfc7f3ddba3 Mon Sep 17 00:00:00 2001 From: Maximilian Kroeg Date: Thu, 27 Feb 2020 10:28:26 +0100 Subject: [PATCH 4/4] Do requeseted changes 3/3 --- src/ol/source/Cluster.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/source/Cluster.js b/src/ol/source/Cluster.js index 35b02010ae..535956bc67 100644 --- a/src/ol/source/Cluster.js +++ b/src/ol/source/Cluster.js @@ -141,7 +141,7 @@ class Cluster extends VectorSource { /** * Replace the wrapped source. - * @param {VectorSource|null} source The new source for this instance. + * @param {VectorSource} source The new source for this instance. * @api */ setSource(source) {