resolves #4059 - throw error msg about "missing style"

etpinard · etpinard · commit a460be044727 · 2019-07-22T15:18:54.000-04:00
... when trying to plot a mapbox subplot w/o a Mapbox access token and
    w/o setting a Mapbox-served style as opposed to simply
    throwing a "missing token" msg.

    Also, rename constants.styles -&gt; constants.stylesNonMapbox
diff --git a/src/plots/mapbox/constants.js b/src/plots/mapbox/constants.js
@@ -10,147 +10,148 @@
 
 var requiredVersion = '1.1.1';
 
-module.exports = {
-    requiredVersion: requiredVersion,
-
-    styleUrlPrefix: 'mapbox://styles/mapbox/',
-    styleUrlSuffix: 'v9',
-
-    styleValuesMapbox: ['basic', 'streets', 'outdoors', 'light', 'dark', 'satellite', 'satellite-streets'],
-    styleValueOSM: 'open-street-map',
-    styleValueDflt: 'basic',
-
-    styles: {
-        'open-street-map': {
-            id: 'osm',
-            version: 8,
-            sources: {
-                'plotly-osm-tiles': {
-                    type: 'raster',
-                    attribution: '<a href="http://www.openstreetmap.org/about/" target="_blank">© OpenStreetMap</a>',
-                    tiles: [
-                        'https://a.tile.openstreetmap.org/{z}/{x}/{y}.png',
-                        'https://b.tile.openstreetmap.org/{z}/{x}/{y}.png'
-                    ],
-                    tileSize: 256
-                }
-            },
-            layers: [{
-                id: 'plotly-osm-tiles',
+var stylesNonMapbox = {
+    'open-street-map': {
+        id: 'osm',
+        version: 8,
+        sources: {
+            'plotly-osm-tiles': {
                 type: 'raster',
-                source: 'plotly-osm-tiles',
-                minzoom: 0,
-                maxzoom: 22
-            }]
+                attribution: '<a href="http://www.openstreetmap.org/about/" target="_blank">© OpenStreetMap</a>',
+                tiles: [
+                    'https://a.tile.openstreetmap.org/{z}/{x}/{y}.png',
+                    'https://b.tile.openstreetmap.org/{z}/{x}/{y}.png'
+                ],
+                tileSize: 256
+            }
         },
-        'white-bg': {
+        layers: [{
+            id: 'plotly-osm-tiles',
+            type: 'raster',
+            source: 'plotly-osm-tiles',
+            minzoom: 0,
+            maxzoom: 22
+        }]
+    },
+    'white-bg': {
+        id: 'white-bg',
+        version: 8,
+        sources: {},
+        layers: [{
             id: 'white-bg',
-            version: 8,
-            sources: {},
-            layers: [{
-                id: 'white-bg',
-                type: 'background',
-                paint: {'background-color': '#FFFFFF'},
-                minzoom: 0,
-                maxzoom: 22
-            }]
-        },
-        'carto-positron': {
-            id: 'carto-positron',
-            version: 8,
-            sources: {
-                'plotly-carto-positron': {
-                    type: 'raster',
-                    attribution: '<a href="https://carto.com/" target="_blank">© CARTO</a>',
-                    tiles: ['https://cartodb-basemaps-c.global.ssl.fastly.net/light_all/{z}/{x}/{y}.png'],
-                    tileSize: 256
-                }
-            },
-            layers: [{
-                id: 'plotly-carto-positron',
+            type: 'background',
+            paint: {'background-color': '#FFFFFF'},
+            minzoom: 0,
+            maxzoom: 22
+        }]
+    },
+    'carto-positron': {
+        id: 'carto-positron',
+        version: 8,
+        sources: {
+            'plotly-carto-positron': {
                 type: 'raster',
-                source: 'plotly-carto-positron',
-                minzoom: 0,
-                maxzoom: 22
-            }]
+                attribution: '<a href="https://carto.com/" target="_blank">© CARTO</a>',
+                tiles: ['https://cartodb-basemaps-c.global.ssl.fastly.net/light_all/{z}/{x}/{y}.png'],
+                tileSize: 256
+            }
         },
-        'carto-darkmatter': {
-            id: 'carto-darkmatter',
-            version: 8,
-            sources: {
-                'plotly-carto-darkmatter': {
-                    type: 'raster',
-                    attribution: '<a href="https://carto.com/" target="_blank">© CARTO</a>',
-                    tiles: ['https://cartodb-basemaps-c.global.ssl.fastly.net/dark_all/{z}/{x}/{y}.png'],
-                    tileSize: 256
-                }
-            },
-            layers: [{
-                id: 'plotly-carto-darkmatter',
+        layers: [{
+            id: 'plotly-carto-positron',
+            type: 'raster',
+            source: 'plotly-carto-positron',
+            minzoom: 0,
+            maxzoom: 22
+        }]
+    },
+    'carto-darkmatter': {
+        id: 'carto-darkmatter',
+        version: 8,
+        sources: {
+            'plotly-carto-darkmatter': {
                 type: 'raster',
-                source: 'plotly-carto-darkmatter',
-                minzoom: 0,
-                maxzoom: 22
-            }]
+                attribution: '<a href="https://carto.com/" target="_blank">© CARTO</a>',
+                tiles: ['https://cartodb-basemaps-c.global.ssl.fastly.net/dark_all/{z}/{x}/{y}.png'],
+                tileSize: 256
+            }
         },
-        'stamen-terrain': {
-            id: 'stamen-terrain',
-            version: 8,
-            sources: {
-                'plotly-stamen-terrain': {
-                    type: 'raster',
-                    attribution: 'Map tiles by <a href="http://stamen.com">Stamen Design</a>, under <a href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a> | Data by <a href="http://openstreetmap.org">OpenStreetMap</a>, under <a href="http://www.openstreetmap.org/copyright">ODbL</a>.',
-                    tiles: ['https://stamen-tiles.a.ssl.fastly.net/terrain/{z}/{x}/{y}.png'],
-                    tileSize: 256
-                }
-            },
-            layers: [{
-                id: 'plotly-stamen-terrain',
+        layers: [{
+            id: 'plotly-carto-darkmatter',
+            type: 'raster',
+            source: 'plotly-carto-darkmatter',
+            minzoom: 0,
+            maxzoom: 22
+        }]
+    },
+    'stamen-terrain': {
+        id: 'stamen-terrain',
+        version: 8,
+        sources: {
+            'plotly-stamen-terrain': {
                 type: 'raster',
-                source: 'plotly-stamen-terrain',
-                minzoom: 0,
-                maxzoom: 22
-            }]
+                attribution: 'Map tiles by <a href="http://stamen.com">Stamen Design</a>, under <a href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a> | Data by <a href="http://openstreetmap.org">OpenStreetMap</a>, under <a href="http://www.openstreetmap.org/copyright">ODbL</a>.',
+                tiles: ['https://stamen-tiles.a.ssl.fastly.net/terrain/{z}/{x}/{y}.png'],
+                tileSize: 256
+            }
         },
-        'stamen-toner': {
-            id: 'stamen-toner',
-            version: 8,
-            sources: {
-                'plotly-stamen-toner': {
-                    type: 'raster',
-                    attribution: 'Map tiles by <a href="http://stamen.com">Stamen Design</a>, under <a href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a> | Data by <a href="http://openstreetmap.org">OpenStreetMap</a>, under <a href="http://www.openstreetmap.org/copyright">ODbL</a>.',
-                    tiles: ['https://stamen-tiles.a.ssl.fastly.net/toner/{z}/{x}/{y}.png'],
-                    tileSize: 256
-                }
-            },
-            layers: [{
-                id: 'plotly-stamen-toner',
+        layers: [{
+            id: 'plotly-stamen-terrain',
+            type: 'raster',
+            source: 'plotly-stamen-terrain',
+            minzoom: 0,
+            maxzoom: 22
+        }]
+    },
+    'stamen-toner': {
+        id: 'stamen-toner',
+        version: 8,
+        sources: {
+            'plotly-stamen-toner': {
                 type: 'raster',
-                source: 'plotly-stamen-toner',
-                minzoom: 0,
-                maxzoom: 22
-            }]
+                attribution: 'Map tiles by <a href="http://stamen.com">Stamen Design</a>, under <a href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a> | Data by <a href="http://openstreetmap.org">OpenStreetMap</a>, under <a href="http://www.openstreetmap.org/copyright">ODbL</a>.',
+                tiles: ['https://stamen-tiles.a.ssl.fastly.net/toner/{z}/{x}/{y}.png'],
+                tileSize: 256
+            }
         },
-        'stamen-watercolor': {
-            id: 'stamen-watercolor',
-            version: 8,
-            sources: {
-                'plotly-stamen-watercolor': {
-                    type: 'raster',
-                    attribution: 'Map tiles by <a href="http://stamen.com">Stamen Design</a>, under <a href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a> | Data by <a href="http://openstreetmap.org">OpenStreetMap</a>, under <a href="http://creativecommons.org/licenses/by-sa/3.0">CC BY SA</a>.',
-                    tiles: ['https://stamen-tiles.a.ssl.fastly.net/watercolor/{z}/{x}/{y}.png'],
-                    tileSize: 256
-                }
-            },
-            layers: [{
-                id: 'plotly-stamen-watercolor',
-                type: 'raster',
-                source: 'plotly-stamen-watercolor',
-                minzoom: 0,
-                maxzoom: 22
-            }]
-        }
+        layers: [{
+            id: 'plotly-stamen-toner',
+            type: 'raster',
+            source: 'plotly-stamen-toner',
+            minzoom: 0,
+            maxzoom: 22
+        }]
     },
+    'stamen-watercolor': {
+        id: 'stamen-watercolor',
+        version: 8,
+        sources: {
+            'plotly-stamen-watercolor': {
+                type: 'raster',
+                attribution: 'Map tiles by <a href="http://stamen.com">Stamen Design</a>, under <a href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a> | Data by <a href="http://openstreetmap.org">OpenStreetMap</a>, under <a href="http://creativecommons.org/licenses/by-sa/3.0">CC BY SA</a>.',
+                tiles: ['https://stamen-tiles.a.ssl.fastly.net/watercolor/{z}/{x}/{y}.png'],
+                tileSize: 256
+            }
+        },
+        layers: [{
+            id: 'plotly-stamen-watercolor',
+            type: 'raster',
+            source: 'plotly-stamen-watercolor',
+            minzoom: 0,
+            maxzoom: 22
+        }]
+    }
+};
+
+module.exports = {
+    requiredVersion: requiredVersion,
+
+    styleUrlPrefix: 'mapbox://styles/mapbox/',
+    styleUrlSuffix: 'v9',
+
+    styleValuesMapbox: ['basic', 'streets', 'outdoors', 'light', 'dark', 'satellite', 'satellite-streets'],
+    styleValueOSM: 'open-street-map',
+    styleValueDflt: 'basic',
+    stylesNonMapbox: stylesNonMapbox,
 
     traceLayerPrefix: 'plotly-trace-layer-',
     layoutLayerPrefix: 'plotly-layout-layer-',
@@ -168,6 +169,12 @@ module.exports = {
         'More info here: https://www.mapbox.com/help/define-access-token/'
     ].join('\n'),
 
+    missingStyleErrorMsg: [
+        'No valid mapbox style found, please set `mapbox.style` to one of:',
+        Object.keys(stylesNonMapbox).join(', '),
+        'or register a Mapbox access token to use a Mapbox-served style.'
+    ].join('\n'),
+
     multipleTokensErrorMsg: [
         'Set multiple mapbox access token across different mapbox subplot,',
         'using first token found as mapbox-gl does not allow multiple' +
diff --git a/src/plots/mapbox/index.js b/src/plots/mapbox/index.js
@@ -220,20 +220,23 @@ function findAccessToken(gd, mapboxIds) {
 
     var tokensUseful = [];
     var tokensListed = [];
+    var hasOneSetMapboxStyle = false;
     var wontWork = false;
 
     // Take the first token we find in a mapbox subplot.
     // These default to the context value but may be overridden.
     for(var i = 0; i < mapboxIds.length; i++) {
         var opts = fullLayout[mapboxIds[i]];
-        var style = opts.style;
         var token = opts.accesstoken;
 
-        if(typeof style === 'string' && constants.styleValuesMapbox.indexOf(style) !== -1) {
+        if(isMapboxStyle(opts.style)) {
             if(token) {
                 Lib.pushUnique(tokensUseful, token);
             } else {
-                Lib.error('Uses Mapbox map style, but did not set an access token.');
+                if(isMapboxStyle(opts._input.style)) {
+                    Lib.error('Uses Mapbox map style, but did not set an access token.');
+                    hasOneSetMapboxStyle = true;
+                }
                 wontWork = true;
             }
         }
@@ -244,7 +247,10 @@ function findAccessToken(gd, mapboxIds) {
     }
 
     if(wontWork) {
-        throw new Error(constants.noAccessTokenErrorMsg);
+        var msg = hasOneSetMapboxStyle ?
+            constants.noAccessTokenErrorMsg :
+            constants.missingStyleErrorMsg;
+        throw new Error(msg);
     }
 
     if(tokensUseful.length) {
@@ -263,6 +269,10 @@ function findAccessToken(gd, mapboxIds) {
     }
 }
 
+function isMapboxStyle(s) {
+    return typeof s === 'string' && constants.styleValuesMapbox.indexOf(s) !== -1;
+}
+
 exports.updateFx = function(gd) {
     var fullLayout = gd._fullLayout;
     var subplotIds = fullLayout._subplots[MAPBOX];
diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js
@@ -769,8 +769,8 @@ function getStyleObj(val) {
 
         if(constants.styleValuesMapbox.indexOf(val) !== -1) {
             styleObj.style = convertStyleVal(val);
-        } else if(constants.styles[val]) {
-            styleObj.style = constants.styles[val];
+        } else if(constants.stylesNonMapbox[val]) {
+            styleObj.style = constants.stylesNonMapbox[val];
         } else {
             styleObj.style = val;
         }
diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js
@@ -283,7 +283,7 @@ describe('mapbox credentials', function() {
         });
     });
 
-    it('@gl should throw error if token is not registered', function() {
+    it('@gl should throw error when no non-mapbox style is set and missing a mapbox access token token', function() {
         spyOn(Lib, 'error');
 
         expect(function() {
@@ -292,6 +292,22 @@ describe('mapbox credentials', function() {
                 lon: [10, 20, 30],
                 lat: [10, 20, 30]
             }]);
+        }).toThrow(new Error(constants.missingStyleErrorMsg));
+
+        expect(Lib.error).toHaveBeenCalledTimes(0);
+    }, LONG_TIMEOUT_INTERVAL);
+
+    it('@gl should throw error when setting a Mapbox style w/o a registered token', function() {
+        spyOn(Lib, 'error');
+
+        expect(function() {
+            Plotly.plot(gd, [{
+                type: 'scattermapbox',
+                lon: [10, 20, 30],
+                lat: [10, 20, 30]
+            }], {
+                mapbox: {style: 'basic'}
+            });
         }).toThrow(new Error(constants.noAccessTokenErrorMsg));
 
         expect(Lib.error).toHaveBeenCalledWith('Uses Mapbox map style, but did not set an access token.');
