[Pkg-javascript-commits] [leaflet-markercluster] 39/128: Manage nested groups in removeLayer and removeLayers as well
Jonas Smedegaard
dr at jones.dk
Sun Apr 16 06:26:01 UTC 2017
This is an automated email from the git hooks/post-receive script.
js pushed a commit to branch master
in repository leaflet-markercluster.
commit 9b451926fdf8e81e34e1e270cf9ebea1ea94113b
Author: ghybs <ghybs1 at gmail.com>
Date: Fri Jan 15 13:31:42 2016 +0400
Manage nested groups in removeLayer and removeLayers as well
Updated corresponding test suites (took the opportunity to re-vamp to avoid PhantomJS memory leak).
Also updated README to warn about the side effects (modified input array, chunkProgress backward jump, hasLayer false on groups).
---
README.md | 5 ++
spec/suites/RemoveLayerSpec.js | 120 ++++++++++++++++++++++++++++++++-------
spec/suites/removeLayersSpec.js | 121 ++++++++++++++++++++++++++++++++++------
src/MarkerClusterGroup.js | 49 ++++++++++++----
4 files changed, 246 insertions(+), 49 deletions(-)
diff --git a/README.md b/README.md
index a0d3cbe..cf00c2c 100644
--- a/README.md
+++ b/README.md
@@ -158,6 +158,11 @@ markers.on('clusterclick', function (a) {
#### Bulk adding and removing Markers
`addLayers` and `removeLayers` are bulk methods for adding and removing markers and should be favoured over the single versions when doing bulk addition/removal of markers. Each takes an array of markers. You can use [dedicated options](#chunked-addlayers) to fine-tune the behaviour of `addLayers`.
+These methods extract non-group layer children from Layer Group types, even deeply nested. _However_, be noted that:
+- The input array is modified. If you need to keep it untouched, clone it before passing it (e.g. `newArray = originalArray.slice()`).
+- `chunkProgress` jumps backward when `addLayers` finds a group (since appending its children to the input array makes the total increase).
+- `hasLayer` method will return `true` for child non-group layers, but `false` on any (possibly parent) Layer Group type.
+
If you are removing a lot of markers it will almost definitely be better to call `clearLayers` then call `addLayers` to add the markers you don't want to remove back in. See [#59](https://github.com/Leaflet/Leaflet.markercluster/issues/59#issuecomment-9320628) for details.
#### Getting the visible parent of a marker
diff --git a/spec/suites/RemoveLayerSpec.js b/spec/suites/RemoveLayerSpec.js
index 52f014c..8782ac9 100644
--- a/spec/suites/RemoveLayerSpec.js
+++ b/spec/suites/RemoveLayerSpec.js
@@ -1,27 +1,72 @@
describe('removeLayer', function () {
- var map, div, clock;
+
+ /**
+ * Avoid as much as possible creating and destroying objects for each test.
+ * Instead, try re-using them, except for the ones under test of course.
+ * PhantomJS does not perform garbage collection for the life of the page,
+ * i.e. during the entire test process (Karma runs all tests in a single page).
+ * http://stackoverflow.com/questions/27239708/how-to-get-around-memory-error-with-karma-phantomjs
+ *
+ * The `beforeEach` and `afterEach do not seem to cause much issue.
+ * => they can still be used to initialize some setup between each test.
+ * Using them keeps a readable spec/index.
+ *
+ * But refrain from re-creating div and map every time. Re-use those objects.
+ */
+
+ /////////////////////////////
+ // SETUP FOR EACH TEST
+ /////////////////////////////
+
beforeEach(function () {
- clock = sinon.useFakeTimers();
- div = document.createElement('div');
- div.style.width = '200px';
- div.style.height = '200px';
- document.body.appendChild(div);
- map = L.map(div, { maxZoom: 18 });
+ clock = sinon.useFakeTimers();
- map.fitBounds(new L.LatLngBounds([
- [1, 1],
- [2, 2]
- ]));
});
+
afterEach(function () {
+
+ if (group instanceof L.MarkerClusterGroup) {
+ group.clearLayers();
+ map.removeLayer(group);
+ }
+
+ // Throw away group as it can be assigned with different configurations between tests.
+ group = null;
+
clock.restore();
- document.body.removeChild(div);
+
});
+
+ /////////////////////////////
+ // PREPARATION CODE
+ /////////////////////////////
+
+ var div, map, group, clock;
+
+ div = document.createElement('div');
+ div.style.width = '200px';
+ div.style.height = '200px';
+ document.body.appendChild(div);
+
+ map = L.map(div, { maxZoom: 18 });
+
+ // Corresponds to zoom level 8 for the above div dimensions.
+ map.fitBounds(new L.LatLngBounds([
+ [1, 1],
+ [2, 2]
+ ]));
+
+
+ /////////////////////////////
+ // TESTS
+ /////////////////////////////
+
it('removes a layer that was added to it', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1.5, 1.5]);
map.addLayer(group);
@@ -37,7 +82,8 @@
it('doesnt remove a layer not added to it', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1.5, 1.5]);
map.addLayer(group);
@@ -53,7 +99,8 @@
it('removes a layer that was added to it (before being on the map) that is shown in a cluster', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1.5, 1.5]);
var marker2 = new L.Marker([1.5, 1.5]);
@@ -68,7 +115,8 @@
it('removes a layer that was added to it (after being on the map) that is shown in a cluster', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1.5, 1.5]);
var marker2 = new L.Marker([1.5, 1.5]);
@@ -84,7 +132,8 @@
it('removes a layer that was added to it (before being on the map) that is individually', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1, 1.5]);
var marker2 = new L.Marker([3, 1.5]);
@@ -103,7 +152,8 @@
it('removes a layer (with animation) that was added to it (after being on the map) that is shown in a cluster', function () {
- var group = new L.MarkerClusterGroup({ animateAddingMarkers: true });
+ group = new L.MarkerClusterGroup({ animateAddingMarkers: true });
+
var marker = new L.Marker([1.5, 1.5]);
var marker2 = new L.Marker([1.5, 1.5]);
@@ -128,7 +178,8 @@
it('removes the layers that are in the given LayerGroup', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1.5, 1.5]);
var marker2 = new L.Marker([1.5, 1.5]);
@@ -145,7 +196,8 @@
it('removes the layers that are in the given LayerGroup when not on the map', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var marker = new L.Marker([1.5, 1.5]);
var marker2 = new L.Marker([1.5, 1.5]);
@@ -158,4 +210,30 @@
expect(group.hasLayer(marker)).to.be(true);
expect(group.hasLayer(marker2)).to.be(false);
});
-});
\ No newline at end of file
+
+ it('passes control to removeLayers when marker is a Layer Group', function () {
+
+ group = new L.MarkerClusterGroup();
+
+ var marker1 = new L.Marker([1.5, 1.5]);
+ var marker2 = new L.Marker([1.5, 1.5]);
+
+ group.addLayers([marker1, marker2]);
+
+ var layer = L.layerGroup();
+ layer.addLayer(marker2);
+ group.removeLayer(new L.LayerGroup([layer]));
+
+ expect(group.hasLayer(marker1)).to.be(true);
+ expect(group.hasLayer(marker2)).to.be(false);
+ });
+
+
+ /////////////////////////////
+ // CLEAN UP CODE
+ /////////////////////////////
+
+ map.remove();
+ document.body.removeChild(div);
+
+});
diff --git a/spec/suites/removeLayersSpec.js b/spec/suites/removeLayersSpec.js
index f4caff6..c7f47c1 100644
--- a/spec/suites/removeLayersSpec.js
+++ b/spec/suites/removeLayersSpec.js
@@ -1,27 +1,72 @@
describe('removeLayers', function () {
- var map, div, clock;
+
+ /**
+ * Avoid as much as possible creating and destroying objects for each test.
+ * Instead, try re-using them, except for the ones under test of course.
+ * PhantomJS does not perform garbage collection for the life of the page,
+ * i.e. during the entire test process (Karma runs all tests in a single page).
+ * http://stackoverflow.com/questions/27239708/how-to-get-around-memory-error-with-karma-phantomjs
+ *
+ * The `beforeEach` and `afterEach do not seem to cause much issue.
+ * => they can still be used to initialize some setup between each test.
+ * Using them keeps a readable spec/index.
+ *
+ * But refrain from re-creating div and map every time. Re-use those objects.
+ */
+
+ /////////////////////////////
+ // SETUP FOR EACH TEST
+ /////////////////////////////
+
beforeEach(function () {
- clock = sinon.useFakeTimers();
- div = document.createElement('div');
- div.style.width = '200px';
- div.style.height = '200px';
- document.body.appendChild(div);
- map = L.map(div, { maxZoom: 18 });
+ clock = sinon.useFakeTimers();
- map.fitBounds(new L.LatLngBounds([
- [1, 1],
- [2, 2]
- ]));
});
+
afterEach(function () {
+
+ if (group instanceof L.MarkerClusterGroup) {
+ group.clearLayers();
+ map.removeLayer(group);
+ }
+
+ // Throw away group as it can be assigned with different configurations between tests.
+ group = null;
+
clock.restore();
- document.body.removeChild(div);
+
});
+
+ /////////////////////////////
+ // PREPARATION CODE
+ /////////////////////////////
+
+ var div, map, group, clock;
+
+ div = document.createElement('div');
+ div.style.width = '200px';
+ div.style.height = '200px';
+ document.body.appendChild(div);
+
+ map = L.map(div, { maxZoom: 18 });
+
+ // Corresponds to zoom level 8 for the above div dimensions.
+ map.fitBounds(new L.LatLngBounds([
+ [1, 1],
+ [2, 2]
+ ]));
+
+
+ /////////////////////////////
+ // TESTS
+ /////////////////////////////
+
it('removes all the layer given to it', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var markers = [
new L.Marker([1.5, 1.5]),
new L.Marker([1.5, 1.5]),
@@ -43,7 +88,8 @@
it('removes all the layer given to it even though they move', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var markers = [
new L.Marker([10, 10]),
new L.Marker([20, 20]),
@@ -67,7 +113,8 @@
it('removes all the layer given to it even if the group is not on the map', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var markers = [
new L.Marker([1.5, 1.5]),
new L.Marker([1.5, 1.5]),
@@ -89,7 +136,8 @@
it('doesnt break if we are spiderfied', function () {
- var group = new L.MarkerClusterGroup();
+ group = new L.MarkerClusterGroup();
+
var markers = [
new L.Marker([1.5, 1.5]),
new L.Marker([1.5, 1.5]),
@@ -117,4 +165,45 @@
expect(group._spiderfied).to.be(null);
});
});
+
+ it('handles nested Layer Groups', function () {
+
+ group = new L.MarkerClusterGroup();
+
+ var marker1 = new L.Marker([1.5, 1.5]);
+ var marker2 = new L.Marker([1.5, 1.5]);
+ var marker3 = new L.Marker([1.5, 1.5]);
+
+ map.addLayer(group);
+
+ group.addLayers([marker1, marker2, marker3]);
+
+ expect(group.hasLayer(marker1)).to.be(true);
+ expect(group.hasLayer(marker2)).to.be(true);
+ expect(group.hasLayer(marker3)).to.be(true);
+
+ group.removeLayers([
+ marker1,
+ new L.LayerGroup([
+ marker2, new L.LayerGroup([
+ marker3
+ ])
+ ])
+ ]);
+
+ expect(group.hasLayer(marker1)).to.be(false);
+ expect(group.hasLayer(marker2)).to.be(false);
+ expect(group.hasLayer(marker3)).to.be(false);
+
+ expect(group.getLayers().length).to.be(0);
+ });
+
+
+ /////////////////////////////
+ // CLEAN UP CODE
+ /////////////////////////////
+
+ map.remove();
+ document.body.removeChild(div);
+
});
diff --git a/src/MarkerClusterGroup.js b/src/MarkerClusterGroup.js
index 1b3766d..b4b6677 100644
--- a/src/MarkerClusterGroup.js
+++ b/src/MarkerClusterGroup.js
@@ -125,13 +125,8 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
removeLayer: function (layer) {
- if (layer instanceof L.LayerGroup)
- {
- var array = [];
- for (var i in layer._layers) {
- array.push(layer._layers[i]);
- }
- return this.removeLayers(array);
+ if (layer instanceof L.LayerGroup) {
+ return this.removeLayers([layer]);
}
//Non point layers
@@ -208,6 +203,8 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
// Side effects:
// - Total increases, so chunkProgress ratio jumps backward.
// - Input array is modified.
+ // Changing array length while looping does not affect performance in current browsers:
+ // http://jsperf.com/for-loop-changing-length/6
if (m instanceof L.LayerGroup) {
this._extractNonGroupLayers(m, layersArray);
l = layersArray.length;
@@ -292,13 +289,22 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
//Takes an array of markers and removes them in bulk
removeLayers: function (layersArray) {
- var i, l, m,
+ var i, m,
+ l = layersArray.length,
fg = this._featureGroup,
npg = this._nonPointGroup;
if (!this._map) {
- for (i = 0, l = layersArray.length; i < l; i++) {
+ for (i = 0; i < l; i++) {
m = layersArray[i];
+
+ // Group of layers, append children to layersArray and skip.
+ if (m instanceof L.LayerGroup) {
+ this._extractNonGroupLayers(m, layersArray);
+ l = layersArray.length;
+ continue;
+ }
+
this._arraySplice(this._needsClustering, m);
npg.removeLayer(m);
if (this.hasLayer(m)) {
@@ -310,15 +316,34 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
if (this._unspiderfy) {
this._unspiderfy();
- for (i = 0, l = layersArray.length; i < l; i++) {
- m = layersArray[i];
+
+ // Work on a copy of the array, so that next loop is not affected.
+ var layersArray2 = layersArray.slice(),
+ l2 = l;
+ for (i = 0; i < l2; i++) {
+ m = layersArray2[i];
+
+ // Group of layers, append children to layersArray and skip.
+ if (m instanceof L.LayerGroup) {
+ this._extractNonGroupLayers(m, layersArray2);
+ l2 = layersArray2.length;
+ continue;
+ }
+
this._unspiderfyLayer(m);
}
}
- for (i = 0, l = layersArray.length; i < l; i++) {
+ for (i = 0; i < l; i++) {
m = layersArray[i];
+ // Group of layers, append children to layersArray and skip.
+ if (m instanceof L.LayerGroup) {
+ this._extractNonGroupLayers(m, layersArray);
+ l = layersArray.length;
+ continue;
+ }
+
if (!m.__parent) {
npg.removeLayer(m);
continue;
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/leaflet-markercluster.git
More information about the Pkg-javascript-commits
mailing list