diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index e1ab1c92463..75bd8e37261 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -15,8 +15,9 @@ var d3 = require('d3'); var Color = require('../../components/color'); var Drawing = require('../../components/drawing'); -var Axes = require('../../plots/cartesian/axes'); -var Fx = require('../../plots/cartesian/graph_interact'); +var Plots = require('../plots'); +var Axes = require('../cartesian/axes'); +var Fx = require('../cartesian/graph_interact'); var addProjectionsToD3 = require('./projections'); var createGeoScale = require('./set_scale'); @@ -170,66 +171,13 @@ proto.plot = function(geoCalcData, fullLayout, promises) { }; proto.onceTopojsonIsLoaded = function(geoCalcData, geoLayout) { - var i; - this.drawLayout(geoLayout); - var traceHashOld = this.traceHash; - var traceHash = {}; - - for(i = 0; i < geoCalcData.length; i++) { - var calcData = geoCalcData[i], - trace = calcData[0].trace; - - traceHash[trace.type] = traceHash[trace.type] || []; - traceHash[trace.type].push(calcData); - } - - var moduleNamesOld = Object.keys(traceHashOld); - var moduleNames = Object.keys(traceHash); - - // when a trace gets deleted, make sure that its module's - // plot method is called so that it is properly - // removed from the DOM. - for(i = 0; i < moduleNamesOld.length; i++) { - var moduleName = moduleNamesOld[i]; - - if(moduleNames.indexOf(moduleName) === -1) { - var fakeCalcTrace = traceHashOld[moduleName][0], - fakeTrace = fakeCalcTrace[0].trace; - - fakeTrace.visible = false; - traceHash[moduleName] = [fakeCalcTrace]; - } - } - - moduleNames = Object.keys(traceHash); - - for(i = 0; i < moduleNames.length; i++) { - var moduleCalcData = traceHash[moduleNames[i]], - _module = moduleCalcData[0][0].trace._module; - - _module.plot(this, filterVisible(moduleCalcData), geoLayout); - } - - this.traceHash = traceHash; + Plots.generalUpdatePerTraceModule(this, geoCalcData, geoLayout); this.render(); }; -function filterVisible(calcDataIn) { - var calcDataOut = []; - - for(var i = 0; i < calcDataIn.length; i++) { - var calcTrace = calcDataIn[i], - trace = calcTrace[0].trace; - - if(trace.visible === true) calcDataOut.push(calcTrace); - } - - return calcDataOut; -} - proto.updateFx = function(hovermode) { this.showHover = (hovermode !== false); diff --git a/src/plots/geo/index.js b/src/plots/geo/index.js index 379783add7d..03cfbdf3393 100644 --- a/src/plots/geo/index.js +++ b/src/plots/geo/index.js @@ -45,11 +45,11 @@ exports.plot = function plotGeo(gd) { for(var i = 0; i < geoIds.length; i++) { var geoId = geoIds[i], - geoCalcData = getSubplotCalcData(calcData, geoId), + geoCalcData = Plots.getSubplotCalcData(calcData, 'geo', geoId), geo = fullLayout[geoId]._subplot; // If geo is not instantiated, create one! - if(geo === undefined) { + if(!geo) { geo = new Geo({ id: geoId, graphDiv: gd, @@ -102,16 +102,3 @@ exports.toSVG = function(gd) { .appendChild(geoFramework.node()); } }; - -function getSubplotCalcData(calcData, id) { - var subplotCalcData = []; - - for(var i = 0; i < calcData.length; i++) { - var calcTrace = calcData[i], - trace = calcTrace[0].trace; - - if(trace.geo === id) subplotCalcData.push(calcTrace); - } - - return subplotCalcData; -} diff --git a/src/plots/mapbox/index.js b/src/plots/mapbox/index.js index 806b23fac70..25d5ef7dafc 100644 --- a/src/plots/mapbox/index.js +++ b/src/plots/mapbox/index.js @@ -56,7 +56,7 @@ exports.plot = function plotMapbox(gd) { for(var i = 0; i < mapboxIds.length; i++) { var id = mapboxIds[i], - subplotCalcData = getSubplotCalcData(calcData, id), + subplotCalcData = Plots.getSubplotCalcData(calcData, 'mapbox', id), opts = fullLayout[id], mapbox = opts._subplot; @@ -118,19 +118,6 @@ exports.toSVG = function(gd) { } }; -function getSubplotCalcData(calcData, id) { - var subplotCalcData = []; - - for(var i = 0; i < calcData.length; i++) { - var calcTrace = calcData[i], - trace = calcTrace[0].trace; - - if(trace.subplot === id) subplotCalcData.push(calcTrace); - } - - return subplotCalcData; -} - function findAccessToken(gd, mapboxIds) { var fullLayout = gd._fullLayout, context = gd._context; diff --git a/src/plots/plots.js b/src/plots/plots.js index a1d853d40ef..3fd99071263 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -63,7 +63,7 @@ plots.hasSimpleAPICommandBindings = commandModule.hasSimpleAPICommandBindings; plots.findSubplotIds = function findSubplotIds(data, type) { var subplotIds = []; - if(plots.subplotsRegistry[type] === undefined) return subplotIds; + if(!plots.subplotsRegistry[type]) return subplotIds; var attr = plots.subplotsRegistry[type].attr; @@ -90,7 +90,7 @@ plots.findSubplotIds = function findSubplotIds(data, type) { plots.getSubplotIds = function getSubplotIds(layout, type) { var _module = plots.subplotsRegistry[type]; - if(_module === undefined) return []; + if(!_module) return []; // layout must be 'fullLayout' here if(type === 'cartesian' && (!layout._has || !layout._has('cartesian'))) return []; @@ -124,14 +124,14 @@ plots.getSubplotIds = function getSubplotIds(layout, type) { * Get the data trace(s) associated with a given subplot. * * @param {array} data plotly full data array. - * @param {object} layout plotly full layout object. - * @param {string} subplotId subplot ids to look for. + * @param {string} type subplot type to look for. + * @param {string} subplotId subplot id to look for. * * @return {array} list of trace objects. * */ plots.getSubplotData = function getSubplotData(data, type, subplotId) { - if(plots.subplotsRegistry[type] === undefined) return []; + if(!plots.subplotsRegistry[type]) return []; var attr = plots.subplotsRegistry[type].attr, subplotData = [], @@ -157,6 +157,31 @@ 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 @@ -2027,3 +2052,67 @@ plots.doCalcdata = function(gd, traces) { calcdata[i] = cd; } }; + +plots.generalUpdatePerTraceModule = function(subplot, subplotCalcData, subplotLayout) { + var traceHashOld = subplot.traceHash, + traceHash = {}, + i; + + function filterVisible(calcDataIn) { + var calcDataOut = []; + + for(var i = 0; i < calcDataIn.length; i++) { + var calcTrace = calcDataIn[i], + trace = calcTrace[0].trace; + + if(trace.visible === true) calcDataOut.push(calcTrace); + } + + return calcDataOut; + } + + // build up moduleName -> calcData hash + for(i = 0; i < subplotCalcData.length; i++) { + var calcTraces = subplotCalcData[i], + trace = calcTraces[0].trace; + + // skip over visible === false traces + // as they don't have `_module` ref + if(trace.visible) { + traceHash[trace.type] = traceHash[trace.type] || []; + traceHash[trace.type].push(calcTraces); + } + } + + var moduleNamesOld = Object.keys(traceHashOld); + var moduleNames = Object.keys(traceHash); + + // when a trace gets deleted, make sure that its module's + // plot method is called so that it is properly + // removed from the DOM. + for(i = 0; i < moduleNamesOld.length; i++) { + var moduleName = moduleNamesOld[i]; + + if(moduleNames.indexOf(moduleName) === -1) { + var fakeCalcTrace = traceHashOld[moduleName][0], + fakeTrace = fakeCalcTrace[0].trace; + + fakeTrace.visible = false; + traceHash[moduleName] = [fakeCalcTrace]; + } + } + + // update list of module names to include 'fake' traces added above + moduleNames = Object.keys(traceHash); + + // call module plot method + for(i = 0; i < moduleNames.length; i++) { + var moduleCalcData = traceHash[moduleNames[i]], + _module = moduleCalcData[0][0].trace._module; + + _module.plot(subplot, filterVisible(moduleCalcData), subplotLayout); + } + + // update moduleName -> calcData hash + subplot.traceHash = traceHash; +}; diff --git a/src/plots/ternary/index.js b/src/plots/ternary/index.js index 4b1350f87e7..e1da80af7b1 100644 --- a/src/plots/ternary/index.js +++ b/src/plots/ternary/index.js @@ -32,16 +32,16 @@ exports.supplyLayoutDefaults = require('./layout/defaults'); exports.plot = function plotTernary(gd) { var fullLayout = gd._fullLayout, - fullData = gd._fullData, + calcData = gd.calcdata, ternaryIds = Plots.getSubplotIds(fullLayout, 'ternary'); for(var i = 0; i < ternaryIds.length; i++) { var ternaryId = ternaryIds[i], - fullTernaryData = Plots.getSubplotData(fullData, 'ternary', ternaryId), + ternaryCalcData = Plots.getSubplotCalcData(calcData, 'ternary', ternaryId), ternary = fullLayout[ternaryId]._subplot; // If ternary is not instantiated, create one! - if(ternary === undefined) { + if(!ternary) { ternary = new Ternary({ id: ternaryId, graphDiv: gd, @@ -53,7 +53,7 @@ exports.plot = function plotTernary(gd) { fullLayout[ternaryId]._subplot = ternary; } - ternary.plot(fullTernaryData, fullLayout, gd._promises); + ternary.plot(ternaryCalcData, fullLayout, gd._promises); } }; diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index 83fad349f27..547e427af99 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -18,6 +18,7 @@ var Color = require('../../components/color'); var Drawing = require('../../components/drawing'); var setConvert = require('../cartesian/set_convert'); var extendFlat = require('../../lib/extend').extendFlat; +var Plots = require('../plots'); var Axes = require('../cartesian/axes'); var dragElement = require('../../components/dragelement'); var Titles = require('../../components/titles'); @@ -44,59 +45,14 @@ proto.init = function(fullLayout) { this.traceHash = {}; }; -proto.plot = function(ternaryData, fullLayout) { +proto.plot = function(ternaryCalcData, fullLayout) { var _this = this, ternaryLayout = fullLayout[_this.id], - graphSize = fullLayout._size, - i; - - if(Lib.getPlotDiv(_this.plotContainer.node()) !== _this.graphDiv) { - // someone deleted the framework - remake it - // TODO: this is getting deleted in (cartesian) makePlotFramework - // turn that into idiomatic d3 (enter/exit, the piece I didn't know - // before was ordering selections) so we don't need this. - _this.init(_this.graphDiv._fullLayout); - _this.makeFramework(); - } + graphSize = fullLayout._size; _this.adjustLayout(ternaryLayout, graphSize); - var traceHashOld = _this.traceHash; - var traceHash = {}; - - for(i = 0; i < ternaryData.length; i++) { - var trace = ternaryData[i]; - - traceHash[trace.type] = traceHash[trace.type] || []; - traceHash[trace.type].push(trace); - } - - var moduleNamesOld = Object.keys(traceHashOld); - var moduleNames = Object.keys(traceHash); - - // when a trace gets deleted, make sure that its module's - // plot method is called so that it is properly - // removed from the DOM. - for(i = 0; i < moduleNamesOld.length; i++) { - var moduleName = moduleNamesOld[i]; - - if(moduleNames.indexOf(moduleName) === -1) { - var fakeModule = traceHashOld[moduleName][0]; - fakeModule.visible = false; - traceHash[moduleName] = [fakeModule]; - } - } - - moduleNames = Object.keys(traceHash); - - for(i = 0; i < moduleNames.length; i++) { - var moduleData = traceHash[moduleNames[i]]; - var _module = moduleData[0]._module; - - _module.plot(_this, Lib.filterVisible(moduleData), ternaryLayout); - } - - _this.traceHash = traceHash; + Plots.generalUpdatePerTraceModule(_this, ternaryCalcData, ternaryLayout); _this.layers.plotbg.select('path').call(Color.fill, ternaryLayout.bgcolor); }; diff --git a/src/traces/scatterternary/plot.js b/src/traces/scatterternary/plot.js index c3bb85340eb..0ecfaa25601 100644 --- a/src/traces/scatterternary/plot.js +++ b/src/traces/scatterternary/plot.js @@ -12,7 +12,7 @@ var scatterPlot = require('../scatter/plot'); -module.exports = function plot(ternary, data) { +module.exports = function plot(ternary, moduleCalcData) { var plotContainer = ternary.plotContainer; // remove all nodes inside the scatter layer @@ -25,20 +25,10 @@ module.exports = function plot(ternary, data) { plot: plotContainer }; - var calcdata = new Array(data.length), - fullCalcdata = ternary.graphDiv.calcdata; - - for(var i = 0; i < fullCalcdata.length; i++) { - var j = data.indexOf(fullCalcdata[i][0].trace); - - if(j === -1) continue; - - calcdata[j] = fullCalcdata[i]; - - // while we're here and have references to both the Ternary object - // and fullData, connect the two (for use by hover) - data[j]._ternary = ternary; + // add ref to ternary subplot object in fullData traces + for(var i = 0; i < moduleCalcData.length; i++) { + moduleCalcData[i][0].trace._ternary = ternary; } - scatterPlot(ternary.graphDiv, plotinfo, calcdata); + scatterPlot(ternary.graphDiv, plotinfo, moduleCalcData); }; diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 60afb54d805..0e702ec5b89 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -1,5 +1,7 @@ var Plotly = require('@lib/index'); var Plots = require('@src/plots/plots'); + +var d3 = require('d3'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); @@ -549,4 +551,165 @@ describe('Test Plots', function() { }); }); }); + + describe('Plots.getSubplotCalcData', function() { + var trace0 = { geo: 'geo2' }; + var trace1 = { subplot: 'ternary10' }; + var trace2 = { subplot: 'ternary10' }; + + var cd = [ + [{ trace: trace0 }], + [{ trace: trace1 }], + [{ trace: trace2}] + ]; + + it('should extract calcdata traces associated with subplot (1)', function() { + var out = Plots.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'); + expect(out).toEqual([[{ trace: trace1 }], [{ trace: trace2 }]]); + }); + + it('should return [] when no calcdata traces where found', function() { + var out = Plots.getSubplotCalcData(cd, 'geo', 'geo'); + expect(out).toEqual([]); + }); + + it('should return [] when subplot type is invalid', function() { + var out = Plots.getSubplotCalcData(cd, 'non-sense', 'geo2'); + expect(out).toEqual([]); + }); + }); + + describe('Plots.generalUpdatePerTraceModule', function() { + + function _update(subplotCalcData, traceHashOld) { + var subplot = { traceHash: traceHashOld || {} }; + var calcDataPerModule = []; + + var plot = function(_, moduleCalcData) { + calcDataPerModule.push(moduleCalcData); + }; + + subplotCalcData.forEach(function(calcTrace) { + calcTrace[0].trace._module = { plot: plot }; + }); + + Plots.generalUpdatePerTraceModule(subplot, subplotCalcData, {}); + + return { + traceHash: subplot.traceHash, + calcDataPerModule: calcDataPerModule + }; + } + + it('should update subplot trace hash and call module plot method with correct calcdata traces', function() { + var out = _update([ + [ { trace: { type: 'A', visible: false } } ], + [ { trace: { type: 'A', visible: true } } ], + [ { trace: { type: 'B', visible: false } } ], + [ { trace: { type: 'C', visible: true } } ] + ]); + + expect(Object.keys(out.traceHash)).toEqual(['A', 'C']); + expect(out.traceHash.A.length).toEqual(1); + expect(out.traceHash.C.length).toEqual(1); + + expect(out.calcDataPerModule.length).toEqual(2); + expect(out.calcDataPerModule[0].length).toEqual(1); + expect(out.calcDataPerModule[1].length).toEqual(1); + + var out2 = _update([ + [ { trace: { type: 'A', visible: false } } ], + [ { trace: { type: 'A', visible: false } } ], + [ { trace: { type: 'B', visible: true } } ], + [ { trace: { type: 'C', visible: false } } ] + ], out.traceHash); + + expect(Object.keys(out2.traceHash)).toEqual(['B', 'A', 'C']); + expect(out2.traceHash.B.length).toEqual(1); + expect(out2.traceHash.A.length).toEqual(1); + expect(out2.traceHash.A[0][0].trace.visible).toBe(false); + expect(out2.traceHash.C.length).toEqual(1); + expect(out2.traceHash.C[0][0].trace.visible).toBe(false); + + expect(out2.calcDataPerModule.length).toEqual(1); + expect(out2.calcDataPerModule[0].length).toEqual(1); + + var out3 = _update([ + [ { trace: { type: 'A', visible: false } } ], + [ { trace: { type: 'A', visible: false } } ], + [ { trace: { type: 'B', visible: false } } ], + [ { trace: { type: 'C', visible: false } } ] + ], out2.traceHash); + + expect(Object.keys(out3.traceHash)).toEqual(['B', 'A', 'C']); + expect(out3.traceHash.B.length).toEqual(1); + expect(out3.traceHash.B[0][0].trace.visible).toBe(false); + expect(out3.traceHash.A.length).toEqual(1); + expect(out3.traceHash.A[0][0].trace.visible).toBe(false); + expect(out3.traceHash.C.length).toEqual(1); + expect(out3.traceHash.C[0][0].trace.visible).toBe(false); + + expect(out3.calcDataPerModule.length).toEqual(0); + + var out4 = _update([ + [ { trace: { type: 'A', visible: true } } ], + [ { trace: { type: 'A', visible: true } } ], + [ { trace: { type: 'B', visible: true } } ], + [ { trace: { type: 'C', visible: true } } ] + ], out3.traceHash); + + expect(Object.keys(out4.traceHash)).toEqual(['A', 'B', 'C']); + expect(out4.traceHash.A.length).toEqual(2); + expect(out4.traceHash.B.length).toEqual(1); + expect(out4.traceHash.C.length).toEqual(1); + + expect(out4.calcDataPerModule.length).toEqual(3); + expect(out4.calcDataPerModule[0].length).toEqual(2); + expect(out4.calcDataPerModule[1].length).toEqual(1); + expect(out4.calcDataPerModule[2].length).toEqual(1); + }); + + it('should handle cases when module plot is not set (geo case)', function(done) { + Plotly.plot(createGraphDiv(), [{ + type: 'scattergeo', + visible: false, + lon: [10, 20], + lat: [20, 10] + }, { + type: 'scattergeo', + lon: [10, 20], + lat: [20, 10] + }]) + .then(function() { + expect(d3.selectAll('g.trace.scattergeo').size()).toEqual(1); + + destroyGraphDiv(); + done(); + }); + }); + + it('should handle cases when module plot is not set (ternary case)', function(done) { + Plotly.plot(createGraphDiv(), [{ + type: 'scatterternary', + visible: false, + a: [0.1, 0.2], + b: [0.2, 0.1] + }, { + type: 'scatterternary', + a: [0.1, 0.2], + b: [0.2, 0.1] + }]) + .then(function() { + expect(d3.selectAll('g.trace.scatter').size()).toEqual(1); + + destroyGraphDiv(); + done(); + }); + }); + }); });