From f0f0247daccfdf95783453b432c0863667434c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 7 Jul 2016 17:29:03 -0400 Subject: [PATCH 1/5] plot: clear promise queue if one of them got rejected --- src/plot_api/plot_api.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 8b3cd559c4b..b6dc3146faa 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -326,6 +326,9 @@ Plotly.plot = function(gd, data, layout, config) { // so that the caller doesn't care which route we took return Promise.all(gd._promises).then(function() { return gd; + }, function() { + // clear the promise queue if one of them got rejected + gd._promises = []; }); }; From 85a8e7dbdf1a7d35b2d2fbfb59df78906f06fd5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 7 Jul 2016 18:17:52 -0400 Subject: [PATCH 2/5] lint: move MOUSE_DELAY to suite scope --- test/jasmine/tests/mapbox_test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 6cde3b89f5e..6aa3b093db2 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -13,6 +13,7 @@ var customMatchers = require('../assets/custom_matchers'); var MAPBOX_ACCESS_TOKEN = require('@build/credentials.json').MAPBOX_ACCESS_TOKEN; var TRANSITION_DELAY = 500; +var MOUSE_DELAY = 100; var noop = function() {}; @@ -763,15 +764,13 @@ describe('mapbox plots', function() { } function _mouseEvent(type, pos, cb) { - var DELAY = 100; - return new Promise(function(resolve) { mouseEvent(type, pos[0], pos[1]); setTimeout(function() { cb(); resolve(); - }, DELAY); + }, MOUSE_DELAY); }); } From 3f6e647e17846eab97d1b42760190b9bacd4999a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 7 Jul 2016 18:36:24 -0400 Subject: [PATCH 3/5] mapbox: allow access token to be set per map - use token in gd._context if not present in fullLayout - make sure map is removed + re-created if access token changes - --- src/plots/mapbox/index.js | 41 +++++++++++++++++++++------ src/plots/mapbox/layout_attributes.js | 11 +++++++ src/plots/mapbox/layout_defaults.js | 1 + src/plots/mapbox/mapbox.js | 17 +++++++++-- test/jasmine/tests/mapbox_test.js | 32 +++++++++++++++++++++ 5 files changed, 91 insertions(+), 11 deletions(-) diff --git a/src/plots/mapbox/index.js b/src/plots/mapbox/index.js index 5faf2f5c3d5..e7ab347d07b 100644 --- a/src/plots/mapbox/index.js +++ b/src/plots/mapbox/index.js @@ -47,22 +47,21 @@ exports.layoutAttributes = require('./layout_attributes'); exports.supplyLayoutDefaults = require('./layout_defaults'); exports.plot = function plotMapbox(gd) { - - if(!gd._context.mapboxAccessToken) { - throw new Error(constants.noAccessTokenErrorMsg); - } - else { - mapboxgl.accessToken = gd._context.mapboxAccessToken; - } - var fullLayout = gd._fullLayout, calcData = gd.calcdata, mapboxIds = Plots.getSubplotIds(fullLayout, 'mapbox'); + var accessToken = findAccessToken(gd, mapboxIds); + mapboxgl.accessToken = accessToken; + for(var i = 0; i < mapboxIds.length; i++) { var id = mapboxIds[i], subplotCalcData = getSubplotCalcData(calcData, id), - mapbox = fullLayout[id]._subplot; + opts = fullLayout[id], + mapbox = opts._subplot; + + // copy access token to fullLayout (to handle the context case) + opts.accesstoken = accessToken; if(!mapbox) { mapbox = createMapbox({ @@ -131,3 +130,27 @@ function getSubplotCalcData(calcData, id) { return subplotCalcData; } + +function findAccessToken(gd, mapboxIds) { + var fullLayout = gd._fullLayout, + context = gd._context; + + // first look for access token in context + var accessToken = context.mapboxAccessToken; + + // allow mapbox layout options to override it + for(var i = 0; i < mapboxIds.length; i++) { + var opts = fullLayout[mapboxIds[i]]; + + if(opts.accesstoken) { + accessToken = opts.accesstoken; + break; + } + } + + if(!accessToken) { + throw new Error(constants.noAccessTokenErrorMsg); + } + + return accessToken; +} diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 20c23b19e64..74663d3e51f 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -45,6 +45,16 @@ module.exports = { } }, + accesstoken: { + valType: 'string', + noBlank: true, + strict: true, + role: 'info', + description: [ + 'Sets the mapbox access token to be set or as a configuration option', + 'under `mapboxAccessToken`.' + ].join(' ') + }, style: { valType: 'string', values: ['basic', 'streets', 'outdoors', 'light', 'dark', 'satellite', 'satellite-streets'], @@ -55,6 +65,7 @@ module.exports = { 'Either input the defaults Mapbox names or the URL to a custom style.' ].join(' ') }, + center: { lon: { valType: 'number', diff --git a/src/plots/mapbox/layout_defaults.js b/src/plots/mapbox/layout_defaults.js index 92eac67f355..59c91057f31 100644 --- a/src/plots/mapbox/layout_defaults.js +++ b/src/plots/mapbox/layout_defaults.js @@ -25,6 +25,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { }; function handleDefaults(containerIn, containerOut, coerce) { + coerce('accesstoken'); coerce('style'); coerce('center.lon'); coerce('center.lat'); diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 89b966a53bc..6d53104d8f4 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -40,6 +40,7 @@ function Mapbox(opts) { // state variables used to infer how and what to update this.map = null; + this.accessToken = null; this.styleUrl = null; this.traceHash = {}; this.layerList = []; @@ -57,7 +58,16 @@ proto.plot = function(calcData, fullLayout, promises) { var self = this; // feed in new mapbox options - self.opts = fullLayout[this.id]; + var opts = self.opts = fullLayout[this.id]; + + // remove map and create a new map if access token has change + if(self.map && (opts.accesstoken !== self.accessToken)) { + self.map.remove(); + self.map = null; + self.styleUrl = null; + self.traceHash = []; + self.layerList = {}; + } var promise; @@ -83,6 +93,9 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { // mapbox doesn't have a way to get the current style URL; do it ourselves var styleUrl = self.styleUrl = convertStyleUrl(opts.style); + // store access token associated with this map + self.accessToken = opts.accesstoken; + var map = self.map = new mapboxgl.Map({ container: self.div, @@ -334,7 +347,7 @@ proto.updateLayers = function() { }; proto.destroy = function() { - this.map.remove(); + if(this.map) this.map.remove(); this.container.removeChild(this.div); }; diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 6aa3b093db2..f8acf99dd69 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -188,6 +188,22 @@ describe('mapbox credentials', function() { mapboxAccessToken: dummyToken }).catch(function(err) { expect(err).toEqual(new Error(constants.mapOnErrorMsg)); + }).then(done); + }); + + it('should use access token in mapbox layout options if present', function(done) { + Plotly.plot(gd, [{ + type: 'scattermapbox', + lon: [10, 20, 30], + lat: [10, 20, 30] + }], { + mapbox: { + accesstoken: MAPBOX_ACCESS_TOKEN + } + }, { + mapboxAccessToken: dummyToken + }).then(function() { + expect(gd._fullLayout.mapbox.accesstoken).toEqual(MAPBOX_ACCESS_TOKEN); done(); }); }); @@ -476,6 +492,22 @@ describe('mapbox plots', function() { }); }); + it('should be able to update the access token', function(done) { + var promise = Plotly.relayout(gd, 'mapbox.accesstoken', 'wont-work'); + + promise.catch(function(err) { + expect(gd._fullLayout.mapbox.accesstoken).toEqual('wont-work'); + expect(err).toEqual(new Error(constants.mapOnErrorMsg)); + }); + + promise.then(function() { + return Plotly.relayout(gd, 'mapbox.accesstoken', MAPBOX_ACCESS_TOKEN); + }).then(function() { + expect(gd._fullLayout.mapbox.accesstoken).toEqual(MAPBOX_ACCESS_TOKEN); + }).then(done); + }); + + it('should be able to update traces', function(done) { function assertDataPts(lengths) { var lines = getGeoJsonData(gd, 'lines'), From 8a9f8aa63e6a5d825d9c633fe6df46c51ecb361c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 21 Jul 2016 13:50:19 -0400 Subject: [PATCH 4/5] log when clearing rejected promises from queue. --- src/plot_api/plot_api.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index b6dc3146faa..cb1f5bd7549 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -328,6 +328,7 @@ Plotly.plot = function(gd, data, layout, config) { return gd; }, function() { // clear the promise queue if one of them got rejected + Lib.log('Clearing previous rejected promises from queue.'); gd._promises = []; }); }; From faf79b5b01b98ed1e1f348c07eff4ebc48907708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 21 Jul 2016 13:50:48 -0400 Subject: [PATCH 5/5] doc: improve 'mapbox.accesstoken' description --- src/plots/mapbox/layout_attributes.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 74663d3e51f..fdf9d97fe07 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -51,8 +51,9 @@ module.exports = { strict: true, role: 'info', description: [ - 'Sets the mapbox access token to be set or as a configuration option', - 'under `mapboxAccessToken`.' + 'Sets the mapbox access token to be used for this mapbox map.', + 'Alternatively, the mapbox access token can be set in the', + 'configuration options under `mapboxAccessToken`.' ].join(' ') }, style: {