Skip to content

Commit 41d167f

Browse files
committed
adapt find-mapbox-access-token for maps that don't use Mapbox styles
- that is if all mapbox subplots on graph do not use Mapbox styles, DO NOT error when mapbox access token isn't set - moreover, add helpful log when multiple distinct tokens are set in same layout
1 parent 889abb6 commit 41d167f

File tree

3 files changed

+141
-6
lines changed

3 files changed

+141
-6
lines changed

Diff for: src/plots/mapbox/constants.js

+6
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ module.exports = {
3131
'More info here: https://www.mapbox.com/help/define-access-token/'
3232
].join('\n'),
3333

34+
multipleTokensErrorMsg: [
35+
'Set multiple mapbox access token across different mapbox subplot,',
36+
'using first token found as mapbox-gl does not allow multiple' +
37+
'access tokens on the same page.'
38+
].join('\n'),
39+
3440
mapOnErrorMsg: 'Mapbox error.',
3541

3642
// a subset of node_modules/mapbox-gl/dist/mapbox-gl.css

Diff for: src/plots/mapbox/index.js

+38-4
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ exports.attributes = {
4646
}
4747
};
4848

49-
exports.layoutAttributes = require('./layout_attributes');
49+
var layoutAttrs = exports.layoutAttributes = require('./layout_attributes');
5050

5151
exports.supplyLayoutDefaults = require('./layout_defaults');
5252

@@ -132,24 +132,58 @@ exports.toSVG = function(gd) {
132132
}
133133
};
134134

135+
// N.B. mapbox-gl only allows one accessToken to be set per page:
136+
// https://github.com/mapbox/mapbox-gl-js/issues/6331
135137
function findAccessToken(gd, mapboxIds) {
136138
var fullLayout = gd._fullLayout;
137139
var context = gd._context;
138140

139141
// special case for Mapbox Atlas users
140142
if(context.mapboxAccessToken === '') return '';
141143

144+
var tokensUseful = [];
145+
var tokensListed = [];
146+
var wontWork = false;
147+
142148
// Take the first token we find in a mapbox subplot.
143149
// These default to the context value but may be overridden.
144150
for(var i = 0; i < mapboxIds.length; i++) {
145151
var opts = fullLayout[mapboxIds[i]];
152+
var style = opts.style;
153+
var token = opts.accesstoken;
154+
155+
if(typeof style === 'string' && layoutAttrs.style.values.indexOf(style) !== -1) {
156+
if(token) {
157+
Lib.pushUnique(tokensUseful, token);
158+
} else {
159+
Lib.error('Uses Mapbox map style, but did not set an access token.');
160+
wontWork = true;
161+
}
162+
}
146163

147-
if(opts.accesstoken) {
148-
return opts.accesstoken;
164+
if(token) {
165+
Lib.pushUnique(tokensListed, token);
149166
}
150167
}
151168

152-
throw new Error(constants.noAccessTokenErrorMsg);
169+
if(wontWork) {
170+
throw new Error(constants.noAccessTokenErrorMsg);
171+
}
172+
173+
if(tokensUseful.length) {
174+
if(tokensUseful.length > 1) {
175+
Lib.warn(constants.multipleTokensErrorMsg);
176+
}
177+
return tokensUseful[0];
178+
} else {
179+
if(tokensListed.length) {
180+
Lib.log([
181+
'Listed mapbox access token(s)', tokensListed.join(','),
182+
'but did not use a Mapbox map style, ignoring token(s).'
183+
].join(' '));
184+
}
185+
return '';
186+
}
153187
}
154188

155189
exports.updateFx = function(gd) {

Diff for: test/jasmine/tests/mapbox_test.js

+97-2
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,31 @@ describe('mapbox defaults', function() {
196196
});
197197

198198
describe('mapbox credentials', function() {
199-
'use strict';
199+
var gd;
200200

201201
var dummyToken = 'asfdsa124331wersdsa1321q3';
202-
var gd;
202+
203+
var osmStyle = {
204+
id: 'osm',
205+
version: 8,
206+
sources: {
207+
'osm-tiles': {
208+
type: 'raster',
209+
tiles: [
210+
'https://a.tile.openstreetmap.org/{z}/{x}/{y}.png',
211+
'https://b.tile.openstreetmap.org/{z}/{x}/{y}.png'
212+
],
213+
tileSize: 256
214+
}
215+
},
216+
layers: [{
217+
id: 'osm-tiles',
218+
type: 'raster',
219+
source: 'osm-tiles',
220+
minzoom: 0,
221+
maxzoom: 22
222+
}]
223+
};
203224

204225
beforeEach(function() {
205226
gd = createGraphDiv();
@@ -219,13 +240,17 @@ describe('mapbox credentials', function() {
219240
});
220241

221242
it('should throw error if token is not registered', function() {
243+
spyOn(Lib, 'error');
244+
222245
expect(function() {
223246
Plotly.plot(gd, [{
224247
type: 'scattermapbox',
225248
lon: [10, 20, 30],
226249
lat: [10, 20, 30]
227250
}]);
228251
}).toThrow(new Error(constants.noAccessTokenErrorMsg));
252+
253+
expect(Lib.error).toHaveBeenCalledWith('Uses Mapbox map style, but did not set an access token.');
229254
}, LONG_TIMEOUT_INTERVAL);
230255

231256
it('should throw error if token is invalid', function(done) {
@@ -270,6 +295,76 @@ describe('mapbox credentials', function() {
270295
});
271296
}, LONG_TIMEOUT_INTERVAL);
272297

298+
it('should warn when multiple tokens in mapbox layout options are present', function(done) {
299+
spyOn(Lib, 'warn');
300+
var cnt = 0;
301+
302+
Plotly.plot(gd, [{
303+
type: 'scattermapbox',
304+
lon: [10, 20, 30],
305+
lat: [10, 20, 30]
306+
}, {
307+
type: 'scattermapbox',
308+
lon: [10, 20, 30],
309+
lat: [10, 20, 30],
310+
subplot: 'mapbox2'
311+
}], {
312+
mapbox: { accesstoken: MAPBOX_ACCESS_TOKEN },
313+
mapbox2: { accesstoken: dummyToken }
314+
}).catch(function() {
315+
cnt++;
316+
}).then(function() {
317+
expect(cnt).toEqual(0);
318+
expect(gd._fullLayout.mapbox.accesstoken).toEqual(MAPBOX_ACCESS_TOKEN);
319+
expect(Lib.warn).toHaveBeenCalledWith(constants.multipleTokensErrorMsg);
320+
done();
321+
});
322+
}, LONG_TIMEOUT_INTERVAL);
323+
324+
it('should not throw when using a custom non-mapbox style', function(done) {
325+
var cnt = 0;
326+
327+
Plotly.plot(gd, [{
328+
type: 'scattermapbox',
329+
lon: [10, 20, 30],
330+
lat: [10, 20, 30]
331+
}], {
332+
mapbox: { style: osmStyle }
333+
}).catch(function() {
334+
cnt++;
335+
}).then(function() {
336+
expect(cnt).toEqual(0);
337+
expect(gd._fullLayout.mapbox.accesstoken).toBe(undefined);
338+
done();
339+
});
340+
}, LONG_TIMEOUT_INTERVAL);
341+
342+
it('should log when an access token is set while using a custom non-mapbox style', function(done) {
343+
spyOn(Lib, 'log');
344+
var cnt = 0;
345+
346+
Plotly.plot(gd, [{
347+
type: 'scattermapbox',
348+
lon: [10, 20, 30],
349+
lat: [10, 20, 30]
350+
}], {
351+
mapbox: {
352+
style: osmStyle,
353+
accesstoken: MAPBOX_ACCESS_TOKEN
354+
}
355+
}).catch(function() {
356+
cnt++;
357+
}).then(function() {
358+
expect(cnt).toEqual(0);
359+
expect(Lib.log).toHaveBeenCalledWith([
360+
'Listed mapbox access token(s)',
361+
MAPBOX_ACCESS_TOKEN,
362+
'but did not use a Mapbox map style, ignoring token(s).'
363+
].join(' '));
364+
done();
365+
});
366+
}, LONG_TIMEOUT_INTERVAL);
367+
273368
it('should bypass access token in mapbox layout options when config points to an Atlas server', function(done) {
274369
var cnt = 0;
275370
var msg = [

0 commit comments

Comments
 (0)