diff --git a/spec/suites/removeOutsideVisibleBoundsSpec.js b/spec/suites/removeOutsideVisibleBoundsSpec.js index e7a633a0..a9ce90f5 100644 --- a/spec/suites/removeOutsideVisibleBoundsSpec.js +++ b/spec/suites/removeOutsideVisibleBoundsSpec.js @@ -1,75 +1,44 @@ describe('Option removeOutsideVisibleBounds', 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 performs badly? - * @param testDescription string - * @param testInstructions function - * @param testFinally function to be executed just before afterEach2, in the `finally` block. + * 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. */ - 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() { + beforeEach(function () { // Nothing for this test suite. - } + }); - /** - * Instructions to be executed after each test called with `it2`. - */ - function afterEach2() { + 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.removeLayers(group.getLayers()); map.removeLayer(group); } - // Throw away group as it can be assigned with different configurations between tests. + // group must be thrown away since we are testing it with a potentially + // different configuration at each test. group = null; - } + + }); ///////////////////////////// @@ -81,7 +50,9 @@ describe('Option removeOutsideVisibleBounds', function () { marker3 = L.marker([1.5, 1.5]), // In view port. marker4 = L.marker([1.5, 2.4]), // 1 screen width away. marker5 = L.marker([1.5, 3.4]), // 2 screens width away. - div, map, group, previousMobileSetting; + markers = [marker1, marker2, marker3, marker4, marker5], + previousMobileSetting = L.Browser.mobile, + div, map, group, i; div = document.createElement('div'); div.style.width = '200px'; @@ -97,24 +68,17 @@ describe('Option removeOutsideVisibleBounds', function () { ])); // Add all markers once to map then remove them immediately so that their icon is null (instead of undefined). - map.removeLayer(marker1.addTo(map)); - map.removeLayer(marker2.addTo(map)); - map.removeLayer(marker3.addTo(map)); - map.removeLayer(marker4.addTo(map)); - map.removeLayer(marker5.addTo(map)); + for (i = 0; i < markers.length; i++) { + map.removeLayer(markers[i].addTo(map)); + } function prepareGroup() { - // Group should be assigned with a Marker Cluster Group before calling this function. + // "group" should be assigned with a Marker Cluster Group before calling this function. group.addTo(map); - // Add markers 1 by 1 to make sure we do not create an async process. - marker1.addTo(group); - marker2.addTo(group); - marker3.addTo(group); - marker4.addTo(group); - marker5.addTo(group); + group.addLayers(markers); } @@ -122,7 +86,7 @@ describe('Option removeOutsideVisibleBounds', function () { // TESTS ///////////////////////////// - it2('removes objects more than 1 screen away from view port by default', function () { + it('removes objects more than 1 screen away from view port by default', function () { group = L.markerClusterGroup(); @@ -134,43 +98,166 @@ describe('Option removeOutsideVisibleBounds', function () { }); - it2( - 'removes objects out of view port by default for mobile device', + it('removes objects out of view port by default for mobile device', function () { - function () { + // Fool Leaflet, make it thinks it runs on a mobile device. + L.Browser.mobile = true; - // Fool Leaflet, make it thinks it runs on a mobile device. - previousMobileSetting = L.Browser.mobile; - L.Browser.mobile = true; + group = L.markerClusterGroup(); - group = L.markerClusterGroup(); + prepareGroup(); + + expect(marker1._icon).to.be(null); + expect(marker2._icon).to.be(null); + expect(map._panes.markerPane.childNodes.length).to.be(1); // marker 3 only. + expect(marker4._icon).to.be(null); + expect(marker5._icon).to.be(null); + + }); + + it('leaves all objects on map when set to false', function () { + + group = L.markerClusterGroup({ + removeOutsideVisibleBounds: false + }); + + prepareGroup(); + + expect(map._panes.markerPane.childNodes.length).to.be(5); // All 5 markers. + + }); + + + // Following tests need markers at very high latitude. + // They test the _checkBoundsMaxLat method against the default Web/Spherical Mercator projection maximum latitude (85.0511287798). + // The actual map view should be '-1.0986328125,84.92929204957956,1.0986328125,85.11983467698401' + // The expdanded bounds without correction should be '-3.2958984375,84.7387494221751,3.2958984375,85.31037730438847' + var latLngsMaxLatDefault = [ + [100, 3], // Impossible in real world, but nothing prevents the user from entering such latitude, and Web/Spherical Mercator projection will still display it at 85.0511287798 + [85.2, 1.5], // 1 "screen" heights away. + [85, 0], // In center of view. + [84.8, -1.5], // 1 "screen" height away. + [84.6, -3] // 2 "screens" height away. + ]; + + function moveMarkersAndMapToMaxLat(latLngs, isSouth) { + for (i = 0; i < markers.length; i++) { + if (isSouth) { + markers[i].setLatLng([-latLngs[i][0], latLngs[i][1]]); + } else { + markers[i].setLatLng(latLngs[i]); + } + } - prepareGroup(); + map.fitBounds([ + [isSouth ? -86 : 85, -1], + [isSouth ? -85 : 86, 1] // The actual map view longitude span will be wider. '-1.0986328125,84.92929204957956,1.0986328125,85.11983467698401' + ]); + } - expect(marker1._icon).to.be(null); - expect(marker2._icon).to.be(null); - expect(map._panes.markerPane.childNodes.length).to.be(1); // marker 3 only. - expect(marker4._icon).to.be(null); - expect(marker5._icon).to.be(null); + function checkProjection(latLngs) { + expect(map.options.crs).to.equal(L.CRS.EPSG3857); + expect(L.CRS.EPSG3857.projection).to.equal(L.Projection.SphericalMercator); + expect(L.Projection.SphericalMercator.MAX_LATITUDE).to.be.a('number'); - }, + var mapZoom = map.getZoom(); - // Extra final instruction to be called even on failure. - function () { - // Restore original setting, so that next tests are unaffected. - L.Browser.mobile = previousMobileSetting; + for (i = 0; i < markers.length; i++) { + markers[i].setLatLng(latLngs[i]); + try { + expect(markers[i].__parent._zoom).to.be.below(mapZoom); + } catch (e) { + console.log("Failed marker: " + (i + 1)); + throw e; + } } - ); + } + + it('includes objects above the Web Mercator projection maximum limit by default', function () { + + moveMarkersAndMapToMaxLat(latLngsMaxLatDefault); + + group = L.markerClusterGroup(); + + prepareGroup(); + + checkProjection(latLngsMaxLatDefault); - it2('leaves all objects on map when set to false', function () { + expect(map._panes.markerPane.childNodes.length).to.be(4); // Markers 1, 2, 3 and 4. + expect(marker5._icon).to.be(null); + + }); + + it('includes objects below the Web Mercator projection minimum limit by default', function () { + + moveMarkersAndMapToMaxLat(latLngsMaxLatDefault, true); + + // Make sure we are really in Southern hemisphere. + expect(map.getBounds().getNorth()).to.be.below(-80); + + group = L.markerClusterGroup(); + + prepareGroup(); + + checkProjection(latLngsMaxLatDefault); + + expect(map._panes.markerPane.childNodes.length).to.be(4); // Markers 1, 2, 3 and 4. + expect(marker5._icon).to.be(null); + + }); + + + // The actual map view should be '-1.0986328125,84.92929204957956,1.0986328125,85.11983467698401' + var latLngsMaxLatMobile = [ + [100, 1], // Impossible in real world, but nothing prevents the user from entering such latitude, and Web/Spherical Mercator projection will still display it at 85.0511287798 + [85.2, 0.5], // 1 "screen" heights away, but should be included by the correction. + [85, 0], // In center of view. + [84.9, -1], // 1 "screen" height away. + [84.8, -1.5] // 2 "screens" height away. + ]; + + it('includes objects above the Web Mercator projection maximum limit for mobile device', function () { + + // Fool Leaflet, make it thinks it runs on a mobile device. + L.Browser.mobile = true; + + moveMarkersAndMapToMaxLat(latLngsMaxLatMobile); group = L.markerClusterGroup({ - removeOutsideVisibleBounds: false + maxClusterRadius: 10 }); prepareGroup(); - expect(map._panes.markerPane.childNodes.length).to.be(5); // All 5 markers. + checkProjection(latLngsMaxLatMobile); + + expect(map._panes.markerPane.childNodes.length).to.be(3); // Markers 1, 2 and 3. + expect(marker4._icon).to.be(null); + expect(marker5._icon).to.be(null); + + }); + + it('includes objects below the Web Mercator projection minimum limit for mobile device', function () { + + // Fool Leaflet, make it thinks it runs on a mobile device. + L.Browser.mobile = true; + + moveMarkersAndMapToMaxLat(latLngsMaxLatMobile, true); + + // Make sure we are really in Southern hemisphere. + expect(map.getBounds().getNorth()).to.be.below(-80); + + group = L.markerClusterGroup({ + maxClusterRadius: 10 + }); + + prepareGroup(); + + checkProjection(latLngsMaxLatMobile); + + expect(map._panes.markerPane.childNodes.length).to.be(3); // Markers 1, 2 and 3. + expect(marker4._icon).to.be(null); + expect(marker5._icon).to.be(null); }); diff --git a/src/MarkerClusterGroup.js b/src/MarkerClusterGroup.js index 099b451c..744b8230 100644 --- a/src/MarkerClusterGroup.js +++ b/src/MarkerClusterGroup.js @@ -503,6 +503,8 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({ this._generateInitialClusters(); } + this._maxLat = map.options.crs.projection.MAX_LATITUDE; + for (i = 0, l = this._needsRemoving.length; i < l; i++) { layer = this._needsRemoving[i]; this._removeLayer(layer, true); @@ -542,7 +544,7 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({ this._spiderfierOnRemove(); } - + delete this._maxLat; //Clean up all the layers we added to the map this._hideCoverage(); @@ -920,10 +922,35 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({ if (!this.options.removeOutsideVisibleBounds) { return this._mapBoundsInfinite; } else if (L.Browser.mobile) { - return this._map.getBounds(); + return this._checkBoundsMaxLat(this._map.getBounds()); } - return this._map.getBounds().pad(1); // Padding expands the bounds by its own dimensions but scaled with the given factor. + return this._checkBoundsMaxLat(this._map.getBounds().pad(1)); // Padding expands the bounds by its own dimensions but scaled with the given factor. + }, + + /** + * Expands the latitude to Infinity (or -Infinity) if the input bounds reach the map projection maximum defined latitude + * (in the case of Web/Spherical Mercator, it is 85.0511287798 / see https://en.wikipedia.org/wiki/Web_Mercator#Formulas). + * Otherwise, the removeOutsideVisibleBounds option will remove markers beyond that limit, whereas the same markers without + * this option (or outside MCG) will have their position floored (ceiled) by the projection and rendered at that limit, + * making the user think that MCG "eats" them and never displays them again. + * @param bounds L.LatLngBounds + * @returns {L.LatLngBounds} + * @private + */ + _checkBoundsMaxLat: function (bounds) { + var maxLat = this._maxLat; + + if (maxLat !== undefined) { + if (bounds.getNorth() >= maxLat) { + bounds._northEast.lat = Infinity; + } + if (bounds.getSouth() <= -maxLat) { + bounds._southWest.lat = -Infinity; + } + } + + return bounds; }, //Shared animation code