From 88f552a1a9ed0331f39de51352d470bc99919c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 27 Apr 2016 18:14:17 -0400 Subject: [PATCH 01/26] mv gd._modules -> fullLayout._modules --- src/plot_api/plot_api.js | 2 +- src/plots/cartesian/index.js | 2 +- src/plots/plots.js | 6 +++--- src/traces/scatterternary/style.js | 19 +++++++++++-------- test/jasmine/tests/plots_test.js | 1 - 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 05c9e657aba..4dfed0aff76 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -205,7 +205,7 @@ Plotly.plot = function(gd, data, layout, config) { if(!recalc) return; var subplots = Plots.getSubplotIds(fullLayout, 'cartesian'), - modules = gd._modules; + modules = fullLayout._modules; // position and range calculations for traces that // depend on each other ie bars (stacked or grouped) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 2e0ceebfe1b..a89cc135b8e 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -30,7 +30,7 @@ exports.plot = function(gd) { var fullLayout = gd._fullLayout, subplots = Plots.getSubplotIds(fullLayout, 'cartesian'), calcdata = gd.calcdata, - modules = gd._modules; + modules = fullLayout._modules; function getCdSubplot(calcdata, subplot) { var cdSubplot = []; diff --git a/src/plots/plots.js b/src/plots/plots.js index 271c18374cf..f45c9c77177 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -447,8 +447,9 @@ plots.supplyDefaults = function(gd) { oldFullData = gd._fullData || [], newFullData = gd._fullData = [], newData = gd.data || [], - modules = gd._modules = []; + newData = gd.data || []; + var modules = newFullLayout._modules = []; var i, trace, fullTrace, _module, axList, ax; @@ -792,7 +793,6 @@ plots.purge = function(gd) { // these get recreated on Plotly.plot anyway, but just to be safe // (and to have a record of them...) - delete gd._modules; delete gd._tester; delete gd._testref; delete gd._promises; @@ -810,7 +810,7 @@ plots.purge = function(gd) { }; plots.style = function(gd) { - var _modules = gd._modules; + var _modules = gd._fullLayout._modules; for(var i = 0; i < _modules.length; i++) { var _module = _modules[i]; diff --git a/src/traces/scatterternary/style.js b/src/traces/scatterternary/style.js index 7b92f4d3200..25459e55e74 100644 --- a/src/traces/scatterternary/style.js +++ b/src/traces/scatterternary/style.js @@ -12,13 +12,16 @@ var scatterStyle = require('../scatter/style'); -module.exports = function style(graphDiv) { - for(var i = 0; i < graphDiv._modules.length; i++) { - // we're just going to call scatter style... if we already - // called it, don't need to redo. - // Later though we may want differences, or we may make style - // more specific in its scope, then we can remove this. - if(graphDiv._modules[i].name === 'scatter') return; +module.exports = function style(gd) { + var modules = gd._fullLayout._modules; + + // we're just going to call scatter style... if we already + // called it, don't need to redo. + // Later though we may want differences, or we may make style + // more specific in its scope, then we can remove this. + for(var i = 0; i < modules.length; i++) { + if(modules[i].name === 'scatter') return; } - scatterStyle(graphDiv); + + scatterStyle(gd); }; diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index c83d6ea7d63..8dfbf9c71d3 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -351,7 +351,6 @@ describe('Test Plots', function() { expect(gd.undonum).toBeUndefined(); expect(gd.autoplay).toBeUndefined(); expect(gd.changed).toBeUndefined(); - expect(gd._modules).toBeUndefined(); expect(gd._tester).toBeUndefined(); expect(gd._testref).toBeUndefined(); expect(gd._promises).toBeUndefined(); From 525e3c7e615858f0c190e5fe13f63b4e33ac0db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 27 Apr 2016 18:15:42 -0400 Subject: [PATCH 02/26] lint --- src/plots/plots.js | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index f45c9c77177..a782ba5a96a 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -435,23 +435,29 @@ plots.sendDataToCloud = function(gd) { return false; }; +// Fill in default values: +// +// gd.data, gd.layout: +// are precisely what the user specified +// +// gd._fullData, gd._fullLayout: +// are complete descriptions of how to draw the plot +// +// gd._fullLayout._modules +// is a list of all the trace modules required to draw the plot +// plots.supplyDefaults = function(gd) { - // fill in default values: - // gd.data, gd.layout: - // are precisely what the user specified - // gd._fullData, gd._fullLayout: - // are complete descriptions of how to draw the plot var oldFullLayout = gd._fullLayout || {}, newFullLayout = gd._fullLayout = {}, - newLayout = gd.layout || {}, - oldFullData = gd._fullData || [], + newLayout = gd.layout || {}; + + var oldFullData = gd._fullData || [], newFullData = gd._fullData = [], - newData = gd.data || [], newData = gd.data || []; var modules = newFullLayout._modules = []; - var i, trace, fullTrace, _module, axList, ax; + var i, _module; // first fill in what we can of layout without looking at data // because fullData needs a few things from layout @@ -462,9 +468,7 @@ plots.supplyDefaults = function(gd) { // then do the data for(i = 0; i < newData.length; i++) { - trace = newData[i]; - - fullTrace = plots.supplyDataDefaults(trace, i, newFullLayout); + var fullTrace = plots.supplyDataDefaults(newData[i], i, newFullLayout); newFullData.push(fullTrace); // detect plot type @@ -476,8 +480,9 @@ plots.supplyDefaults = function(gd) { else if(plots.traceIs(fullTrace, 'ternary')) newFullLayout._hasTernary = true; else if('r' in fullTrace) newFullLayout._hasPolar = true; + // fill in modules list _module = fullTrace._module; - if(_module && modules.indexOf(_module)===-1) modules.push(_module); + if(_module && modules.indexOf(_module) === -1) modules.push(_module); } // special cases that introduce interactions between traces @@ -498,19 +503,16 @@ plots.supplyDefaults = function(gd) { // clean subplots and other artifacts from previous plot calls plots.cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout); - /* - * Relink functions and underscore attributes to promote consistency between - * plots. - */ + // relink functions and _ attributes to promote consistency between plots relinkPrivateKeys(newFullLayout, oldFullLayout); plots.doAutoMargin(gd); // can't quite figure out how to get rid of this... each axis needs // a reference back to the DOM object for just a few purposes - axList = Plotly.Axes.list(gd); + var axList = Plotly.Axes.list(gd); for(i = 0; i < axList.length; i++) { - ax = axList[i]; + var ax = axList[i]; ax._gd = gd; ax.setScale(); } @@ -518,7 +520,7 @@ plots.supplyDefaults = function(gd) { // update object references in calcdata if((gd.calcdata || []).length === newFullData.length) { for(i = 0; i < newFullData.length; i++) { - trace = newFullData[i]; + var trace = newFullData[i]; (gd.calcdata[i][0] || {}).trace = trace; } } @@ -725,7 +727,7 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut) { plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) { var i, _module; - // TODO incorporate into subplotRegistry + // TODO incorporate into subplotsRegistry Plotly.Axes.supplyLayoutDefaults(layoutIn, layoutOut, fullData); // plot module layout defaults From 876ce4b01fce2c34bebcdec379f0c9819b721f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 27 Apr 2016 18:30:46 -0400 Subject: [PATCH 03/26] attach has (plot type) method to fullLayout, - as a replacement for _hasCartesian, _hasGeo, _hadGL3D, etc. --- src/plots/plots.js | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index a782ba5a96a..10a08a9e2e2 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -471,20 +471,16 @@ plots.supplyDefaults = function(gd) { var fullTrace = plots.supplyDataDefaults(newData[i], i, newFullLayout); newFullData.push(fullTrace); - // detect plot type - if(plots.traceIs(fullTrace, 'cartesian')) newFullLayout._hasCartesian = true; - else if(plots.traceIs(fullTrace, 'gl3d')) newFullLayout._hasGL3D = true; - else if(plots.traceIs(fullTrace, 'geo')) newFullLayout._hasGeo = true; - else if(plots.traceIs(fullTrace, 'pie')) newFullLayout._hasPie = true; - else if(plots.traceIs(fullTrace, 'gl2d')) newFullLayout._hasGL2D = true; - else if(plots.traceIs(fullTrace, 'ternary')) newFullLayout._hasTernary = true; - else if('r' in fullTrace) newFullLayout._hasPolar = true; + // detect polar + if('r' in fullTrace) newFullLayout._hasPolar = true; // fill in modules list _module = fullTrace._module; if(_module && modules.indexOf(_module) === -1) modules.push(_module); } + // attach helper method + newFullLayout._has = hasPlotType.bind(newFullLayout); // special cases that introduce interactions between traces for(i = 0; i < modules.length; i++) { _module = modules[i]; @@ -526,6 +522,23 @@ plots.supplyDefaults = function(gd) { } }; +// helper function to be bound to fullLayout to check +// whether a certain plot type or layout categories is present on plot +function hasPlotType(category) { + var modules = this._modules || []; + + for(var i = 0; i < modules.length; i++) { + var _module = modules[i]; + + if(_module.basePlotModule.name === category) return true; + + var layoutCategories = _module.layoutCategories || []; + if(layoutCategories.indexOf(category) !== -1) return true; + } + + return false; +} + plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { var i, j; From f45ffec98f96a718429f08d283b15f821e370a81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 27 Apr 2016 18:31:37 -0400 Subject: [PATCH 04/26] add tempory block _has() -> _hasCartesian etc. - revert after replacing all _has with _has() --- src/plots/plots.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/plots/plots.js b/src/plots/plots.js index 10a08a9e2e2..319f5b27ca3 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -481,6 +481,15 @@ plots.supplyDefaults = function(gd) { // attach helper method newFullLayout._has = hasPlotType.bind(newFullLayout); + + // temporary block (before replace all _has??? with _hasPlotType() ? + newFullLayout._hasCartesian = newFullLayout._has('cartesian'); + newFullLayout._hasGeo = newFullLayout._has('geo'); + newFullLayout._hasGL3D = newFullLayout._has('gl3d'); + newFullLayout._hasGL2D = newFullLayout._has('gl2d'); + newFullLayout._hasTernary = newFullLayout._has('ternary'); + newFullLayout._hasPie = newFullLayout._has('pie'); + // special cases that introduce interactions between traces for(i = 0; i < modules.length; i++) { _module = modules[i]; From 4e0c6dacfca75419593f0b60e90458f82514714d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 27 Apr 2016 18:32:10 -0400 Subject: [PATCH 05/26] rm _has??? attributes --- src/plots/layout_attributes.js | 25 +------------------------ src/plots/plots.js | 6 ------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index 601251a1a8d..52f016e8af5 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -168,30 +168,7 @@ module.exports = { role: 'info', description: 'Determines whether or not a legend is drawn.' }, - _hasCartesian: { - valType: 'boolean', - dflt: false - }, - _hasGL3D: { - valType: 'boolean', - dflt: false - }, - _hasGeo: { - valType: 'boolean', - dflt: false - }, - _hasPie: { - valType: 'boolean', - dflt: false - }, - _hasGL2D: { - valType: 'boolean', - dflt: false - }, - _hasTernary: { - valType: 'boolean', - dflt: false - }, + _composedModules: { '*': 'Fx' }, diff --git a/src/plots/plots.js b/src/plots/plots.js index 319f5b27ca3..e1e84a2c14c 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -738,12 +738,6 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut) { coerce('separators'); coerce('hidesources'); coerce('smith'); - coerce('_hasCartesian'); - coerce('_hasGL3D'); - coerce('_hasGeo'); - coerce('_hasPie'); - coerce('_hasGL2D'); - coerce('_hasTernary'); }; plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) { From 93493a69c16e379b36b01a9cc3827dc106ef2bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 27 Apr 2016 18:33:15 -0400 Subject: [PATCH 06/26] make 'pie' a layout category, - so that fullLayout.has('pie') returns true - note that fullLayout.has('cartesian') also returns true now as pie trace use the cartesian plot module. --- src/plots/plots.js | 2 +- src/traces/pie/index.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index e1e84a2c14c..19d2fe65aac 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -193,7 +193,7 @@ plots.getSubplotIds = function getSubplotIds(layout, type) { if(type === 'cartesian' && !layout._hasCartesian) return []; if(type === 'gl2d' && !layout._hasGL2D) return []; if(type === 'cartesian' || type === 'gl2d') { - return Object.keys(layout._plots); + return Object.keys(layout._plots || {}); } var idRegex = _module.idRegex, diff --git a/src/traces/pie/index.js b/src/traces/pie/index.js index c5344071d8e..fb3ff9e2e58 100644 --- a/src/traces/pie/index.js +++ b/src/traces/pie/index.js @@ -23,6 +23,7 @@ Pie.moduleType = 'trace'; Pie.name = 'pie'; Pie.basePlotModule = require('../../plots/cartesian'); Pie.categories = ['pie', 'showLegend']; +Pie.layoutCategories = ['pie']; Pie.meta = { description: [ 'A data visualized by the sectors of the pie is set in `values`.', From e63ba4ae59f022435f8cc6d5221f6ee748391896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 28 Apr 2016 14:25:18 -0400 Subject: [PATCH 07/26] check for hasCartesian && !hasPie for 'hovermode', - so that 'hovermode' is set and toggled properly. --- src/components/modebar/buttons.js | 2 +- src/plots/cartesian/graph_interact.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index b545faff14b..c6c0a4fbb0d 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -500,7 +500,7 @@ function toggleHover(gd) { var fullLayout = gd._fullLayout; var onHoverVal; - if(fullLayout._hasCartesian) { + if(fullLayout._hasCartesian && !fullLayout._hasPie) { onHoverVal = fullLayout._isHoriz ? 'y' : 'x'; } else onHoverVal = 'closest'; diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index b36f5952ce8..9eb41eda29a 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -60,7 +60,7 @@ fx.supplyLayoutDefaults = function(layoutIn, layoutOut, fullData) { coerce('dragmode'); var hovermodeDflt; - if(layoutOut._hasCartesian) { + if(layoutOut._hasCartesian && !layoutOut._hasPie) { // flag for 'horizontal' plots: // determines the state of the mode bar 'compare' hovermode button var isHoriz = layoutOut._isHoriz = fx.isHoriz(fullData); From b655cb059609c9cb11edf561bc2307ba50b71f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 3 May 2016 14:25:34 -0400 Subject: [PATCH 08/26] Revert "check for hasCartesian && !hasPie for 'hovermode'," This reverts commit e63ba4ae59f022435f8cc6d5221f6ee748391896. --- src/components/modebar/buttons.js | 2 +- src/plots/cartesian/graph_interact.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index c6c0a4fbb0d..b545faff14b 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -500,7 +500,7 @@ function toggleHover(gd) { var fullLayout = gd._fullLayout; var onHoverVal; - if(fullLayout._hasCartesian && !fullLayout._hasPie) { + if(fullLayout._hasCartesian) { onHoverVal = fullLayout._isHoriz ? 'y' : 'x'; } else onHoverVal = 'closest'; diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 9eb41eda29a..b36f5952ce8 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -60,7 +60,7 @@ fx.supplyLayoutDefaults = function(layoutIn, layoutOut, fullData) { coerce('dragmode'); var hovermodeDflt; - if(layoutOut._hasCartesian && !layoutOut._hasPie) { + if(layoutOut._hasCartesian) { // flag for 'horizontal' plots: // determines the state of the mode bar 'compare' hovermode button var isHoriz = layoutOut._isHoriz = fx.isHoriz(fullData); From f476e05b09a7a9af1bb2689fb10f1121c9c1c72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 3 May 2016 15:33:27 -0400 Subject: [PATCH 09/26] split pie trace base plot module from cartesian: - so that _has??? are one-to-one with base plot modules - handle case where base plot module attrRegex isn't defined in plot schema. --- src/plot_api/plot_api.js | 5 ++--- src/plot_api/plot_schema.js | 2 ++ src/plots/cartesian/index.js | 17 --------------- src/traces/pie/base_plot.js | 42 ++++++++++++++++++++++++++++++++++++ src/traces/pie/index.js | 2 +- 5 files changed, 47 insertions(+), 21 deletions(-) create mode 100644 src/traces/pie/base_plot.js diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 4dfed0aff76..b34dbf1e391 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -277,9 +277,8 @@ Plotly.plot = function(gd, data, layout, config) { if(fullLayout._hasGL3D) plotRegistry.gl3d.plot(gd); if(fullLayout._hasGeo) plotRegistry.geo.plot(gd); if(fullLayout._hasGL2D) plotRegistry.gl2d.plot(gd); - if(fullLayout._hasCartesian || fullLayout._hasPie) { - plotRegistry.cartesian.plot(gd); - } + if(fullLayout._hasCartesian) plotRegistry.cartesian.plot(gd); + if(fullLayout._hasPie) plotRegistry.pie.plot(gd); if(fullLayout._hasTernary) plotRegistry.ternary.plot(gd); // clean up old scenes that no longer have associated data diff --git a/src/plot_api/plot_schema.js b/src/plot_api/plot_schema.js index c7a3c9b2f84..36d83861831 100644 --- a/src/plot_api/plot_schema.js +++ b/src/plot_api/plot_schema.js @@ -286,6 +286,8 @@ function handleSubplotObjs(layoutAttributes) { var subplotRegistry = subplotsRegistry[subplotType], isSubplotObj; + if(!subplotRegistry.attrRegex) return; + if(subplotType === 'cartesian' || subplotType === 'gl2d') { isSubplotObj = ( subplotRegistry.attrRegex.x.test(k) || diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index a89cc135b8e..56c1ac40946 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -78,9 +78,6 @@ exports.plot = function(gd) { // skip over non-cartesian trace modules if(_module.basePlotModule.name !== 'cartesian') continue; - // skip over pies, there are drawn below - if(_module.name === 'pie') continue; - // plot all traces of this type on this subplot at once var cdModule = getCdModule(cdSubplot, _module); _module.plot(gd, subplotInfo, cdModule); @@ -88,18 +85,4 @@ exports.plot = function(gd) { Lib.markTime('done ' + (cdModule[0] && cdModule[0][0].trace.type)); } } - - // now draw stuff not on subplots (ie, only pies at the moment) - if(fullLayout._hasPie) { - var Pie = Plots.getModule('pie'); - var cdPie = getCdModule(calcdata, Pie); - - if(cdPie.length) Pie.plot(gd, cdPie); - } -}; - -exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { - if(oldFullLayout._hasPie && !newFullLayout._hasPie) { - oldFullLayout._pielayer.selectAll('g.trace').remove(); - } }; diff --git a/src/traces/pie/base_plot.js b/src/traces/pie/base_plot.js new file mode 100644 index 00000000000..811060fff4d --- /dev/null +++ b/src/traces/pie/base_plot.js @@ -0,0 +1,42 @@ +/** +* Copyright 2012-2016, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +var Plots = require('../../plots/plots'); + + +exports.name = 'pie'; + +exports.plot = function(gd) { + var Pie = Plots.getModule('pie'); + var cdPie = getCdModule(gd.calcdata, Pie); + + if(cdPie.length) Pie.plot(gd, cdPie); +}; + +exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { + if(oldFullLayout._hasPie && !newFullLayout._hasPie) { + oldFullLayout._pielayer.selectAll('g.trace').remove(); + } +}; + +function getCdModule(calcdata, _module) { + var cdModule = []; + + for(var i = 0; i < calcdata.length; i++) { + var cd = calcdata[i]; + var trace = cd[0].trace; + + if((trace._module === _module) && (trace.visible === true)) { + cdModule.push(cd); + } + } + + return cdModule; +} diff --git a/src/traces/pie/index.js b/src/traces/pie/index.js index fb3ff9e2e58..1d375996ae3 100644 --- a/src/traces/pie/index.js +++ b/src/traces/pie/index.js @@ -21,7 +21,7 @@ Pie.styleOne = require('./style_one'); Pie.moduleType = 'trace'; Pie.name = 'pie'; -Pie.basePlotModule = require('../../plots/cartesian'); +Pie.basePlotModule = require('./base_plot'); Pie.categories = ['pie', 'showLegend']; Pie.layoutCategories = ['pie']; Pie.meta = { From 1c41c2e24c9426cf3886090b15d57f51de32216c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 3 May 2016 15:33:45 -0400 Subject: [PATCH 10/26] rm (now useless) trace module layoutCategories --- src/plots/plots.js | 3 --- src/traces/pie/index.js | 1 - 2 files changed, 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 19d2fe65aac..f4293b04fa9 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -540,9 +540,6 @@ function hasPlotType(category) { var _module = modules[i]; if(_module.basePlotModule.name === category) return true; - - var layoutCategories = _module.layoutCategories || []; - if(layoutCategories.indexOf(category) !== -1) return true; } return false; diff --git a/src/traces/pie/index.js b/src/traces/pie/index.js index 1d375996ae3..d71ae74207a 100644 --- a/src/traces/pie/index.js +++ b/src/traces/pie/index.js @@ -23,7 +23,6 @@ Pie.moduleType = 'trace'; Pie.name = 'pie'; Pie.basePlotModule = require('./base_plot'); Pie.categories = ['pie', 'showLegend']; -Pie.layoutCategories = ['pie']; Pie.meta = { description: [ 'A data visualized by the sectors of the pie is set in `values`.', From 313150eaa68ee02c09423af8b9aca7efafc61b0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:04:44 -0400 Subject: [PATCH 11/26] add fill unique lib function, - useful to fill fullLayout._modules and fullLayout._basePlotModules --- src/lib/index.js | 17 +++++++++++++++ test/jasmine/tests/lib_test.js | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/lib/index.js b/src/lib/index.js index 8f2235928cd..2ac0e38d48f 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -335,6 +335,23 @@ lib.noneOrAll = function(containerIn, containerOut, attrList) { } }; +/** + * Fill with unique items + * + * @param {array} array + * array to be filled + * @param {any} item + * item to be or not to be inserted + * @return {array} + * ref to array (now possibly containing one more item) + * + */ +lib.fillUnique = function(array, item) { + if(item && array.indexOf(item) === -1) array.push(item); + + return array; +}; + lib.mergeArray = function(traceAttr, cd, cdAttr) { if(Array.isArray(traceAttr)) { var imax = Math.min(traceAttr.length, cd.length); diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 3b11c1868d9..c635064c7df 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -770,4 +770,44 @@ describe('Test lib.js:', function() { }); }); + describe('fillUnique', function() { + + beforeEach(function() { + this.obj = { a: 'A' }; + this.array = ['a', 'b', 'c', this.obj]; + }); + + it('should fill new items in array', function() { + var out = Lib.fillUnique(this.array, 'd'); + + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }, 'd']); + expect(this.array).toBe(out); + }); + + it('should ignore falsy items', function() { + Lib.fillUnique(this.array, false); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + + Lib.fillUnique(this.array, undefined); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + + Lib.fillUnique(this.array, 0); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + + Lib.fillUnique(this.array, null); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + + Lib.fillUnique(this.array, ''); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + }); + + it('should ignore item already in array', function() { + Lib.fillUnique(this.array, 'a'); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + + Lib.fillUnique(this.array, this.obj); + expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); + }); + }); + }); From 392619c645990d78793ad8388b172b54c693d451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:09:49 -0400 Subject: [PATCH 12/26] fill fullLayout._basePlotModules list in default trace loop, - and in _basePlotModules in fullLayout._has() instead of loop over _modules --- src/plots/plots.js | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index f4293b04fa9..939c5df4ca9 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -446,6 +446,9 @@ plots.sendDataToCloud = function(gd) { // gd._fullLayout._modules // is a list of all the trace modules required to draw the plot // +// gd._fullLayout._basePlotModules +// is a list of all the plot modules required to draw the plot +// plots.supplyDefaults = function(gd) { var oldFullLayout = gd._fullLayout || {}, newFullLayout = gd._fullLayout = {}, @@ -455,7 +458,8 @@ plots.supplyDefaults = function(gd) { newFullData = gd._fullData = [], newData = gd.data || []; - var modules = newFullLayout._modules = []; + var modules = newFullLayout._modules = [], + basePlotModules = newFullLayout._basePlotModules = []; var i, _module; @@ -474,11 +478,13 @@ plots.supplyDefaults = function(gd) { // detect polar if('r' in fullTrace) newFullLayout._hasPolar = true; - // fill in modules list _module = fullTrace._module; - if(_module && modules.indexOf(_module) === -1) modules.push(_module); - } + if(!_module) continue; + // fill in module lists + Lib.fillUnique(modules, _module); + Lib.fillUnique(basePlotModules, fullTrace._module.basePlotModule); + } // attach helper method newFullLayout._has = hasPlotType.bind(newFullLayout); @@ -532,18 +538,17 @@ plots.supplyDefaults = function(gd) { }; // helper function to be bound to fullLayout to check -// whether a certain plot type or layout categories is present on plot +// whether a certain plot type is present on plot function hasPlotType(category) { - var modules = this._modules || []; + var basePlotModules = this._basePlotModules || []; - for(var i = 0; i < modules.length; i++) { - var _module = modules[i]; + for(var i = 0; i < basePlotModules.length; i++) { + var _module = basePlotModules[i]; - if(_module.basePlotModule.name === category) return true; + if(_module.name === category) return true; } return false; -} plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { var i, j; From 3787d4734ec7269b5085b2e846e68fb015743d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:15:43 -0400 Subject: [PATCH 13/26] loop over basePlotModules instead of subplotRegistry in: - in Plots.cleanPlot (use basePlotModules list in old fullLayout) - in Plots.supplyLayoutModulesDefaults (making those !_has??? obsolete) - in main drawData function (instead of _has??? switch block) --- src/plot_api/plot_api.js | 21 ++++++++------------- src/plots/geo/layout/defaults.js | 2 -- src/plots/gl3d/layout/defaults.js | 2 -- src/plots/plots.js | 20 ++++++++++++-------- src/plots/ternary/layout/defaults.js | 2 -- 5 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index b34dbf1e391..51d862281da 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -250,11 +250,12 @@ Plotly.plot = function(gd, data, layout, config) { // Now plot the data function drawData() { - var calcdata = gd.calcdata; + var calcdata = gd.calcdata, + i; // in case of traces that were heatmaps or contour maps // previously, remove them and their colorbars explicitly - for(var i = 0; i < calcdata.length; i++) { + for(i = 0; i < calcdata.length; i++) { var trace = calcdata[i][0].trace, isVisible = (trace.visible === true), uid = trace.uid; @@ -272,17 +273,11 @@ Plotly.plot = function(gd, data, layout, config) { } } - var plotRegistry = Plots.subplotsRegistry; - - if(fullLayout._hasGL3D) plotRegistry.gl3d.plot(gd); - if(fullLayout._hasGeo) plotRegistry.geo.plot(gd); - if(fullLayout._hasGL2D) plotRegistry.gl2d.plot(gd); - if(fullLayout._hasCartesian) plotRegistry.cartesian.plot(gd); - if(fullLayout._hasPie) plotRegistry.pie.plot(gd); - if(fullLayout._hasTernary) plotRegistry.ternary.plot(gd); - - // clean up old scenes that no longer have associated data - // will this be a performance hit? + // loop over the base plot modules present on graph + var basePlotModules = fullLayout._basePlotModules; + for(i = 0; i < basePlotModules.length; i++) { + basePlotModules[i].plot(gd); + } // styling separate from drawing Plots.style(gd); diff --git a/src/plots/geo/layout/defaults.js b/src/plots/geo/layout/defaults.js index 5919cbfafcb..f149d6b1191 100644 --- a/src/plots/geo/layout/defaults.js +++ b/src/plots/geo/layout/defaults.js @@ -16,8 +16,6 @@ var supplyGeoAxisLayoutDefaults = require('./axis_defaults'); module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { - if(!layoutOut._hasGeo) return; - handleSubplotDefaults(layoutIn, layoutOut, fullData, { type: 'geo', attributes: layoutAttributes, diff --git a/src/plots/gl3d/layout/defaults.js b/src/plots/gl3d/layout/defaults.js index 066f7ec2fa4..006ed9b761c 100644 --- a/src/plots/gl3d/layout/defaults.js +++ b/src/plots/gl3d/layout/defaults.js @@ -17,8 +17,6 @@ var supplyGl3dAxisLayoutDefaults = require('./axis_defaults'); module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { - if(!layoutOut._hasGL3D) return; - var hasNon3D = ( layoutOut._hasCartesian || layoutOut._hasGeo || diff --git a/src/plots/plots.js b/src/plots/plots.js index 939c5df4ca9..15b631a6f8b 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -553,9 +553,9 @@ function hasPlotType(category) { plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { var i, j; - var plotTypes = Object.keys(subplotsRegistry); - for(i = 0; i < plotTypes.length; i++) { - var _module = subplotsRegistry[plotTypes[i]]; + var basePlotModules = oldFullLayout._basePlotModules || []; + for(i = 0; i < basePlotModules.length; i++) { + var _module = basePlotModules[i]; if(_module.clean) { _module.clean(newFullData, newFullLayout, oldFullData, oldFullLayout); @@ -745,13 +745,17 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut) { plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) { var i, _module; - // TODO incorporate into subplotsRegistry + // can't be be part of basePlotModules loop + // in order to handle the orphan axes case Plotly.Axes.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - // plot module layout defaults - var plotTypes = Object.keys(subplotsRegistry); - for(i = 0; i < plotTypes.length; i++) { - _module = subplotsRegistry[plotTypes[i]]; + // base plot module layout defaults + var basePlotModules = layoutOut._basePlotModules; + for(i = 0; i < basePlotModules.length; i++) { + _module = basePlotModules[i]; + + // done above already + if(_module.name === 'cartesian') continue; // e.g. gl2d does not have a layout-defaults step if(_module.supplyLayoutDefaults) { diff --git a/src/plots/ternary/layout/defaults.js b/src/plots/ternary/layout/defaults.js index 047208ec562..3564d84b812 100644 --- a/src/plots/ternary/layout/defaults.js +++ b/src/plots/ternary/layout/defaults.js @@ -18,8 +18,6 @@ var handleAxisDefaults = require('./axis_defaults'); var axesNames = ['aaxis', 'baxis', 'caxis']; module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { - if(!layoutOut._hasTernary) return; - handleSubplotDefaults(layoutIn, layoutOut, fullData, { type: 'ternary', attributes: layoutAttributes, From 68b12c4da0de16ab3d66d646c36ab7e44ecc3be7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:16:34 -0400 Subject: [PATCH 14/26] make sure to add cartesian base plot module to orphan axis graphs: - so that Cartesian.plot gets called --- src/plots/cartesian/layout_defaults.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 2fae5aaace3..b49fcd379fc 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -95,7 +95,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { // make sure that plots with orphan cartesian axes // are considered 'cartesian' if(xaListCartesian.length && yaListCartesian.length) { - layoutOut._hasCartesian = true; + Lib.fillUnique(layoutOut._basePlotModules, Plots.subplotsRegistry.cartesian); } function axSort(a, b) { From a41427cd7500a6b19da9a4d1afab1e7918de2957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:18:18 -0400 Subject: [PATCH 15/26] move 3d axes init step from makePlotFrameork -> Gl3d.plot - making for one less _has call --- src/plot_api/plot_api.js | 3 --- src/plots/gl3d/index.js | 28 ++++++++++++---------------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 51d862281da..aa95bc762ff 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2569,9 +2569,6 @@ function makePlotFramework(gd) { var gd3 = d3.select(gd), fullLayout = gd._fullLayout; - // TODO - find a better place for 3D to initialize axes - if(fullLayout._hasGL3D) Plots.subplotsRegistry.gl3d.initAxes(gd); - // Plot container fullLayout._container = gd3.selectAll('.plot-container').data([0]); fullLayout._container.enter().insert('div', ':first-child') diff --git a/src/plots/gl3d/index.js b/src/plots/gl3d/index.js index d0286ee52c7..be90dab79d3 100644 --- a/src/plots/gl3d/index.js +++ b/src/plots/gl3d/index.js @@ -46,10 +46,13 @@ exports.plot = function plotGl3d(gd) { for(var i = 0; i < sceneIds.length; i++) { var sceneId = sceneIds[i], fullSceneData = Plots.getSubplotData(fullData, 'gl3d', sceneId), - scene = fullLayout[sceneId]._scene; // ref. to corresp. Scene instance + sceneLayout = fullLayout[sceneId], + scene = sceneLayout._scene; // If Scene is not instantiated, create one! if(scene === undefined) { + initAxes(gd, sceneLayout); + scene = new Scene({ id: sceneId, graphDiv: gd, @@ -60,10 +63,11 @@ exports.plot = function plotGl3d(gd) { fullLayout ); - fullLayout[sceneId]._scene = scene; // set ref to Scene instance + // set ref to Scene instance + sceneLayout._scene = scene; } - scene.plot(fullSceneData, fullLayout, gd.layout); // takes care of business + scene.plot(fullSceneData, fullLayout, gd.layout); } }; @@ -91,18 +95,10 @@ exports.cleanId = function cleanId(id) { exports.setConvert = require('./set_convert'); -exports.initAxes = function(gd) { - var fullLayout = gd._fullLayout; - var sceneIds = Plots.getSubplotIds(fullLayout, 'gl3d'); - - for(var i = 0; i < sceneIds.length; ++i) { - var sceneId = sceneIds[i]; - var sceneLayout = fullLayout[sceneId]; +function initAxes(gd, sceneLayout) { + for(var j = 0; j < 3; ++j) { + var axisName = axesNames[j]; - for(var j = 0; j < 3; ++j) { - var axisName = axesNames[j]; - var ax = sceneLayout[axisName]; - ax._gd = gd; - } + sceneLayout[axisName]._gd = gd; } -}; +} From a48d071e98203ec8696ba9a249d8251e52ba4cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:22:08 -0400 Subject: [PATCH 16/26] check for non-cartesian/pie/ternary base plot module in restyle, - if found, the restyle calls required a full replot --- src/plot_api/plot_api.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index aa95bc762ff..f88664c7543 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1654,10 +1654,12 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { axlist, flagAxForDelete = {}; - // for now, if we detect gl or geo stuff, just re-do the plot - if(fullLayout._hasGL3D || fullLayout._hasGeo || fullLayout._hasGL2D) { - doplot = true; - } + // At the moment, only cartesian, pie and ternary plot types can afford + // to not go through a full replot + var doPlotWhiteList = ['cartesian', 'pie', 'ternary']; + fullLayout._basePlotModules.forEach(function(_module) { + if(doPlotWhiteList.indexOf(_module.name) === -1) doplot = true; + }); // make a new empty vals array for undoit function a0() { return traces.map(function() { return undefined; }); } From a78521c438bcb0e5fde2f23590bca25a03295fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:28:47 -0400 Subject: [PATCH 17/26] handle a few polar-only cases: - Plots.redrawText and Snapshot redraw work with every plot type except polar - continue if polar trace is detected in main supply default trace loop - don't assign _hasPolar (it didn't have to correct value anyway) --- src/plots/plots.js | 14 +++++++------- src/snapshot/index.js | 16 +++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 15b631a6f8b..e1811e7c628 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -259,18 +259,18 @@ plots.getSubplotData = function getSubplotData(data, type, subplotId) { // then wait a little, then draw it again plots.redrawText = function(gd) { - // doesn't work presently (and not needed) for polar or 3d - if(gd._fullLayout._hasGL3D || (gd.data && gd.data[0] && gd.data[0].r)) { - return; - } + // do not work if polar is present + if((gd.data && gd.data[0] && gd.data[0].r)) return; return new Promise(function(resolve) { setTimeout(function() { Plotly.Annotations.drawAll(gd); Plotly.Legend.draw(gd); - (gd.calcdata||[]).forEach(function(d) { - if(d[0]&&d[0].t&&d[0].t.cb) d[0].t.cb(); + + (gd.calcdata || []).forEach(function(d) { + if(d[0] && d[0].t && d[0].t.cb) d[0].t.cb(); }); + resolve(plots.previousPromises(gd)); },300); }); @@ -476,7 +476,7 @@ plots.supplyDefaults = function(gd) { newFullData.push(fullTrace); // detect polar - if('r' in fullTrace) newFullLayout._hasPolar = true; + if('r' in newData[i]) continue; _module = fullTrace._module; if(!_module) continue; diff --git a/src/snapshot/index.js b/src/snapshot/index.js index 2b662846928..86d3dd104ca 100644 --- a/src/snapshot/index.js +++ b/src/snapshot/index.js @@ -10,18 +10,20 @@ 'use strict'; function getDelay(fullLayout) { - return (fullLayout._hasGL3D || fullLayout._hasGL2D) ? 500 : 0; + + // polar clears fullLayout._has for some reason + if(!fullLayout._has) return 0; + + // maybe we should add a 'gl' (and 'svg') layoutCategory ?? + return (fullLayout._has('gl3d')|| fullLayout._has('gl2d')) ? 500 : 0; } function getRedrawFunc(gd) { - return function() { - var fullLayout = gd._fullLayout; - // doesn't work presently (and not needed) for polar or gl - if(fullLayout._hasGL3D || fullLayout._hasGL2D || - (gd.data && gd.data[0] && gd.data[0].r) - ) return; + // do not work if polar is present + if((gd.data && gd.data[0] && gd.data[0].r)) return; + return function() { (gd.calcdata || []).forEach(function(d) { if(d[0] && d[0].t && d[0].t.cb) d[0].t.cb(); }); From aa2ec8bd7bee480727fd1514fd354012efc950ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:33:02 -0400 Subject: [PATCH 18/26] sub has??? -> has('???') --- src/components/modebar/buttons.js | 4 ++-- src/components/modebar/manage.js | 12 ++++++------ src/components/rangeslider/index.js | 2 +- src/plot_api/plot_api.js | 4 ++-- src/plots/cartesian/axes.js | 4 ++-- src/plots/cartesian/graph_interact.js | 6 +++--- src/plots/cartesian/layout_defaults.js | 2 +- src/plots/gl3d/layout/defaults.js | 10 +++++----- src/plots/plots.js | 4 ++-- 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index b545faff14b..25da2554b2a 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -256,7 +256,7 @@ function handleCartesian(gd, ev) { Plotly.relayout(gd, aobj).then(function() { if(astr === 'dragmode') { - if(fullLayout._hasCartesian) { + if(fullLayout._has('cartesian')) { setCursor( fullLayout._paper.select('.nsewdrag'), DRAGCURSORS[val] @@ -500,7 +500,7 @@ function toggleHover(gd) { var fullLayout = gd._fullLayout; var onHoverVal; - if(fullLayout._hasCartesian) { + if(fullLayout._has('cartesian')) { onHoverVal = fullLayout._isHoriz ? 'y' : 'x'; } else onHoverVal = 'closest'; diff --git a/src/components/modebar/manage.js b/src/components/modebar/manage.js index e0c6ba3f6bf..7abc6cb330c 100644 --- a/src/components/modebar/manage.js +++ b/src/components/modebar/manage.js @@ -73,12 +73,12 @@ function getButtonGroups(gd, buttonsToRemove, buttonsToAdd) { var fullLayout = gd._fullLayout, fullData = gd._fullData; - var hasCartesian = !!fullLayout._hasCartesian, - hasGL3D = !!fullLayout._hasGL3D, - hasGeo = !!fullLayout._hasGeo, - hasPie = !!fullLayout._hasPie, - hasGL2D = !!fullLayout._hasGL2D, - hasTernary = !!fullLayout._hasTernary; + var hasCartesian = !!fullLayout._has('cartesian'), + hasGL3D = !!fullLayout._has('gl3d'), + hasGeo = !!fullLayout._has('geo'), + hasPie = !!fullLayout._has('pie'), + hasGL2D = !!fullLayout._has('gl2d'), + hasTernary = !!fullLayout._has('ternary'); var groups = []; diff --git a/src/components/rangeslider/index.js b/src/components/rangeslider/index.js index 31bd1175c8e..26ef6b1d069 100644 --- a/src/components/rangeslider/index.js +++ b/src/components/rangeslider/index.js @@ -41,7 +41,7 @@ function draw(gd) { var height = (fullLayout.height - fullLayout.margin.b - fullLayout.margin.t) * options.thickness, offsetShift = Math.floor(options.borderwidth / 2); - if(sliderContainer[0].length === 0 && !fullLayout._hasGL2D) createSlider(gd); + if(sliderContainer[0].length === 0 && !fullLayout._has('gl2d')) createSlider(gd); // Need to default to 0 for when making gl plots var bb = fullLayout.xaxis._boundingBox ? diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index f88664c7543..208c5460552 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2298,7 +2298,7 @@ Plotly.relayout = function relayout(gd, astr, val) { if(p.parts[0].indexOf('scene') === 0) doplot = true; else if(p.parts[0].indexOf('geo') === 0) doplot = true; else if(p.parts[0].indexOf('ternary') === 0) doplot = true; - else if(fullLayout._hasGL2D && + else if(fullLayout._has('gl2d') && (ai.indexOf('axis') !== -1 || p.parts[0] === 'plot_bgcolor') ) doplot = true; else if(ai === 'hiddenlabels') docalc = true; @@ -2643,7 +2643,7 @@ function makePlotFramework(gd) { makeSubplots(gd, subplots); } - if(fullLayout._hasCartesian) makeCartesianPlotFramwork(gd, subplots); + if(fullLayout._has('cartesian')) makeCartesianPlotFramwork(gd, subplots); // single ternary layer for the whole plot fullLayout._ternarylayer = fullLayout._paper.append('g').classed('ternarylayer', true); diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 4440e2b54d6..42f898387e7 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -39,7 +39,7 @@ axes.getFromTrace = axisIds.getFromTrace; // find the list of possible axes to reference with an xref or yref attribute // and coerce it to that list axes.coerceRef = function(containerIn, containerOut, gd, axLetter) { - var axlist = gd._fullLayout._hasGL2D ? [] : axes.listIds(gd, axLetter), + var axlist = gd._fullLayout._has('gl2d') ? [] : axes.listIds(gd, axLetter), refAttr = axLetter + 'ref', attrDef = {}; @@ -1797,7 +1797,7 @@ axes.doTicks = function(gd, axid, skipTitle) { var alldone = axes.getSubplots(gd, ax).map(function(subplot) { var plotinfo = fullLayout._plots[subplot]; - if(!fullLayout._hasCartesian) return; + if(!fullLayout._has('cartesian')) return; var container = plotinfo[axletter + 'axislayer'], diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index b36f5952ce8..f0a67f65b26 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -60,7 +60,7 @@ fx.supplyLayoutDefaults = function(layoutIn, layoutOut, fullData) { coerce('dragmode'); var hovermodeDflt; - if(layoutOut._hasCartesian) { + if(layoutOut._has('cartesian')) { // flag for 'horizontal' plots: // determines the state of the mode bar 'compare' hovermode button var isHoriz = layoutOut._isHoriz = fx.isHoriz(fullData); @@ -89,7 +89,7 @@ fx.isHoriz = function(fullData) { fx.init = function(gd) { var fullLayout = gd._fullLayout; - if(!fullLayout._hasCartesian || gd._context.staticPlot) return; + if(!fullLayout._has('cartesian') || gd._context.staticPlot) return; var subplots = Object.keys(fullLayout._plots || {}).sort(function(a,b) { // sort overlays last, then by x axis number, then y axis number @@ -107,7 +107,7 @@ fx.init = function(gd) { subplots.forEach(function(subplot) { var plotinfo = fullLayout._plots[subplot]; - if(!fullLayout._hasCartesian) return; + if(!fullLayout._has('cartesian')) return; var xa = plotinfo.x(), ya = plotinfo.y(), diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index b49fcd379fc..2ea5a2d52bd 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -71,7 +71,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { // if gl3d or geo is present on graph. This is retain backward compatible. // // TODO drop this in version 2.0 - var ignoreOrphan = (layoutOut._hasGL3D || layoutOut._hasGeo); + var ignoreOrphan = (layoutOut._has('gl3d') || layoutOut._has('geo')); if(!ignoreOrphan) { for(i = 0; i < layoutKeys.length; i++) { diff --git a/src/plots/gl3d/layout/defaults.js b/src/plots/gl3d/layout/defaults.js index 006ed9b761c..291d8dcd572 100644 --- a/src/plots/gl3d/layout/defaults.js +++ b/src/plots/gl3d/layout/defaults.js @@ -18,11 +18,11 @@ var supplyGl3dAxisLayoutDefaults = require('./axis_defaults'); module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { var hasNon3D = ( - layoutOut._hasCartesian || - layoutOut._hasGeo || - layoutOut._hasGL2D || - layoutOut._hasPie || - layoutOut._hasTernary + layoutOut._has('cartesian') || + layoutOut._has('geo') || + layoutOut._has('gl2d') || + layoutOut._has('pie') || + layoutOut._has('ternary') ); // some layout-wide attribute are used in all scenes diff --git a/src/plots/plots.js b/src/plots/plots.js index e1811e7c628..8648a7b4603 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -190,8 +190,8 @@ plots.getSubplotIds = function getSubplotIds(layout, type) { if(_module === undefined) return []; // layout must be 'fullLayout' here - if(type === 'cartesian' && !layout._hasCartesian) return []; - if(type === 'gl2d' && !layout._hasGL2D) return []; + if(type === 'cartesian' && (!layout._has || !layout._has('cartesian'))) return []; + if(type === 'gl2d' && (!layout._has || !layout._has('gl2d'))) return []; if(type === 'cartesian' || type === 'gl2d') { return Object.keys(layout._plots || {}); } From 77fb7e20ccd97985fc834d936b1251d94741711d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:33:58 -0400 Subject: [PATCH 19/26] loop over (trace) modules in trace supplyLayoutDefaults: - no need to loop over all the registered module, only the ones that appear in the data suffices. --- src/plots/plots.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 8648a7b4603..6320641f41b 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -764,9 +764,9 @@ plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) { } // trace module layout defaults - var traceTypes = Object.keys(modules); - for(i = 0; i < traceTypes.length; i++) { - _module = modules[allTypes[i]]._module; + var modules = layoutOut._modules; + for(i = 0; i < modules.length; i++) { + _module = modules[i]; if(_module.supplyLayoutDefaults) { _module.supplyLayoutDefaults(layoutIn, layoutOut, fullData); From a141d78cc6767769e1b88d19ae2abe9849b31fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:34:42 -0400 Subject: [PATCH 20/26] robustify pie clean step: - so that it works in cases where _has isn't defined e.g. via Plotly.purge(). --- src/plots/plots.js | 2 +- src/traces/pie/base_plot.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 6320641f41b..3a21119ac92 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -193,7 +193,7 @@ plots.getSubplotIds = function getSubplotIds(layout, type) { if(type === 'cartesian' && (!layout._has || !layout._has('cartesian'))) return []; if(type === 'gl2d' && (!layout._has || !layout._has('gl2d'))) return []; if(type === 'cartesian' || type === 'gl2d') { - return Object.keys(layout._plots || {}); + return Object.keys(layout._plots); } var idRegex = _module.idRegex, diff --git a/src/traces/pie/base_plot.js b/src/traces/pie/base_plot.js index 811060fff4d..c53f3e8ae8f 100644 --- a/src/traces/pie/base_plot.js +++ b/src/traces/pie/base_plot.js @@ -21,7 +21,10 @@ exports.plot = function(gd) { }; exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { - if(oldFullLayout._hasPie && !newFullLayout._hasPie) { + var hadPie = (oldFullLayout._has && oldFullLayout._has('pie')); + var hasPie = (newFullLayout._has && newFullLayout._has('pie')); + + if(hadPie && !hasPie) { oldFullLayout._pielayer.selectAll('g.trace').remove(); } }; From dc297ca1913c3ed61a559c5b679c718a7007ce08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:36:00 -0400 Subject: [PATCH 21/26] put ref of hasPlotType method on Plots: - to more easily mock _has in test layouts --- src/plots/plots.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 3a21119ac92..285066180e4 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -485,9 +485,9 @@ plots.supplyDefaults = function(gd) { Lib.fillUnique(modules, _module); Lib.fillUnique(basePlotModules, fullTrace._module.basePlotModule); } - // attach helper method - newFullLayout._has = hasPlotType.bind(newFullLayout); + // attach helper method to check whether a plot type is present on graph + newFullLayout._has = plots._hasPlotType.bind(newFullLayout); // temporary block (before replace all _has??? with _hasPlotType() ? newFullLayout._hasCartesian = newFullLayout._has('cartesian'); newFullLayout._hasGeo = newFullLayout._has('geo'); @@ -539,7 +539,7 @@ plots.supplyDefaults = function(gd) { // helper function to be bound to fullLayout to check // whether a certain plot type is present on plot -function hasPlotType(category) { +plots._hasPlotType = function(category) { var basePlotModules = this._basePlotModules || []; for(var i = 0; i < basePlotModules.length; i++) { @@ -549,6 +549,7 @@ function hasPlotType(category) { } return false; +}; plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { var i, j; From e3f72f092f16e67345cbabb72da742ced607f4a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:37:18 -0400 Subject: [PATCH 22/26] move block of _has?? = _has('') after all defaults are filled: - so that orphan axes are considered hasCartesian still - IMPORTANT: we must keep ref to _has??? for backward compatibility --- src/plots/plots.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 285066180e4..b99606c0da4 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -488,13 +488,6 @@ plots.supplyDefaults = function(gd) { // attach helper method to check whether a plot type is present on graph newFullLayout._has = plots._hasPlotType.bind(newFullLayout); - // temporary block (before replace all _has??? with _hasPlotType() ? - newFullLayout._hasCartesian = newFullLayout._has('cartesian'); - newFullLayout._hasGeo = newFullLayout._has('geo'); - newFullLayout._hasGL3D = newFullLayout._has('gl3d'); - newFullLayout._hasGL2D = newFullLayout._has('gl2d'); - newFullLayout._hasTernary = newFullLayout._has('ternary'); - newFullLayout._hasPie = newFullLayout._has('pie'); // special cases that introduce interactions between traces for(i = 0; i < modules.length; i++) { @@ -511,6 +504,15 @@ plots.supplyDefaults = function(gd) { // finally, fill in the pieces of layout that may need to look at data plots.supplyLayoutModuleDefaults(newLayout, newFullLayout, newFullData); + // TODO remove in v2.0.0 + // add has-plot-type refs to fullLayout for backward compatibility + newFullLayout._hasCartesian = newFullLayout._has('cartesian'); + newFullLayout._hasGeo = newFullLayout._has('geo'); + newFullLayout._hasGL3D = newFullLayout._has('gl3d'); + newFullLayout._hasGL2D = newFullLayout._has('gl2d'); + newFullLayout._hasTernary = newFullLayout._has('ternary'); + newFullLayout._hasPie = newFullLayout._has('pie'); + // clean subplots and other artifacts from previous plot calls plots.cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout); From c59a9607bf020198fc011259ac04937c491bfa38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 14:37:26 -0400 Subject: [PATCH 23/26] update tests --- test/jasmine/tests/axes_test.js | 21 +++++---- test/jasmine/tests/fx_test.js | 49 +++++++-------------- test/jasmine/tests/geolayout_test.js | 3 +- test/jasmine/tests/gl3dlayout_test.js | 10 +++-- test/jasmine/tests/gl_plot_interact_test.js | 8 ++-- test/jasmine/tests/modebar_test.js | 45 +++++++++---------- test/jasmine/tests/plots_test.js | 6 +-- test/jasmine/tests/ternary_test.js | 2 - 8 files changed, 63 insertions(+), 81 deletions(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index bc61b5603ed..97431237c2f 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -171,7 +171,10 @@ describe('Test axes', function() { var layoutIn, layoutOut, fullData; beforeEach(function() { - layoutOut = {}; + layoutOut = { + _has: Plots._hasPlotType, + _basePlotModules: [] + }; fullData = []; }); @@ -267,7 +270,7 @@ describe('Test axes', function() { fullData = []; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut._hasCartesian).toBe(true); + expect(layoutOut._basePlotModules[0].name).toEqual('cartesian'); }); it('should detect orphan axes (gl2d trace conflict case)', function() { @@ -282,7 +285,7 @@ describe('Test axes', function() { }]; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut._hasCartesian).toBe(undefined); + expect(layoutOut._basePlotModules).toEqual([]); }); it('should detect orphan axes (gl2d + cartesian case)', function() { @@ -297,7 +300,7 @@ describe('Test axes', function() { }]; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut._hasCartesian).toBe(true); + expect(layoutOut._basePlotModules[0].name).toEqual('cartesian'); }); it('should detect orphan axes (gl3d present case)', function() { @@ -305,21 +308,21 @@ describe('Test axes', function() { xaxis: {}, yaxis: {} }; - layoutOut._hasGL3D = true; + layoutOut._basePlotModules = [ { name: 'gl3d' }]; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut._hasCartesian).toBe(undefined); + expect(layoutOut._basePlotModules).toEqual([ { name: 'gl3d' }]); }); - it('should detect orphan axes (gl3d present case)', function() { + it('should detect orphan axes (geo present case)', function() { layoutIn = { xaxis: {}, yaxis: {} }; - layoutOut._hasGeo = true; + layoutOut._basePlotModules = [ { name: 'geo' }]; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut._hasCartesian).toBe(undefined); + expect(layoutOut._basePlotModules).toEqual([ { name: 'geo' }]); }); it('should use \'axis.color\' as default for \'axis.titlefont.color\'', function() { diff --git a/test/jasmine/tests/fx_test.js b/test/jasmine/tests/fx_test.js index b4a27fadc17..59ac3a146fc 100644 --- a/test/jasmine/tests/fx_test.js +++ b/test/jasmine/tests/fx_test.js @@ -1,4 +1,5 @@ var Fx = require('@src/plots/cartesian/graph_interact'); +var Plots = require('@src/plots/plots'); describe('Test FX', function() { @@ -6,22 +7,24 @@ describe('Test FX', function() { describe('defaults', function() { - it('should default (blank version)', function() { - var layoutIn = {}; - var layoutOut = {}; - var fullData = [{}]; + var layoutIn, layoutOut, fullData; + beforeEach(function() { + layoutIn = {}; + layoutOut = { + _has: Plots._hasPlotType + }; + fullData = [{}]; + }); + + it('should default (blank version)', function() { Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); }); it('should default (cartesian version)', function() { - var layoutIn = {}; - var layoutOut = { - _hasCartesian: true - }; - var fullData = [{}]; + layoutOut._basePlotModules = [{ name: 'cartesian' }]; Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.hovermode).toBe('x', 'hovermode to x'); @@ -30,13 +33,8 @@ describe('Test FX', function() { }); it('should default (cartesian horizontal version)', function() { - var layoutIn = {}; - var layoutOut = { - _hasCartesian: true - }; - var fullData = [{ - orientation: 'h' - }]; + layoutOut._basePlotModules = [{ name: 'cartesian' }]; + fullData[0] = { orientation: 'h' }; Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.hovermode).toBe('y', 'hovermode to y'); @@ -45,11 +43,7 @@ describe('Test FX', function() { }); it('should default (gl3d version)', function() { - var layoutIn = {}; - var layoutOut = { - _hasGL3D: true - }; - var fullData = [{}]; + layoutOut._basePlotModules = [{ name: 'gl3d' }]; Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); @@ -57,11 +51,7 @@ describe('Test FX', function() { }); it('should default (geo version)', function() { - var layoutIn = {}; - var layoutOut = { - _hasGeo: true - }; - var fullData = [{}]; + layoutOut._basePlotModules = [{ name: 'geo' }]; Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); @@ -69,12 +59,7 @@ describe('Test FX', function() { }); it('should default (multi plot type version)', function() { - var layoutIn = {}; - var layoutOut = { - _hasCartesian: true, - _hasGL3D: true - }; - var fullData = [{}]; + layoutOut._basePlotModules = [{ name: 'cartesian' }, { name: 'gl3d' }]; Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.hovermode).toBe('x', 'hovermode to x'); diff --git a/test/jasmine/tests/geolayout_test.js b/test/jasmine/tests/geolayout_test.js index 057897cdd4d..2c22715a056 100644 --- a/test/jasmine/tests/geolayout_test.js +++ b/test/jasmine/tests/geolayout_test.js @@ -11,8 +11,7 @@ describe('Test Geo layout defaults', function() { var layoutIn, layoutOut, fullData; beforeEach(function() { - // if hasGeo is not at this stage, the default step is skipped - layoutOut = { _hasGeo: true }; + layoutOut = {}; // needs a geo-ref in a trace in order to be detected fullData = [{ type: 'scattergeo', geo: 'geo' }]; diff --git a/test/jasmine/tests/gl3dlayout_test.js b/test/jasmine/tests/gl3dlayout_test.js index 8a2f4f81cdc..f6e2fd24d07 100644 --- a/test/jasmine/tests/gl3dlayout_test.js +++ b/test/jasmine/tests/gl3dlayout_test.js @@ -1,4 +1,5 @@ var Gl3d = require('@src/plots/gl3d'); +var Plots = require('@src/plots/plots'); var tinycolor = require('tinycolor2'); var Color = require('@src/components/color'); @@ -13,8 +14,9 @@ describe('Test Gl3d layout defaults', function() { var supplyLayoutDefaults = Gl3d.supplyLayoutDefaults; beforeEach(function() { - // if hasGL3D is not at this stage, the default step is skipped - layoutOut = { _hasGL3D: true }; + layoutOut = { + _has: Plots._hasPlotType + }; // needs a scene-ref in a trace in order to be detected fullData = [ { type: 'scatter3d', scene: 'scene' }]; @@ -173,7 +175,7 @@ describe('Test Gl3d layout defaults', function() { .toBe('orbit', 'to user layout val if valid and 3d only'); layoutIn = { scene: {}, dragmode: 'orbit' }; - layoutOut._hasCartesian = true; + layoutOut._basePlotModules = [{ name: 'cartesian' }]; supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.scene.dragmode) .toBe('turntable', 'to default if not 3d only'); @@ -201,7 +203,7 @@ describe('Test Gl3d layout defaults', function() { .toBe(false, 'to user layout val if valid and 3d only'); layoutIn = { scene: {}, hovermode: false }; - layoutOut._hasCartesian = true; + layoutOut._basePlotModules = [{ name: 'cartesian' }]; supplyLayoutDefaults(layoutIn, layoutOut, fullData); expect(layoutOut.scene.hovermode) .toBe('closest', 'to default if not 3d only'); diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index f59a84f3875..70ce1fe0cd3 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -159,7 +159,7 @@ describe('Test gl plot interactions', function() { expect(gd.layout.scene).toEqual(sceneLayout); expect(gd.layout.xaxis).toBeUndefined(); expect(gd.layout.yaxis).toBeUndefined(); - expect(gd._fullLayout._hasGL3D).toBe(true); + expect(gd._fullLayout._has('gl3d')).toBe(true); expect(gd._fullLayout.scene._scene).toBeDefined(); Plotly.restyle(gd, 'type', 'scatter').then(function() { @@ -167,7 +167,7 @@ describe('Test gl plot interactions', function() { expect(gd.layout.scene).toEqual(sceneLayout); expect(gd.layout.xaxis).toBeDefined(); expect(gd.layout.yaxis).toBeDefined(); - expect(gd._fullLayout._hasGL3D).toBe(false); + expect(gd._fullLayout._has('gl3d')).toBe(false); expect(gd._fullLayout.scene).toBeUndefined(); return Plotly.restyle(gd, 'type', 'scatter3d'); @@ -176,7 +176,7 @@ describe('Test gl plot interactions', function() { expect(gd.layout.scene).toEqual(sceneLayout); expect(gd.layout.xaxis).toBeDefined(); expect(gd.layout.yaxis).toBeDefined(); - expect(gd._fullLayout._hasGL3D).toBe(true); + expect(gd._fullLayout._has('gl3d')).toBe(true); expect(gd._fullLayout.scene._scene).toBeDefined(); done(); @@ -186,7 +186,7 @@ describe('Test gl plot interactions', function() { it('should be able to delete the last trace', function(done) { Plotly.deleteTraces(gd, [0]).then(function() { expect(countCanvases()).toEqual(0); - expect(gd._fullLayout._hasGL3D).toBe(false); + expect(gd._fullLayout._has('gl3d')).toBe(false); expect(gd._fullLayout.scene).toBeUndefined(); done(); diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js index 9931091d431..b8a5c460e12 100644 --- a/test/jasmine/tests/modebar_test.js +++ b/test/jasmine/tests/modebar_test.js @@ -4,6 +4,7 @@ var createModeBar = require('@src/components/modebar'); var manageModeBar = require('@src/components/modebar/manage'); var Plotly = require('@lib/index'); +var Plots = require('@src/plots/plots'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var selectButton = require('../assets/modebar_button'); @@ -28,7 +29,8 @@ describe('ModeBar', function() { return { _fullLayout: { dragmode: 'zoom', - _paperdiv: d3.select(getMockContainerTree()) + _paperdiv: d3.select(getMockContainerTree()), + _has: Plots._hasPlotType }, _fullData: [], _context: { @@ -186,7 +188,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }]; gd._fullLayout.xaxis = {fixedrange: false}; manageModeBar(gd); @@ -204,7 +206,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }]; gd._fullLayout.xaxis = {fixedrange: false}; gd._fullData = [{ type: 'scatter', @@ -226,7 +228,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -243,7 +245,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasGL3D = true; + gd._fullLayout._basePlotModules = [{ name: 'gl3d' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -259,7 +261,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasGeo = true; + gd._fullLayout._basePlotModules = [{ name: 'geo' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -276,7 +278,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasGL2D = true; + gd._fullLayout._basePlotModules = [{ name: 'gl2d' }]; gd._fullLayout.xaxis = {fixedrange: false}; manageModeBar(gd); @@ -292,7 +294,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasPie = true; + gd._fullLayout._basePlotModules = [{ name: 'pie' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -307,8 +309,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; - gd._fullLayout._hasGL3D = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }, { name: 'gl3d' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -323,8 +324,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; - gd._fullLayout._hasGeo = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }, { name: 'geo' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -341,7 +341,6 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; gd._fullData = [{ type: 'scatter', visible: true, @@ -349,7 +348,7 @@ describe('ModeBar', function() { _module: {selectPoints: true} }]; gd._fullLayout.xaxis = {fixedrange: false}; - gd._fullLayout._hasPie = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }, { name: 'pie' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -364,8 +363,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasGL3D = true; - gd._fullLayout._hasGeo = true; + gd._fullLayout._basePlotModules = [{ name: 'geo' }, { name: 'gl3d' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -380,7 +378,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasTernary = true; + gd._fullLayout._basePlotModules = [{ name: 'ternary' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -395,13 +393,13 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasTernary = true; gd._fullData = [{ type: 'scatterternary', visible: true, mode: 'markers', _module: {selectPoints: true} }]; + gd._fullLayout._basePlotModules = [{ name: 'ternary' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -417,8 +415,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasTernary = true; - gd._fullLayout._hasCartesian = true; + gd._fullLayout._basePlotModules = [{ name: 'ternary' }, { name: 'cartesian' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -433,8 +430,7 @@ describe('ModeBar', function() { ]); var gd = getMockGraphInfo(); - gd._fullLayout._hasTernary = true; - gd._fullLayout._hasGL3D = true; + gd._fullLayout._basePlotModules = [{ name: 'ternary' }, { name: 'gl3d' }]; manageModeBar(gd); var modeBar = gd._fullLayout._modeBar; @@ -487,7 +483,7 @@ describe('ModeBar', function() { // gives 11 buttons in 5 groups by default function setupGraphInfo() { var gd = getMockGraphInfo(); - gd._fullLayout._hasCartesian = true; + gd._fullLayout._basePlotModules = [{ name: 'cartesian' }]; gd._fullLayout.xaxis = {fixedrange: false}; return gd; } @@ -496,8 +492,7 @@ describe('ModeBar', function() { var gd = setupGraphInfo(); manageModeBar(gd); - gd._fullLayout._hasCartesian = false; - gd._fullLayout._hasGL3D = true; + gd._fullLayout._basePlotModules = [{ name: 'gl3d' }]; manageModeBar(gd); expect(countButtons(gd._fullLayout._modeBar)).toEqual(10); diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 8dfbf9c71d3..edd2254d557 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -140,20 +140,20 @@ describe('Test Plots', function() { it('returns cartesian ids', function() { var layout = { + _has: Plots._hasPlotType, _plots: { xy: {}, x2y2: {} } }; expect(getSubplotIds(layout, 'cartesian')) .toEqual([]); - layout._hasCartesian = true; + layout._basePlotModules = [{ name: 'cartesian' }]; expect(getSubplotIds(layout, 'cartesian')) .toEqual(['xy', 'x2y2']); expect(getSubplotIds(layout, 'gl2d')) .toEqual([]); - delete layout._hasCartesian; - layout._hasGL2D = true; + layout._basePlotModules = [{ name: 'gl2d' }]; expect(getSubplotIds(layout, 'gl2d')) .toEqual(['xy', 'x2y2']); expect(getSubplotIds(layout, 'cartesian')) diff --git a/test/jasmine/tests/ternary_test.js b/test/jasmine/tests/ternary_test.js index 167d46c4b54..b933dae528c 100644 --- a/test/jasmine/tests/ternary_test.js +++ b/test/jasmine/tests/ternary_test.js @@ -230,9 +230,7 @@ describe('ternary defaults', function() { var layoutIn, layoutOut, fullData; beforeEach(function() { - // if hasTernary is not at this stage, the default step is skipped layoutOut = { - _hasTernary: true, font: { color: 'red' } }; From 9f94494fd4dc4c9d08c72e40c9f3ab53a9e2e96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 6 May 2016 15:33:07 -0400 Subject: [PATCH 24/26] add info about data/layout and fullData/fullLayout --- src/plots/plots.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index b99606c0da4..3f3bf129853 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -438,16 +438,19 @@ plots.sendDataToCloud = function(gd) { // Fill in default values: // // gd.data, gd.layout: -// are precisely what the user specified +// are precisely what the user specified, +// these fields shouldn't be modified nor used directly +// after the supply defaults step. // // gd._fullData, gd._fullLayout: -// are complete descriptions of how to draw the plot +// are complete descriptions of how to draw the plot, +// use these fields in all required computations. // // gd._fullLayout._modules -// is a list of all the trace modules required to draw the plot +// is a list of all the trace modules required to draw the plot. // // gd._fullLayout._basePlotModules -// is a list of all the plot modules required to draw the plot +// is a list of all the plot modules required to draw the plot. // plots.supplyDefaults = function(gd) { var oldFullLayout = gd._fullLayout || {}, From 0c7b2f800ca21e0ddbc3f7e72189231150b00bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 9 May 2016 17:16:13 -0400 Subject: [PATCH 25/26] rename Lib.fillUnique -> Lib.pushUnique --- src/lib/index.js | 4 ++-- src/plots/cartesian/layout_defaults.js | 2 +- src/plots/plots.js | 4 ++-- test/jasmine/tests/lib_test.js | 18 +++++++++--------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lib/index.js b/src/lib/index.js index b087e81c3f8..9f8d40b0928 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -336,7 +336,7 @@ lib.noneOrAll = function(containerIn, containerOut, attrList) { }; /** - * Fill with unique items + * Push array with unique items * * @param {array} array * array to be filled @@ -346,7 +346,7 @@ lib.noneOrAll = function(containerIn, containerOut, attrList) { * ref to array (now possibly containing one more item) * */ -lib.fillUnique = function(array, item) { +lib.pushUnique = function(array, item) { if(item && array.indexOf(item) === -1) array.push(item); return array; diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 2ea5a2d52bd..15273f50b33 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -95,7 +95,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { // make sure that plots with orphan cartesian axes // are considered 'cartesian' if(xaListCartesian.length && yaListCartesian.length) { - Lib.fillUnique(layoutOut._basePlotModules, Plots.subplotsRegistry.cartesian); + Lib.pushUnique(layoutOut._basePlotModules, Plots.subplotsRegistry.cartesian); } function axSort(a, b) { diff --git a/src/plots/plots.js b/src/plots/plots.js index 3f3bf129853..f71e4733d72 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -485,8 +485,8 @@ plots.supplyDefaults = function(gd) { if(!_module) continue; // fill in module lists - Lib.fillUnique(modules, _module); - Lib.fillUnique(basePlotModules, fullTrace._module.basePlotModule); + Lib.pushUnique(modules, _module); + Lib.pushUnique(basePlotModules, fullTrace._module.basePlotModule); } // attach helper method to check whether a plot type is present on graph diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 234aeb406da..e30a7378e6f 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -857,7 +857,7 @@ describe('Test lib.js:', function() { }); }); - describe('fillUnique', function() { + describe('pushUnique', function() { beforeEach(function() { this.obj = { a: 'A' }; @@ -865,34 +865,34 @@ describe('Test lib.js:', function() { }); it('should fill new items in array', function() { - var out = Lib.fillUnique(this.array, 'd'); + var out = Lib.pushUnique(this.array, 'd'); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }, 'd']); expect(this.array).toBe(out); }); it('should ignore falsy items', function() { - Lib.fillUnique(this.array, false); + Lib.pushUnique(this.array, false); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); - Lib.fillUnique(this.array, undefined); + Lib.pushUnique(this.array, undefined); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); - Lib.fillUnique(this.array, 0); + Lib.pushUnique(this.array, 0); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); - Lib.fillUnique(this.array, null); + Lib.pushUnique(this.array, null); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); - Lib.fillUnique(this.array, ''); + Lib.pushUnique(this.array, ''); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); }); it('should ignore item already in array', function() { - Lib.fillUnique(this.array, 'a'); + Lib.pushUnique(this.array, 'a'); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); - Lib.fillUnique(this.array, this.obj); + Lib.pushUnique(this.array, this.obj); expect(this.array).toEqual(['a', 'b', 'c', { a: 'A' }]); }); From 5e6759b10d47fe7d8dd11e641b019bb0169930da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 9 May 2016 17:23:32 -0400 Subject: [PATCH 26/26] rm unnecessary boolean type coercion --- src/components/modebar/manage.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/modebar/manage.js b/src/components/modebar/manage.js index 7abc6cb330c..eacfabbfe3b 100644 --- a/src/components/modebar/manage.js +++ b/src/components/modebar/manage.js @@ -73,12 +73,12 @@ function getButtonGroups(gd, buttonsToRemove, buttonsToAdd) { var fullLayout = gd._fullLayout, fullData = gd._fullData; - var hasCartesian = !!fullLayout._has('cartesian'), - hasGL3D = !!fullLayout._has('gl3d'), - hasGeo = !!fullLayout._has('geo'), - hasPie = !!fullLayout._has('pie'), - hasGL2D = !!fullLayout._has('gl2d'), - hasTernary = !!fullLayout._has('ternary'); + var hasCartesian = fullLayout._has('cartesian'), + hasGL3D = fullLayout._has('gl3d'), + hasGeo = fullLayout._has('geo'), + hasPie = fullLayout._has('pie'), + hasGL2D = fullLayout._has('gl2d'), + hasTernary = fullLayout._has('ternary'); var groups = [];