From b2f6b22220e49959528a76e682f2b80e89b87756 Mon Sep 17 00:00:00 2001 From: etpinard Date: Mon, 30 Nov 2015 18:03:20 -0500 Subject: [PATCH 1/7] wrap d3.json requet in geo.plot in promise, - so that Plotly.plot().then() works as expected. --- src/plot_api/plot_api.js | 2 +- src/plots/geo/geo.js | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index c59f677619d..5cb12ade50d 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -484,7 +484,7 @@ function plotGeo(gd) { fullLayout[geoId]._geo = geo; } - geo.plot(fullGeoData, fullLayout); + geo.plot(fullGeoData, fullLayout, gd._promises); } } diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index 9d893276e5c..4cca9c12a6b 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -60,7 +60,7 @@ module.exports = Geo; var proto = Geo.prototype; -proto.plot = function(geoData, fullLayout) { +proto.plot = function(geoData, fullLayout, promises) { var _this = this, geoLayout = fullLayout[_this.id], graphSize = fullLayout._size; @@ -100,12 +100,16 @@ proto.plot = function(geoData, fullLayout) { _this.topojsonName ); - // N.B this is async - d3.json(topojsonPath, function(error, topojson) { - _this.topojson = topojson; - PlotlyGeoAssets.topojson[_this.topojsonName] = topojson; - _this.onceTopojsonIsLoaded(geoData, geoLayout); - }); + promises.push(new Promise(function(resolve) { + d3.json(topojsonPath, function(error, topojson) { + + _this.topojson = topojson; + PlotlyGeoAssets.topojson[_this.topojsonName] = topojson; + + _this.onceTopojsonIsLoaded(geoData, geoLayout); + resolve(); + }); + })); } } else _this.onceTopojsonIsLoaded(geoData, geoLayout); From e84932773b16b739a4bc860a1384b62034764ee7 Mon Sep 17 00:00:00 2001 From: etpinard Date: Mon, 30 Nov 2015 18:04:29 -0500 Subject: [PATCH 2/7] make Plotly.newPlot return a promise: - so that Plotly.newPlot().then() works. --- src/plot_api/plot_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 5cb12ade50d..f6be56b09da 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -940,7 +940,7 @@ Plotly.redraw = function(gd) { Plotly.newPlot = function (gd, data, layout, config) { gd = getGraphDiv(gd); plots.purge(gd); - Plotly.plot(gd, data, layout, config); + return Plotly.plot(gd, data, layout, config); }; function doCalcdata(gd) { From cf95a0631555838b85bd8d311f168924650fa8d8 Mon Sep 17 00:00:00 2001 From: etpinard Date: Mon, 30 Nov 2015 18:07:10 -0500 Subject: [PATCH 3/7] lint --- src/plot_api/plot_api.js | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index f6be56b09da..da47a6eb41b 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -178,7 +178,7 @@ Plotly.plot = function(gd, data, layout, config) { return plots.previousPromises(gd); } - function marginPushersAgain(){ + function marginPushersAgain() { // in case the margins changed, draw margin pushers again var seq = JSON.stringify(fullLayout._size)===oldmargins ? [] : [marginPushers, layoutStyles]; @@ -335,7 +335,7 @@ Plotly.plot = function(gd, data, layout, config) { return plots.previousPromises(gd); } - function cleanUp(){ + function cleanUp() { // now we're REALLY TRULY done plotting... // so mark it as done and let other procedures call a replot gd._replotting = false; @@ -570,7 +570,7 @@ function plotPolar(gd, data, layout) { if(txt === '' || !txt) opacity = 0; var placeholderText = 'Click to enter title'; - var titleLayout = function(){ + var titleLayout = function() { this.call(Plotly.util.convertToTspans); //TODO: html/mathjax //TODO: center title @@ -586,17 +586,17 @@ function plotPolar(gd, data, layout) { title.attr({'data-unformatted': placeholderText}) .text(placeholderText) .style({opacity: opacity}) - .on('mouseover.opacity',function(){ + .on('mouseover.opacity',function() { d3.select(this).transition().duration(100) .style('opacity',1); }) - .on('mouseout.opacity',function(){ + .on('mouseout.opacity',function() { d3.select(this).transition().duration(1000) .style('opacity',0); }); } - var setContenteditable = function(){ + var setContenteditable = function() { this.call(Plotly.util.makeEditable) .on('edit', function(text){ gd.framework({layout: {title: text}}); @@ -605,7 +605,7 @@ function plotPolar(gd, data, layout) { .call(titleLayout); this.call(setContenteditable); }) - .on('cancel', function(){ + .on('cancel', function() { var txt = this.attr('data-unformatted'); this.text(txt).call(titleLayout); }); @@ -1747,7 +1747,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { } // make a new empty vals array for undoit - function a0(){ return traces.map(function(){ return undefined; }); } + function a0() { return traces.map(function() { return undefined; }); } // for autoranging multiple axes function addToAxlist(axid) { @@ -1764,11 +1764,11 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { // attr can be an array to set several at once (all to the same val) function doextra(attr,val,i) { if(Array.isArray(attr)) { - attr.forEach(function(a){ doextra(a,val,i); }); + attr.forEach(function(a) { doextra(a,val,i); }); return; } // quit if explicitly setting this elsewhere - if(attr in aobj) { return; } + if(attr in aobj) return; var extraparam; if(attr.substr(0, 6) === 'LAYOUT') { @@ -2055,7 +2055,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { // a complete layout redraw takes care of plot and var seq; if(dolayout) { - seq = [function changeLayout(){ + seq = [function changeLayout() { var copyLayout = gd.layout; gd.layout = undefined; return Plotly.plot(gd, '', copyLayout); @@ -2068,7 +2068,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { plots.supplyDefaults(gd); seq = [plots.previousPromises]; if(dostyle) { - seq.push(function doStyle(){ + seq.push(function doStyle() { // first see if we need to do arraysToCalcdata // call it regardless of what change we made, in case // supplyDefaults brought in an array that was already @@ -2085,7 +2085,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { }); } if(docolorbars) { - seq.push(function doColorBars(){ + seq.push(function doColorBars() { gd.calcdata.forEach(function(cd) { if((cd[0].t || {}).cb) { var trace = cd[0].trace, @@ -2114,7 +2114,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { if(!plotDone || !plotDone.then) plotDone = Promise.resolve(); - return plotDone.then(function(){ + return plotDone.then(function() { gd.emit('plotly_restyle', Plotly.Lib.extendDeep([], [redoit, traces])); }); @@ -2432,7 +2432,7 @@ Plotly.relayout = function relayout(gd, astr, val) { seq = [plots.previousPromises]; if(doplot || docalc) { - seq.push(function layoutReplot(){ + seq.push(function layoutReplot() { // force plot() to redo the layout gd.layout = undefined; // force it to redo calcdata? @@ -2447,7 +2447,7 @@ Plotly.relayout = function relayout(gd, astr, val) { fullLayout = gd._fullLayout; if(dolegend) { - seq.push(function doLegend(){ + seq.push(function doLegend() { Plotly.Legend.draw(gd); return plots.previousPromises(gd); }); @@ -2456,7 +2456,7 @@ Plotly.relayout = function relayout(gd, astr, val) { if(dolayoutstyle) seq.push(layoutStyles); if(doticks) { - seq.push(function(){ + seq.push(function() { Plotly.Axes.doTicks(gd,'redraw'); Plotly.Titles.draw(gd, 'gtitle'); return plots.previousPromises(gd); @@ -2486,7 +2486,7 @@ Plotly.relayout = function relayout(gd, astr, val) { if(!plotDone || !plotDone.then) plotDone = Promise.resolve(); - return plotDone.then(function(){ + return plotDone.then(function() { gd.emit('plotly_relayout', Plotly.Lib.extendDeep({}, redoit)); }); @@ -2671,7 +2671,7 @@ function makePlotFramework(gd) { // position and style the containers, make main title var frameWorkDone = Plotly.Lib.syncOrAsync([ layoutStyles, - function goAxes(){ return Plotly.Axes.doTicks(gd,'redraw'); }, + function goAxes() { return Plotly.Axes.doTicks(gd,'redraw'); }, Plotly.Fx.init ], gd); From f09911780e5a9e04f80c645fb48ed76bb2213a22 Mon Sep 17 00:00:00 2001 From: etpinard Date: Mon, 30 Nov 2015 18:03:56 -0500 Subject: [PATCH 4/7] add error message when topojson file isn't found --- src/plots/geo/geo.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index 4cca9c12a6b..0f9b6a3549a 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -102,6 +102,17 @@ proto.plot = function(geoData, fullLayout, promises) { promises.push(new Promise(function(resolve) { d3.json(topojsonPath, function(error, topojson) { + if(error) { + if(error.status === 404) { + console.error([ + 'plotly.js could not find topojson file at', + topojsonPath, '.', + 'Make sure the *topojsonURL* plot config option', + 'is set properly.' + ].join(' ')); + } + return; + } _this.topojson = topojson; PlotlyGeoAssets.topojson[_this.topojsonName] = topojson; From ecc0395733edbfb527d4453196a848e2b344f461 Mon Sep 17 00:00:00 2001 From: etpinard Date: Wed, 2 Dec 2015 10:36:11 -0500 Subject: [PATCH 5/7] handler error in get-topojson Promise with reject --- src/plots/geo/geo.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index 0f9b6a3549a..bc2baabd25f 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -100,16 +100,22 @@ proto.plot = function(geoData, fullLayout, promises) { _this.topojsonName ); - promises.push(new Promise(function(resolve) { + promises.push(new Promise(function(resolve, reject) { d3.json(topojsonPath, function(error, topojson) { if(error) { if(error.status === 404) { - console.error([ + reject(Error([ 'plotly.js could not find topojson file at', topojsonPath, '.', 'Make sure the *topojsonURL* plot config option', 'is set properly.' - ].join(' ')); + ].join(' '))); + } + else { + reject(Error([ + 'unexpected error while fetching topojson file at', + topojsonPath + ].join(' '))); } return; } From f5384ba6732cb16b58d62234d53bed36ed0da3b3 Mon Sep 17 00:00:00 2001 From: etpinard Date: Wed, 2 Dec 2015 10:36:28 -0500 Subject: [PATCH 6/7] update comment about streaming on maps --- src/plots/geo/geo.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index bc2baabd25f..b07597bfd92 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -131,7 +131,8 @@ proto.plot = function(geoData, fullLayout, promises) { } else _this.onceTopojsonIsLoaded(geoData, geoLayout); - // TODO handle topojson-is-loading case (for streaming) + // TODO handle topojson-is-loading case + // to avoid making multiple request while streaming }; proto.onceTopojsonIsLoaded = function(geoData, geoLayout) { From c2969dad884ac0ab41310ccf1a9f08c4b589e1b1 Mon Sep 17 00:00:00 2001 From: etpinard Date: Wed, 2 Dec 2015 11:10:52 -0500 Subject: [PATCH 7/7] replace reject(Error()) by reject(new Error()) --- src/plots/geo/geo.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index b07597bfd92..8800696dc21 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -104,7 +104,7 @@ proto.plot = function(geoData, fullLayout, promises) { d3.json(topojsonPath, function(error, topojson) { if(error) { if(error.status === 404) { - reject(Error([ + reject(new Error([ 'plotly.js could not find topojson file at', topojsonPath, '.', 'Make sure the *topojsonURL* plot config option', @@ -112,7 +112,7 @@ proto.plot = function(geoData, fullLayout, promises) { ].join(' '))); } else { - reject(Error([ + reject(new Error([ 'unexpected error while fetching topojson file at', topojsonPath ].join(' ')));