refactor calcdata filtering

more efficient, DRY, and without the hack of basePlot.attr='type'

Author: alexcjohnson

src/plots/cartesian/index.js

@@ -12,6 +12,7 @@
 var d3 = require('d3');
 var Lib = require('../../lib');
 var Plots = require('../plots');
+var getModuleCalcData = require('../get_calcdata').getModuleCalcData;
 
 var axisIds = require('./axis_ids');
 var constants = require('./constants');
@@ -127,15 +128,7 @@ function plotOne(gd, plotinfo, cdSubplot, transitionOpts, makeOnCompleteCallback
         if(_module.basePlotModule.name !== 'cartesian') continue;
 
         // plot all traces of this type on this subplot at once
-        var cdModule = [];
-        for(var k = 0; k < cdSubplot.length; k++) {
-            var cd = cdSubplot[k],
-                trace = cd[0].trace;
-
-            if((trace._module === _module) && (trace.visible === true)) {
-                cdModule.push(cd);
-            }
-        }
+        var cdModule = getModuleCalcData(cdSubplot, _module);
 
         _module.plot(gd, plotinfo, cdModule, transitionOpts, makeOnCompleteCallback);
     }

src/plots/geo/index.js

@@ -11,6 +11,7 @@
 
 var createGeo = require('./geo');
 var Plots = require('../../plots/plots');
+var getSubplotCalcData = require('../../plots/get_calcdata').getSubplotCalcData;
 var counterRegex = require('../../lib').counterRegex;
 
 var GEO = 'geo';
@@ -44,7 +45,7 @@ exports.plot = function plotGeo(gd) {
 
     for(var i = 0; i < geoIds.length; i++) {
         var geoId = geoIds[i];
-        var geoCalcData = Plots.getSubplotCalcData(calcData, GEO, geoId);
+        var geoCalcData = getSubplotCalcData(calcData, GEO, geoId);
         var geoLayout = fullLayout[geoId];
         var geo = geoLayout._subplot;
 

src/plots/get_calcdata.js

@@ -0,0 +1,52 @@
+/**
+* Copyright 2012-2017, 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 Registry = require('../registry');
+
+/**
+ * Get calcdata traces(s) associated with a given subplot
+ *
+ * @param {array} calcData (as in gd.calcdata)
+ * @param {string} type subplot type
+ * @param {string} subplotId subplot id to look for
+ *
+ * @return {array} array of calcdata traces
+ */
+exports.getSubplotCalcData = function(calcData, type, subplotId) {
+    var basePlotModule = Registry.subplotsRegistry[type];
+    if(!basePlotModule) return [];
+
+    var attr = basePlotModule.attr;
+    var subplotCalcData = [];
+
+    for(var i = 0; i < calcData.length; i++) {
+        var calcTrace = calcData[i];
+        var trace = calcTrace[0].trace;
+
+        if(trace[attr] === subplotId) subplotCalcData.push(calcTrace);
+    }
+
+    return subplotCalcData;
+};
+
+exports.getModuleCalcData = function(calcdata, typeOrModule) {
+    var moduleCalcData = [];
+    var _module = typeof typeOrModule === 'string' ? Registry.getModule(typeOrModule) : typeOrModule;
+    if(!_module) return moduleCalcData;
+
+    for(var i = 0; i < calcdata.length; i++) {
+        var cd = calcdata[i];
+        var trace = cd[0].trace;
+
+        if((trace._module === _module) && (trace.visible === true)) moduleCalcData.push(cd);
+    }
+
+    return moduleCalcData;
+};

src/plots/mapbox/index.js

@@ -13,6 +13,7 @@ var mapboxgl = require('mapbox-gl');
 
 var Lib = require('../../lib');
 var Plots = require('../plots');
+var getSubplotCalcData = require('../../plots/get_calcdata').getSubplotCalcData;
 var xmlnsNamespaces = require('../../constants/xmlns_namespaces');
 
 var createMapbox = require('./mapbox');
@@ -58,7 +59,7 @@ exports.plot = function plotMapbox(gd) {
 
     for(var i = 0; i < mapboxIds.length; i++) {
         var id = mapboxIds[i],
-            subplotCalcData = Plots.getSubplotCalcData(calcData, MAPBOX, id),
+            subplotCalcData = getSubplotCalcData(calcData, MAPBOX, id),
             opts = fullLayout[id],
             mapbox = opts._subplot;
 

src/plots/plots.js

@@ -162,31 +162,6 @@ plots.getSubplotData = function getSubplotData(data, type, subplotId) {
     return subplotData;
 };
 
-/**
- * Get calcdata traces(s) associated with a given subplot
- *
- * @param {array} calcData (as in gd.calcdata)
- * @param {string} type subplot type
- * @param {string} subplotId subplot id to look for
- *
- * @return {array} array of calcdata traces
- */
-plots.getSubplotCalcData = function(calcData, type, subplotId) {
-    if(!plots.subplotsRegistry[type]) return [];
-
-    var attr = plots.subplotsRegistry[type].attr;
-    var subplotCalcData = [];
-
-    for(var i = 0; i < calcData.length; i++) {
-        var calcTrace = calcData[i],
-            trace = calcTrace[0].trace;
-
-        if(trace[attr] === subplotId) subplotCalcData.push(calcTrace);
-    }
-
-    return subplotCalcData;
-};
-
 // in some cases the browser doesn't seem to know how big
 // the text is at first, so it needs to draw it,
 // then wait a little, then draw it again

src/plots/ternary/index.js

@@ -12,6 +12,7 @@
 var Ternary = require('./ternary');
 
 var Plots = require('../../plots/plots');
+var getSubplotCalcData = require('../../plots/get_calcdata').getSubplotCalcData;
 var counterRegex = require('../../lib').counterRegex;
 var TERNARY = 'ternary';
 
@@ -36,7 +37,7 @@ exports.plot = function plotTernary(gd) {
 
     for(var i = 0; i < ternaryIds.length; i++) {
         var ternaryId = ternaryIds[i],
-            ternaryCalcData = Plots.getSubplotCalcData(calcData, TERNARY, ternaryId),
+            ternaryCalcData = getSubplotCalcData(calcData, TERNARY, ternaryId),
             ternary = fullLayout[ternaryId]._subplot;
 
         // If ternary is not instantiated, create one!

src/traces/parcoords/base_plot.js

@@ -9,22 +9,22 @@
 'use strict';
 
 var d3 = require('d3');
-var Plots = require('../../plots/plots');
+var getModuleCalcData = require('../../plots/get_calcdata').getModuleCalcData;
 var parcoordsPlot = require('./plot');
 var xmlnsNamespaces = require('../../constants/xmlns_namespaces');
 
-exports.name = 'parcoords';
+var PARCOORDS = 'parcoords';
 
-exports.attr = 'type';
+exports.name = PARCOORDS;
 
 exports.plot = function(gd) {
-    var calcData = Plots.getSubplotCalcData(gd.calcdata, 'parcoords', 'parcoords');
+    var calcData = getModuleCalcData(gd.calcdata, PARCOORDS);
     if(calcData.length) parcoordsPlot(gd, calcData);
 };
 
 exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
-    var hadParcoords = (oldFullLayout._has && oldFullLayout._has('parcoords'));
-    var hasParcoords = (newFullLayout._has && newFullLayout._has('parcoords'));
+    var hadParcoords = (oldFullLayout._has && oldFullLayout._has(PARCOORDS));
+    var hasParcoords = (newFullLayout._has && newFullLayout._has(PARCOORDS));
 
     if(hadParcoords && !hasParcoords) {
         oldFullLayout._paperdiv.selectAll('.parcoords').remove();

src/traces/sankey/base_plot.js

@@ -9,26 +9,26 @@
 'use strict';
 
 var overrideAll = require('../../plot_api/edit_types').overrideAll;
-var Plots = require('../../plots/plots');
+var getModuleCalcData = require('../../plots/get_calcdata').getModuleCalcData;
 var plot = require('./plot');
 var fxAttrs = require('../../components/fx/layout_attributes');
 
-exports.name = 'sankey';
+var SANKEY = 'sankey';
 
-exports.attr = 'type';
+exports.name = SANKEY;
 
 exports.baseLayoutAttrOverrides = overrideAll({
     hoverlabel: fxAttrs.hoverlabel
 }, 'plot', 'nested');
 
 exports.plot = function(gd) {
-    var calcData = Plots.getSubplotCalcData(gd.calcdata, 'sankey', 'sankey');
+    var calcData = getModuleCalcData(gd.calcdata, SANKEY);
     if(calcData.length) plot(gd, calcData);
 };
 
 exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
-    var hadPlot = (oldFullLayout._has && oldFullLayout._has('sankey'));
-    var hasPlot = (newFullLayout._has && newFullLayout._has('sankey'));
+    var hadPlot = (oldFullLayout._has && oldFullLayout._has(SANKEY));
+    var hasPlot = (newFullLayout._has && newFullLayout._has(SANKEY));
 
     if(hadPlot && !hasPlot) {
         oldFullLayout._paperdiv.selectAll('.sankey').remove();

src/traces/table/base_plot.js

@@ -8,21 +8,21 @@
 
 'use strict';
 
-var Plots = require('../../plots/plots');
+var getModuleCalcData = require('../../plots/get_calcdata').getModuleCalcData;
 var tablePlot = require('./plot');
 
-exports.name = 'table';
+var TABLE = 'table';
 
-exports.attr = 'type';
+exports.name = TABLE;
 
 exports.plot = function(gd) {
-    var calcData = Plots.getSubplotCalcData(gd.calcdata, 'table', 'table');
+    var calcData = getModuleCalcData(gd.calcdata, TABLE);
     if(calcData.length) tablePlot(gd, calcData);
 };
 
 exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
-    var hadTable = (oldFullLayout._has && oldFullLayout._has('table'));
-    var hasTable = (newFullLayout._has && newFullLayout._has('table'));
+    var hadTable = (oldFullLayout._has && oldFullLayout._has(TABLE));
+    var hasTable = (newFullLayout._has && newFullLayout._has(TABLE));
 
     if(hadTable && !hasTable) {
         oldFullLayout._paperdiv.selectAll('.table').remove();

test/jasmine/tests/plots_test.js

@@ -596,7 +596,9 @@ describe('Test Plots', function() {
         });
     });
 
-    describe('Plots.getSubplotCalcData', function() {
+    describe('getSubplotCalcData', function() {
+        var getSubplotCalcData = require('@src/plots/get_calcdata').getSubplotCalcData;
+
         var trace0 = { geo: 'geo2' };
         var trace1 = { subplot: 'ternary10' };
         var trace2 = { subplot: 'ternary10' };
@@ -608,22 +610,22 @@ describe('Test Plots', function() {
         ];
 
         it('should extract calcdata traces associated with subplot (1)', function() {
-            var out = Plots.getSubplotCalcData(cd, 'geo', 'geo2');
+            var out = getSubplotCalcData(cd, 'geo', 'geo2');
             expect(out).toEqual([[{ trace: trace0 }]]);
         });
 
         it('should extract calcdata traces associated with subplot (2)', function() {
-            var out = Plots.getSubplotCalcData(cd, 'ternary', 'ternary10');
+            var out = getSubplotCalcData(cd, 'ternary', 'ternary10');
             expect(out).toEqual([[{ trace: trace1 }], [{ trace: trace2 }]]);
         });
 
         it('should return [] when no calcdata traces where found', function() {
-            var out = Plots.getSubplotCalcData(cd, 'geo', 'geo');
+            var out = getSubplotCalcData(cd, 'geo', 'geo');
             expect(out).toEqual([]);
         });
 
         it('should return [] when subplot type is invalid', function() {
-            var out = Plots.getSubplotCalcData(cd, 'non-sense', 'geo2');
+            var out = getSubplotCalcData(cd, 'non-sense', 'geo2');
             expect(out).toEqual([]);
         });
     });