[Pkg-javascript-commits] [leaflet-markercluster] 46/128: Corrected #529 in master (Leaflet 1.0.0)

Jonas Smedegaard dr at jones.dk
Sun Apr 16 06:26:02 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 44963c185116e38b1dc54fc9b8a42e1e118f66e6
Author: ghybs <ghybs1 at gmail.com>
Date:   Fri Mar 4 16:07:10 2016 +0400

    Corrected #529 in master (Leaflet 1.0.0)
    
    Same as #344, which was corrected by commit 254198ad47d73ddb7177c6aefde78561046d8c0c in leaflet-0.7 branch only. Do the same in master branch (for Leaflet 1.0.0) as well.
    Also added a dedicated test spec in zoomAnimationSpec test suite.
    
    Took the opportunity to refactor the test suite to avoid PhantomJS memory leak.
---
 spec/suites/zoomAnimationSpec.js | 164 +++++++++++++++++++++++++++++++++------
 src/MarkerClusterGroup.js        |   4 +-
 2 files changed, 143 insertions(+), 25 deletions(-)

diff --git a/spec/suites/zoomAnimationSpec.js b/spec/suites/zoomAnimationSpec.js
index dbf60f9..b57348b 100644
--- a/spec/suites/zoomAnimationSpec.js
+++ b/spec/suites/zoomAnimationSpec.js
@@ -1,33 +1,86 @@
 describe('zoomAnimation', 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);
+		clock = sinon.useFakeTimers();
 
-		map = L.map(div, { maxZoom: 18 });
 	});
+
 	afterEach(function () {
+
+		// Restore the previous setting, so that even in case of test failure, next tests are not affected.
+		L.Browser.mobile = previousMobileSetting;
+
+		if (group instanceof L.MarkerClusterGroup) {
+			group.clearLayers();
+			map.removeLayer(group);
+		}
+
+		// group must be thrown away since we are testing it with a potentially
+		// different configuration at each test.
+		group = null;
+
 		clock.restore();
+		clock = null;
 
-		document.body.removeChild(div);
 	});
 
+
+	/////////////////////////////
+	// PREPARATION CODE
+	/////////////////////////////
+
+	var previousMobileSetting = L.Browser.mobile,
+		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('adds the visible marker to the map when zooming in', function () {
 		map.setView(new L.LatLng(-37.36142550190516, 174.254150390625), 7);
 
-		var markers = new L.MarkerClusterGroup({
+		group = new L.MarkerClusterGroup({
 			showCoverageOnHover: true,
 			maxClusterRadius: 20,
 			disableClusteringAtZoom: 15
 		});
 		var marker = new L.Marker([-37.77852090603777, 175.3103667497635]);
-		markers.addLayer(marker); //The one we zoom in on
-		markers.addLayer(new L.Marker([-37.711800591811055, 174.50034790039062])); //Marker that we cluster with at the top zoom level, but not 1 level down
-		map.addLayer(markers);
+		group.addLayer(marker); //The one we zoom in on
+		group.addLayer(new L.Marker([-37.711800591811055, 174.50034790039062])); //Marker that we cluster with at the top zoom level, but not 1 level down
+		map.addLayer(group);
 
 		clock.tick(1000);
 		map.setView([-37.77852090603777, 175.3103667497635], 15);
@@ -41,12 +94,12 @@
 
 	it('adds the visible marker to the map when jumping around', function () {
 
-		var markers = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
 		var marker1 = new L.Marker([48.858280181884766, 2.2945759296417236]);
 		var marker2 = new L.Marker([16.02359962463379, -61.70280075073242]);
-		markers.addLayer(marker1); //The one we zoom in on first
-		markers.addLayer(marker2); //Marker that we cluster with at the top zoom level, but not 1 level down
-		map.addLayer(markers);
+		group.addLayer(marker1); //The one we zoom in on first
+		group.addLayer(marker2); //Marker that we cluster with at the top zoom level, but not 1 level down
+		map.addLayer(group);
 
 		//show the first
 		map.fitBounds(new L.LatLngBounds(new L.LatLng(41.371582, -5.142222), new L.LatLng(51.092804, 9.561556)));
@@ -65,15 +118,16 @@
 
 	it('adds the visible markers to the map, but not parent clusters when jumping around', function () {
 
-		var markers = new L.MarkerClusterGroup(),
-			marker1 = new L.Marker([59.9520, 30.3307]),
+		group = new L.MarkerClusterGroup();
+
+		var marker1 = new L.Marker([59.9520, 30.3307]),
 			marker2 = new L.Marker([59.9516, 30.3308]),
 			marker3 = new L.Marker([59.9513, 30.3312]);
 
-		markers.addLayer(marker1);
-		markers.addLayer(marker2);
-		markers.addLayer(marker3);
-		map.addLayer(markers);
+		group.addLayer(marker1);
+		group.addLayer(marker2);
+		group.addLayer(marker3);
+		map.addLayer(group);
 
 		//Show none of them
 		map.setView([53.0676, 170.6835], 16);
@@ -92,4 +146,68 @@
 		expect(marker3._icon.parentNode).to.be(map._panes.markerPane);
 		expect(map._panes.markerPane.childNodes.length).to.be(3);
 	});
-});
\ No newline at end of file
+
+	it('removes clicked clusters on the edge of a mobile screen', function () {
+
+		L.Browser.mobile = true;
+
+		// Corresponds to zoom level 8 for the above div dimensions.
+		map.fitBounds(new L.LatLngBounds([
+			[1, 1],
+			[2, 2]
+		]));
+
+		group = new L.MarkerClusterGroup({
+			maxClusterRadius: 80
+		}).addTo(map);
+
+		// Add a marker 1 pixel below the initial screen bottom edge.
+		var bottomPoint = map.getPixelBounds().max.add([0, 1]),
+			bottomLatLng = map.unproject(bottomPoint),
+			centerLng = map.getCenter().lng,
+			bottomPosition = new L.LatLng(
+				bottomLatLng.lat,
+				centerLng
+			),
+			bottomMarker = new L.Marker(bottomPosition).addTo(group),
+			initialZoom = map.getZoom();
+
+		expect(bottomMarker._icon).to.be(undefined);
+
+		// Add many markers 79 pixels above the first one, so they cluster with it.
+		var newPoint = bottomPoint.add([0, -79]),
+			newLatLng = L.latLng(
+				map.unproject(newPoint).lat,
+				centerLng
+			);
+
+		for (var i = 0; i < 10; i += 1) {
+			group.addLayer(new L.Marker(newLatLng));
+		}
+
+		var parentCluster = bottomMarker.__parent;
+
+		expect(parentCluster._icon.parentNode).to.be(map._panes.markerPane);
+
+		parentCluster.fireEvent('click', null, true);
+
+		//Run the the animation
+		clock.tick(1000);
+
+		expect(map.getZoom()).to.equal(initialZoom + 1); // The fitBounds with 200px height should result in zooming 1 level in.
+
+		// Finally make sure that the cluster has been removed from map.
+		expect(parentCluster._icon).to.be(null);
+		expect(map._panes.markerPane.childNodes.length).to.be(2); // The bottomMarker + cluster for the 10 above markers.
+
+	});
+
+
+	/////////////////////////////
+	// CLEAN UP CODE
+	/////////////////////////////
+
+	map.remove();
+	document.body.removeChild(div);
+
+});
diff --git a/src/MarkerClusterGroup.js b/src/MarkerClusterGroup.js
index d2d1802..22cb01a 100644
--- a/src/MarkerClusterGroup.js
+++ b/src/MarkerClusterGroup.js
@@ -987,10 +987,10 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 	_mergeSplitClusters: function () {
 		var mapZoom = Math.round(this._map._zoom);
 
-		//Incase we are starting to split before the animation finished
+		//In case we are starting to split before the animation finished
 		this._processQueue();
 
-		if (this._zoom < mapZoom && this._currentShownBounds.contains(this._getExpandedVisibleBounds())) { //Zoom in, split
+		if (this._zoom < mapZoom && this._currentShownBounds.intersects(this._getExpandedVisibleBounds())) { //Zoom in, split
 			this._animationStart();
 			//Remove clusters now off screen
 			this._topClusterLevel._recursivelyRemoveChildrenFromMap(this._currentShownBounds, this._zoom, this._getExpandedVisibleBounds());

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