Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean subplots #2227

Merged
merged 17 commits into from
Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion src/plots/geo/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;

Expand Down
52 changes: 52 additions & 0 deletions src/plots/get_calcdata.js
Original file line number Diff line number Diff line change
@@ -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;
};
3 changes: 2 additions & 1 deletion src/plots/mapbox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;

Expand Down
25 changes: 0 additions & 25 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious. Why did you move this to a separate file? Does leaving it in plots/plots.js lead to a circular dependency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you move this to a separate file?

I don't recall if it made more circular deps or not, but I don't like plots/plots - it has its fingers in everything. Plus I was making 2 (3 as of the next commit) similar simple functions for filtering down data or calcdata that only depend on Registry so it seemed natural to group them in their own file.

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
Expand Down
3 changes: 2 additions & 1 deletion src/plots/ternary/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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!
Expand Down
12 changes: 6 additions & 6 deletions src/traces/parcoords/base_plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions src/traces/sankey/base_plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions src/traces/table/base_plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 7 additions & 5 deletions test/jasmine/tests/plots_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
Expand All @@ -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([]);
});
});
Expand Down