Skip to content

Commit bfb05d4

Browse files
committed
make ternary use getSubplotCalcData & generalUpdatePerTraceModule
- fix bug occurring when the first trace on a given ternary subplot had `visible: false` (which yielded an exception)
1 parent fc0af0c commit bfb05d4

File tree

4 files changed

+32
-58
lines changed

4 files changed

+32
-58
lines changed

src/plots/ternary/index.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ exports.supplyLayoutDefaults = require('./layout/defaults');
3232

3333
exports.plot = function plotTernary(gd) {
3434
var fullLayout = gd._fullLayout,
35-
fullData = gd._fullData,
35+
calcData = gd.calcdata,
3636
ternaryIds = Plots.getSubplotIds(fullLayout, 'ternary');
3737

3838
for(var i = 0; i < ternaryIds.length; i++) {
3939
var ternaryId = ternaryIds[i],
40-
fullTernaryData = Plots.getSubplotData(fullData, 'ternary', ternaryId),
40+
ternaryCalcData = Plots.getSubplotCalcData(calcData, 'ternary', ternaryId),
4141
ternary = fullLayout[ternaryId]._subplot;
4242

4343
// If ternary is not instantiated, create one!
@@ -53,7 +53,7 @@ exports.plot = function plotTernary(gd) {
5353
fullLayout[ternaryId]._subplot = ternary;
5454
}
5555

56-
ternary.plot(fullTernaryData, fullLayout, gd._promises);
56+
ternary.plot(ternaryCalcData, fullLayout, gd._promises);
5757
}
5858
};
5959

src/plots/ternary/ternary.js

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ var Color = require('../../components/color');
1818
var Drawing = require('../../components/drawing');
1919
var setConvert = require('../cartesian/set_convert');
2020
var extendFlat = require('../../lib/extend').extendFlat;
21+
var Plots = require('../plots');
2122
var Axes = require('../cartesian/axes');
2223
var dragElement = require('../../components/dragelement');
2324
var Titles = require('../../components/titles');
@@ -44,51 +45,14 @@ proto.init = function(fullLayout) {
4445
this.traceHash = {};
4546
};
4647

47-
proto.plot = function(ternaryData, fullLayout) {
48+
proto.plot = function(ternaryCalcData, fullLayout) {
4849
var _this = this,
4950
ternaryLayout = fullLayout[_this.id],
50-
graphSize = fullLayout._size,
51-
i;
52-
51+
graphSize = fullLayout._size;
5352

5453
_this.adjustLayout(ternaryLayout, graphSize);
5554

56-
var traceHashOld = _this.traceHash;
57-
var traceHash = {};
58-
59-
for(i = 0; i < ternaryData.length; i++) {
60-
var trace = ternaryData[i];
61-
62-
traceHash[trace.type] = traceHash[trace.type] || [];
63-
traceHash[trace.type].push(trace);
64-
}
65-
66-
var moduleNamesOld = Object.keys(traceHashOld);
67-
var moduleNames = Object.keys(traceHash);
68-
69-
// when a trace gets deleted, make sure that its module's
70-
// plot method is called so that it is properly
71-
// removed from the DOM.
72-
for(i = 0; i < moduleNamesOld.length; i++) {
73-
var moduleName = moduleNamesOld[i];
74-
75-
if(moduleNames.indexOf(moduleName) === -1) {
76-
var fakeModule = traceHashOld[moduleName][0];
77-
fakeModule.visible = false;
78-
traceHash[moduleName] = [fakeModule];
79-
}
80-
}
81-
82-
moduleNames = Object.keys(traceHash);
83-
84-
for(i = 0; i < moduleNames.length; i++) {
85-
var moduleData = traceHash[moduleNames[i]];
86-
var _module = moduleData[0]._module;
87-
88-
_module.plot(_this, Lib.filterVisible(moduleData), ternaryLayout);
89-
}
90-
91-
_this.traceHash = traceHash;
55+
Plots.generalUpdatePerTraceModule(_this, ternaryCalcData, ternaryLayout);
9256

9357
_this.layers.plotbg.select('path').call(Color.fill, ternaryLayout.bgcolor);
9458
};

src/traces/scatterternary/plot.js

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
var scatterPlot = require('../scatter/plot');
1313

1414

15-
module.exports = function plot(ternary, data) {
15+
module.exports = function plot(ternary, moduleCalcData) {
1616
var plotContainer = ternary.plotContainer;
1717

1818
// remove all nodes inside the scatter layer
@@ -25,20 +25,10 @@ module.exports = function plot(ternary, data) {
2525
plot: plotContainer
2626
};
2727

28-
var calcdata = new Array(data.length),
29-
fullCalcdata = ternary.graphDiv.calcdata;
30-
31-
for(var i = 0; i < fullCalcdata.length; i++) {
32-
var j = data.indexOf(fullCalcdata[i][0].trace);
33-
34-
if(j === -1) continue;
35-
36-
calcdata[j] = fullCalcdata[i];
37-
38-
// while we're here and have references to both the Ternary object
39-
// and fullData, connect the two (for use by hover)
40-
data[j]._ternary = ternary;
28+
// add ref to ternary subplot object in fullData traces
29+
for(var i = 0; i < moduleCalcData.length; i++) {
30+
moduleCalcData[i][0].trace._ternary = ternary;
4131
}
4232

43-
scatterPlot(ternary.graphDiv, plotinfo, calcdata);
33+
scatterPlot(ternary.graphDiv, plotinfo, moduleCalcData);
4434
};

test/jasmine/tests/plots_test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
var Plotly = require('@lib/index');
22
var Plots = require('@src/plots/plots');
3+
4+
var d3 = require('d3');
35
var createGraphDiv = require('../assets/create_graph_div');
46
var destroyGraphDiv = require('../assets/destroy_graph_div');
57

@@ -691,5 +693,23 @@ describe('Test Plots', function() {
691693
});
692694
});
693695

696+
it('should handle cases when module plot is not set (ternary case)', function(done) {
697+
Plotly.plot(createGraphDiv(), [{
698+
type: 'scatterternary',
699+
visible: false,
700+
a: [0.1, 0.2],
701+
b: [0.2, 0.1]
702+
}, {
703+
type: 'scatterternary',
704+
a: [0.1, 0.2],
705+
b: [0.2, 0.1]
706+
}])
707+
.then(function() {
708+
expect(d3.selectAll('g.trace.scatter').size()).toEqual(1);
709+
710+
destroyGraphDiv();
711+
done();
712+
});
713+
});
694714
});
695715
});

0 commit comments

Comments
 (0)