[Pkg-javascript-commits] [leaflet-markercluster] 147/219: Made refreshClusters to re-draw markers in singleMarkerMode

Jonas Smedegaard dr at jones.dk
Sat May 7 09:39:29 UTC 2016


This is an automated email from the git hooks/post-receive script.

js pushed a commit to branch master
in repository leaflet-markercluster.

commit e6f36e0da05748b3e21995d2bb3e66fa60db8a66
Author: ghybs <ghybs1 at gmail.com>
Date:   Mon Oct 19 12:44:53 2015 +0400

    Made refreshClusters to re-draw markers in singleMarkerMode
    
    Following issue #581, improved refreshClusters() method so that it detects singleMarkerMode and re-draws all passed markers as well, not only parent clusters.
    
    Also created the singleMarkerModeSpec test suite to test singleMarkerMode option, added an extra test in RefreshSpec test suite to cover this case, and added the new suite into spec/index.
---
 spec/index.html                     |   1 +
 spec/suites/RefreshSpec.js          | 203 +++++++++++++++++++++++++-----------
 spec/suites/singleMarkerModeSpec.js | 146 ++++++++++++++++++++++++++
 src/MarkerClusterGroup.Refresh.js   |  15 ++-
 src/MarkerClusterGroup.js           |  28 +++--
 5 files changed, 324 insertions(+), 69 deletions(-)

diff --git a/spec/index.html b/spec/index.html
index f25b59a..68b04e2 100644
--- a/spec/index.html
+++ b/spec/index.html
@@ -37,6 +37,7 @@
 	<script type="text/javascript" src="suites/AddLayer.SingleSpec.js"></script>
 	<script type="text/javascript" src="suites/AddLayersSpec.js"></script>
 	<script type="text/javascript" src="suites/animateOptionSpec.js"></script>
+	<script type="text/javascript" src="suites/singleMarkerModeSpec.js"></script>
 
 	<script type="text/javascript" src="suites/ChildChangingIconSupportSpec.js"></script>
 
diff --git a/spec/suites/RefreshSpec.js b/spec/suites/RefreshSpec.js
index d11eea1..40cf442 100644
--- a/spec/suites/RefreshSpec.js
+++ b/spec/suites/RefreshSpec.js
@@ -1,21 +1,95 @@
 describe('refreshClusters', function () {
-	var map, div, clock, group;
 
-	function getClusterAtZoom(marker, zoom) {
-		var parent = marker.__parent;
+	/**
+	 * Wrapper for Mocha's `it` function, to avoid using `beforeEach` and `afterEach`
+	 * which create problems with PhantomJS when total number of tests (across all suites)
+	 * increases. Might be due to use of promises for which PhantomJS would perform badly?
+	 * NOTE: works only with synchronous code.
+	 * @param testDescription string
+	 * @param testInstructions function
+	 * @param testFinally function to be executed just before afterEach2, in the `finally` block.
+	 */
+	function it2(testDescription, testInstructions, testFinally) {
+
+		it(testDescription, function () {
+
+			// Before each test.
+			if (typeof beforeEach2 === "function") {
+				beforeEach2();
+			}
 
-		while (parent && parent._zoom !== zoom) {
-			parent = parent.__parent;
+			try {
+
+				// Perform the actual test instructions.
+				testInstructions();
+
+			} catch (e) {
+
+				// Re-throw the exception so that Mocha sees the failed test.
+				throw e;
+
+			} finally {
+
+				// If specific final instructions are provided.
+				if (typeof testFinally === "function") {
+					testFinally();
+				}
+
+				// After each test.
+				if (typeof afterEach2 === "function") {
+					afterEach2();
+				}
+
+			}
+		});
+	}
+
+
+	/////////////////////////////
+	// SETUP FOR EACH TEST
+	/////////////////////////////
+
+	/**
+	 * Instructions to be executed before each test called with `it2`.
+	 */
+	function beforeEach2() {
+
+		clock = sinon.useFakeTimers();
+
+		// Look away to avoid refreshing the display while adding markers.
+		// By adding markers one by one (instead of using addLayers) we make
+		// sure we never start an async process.
+		map.fitBounds(new L.LatLngBounds([
+			[-11, -11],
+			[-10, -10]
+		]))
+
+	}
+
+	/**
+	 * Instructions to be executed after each test called with `it2`.
+	 */
+	function afterEach2() {
+
+		if (group instanceof L.MarkerClusterGroup) {
+			group.removeLayers(group.getLayers());
+			map.removeLayer(group);
 		}
 
-		return parent;
+		// group must be thrown away since we are testing it with a potentially
+		// different configuration at each test.
+		group = null;
+
+		clock.restore();
+		clock = null;
 	}
 
-	// It looks like using beforeEach and afterEach generates problems when
-	// total number (across all spec suites) of tests increases.
-	// It could be related to PhantomJS memory leak / bad garbage collection.
-	// This problem seems to affect only PhantomJS, no other browsers?
-	// So let's implement beforeEach and afterEach manually and try re-using objects.
+
+	/////////////////////////////
+	// PREPARATION CODE
+	/////////////////////////////
+
+	var div, map, group, clock;
 
 	div = document.createElement('div');
 	div.style.width = '200px';
@@ -30,16 +104,14 @@
 		[2, 2]
 	]));
 
-	function init() {
-		clock = sinon.useFakeTimers();
+	function getClusterAtZoom(marker, zoom) {
+		var parent = marker.__parent;
 
-		// Look away to avoid refreshing the display while adding markers.
-		// By adding markers one by one (instead of using addLayers) we make
-		// sure we never start an async process.
-		map.fitBounds(new L.LatLngBounds([
-			[-11, -11],
-			[-10, -10]
-		]))
+		while (parent && parent._zoom !== zoom) {
+			parent = parent.__parent;
+		}
+
+		return parent;
 	}
 
 	function setMapView() {
@@ -51,21 +123,12 @@
 		]));
 	}
 
-	function reset() {
-		// Keep map and div setup to avoid potential memory leak.
-		map.removeLayer(group);
 
-		// group must be thrown away since we are testing it with a potentially
-		// different configuration at each test.
-		group = null;
+	/////////////////////////////
+	// TESTS
+	/////////////////////////////
 
-		clock.restore();
-		clock = null;
-	}
-
-	it('flags all non-visible parent clusters of a given marker', function () {
-
-		init();
+	it2('flags all non-visible parent clusters of a given marker', function () {
 
 		group = L.markerClusterGroup().addTo(map);
 
@@ -106,13 +169,9 @@
 		// Also check that visible clusters are "un-flagged" since they should be re-drawn.
 		expect(marker1cluster5._iconNeedsUpdate).to.not.be.ok();
 
-		reset();
-
 	});
 
-	it('re-draws visible clusters', function () {
-
-		init();
+	it2('re-draws visible clusters', function () {
 
 		group = L.markerClusterGroup({
 			iconCreateFunction: function (cluster) {
@@ -159,7 +218,42 @@
 		expect(marker1cluster9._icon.className).to.contain("changed");
 		expect(marker1cluster9._icon.className).to.not.contain("original");
 
-		reset();
+	});
+
+	it2('re-draws markers in singleMarkerMode', function () {
+
+		group = L.markerClusterGroup({
+			singleMarkerMode: true,
+			iconCreateFunction: function (cluster) {
+				var markers = cluster.getAllChildMarkers();
+
+				for(var i in markers) {
+					if (markers[i].changed) {
+						return new L.DivIcon({
+							className: "changed"
+						});
+					}
+				}
+				return new L.DivIcon({
+					className: "original"
+				});
+			}
+		}).addTo(map);
+
+		var marker1 = L.marker([1.5, 1.5]).addTo(group);
+
+		setMapView();
+
+		expect(marker1._icon.className).to.contain("original");
+
+		// Alter the marker.
+		marker1.changed = true;
+
+		// Then request clusters refresh.
+		group.refreshClusters(marker1);
+
+		expect(marker1._icon.className).to.contain("changed");
+		expect(marker1._icon.className).to.not.contain("original");
 
 	});
 
@@ -183,8 +277,6 @@
 
 	function init3clusterBranches() {
 
-		init();
-
 		group = L.markerClusterGroup({
 			maxClusterRadius: 2 // Make sure we keep distinct clusters.
 		}).addTo(map);
@@ -242,7 +334,7 @@
 		// Ready to refresh clusters with method of choice and assess result.
 	}
 
-	it('does not flag clusters of other markers', function () {
+	it2('does not flag clusters of other markers', function () {
 
 		init3clusterBranches();
 
@@ -261,10 +353,9 @@
 		expect(marker5cluster8._iconNeedsUpdate).to.not.be.ok();
 		expect(marker5cluster3._iconNeedsUpdate).to.not.be.ok();
 
-		reset();
 	});
 
-	it('processes itself when no argument is passed', function () {
+	it2('processes itself when no argument is passed', function () {
 
 		init3clusterBranches();
 
@@ -282,11 +373,9 @@
 		expect(marker5cluster8._iconNeedsUpdate).to.be.ok();
 		expect(marker5cluster3._iconNeedsUpdate).to.be.ok();
 
-		reset();
-
 	});
 
-	it('accepts an array of markers', function () {
+	it2('accepts an array of markers', function () {
 
 		init3clusterBranches();
 
@@ -306,11 +395,9 @@
 		expect(marker3cluster8._iconNeedsUpdate).to.not.be.ok();
 		expect(marker3cluster3._iconNeedsUpdate).to.not.be.ok();
 
-		reset();
-
 	});
 
-	it('accepts a mapping of markers', function () {
+	it2('accepts a mapping of markers', function () {
 
 		init3clusterBranches();
 
@@ -332,11 +419,9 @@
 		expect(marker3cluster8._iconNeedsUpdate).to.not.be.ok();
 		expect(marker3cluster3._iconNeedsUpdate).to.not.be.ok();
 
-		reset();
-
 	});
 
-	it('accepts an L.LayerGroup', function () {
+	it2('accepts an L.LayerGroup', function () {
 
 		init3clusterBranches();
 
@@ -357,11 +442,9 @@
 		expect(marker3cluster8._iconNeedsUpdate).to.not.be.ok();
 		expect(marker3cluster3._iconNeedsUpdate).to.not.be.ok();
 
-		reset();
-
 	});
 
-	it('accepts an L.MarkerCluster', function () {
+	it2('accepts an L.MarkerCluster', function () {
 
 		init3clusterBranches();
 
@@ -381,11 +464,13 @@
 		expect(marker5cluster8._iconNeedsUpdate).to.not.be.ok();
 		expect(marker5cluster3._iconNeedsUpdate).to.not.be.ok();
 
-		reset();
-
 	});
 
-	// Now we can throw away the map and div.
+
+	/////////////////////////////
+	// CLEAN UP CODE
+	/////////////////////////////
+
 	map.remove();
 	document.body.removeChild(div);
 
diff --git a/spec/suites/singleMarkerModeSpec.js b/spec/suites/singleMarkerModeSpec.js
new file mode 100644
index 0000000..0c27b7d
--- /dev/null
+++ b/spec/suites/singleMarkerModeSpec.js
@@ -0,0 +1,146 @@
+describe('singleMarkerMode option', function () {
+
+	/**
+	 * Wrapper for Mocha's `it` function, to avoid using `beforeEach` and `afterEach`
+	 * which create problems with PhantomJS when total number of tests (across all suites)
+	 * increases. Might be due to use of promises for which PhantomJS would perform badly?
+	 * NOTE: works only with synchronous code.
+	 * @param testDescription string
+	 * @param testInstructions function
+	 * @param testFinally function to be executed just before afterEach2, in the `finally` block.
+	 */
+	function it2(testDescription, testInstructions, testFinally) {
+
+		it(testDescription, function () {
+
+			// Before each test.
+			if (typeof beforeEach2 === "function") {
+				beforeEach2();
+			}
+
+			try {
+
+				// Perform the actual test instructions.
+				testInstructions();
+
+			} catch (e) {
+
+				// Re-throw the exception so that Mocha sees the failed test.
+				throw e;
+
+			} finally {
+
+				// If specific final instructions are provided.
+				if (typeof testFinally === "function") {
+					testFinally();
+				}
+
+				// After each test.
+				if (typeof afterEach2 === "function") {
+					afterEach2();
+				}
+
+			}
+		});
+	}
+
+
+	/////////////////////////////
+	// SETUP FOR EACH TEST
+	/////////////////////////////
+
+	/**
+	 * Instructions to be executed before each test called with `it2`.
+	 */
+	function beforeEach2() {
+
+		// Reset the marker icon.
+		marker.setIcon(defaultIcon);
+
+	}
+
+	/**
+	 * Instructions to be executed after each test called with `it2`.
+	 */
+	function afterEach2() {
+
+		if (group instanceof L.MarkerClusterGroup) {
+			group.removeLayers(group.getLayers());
+			map.removeLayer(group);
+		}
+
+		// Throw away group as it can be assigned with different configurations between tests.
+		group = null;
+	}
+
+
+	/////////////////////////////
+	// PREPARATION CODE
+	/////////////////////////////
+
+	var div, map, group;
+
+	var defaultIcon = new L.Icon.Default(),
+	    clusterIcon = new L.Icon.Default(),
+		marker = L.marker([1.5, 1.5]);
+
+	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
+	/////////////////////////////
+
+	it2('overrides marker icons when set to true', function () {
+
+		group = L.markerClusterGroup({
+			singleMarkerMode: true,
+			iconCreateFunction: function (layer) {
+				return clusterIcon;
+			}
+		}).addTo(map);
+
+		expect(marker.options.icon).to.equal(defaultIcon);
+
+		marker.addTo(group);
+
+		expect(marker.options.icon).to.equal(clusterIcon);
+
+	});
+
+	it2('does not modify marker icons by default (or set to false)', function () {
+
+		group = L.markerClusterGroup({
+			iconCreateFunction: function (layer) {
+				return clusterIcon;
+			}
+		}).addTo(map);
+
+		expect(marker.options.icon).to.equal(defaultIcon);
+
+		marker.addTo(group);
+
+		expect(marker.options.icon).to.equal(defaultIcon);
+
+	});
+
+
+	/////////////////////////////
+	// CLEAN UP CODE
+	/////////////////////////////
+
+	map.remove();
+	document.body.removeChild(div);
+
+});
diff --git a/src/MarkerClusterGroup.Refresh.js b/src/MarkerClusterGroup.Refresh.js
index 8042a35..c04838f 100644
--- a/src/MarkerClusterGroup.Refresh.js
+++ b/src/MarkerClusterGroup.Refresh.js
@@ -38,10 +38,10 @@ L.MarkerClusterGroup.include({
 	 * @private
 	 */
 	_flagParentsIconsNeedUpdate: function (layers) {
-		var parent;
+		var parent, id;
 
 		// Assumes layers is an Array or an Object whose prototype is non-enumerable.
-		for (var id in layers) {
+		for (id in layers) {
 			// Flag parent clusters' icon as "dirty", all the way up.
 			// Dumb process that flags multiple times upper parents, but still
 			// much more efficient than trying to be smart and make short lists,
@@ -53,6 +53,17 @@ L.MarkerClusterGroup.include({
 				parent = parent.__parent;
 			}
 		}
+
+		// In case of singleMarkerMode, also re-draw the markers.
+		if (this.options.singleMarkerMode) {
+			var layer;
+
+			for (id in layers) {
+				layer = layers[id];
+				// Need to re-create the icon first, then re-draw the marker.
+				layer.setIcon(this._overrideMarkerIcon(layer));
+			}
+		}
 	},
 
 	/**
diff --git a/src/MarkerClusterGroup.js b/src/MarkerClusterGroup.js
index f801547..6369297 100644
--- a/src/MarkerClusterGroup.js
+++ b/src/MarkerClusterGroup.js
@@ -796,14 +796,7 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 		    markerPoint, z;
 
 		if (this.options.singleMarkerMode) {
-			layer.options.icon = this.options.iconCreateFunction({
-				getChildCount: function () {
-					return 1;
-				},
-				getAllChildMarkers: function () {
-					return [layer];
-				}
-			});
+			this._overrideMarkerIcon(layer);
 		}
 
 		//Find the lowest zoom level to slot this one in
@@ -923,6 +916,25 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 		} else {
 			newCluster._updateIcon();
 		}
+	},
+
+	/**
+	 * Implements the singleMarkerMode option.
+	 * @param layer Marker to re-style using the Clusters iconCreateFunction.
+	 * @returns {L.Icon} The newly created icon.
+	 * @private
+	 */
+	_overrideMarkerIcon: function (layer) {
+		var icon = layer.options.icon = this.options.iconCreateFunction({
+			getChildCount: function () {
+				return 1;
+			},
+			getAllChildMarkers: function () {
+				return [layer];
+			}
+		});
+
+		return icon;
 	}
 });
 

-- 
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