From b3e25ac115f1241a60dd7df3a7f7207adb8e21f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:10:12 -0500 Subject: [PATCH 1/8] fix jsdoc for Plots.getSubplotData --- src/plots/plots.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index a1d853d40ef..b3c0044e5b0 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -124,8 +124,8 @@ 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. * From 447abec9697cf5e8a1d5ddf5fd30c67031c44bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:10:42 -0500 Subject: [PATCH 2/8] shorten parts to cartesian modules in geo.js --- src/plots/geo/geo.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index e1ab1c92463..c897774a978 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -15,8 +15,8 @@ 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 Axes = require('../cartesian/axes'); +var Fx = require('../cartesian/graph_interact'); var addProjectionsToD3 = require('./projections'); var createGeoScale = require('./set_scale'); From fe8a3f9f320fb6df4a6f126aab165f621f7f86c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:11:45 -0500 Subject: [PATCH 3/8] simplify subplot creation if-statement --- src/plots/geo/index.js | 2 +- src/plots/ternary/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/geo/index.js b/src/plots/geo/index.js index 379783add7d..ef0f282c76d 100644 --- a/src/plots/geo/index.js +++ b/src/plots/geo/index.js @@ -49,7 +49,7 @@ exports.plot = function plotGeo(gd) { geo = fullLayout[geoId]._subplot; // If geo is not instantiated, create one! - if(geo === undefined) { + if(!geo) { geo = new Geo({ id: geoId, graphDiv: gd, diff --git a/src/plots/ternary/index.js b/src/plots/ternary/index.js index 4b1350f87e7..6c3daefb45e 100644 --- a/src/plots/ternary/index.js +++ b/src/plots/ternary/index.js @@ -41,7 +41,7 @@ exports.plot = function plotTernary(gd) { ternary = fullLayout[ternaryId]._subplot; // If ternary is not instantiated, create one! - if(ternary === undefined) { + if(!ternary) { ternary = new Ternary({ id: ternaryId, graphDiv: gd, From 92119bb6593c87b6bf4e8a1f0ab3468b233cd4cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:13:07 -0500 Subject: [PATCH 4/8] rm (now useless) logic for ternary make plot framework - made useless by https://github.com/plotly/plotly.js/pull/946 --- src/plots/ternary/ternary.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index 83fad349f27..bde82a31a52 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -50,14 +50,6 @@ proto.plot = function(ternaryData, fullLayout) { 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(); - } _this.adjustLayout(ternaryLayout, graphSize); From a7cca2e687e143acf6903bb5e7670e974049eced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:14:11 -0500 Subject: [PATCH 5/8] move getSubplotCalcdata method into Plots --- src/plots/geo/index.js | 15 +-------------- src/plots/mapbox/index.js | 15 +-------------- src/plots/plots.js | 25 ++++++++++++++++++++++++ test/jasmine/tests/plots_test.js | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/plots/geo/index.js b/src/plots/geo/index.js index ef0f282c76d..03cfbdf3393 100644 --- a/src/plots/geo/index.js +++ b/src/plots/geo/index.js @@ -45,7 +45,7 @@ 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! @@ -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 b3c0044e5b0..15700bcde7b 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -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] === undefined) 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 diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 60afb54d805..1c7bb15458c 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -549,4 +549,37 @@ 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([]); + }); + }); + }); From fc0af0c9a759decb6f1102dad83bfd5873b99457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:18:50 -0500 Subject: [PATCH 6/8] add generalUpdatePerTraceModule method in Plots - so that the same logic can be share for geo and ternary subplots (and maybe cartesian subplots down the road) - fix bug occurring when the first trace on a given subplot had `visible: false` (which yielded an exception) --- src/plots/geo/geo.js | 56 +--------------- src/plots/plots.js | 64 ++++++++++++++++++ test/jasmine/tests/plots_test.js | 110 +++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 54 deletions(-) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index c897774a978..75bd8e37261 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -15,6 +15,7 @@ var d3 = require('d3'); var Color = require('../../components/color'); var Drawing = require('../../components/drawing'); +var Plots = require('../plots'); var Axes = require('../cartesian/axes'); var Fx = require('../cartesian/graph_interact'); @@ -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/plots.js b/src/plots/plots.js index 15700bcde7b..02b7633ad28 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2052,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/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 1c7bb15458c..dc3ad46d28e 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -582,4 +582,114 @@ describe('Test Plots', function() { }); }); + 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(); + }); + }); + + }); }); From bfb05d48884a29e70192c69abebfa194e4c292e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 10 Jan 2017 18:20:22 -0500 Subject: [PATCH 7/8] make ternary use getSubplotCalcData & generalUpdatePerTraceModule - fix bug occurring when the first trace on a given ternary subplot had `visible: false` (which yielded an exception) --- src/plots/ternary/index.js | 6 ++--- src/plots/ternary/ternary.js | 44 +++---------------------------- src/traces/scatterternary/plot.js | 20 ++++---------- test/jasmine/tests/plots_test.js | 20 ++++++++++++++ 4 files changed, 32 insertions(+), 58 deletions(-) diff --git a/src/plots/ternary/index.js b/src/plots/ternary/index.js index 6c3daefb45e..e1da80af7b1 100644 --- a/src/plots/ternary/index.js +++ b/src/plots/ternary/index.js @@ -32,12 +32,12 @@ 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! @@ -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 bde82a31a52..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,51 +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; - + 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 dc3ad46d28e..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'); @@ -691,5 +693,23 @@ describe('Test Plots', function() { }); }); + 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(); + }); + }); }); }); From 83c2722b496a00f91619c0eddba866524206760e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 16 Jan 2017 14:01:38 -0500 Subject: [PATCH 8/8] replace unnecessary `=== undefined` check in Plots by simple `!` checks --- src/plots/plots.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 02b7633ad28..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 []; @@ -131,7 +131,7 @@ plots.getSubplotIds = function getSubplotIds(layout, type) { * */ 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 = [], @@ -167,7 +167,7 @@ plots.getSubplotData = function getSubplotData(data, type, subplotId) { * @return {array} array of calcdata traces */ plots.getSubplotCalcData = function(calcData, type, subplotId) { - if(plots.subplotsRegistry[type] === undefined) return []; + if(!plots.subplotsRegistry[type]) return []; var attr = plots.subplotsRegistry[type].attr; var subplotCalcData = [];