diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cd180ce0..74d27afe7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add `enable`/`disable` methods and `isEnabled` property to `Interactivity` - Enable user-defined buckets in `viewportHistogram` - Add and improve examples (legends, widgets, labeling and basemaps) +- Add helpers to validate browser support (`isBrowserSupported` & `unsupportedBrowserReasons`) ### Fixed - Fix `linear` error when evaluating a date property in a feature @@ -26,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fix unwanted `Interactivity` events while using dragPan (and performance improvement) - Fix `linear` applied to cluster aggregations without explicit limits - Fix `MVT` category property returning a numeric value +- Fix `Layer.addTo` for MGL v0.52 ### Changed - Change examples to use Mapbox GL version 0.52 diff --git a/src/Layer.js b/src/Layer.js index 98e28d53a..6846e5a38 100644 --- a/src/Layer.js +++ b/src/Layer.js @@ -157,21 +157,29 @@ export default class Layer { * @api */ addTo (map, beforeLayerID) { - const STYLE_ERROR_REGEX = /Style is not done loading/; - - const addTheLayer = () => { map.addLayer(this, beforeLayerID); }; + // Manage errors, whether they are an Evented Error or a common Error try { - addTheLayer(); - // Note: map.isStyleLoaded() has been tested here without success + map.once('error', (data) => { + console.warn(data.error.message); + this._waitForMapToLoad(map, beforeLayerID); + }); + map.addLayer(this, beforeLayerID); } catch (error) { - if (!STYLE_ERROR_REGEX.test(error)) { + const STYLE_ERROR_REGEX = /Style is not done loading/; + const NO_STYLE_AT_ALL = /Cannot read property 'addLayer' of undefined/; + if (!(STYLE_ERROR_REGEX.test(error) || NO_STYLE_AT_ALL.test(error))) { throw new CartoRuntimeError(`Error adding layer to map: ${error}`); } - - map.on('load', addTheLayer); + this._waitForMapToLoad(map, beforeLayerID); } } + _waitForMapToLoad (map, beforeLayerID) { + map.on('load', () => { + map.addLayer(this, beforeLayerID); + }); + } + /** * Remove this layer from the map. It should be called after the layer is loaded. Otherwise, it will not delete the layer. * diff --git a/test/CartoMap.js b/test/CartoMap.js index 6d2bdf9de..d80d1743e 100644 --- a/test/CartoMap.js +++ b/test/CartoMap.js @@ -42,6 +42,10 @@ class CartoMap { // eslint-disable-line no-unused-vars } } + once () { + + } + addLayer (layer, beforeLayerID) { if (!this._layers.has(layer)) { this._layers.add(layer); diff --git a/test/integration/render/scenarios/regression/layer-addTo/reference.png b/test/integration/render/scenarios/regression/layer-addTo/reference.png new file mode 100644 index 000000000..4b2e08591 Binary files /dev/null and b/test/integration/render/scenarios/regression/layer-addTo/reference.png differ diff --git a/test/integration/render/scenarios/regression/layer-addTo/scenario.js b/test/integration/render/scenarios/regression/layer-addTo/scenario.js new file mode 100644 index 000000000..440bc2ba2 --- /dev/null +++ b/test/integration/render/scenarios/regression/layer-addTo/scenario.js @@ -0,0 +1,30 @@ +const map = new mapboxgl.Map({ + container: 'map', + center: [0, 0], + style: { version: 8, sources: {}, layers: [] }, + zoom: 0 +}); + +const source = new carto.source.GeoJSON(sources['points']); +const viz = new carto.Viz(); +const layer = new carto.Layer('layer', source, viz); + +layer.addTo(map, 'background'); + +// Setting map's style after trying to add the layer... +const style = { + version: 8, + sources: {}, + layers: [{ + id: 'background', + type: 'background', + paint: { + 'background-color': 'black' + } + }] +}; +map.setStyle(style); + +layer.on('loaded', () => { + window.loaded = true; +}); diff --git a/test/integration/user/test/layer-waits-map.test.js b/test/integration/user/test/layer-waits-map.test.js new file mode 100644 index 000000000..425a6e18c --- /dev/null +++ b/test/integration/user/test/layer-waits-map.test.js @@ -0,0 +1,41 @@ +import carto from '../../../../src/'; +import * as util from '../../util'; +import mapboxgl from 'mapbox-gl'; + +const featureData = { + type: 'Feature', + geometry: { + type: 'Point', + coordinates: [0, 0] + }, + properties: {} +}; + +describe('when map is not ready', () => { + let div, map; + + it('Layer should wait for it load', (done) => { + const name = 'map'; + div = util.createMapDivHolder(name); + map = new mapboxgl.Map({ + container: name, + center: [0, 0], + zoom: 0 + }); + + const source = new carto.source.GeoJSON(featureData); + const layer = new carto.Layer('layer', source, new carto.Viz()); + + const waitForMapToLoad = spyOn(layer, '_waitForMapToLoad').and.callThrough(); + layer.addTo(map, 'watername_ocean'); // << adding before setting basemap + map.setStyle(carto.basemaps.voyager); + + expect(waitForMapToLoad).toHaveBeenCalled(); + done(); + }); + + afterEach(() => { + map.remove(); + document.body.removeChild(div); + }); +}); diff --git a/test/integration/util.js b/test/integration/util.js index 122be1d8b..61085ce2f 100644 --- a/test/integration/util.js +++ b/test/integration/util.js @@ -4,6 +4,19 @@ import { projectToWebMercator, WM_2R } from '../../src/utils/util'; const mapSize = 600; export function createMap (name, size) { + const div = createMapDivHolder(name, size); + + const map = new mapboxgl.Map({ + container: name, + style: { version: 8, sources: {}, layers: [] }, + center: [0, 0], + zoom: 0 + }); + + return { div, map }; +} + +export function createMapDivHolder (name, size) { size = size || mapSize; const div = document.createElement('div'); @@ -16,14 +29,7 @@ export function createMap (name, size) { document.body.style.padding = '0'; document.body.appendChild(div); - const map = new mapboxgl.Map({ - container: name, - style: { version: 8, sources: {}, layers: [] }, - center: [0, 0], - zoom: 0 - }); - - return { div, map }; + return div; } function _mouseParamsFromCoords (coordinates) { diff --git a/test/unit/layer.test.js b/test/unit/layer.test.js index d1efaa274..a1bcea12e 100644 --- a/test/unit/layer.test.js +++ b/test/unit/layer.test.js @@ -146,7 +146,8 @@ describe('api/layer', () => { it('should call onMapLoaded when the map is loaded', () => { const layer = new Layer('layer0', source, viz); const mapMock = { - addLayer: jasmine.createSpy('addLayer') + addLayer: jasmine.createSpy('addLayer'), + once: () => { } }; layer.addTo(mapMock, 'beforeLayer'); expect(mapMock.addLayer).toHaveBeenCalledWith(layer, 'beforeLayer'); @@ -159,7 +160,8 @@ describe('api/layer', () => { layer.onAdd = (map) => { layer.map = map; }; const mapMock = { addLayer: (layer) => { layer.onAdd(mapMock); }, - removeLayer: jasmine.createSpy('removeLayer') + removeLayer: jasmine.createSpy('removeLayer'), + once: () => { } }; layer.addTo(mapMock); layer.remove();