Skip to content

Commit 6fae229

Browse files
alexcjohnsonalexcjohnson
committed
some fixes and tests for empty data arrays
1 parent 965bcfb commit 6fae229

File tree

8 files changed

+139
-9
lines changed

8 files changed

+139
-9
lines changed

‎src/traces/histogram/defaults.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
2424

2525
var x = coerce('x');
2626
var y = coerce('y');
27-
var hasX = x && x.length;
28-
var hasY = y && y.length;
2927

3028
var cumulative = coerce('cumulative.enabled');
3129
if(cumulative) {
@@ -35,12 +33,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
3533

3634
coerce('text');
3735

38-
var orientation = coerce('orientation', (hasY && !hasX) ? 'h' : 'v');
36+
var orientation = coerce('orientation', (y && !x) ? 'h' : 'v');
3937
var sampleLetter = orientation === 'v' ? 'x' : 'y';
4038
var aggLetter = orientation === 'v' ? 'y' : 'x';
41-
var sample = traceOut[sampleLetter];
4239

43-
var len = (hasX && hasY) ? Math.min(x.length && y.length) : (sample || []).length;
40+
var len = (x && y) ? Math.min(x.length && y.length) : (traceOut[sampleLetter] || []).length;
4441

4542
if(!len) {
4643
traceOut.visible = false;

‎src/traces/pie/defaults.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
2121
var len;
2222

2323
var vals = coerce('values');
24-
var hasVals = Lib.isArrayOrTypedArray(vals) && vals.length;
24+
var hasVals = Lib.isArrayOrTypedArray(vals);
2525
var labels = coerce('labels');
26-
if(Array.isArray(labels) && labels.length) {
26+
if(Array.isArray(labels)) {
2727
len = labels.length;
2828
if(hasVals) len = Math.min(len, vals.length);
2929
}
@@ -39,6 +39,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
3939
coerce('label0');
4040
coerce('dlabel');
4141
}
42+
43+
if(!len) {
44+
traceOut.visible = false;
45+
return;
46+
}
4247
traceOut._length = len;
4348

4449
var lineWidth = coerce('marker.line.width');

‎test/jasmine/tests/bar_test.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,23 @@ describe('Bar.supplyDefaults', function() {
7474
expect(traceOut.visible).toBe(false);
7575
});
7676

77+
[{letter: 'y', counter: 'x'}, {letter: 'x', counter: 'y'}].forEach(function(spec) {
78+
var l = spec.letter;
79+
var c = spec.counter;
80+
var c0 = c + '0';
81+
var dc = 'd' + c;
82+
it('should be visible using ' + c0 + '/' + dc + ' if ' + c + ' is missing completely but ' + l + ' is present', function() {
83+
traceIn = {};
84+
traceIn[l] = [1, 2];
85+
supplyDefaults(traceIn, traceOut, defaultColor, {});
86+
expect(traceOut.visible).toBe(undefined, l); // visible: true gets set above the module level
87+
expect(traceOut._length).toBe(2, l);
88+
expect(traceOut[c0]).toBe(0, c0);
89+
expect(traceOut[dc]).toBe(1, dc);
90+
expect(traceOut.orientation).toBe(l === 'x' ? 'h' : 'v', l);
91+
});
92+
});
93+
7794
it('should not set base, offset or width', function() {
7895
traceIn = {
7996
y: [1, 2, 3]

‎test/jasmine/tests/finance_test.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ describe('finance charts defaults:', function() {
141141
expect(out._fullData[1]._fullInput.x).toBeDefined();
142142
});
143143

144-
it('should set visible to *false* when minimum supplied length is 0', function() {
144+
it('should set visible to *false* when a component (other than x) is missing', function() {
145145
var trace0 = Lib.extendDeep({}, mock0, { type: 'ohlc' });
146146
trace0.close = undefined;
147147

@@ -160,6 +160,17 @@ describe('finance charts defaults:', function() {
160160
expect(visibilities).toEqual([false, false]);
161161
});
162162

163+
it('should return visible: false if any data component is empty', function() {
164+
['ohlc', 'candlestick'].forEach(function(type) {
165+
['open', 'high', 'low', 'close', 'x'].forEach(function(attr) {
166+
var trace = Lib.extendDeep({}, mock1, {type: type});
167+
trace[attr] = [];
168+
var out = _supply([trace]);
169+
expect(out._fullData[0].visible).toBe(false, type + ' - ' + attr);
170+
});
171+
});
172+
});
173+
163174
it('direction *showlegend* should be inherited from trace-wide *showlegend*', function() {
164175
var trace0 = Lib.extendDeep({}, mock0, {
165176
type: 'ohlc',

‎test/jasmine/tests/histogram_test.js‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,30 @@ describe('Test histogram', function() {
3434
traceIn = {
3535
y: []
3636
};
37+
traceOut = {};
38+
supplyDefaults(traceIn, traceOut, '', {});
39+
expect(traceOut.visible).toBe(false);
40+
});
41+
42+
it('should set visible to false when x or y is empty AND the other is present', function() {
43+
traceIn = {
44+
x: [],
45+
y: [1, 2, 2]
46+
};
47+
supplyDefaults(traceIn, traceOut, '', {});
48+
expect(traceOut.visible).toBe(false);
49+
50+
traceIn = {
51+
x: [1, 2, 2],
52+
y: []
53+
};
54+
traceOut = {};
3755
supplyDefaults(traceIn, traceOut, '', {});
3856
expect(traceOut.visible).toBe(false);
3957
});
4058

4159
it('should set visible to false when type is histogram2d(contour) and x or y are empty', function() {
4260
traceIn = {
43-
type: 'histogram2d',
4461
x: [],
4562
y: [1, 2, 2]
4663
};
@@ -51,20 +68,23 @@ describe('Test histogram', function() {
5168
x: [1, 2, 2],
5269
y: []
5370
};
71+
traceOut = {};
5472
supplyDefaults2D(traceIn, traceOut, '', {});
5573
expect(traceOut.visible).toBe(false);
5674

5775
traceIn = {
5876
x: [],
5977
y: []
6078
};
79+
traceOut = {};
6180
supplyDefaults2D(traceIn, traceOut, '', {});
6281
expect(traceOut.visible).toBe(false);
6382

6483
traceIn = {
6584
x: [],
6685
y: [1, 2, 2]
6786
};
87+
traceOut = {};
6888
supplyDefaults2DC(traceIn, traceOut, '', {});
6989
expect(traceOut.visible).toBe(false);
7090
});
@@ -80,6 +100,7 @@ describe('Test histogram', function() {
80100
x: [1, 2, 2],
81101
y: [1, 2, 2]
82102
};
103+
traceOut = {};
83104
supplyDefaults(traceIn, traceOut, '', {});
84105
expect(traceOut.orientation).toBe('v');
85106
});
@@ -111,6 +132,7 @@ describe('Test histogram', function() {
111132
traceIn = {
112133
x: [1, 2, 2]
113134
};
135+
traceOut = {};
114136
supplyDefaults(traceIn, traceOut, '', {});
115137
expect(traceOut.autobinx).toBeUndefined();
116138
});
@@ -130,6 +152,7 @@ describe('Test histogram', function() {
130152
traceIn = {
131153
y: [1, 2, 2]
132154
};
155+
traceOut = {};
133156
supplyDefaults(traceIn, traceOut, '', {});
134157
expect(traceOut.autobiny).toBeUndefined();
135158
});

‎test/jasmine/tests/pie_test.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,57 @@ var failTest = require('../assets/fail_test');
88
var click = require('../assets/click');
99
var getClientPosition = require('../assets/get_client_position');
1010
var mouseEvent = require('../assets/mouse_event');
11+
var supplyAllDefaults = require('../assets/supply_defaults');
1112

1213
var customAssertions = require('../assets/custom_assertions');
1314
var assertHoverLabelStyle = customAssertions.assertHoverLabelStyle;
1415
var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
1516

17+
18+
describe('Pie defaults', function() {
19+
function _supply(trace) {
20+
var gd = {
21+
data: [trace],
22+
layout: {}
23+
};
24+
25+
supplyAllDefaults(gd);
26+
27+
return gd._fullData[0];
28+
}
29+
30+
it('finds the minimum length of labels & values', function() {
31+
var out = _supply({type: 'pie', labels: ['A', 'B'], values: [1, 2, 3]});
32+
expect(out._length).toBe(2);
33+
34+
out = _supply({type: 'pie', labels: ['A', 'B', 'C'], values: [1, 2]});
35+
expect(out._length).toBe(2);
36+
});
37+
38+
it('allows labels or values to be missing but not both', function() {
39+
var out = _supply({type: 'pie', values: [1, 2]});
40+
expect(out.visible).toBe(true);
41+
expect(out._length).toBe(2);
42+
expect(out.label0).toBe(0);
43+
expect(out.dlabel).toBe(1);
44+
45+
out = _supply({type: 'pie', labels: ['A', 'B']});
46+
expect(out.visible).toBe(true);
47+
expect(out._length).toBe(2);
48+
49+
out = _supply({type: 'pie'});
50+
expect(out.visible).toBe(false);
51+
});
52+
53+
it('is marked invisible if either labels or values is empty', function() {
54+
var out = _supply({type: 'pie', labels: [], values: [1, 2]});
55+
expect(out.visible).toBe(false);
56+
57+
out = _supply({type: 'pie', labels: ['A', 'B'], values: []});
58+
expect(out.visible).toBe(false);
59+
});
60+
});
61+
1662
describe('Pie traces:', function() {
1763
'use strict';
1864

‎test/jasmine/tests/scatter_test.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,22 @@ describe('Test scatter', function() {
7979
expect(traceOut.visible).toBe(false);
8080
});
8181

82+
[{letter: 'y', counter: 'x'}, {letter: 'x', counter: 'y'}].forEach(function(spec) {
83+
var l = spec.letter;
84+
var c = spec.counter;
85+
var c0 = c + '0';
86+
var dc = 'd' + c;
87+
it('should be visible using ' + c0 + '/' + dc + ' if ' + c + ' is missing completely but ' + l + ' is present', function() {
88+
traceIn = {};
89+
traceIn[spec.letter] = [1, 2];
90+
supplyDefaults(traceIn, traceOut, defaultColor, layout);
91+
expect(traceOut.visible).toBe(undefined, l); // visible: true gets set above the module level
92+
expect(traceOut._length).toBe(2, l);
93+
expect(traceOut[c0]).toBe(0, c0);
94+
expect(traceOut[dc]).toBe(1, dc);
95+
});
96+
});
97+
8298
it('should correctly assign \'hoveron\' default', function() {
8399
traceIn = {
84100
x: [1, 2, 3],

‎test/jasmine/tests/scatterternary_test.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,21 @@ describe('scatterternary defaults', function() {
146146
expect(traceOut._length).toBe(1);
147147
});
148148

149+
it('is set visible: false if a, b, or c is empty', function() {
150+
var trace0 = {
151+
a: [1, 2],
152+
b: [2, 1],
153+
c: [2, 2]
154+
};
155+
156+
['a', 'b', 'c'].forEach(function(letter) {
157+
traceIn = Lib.extendDeep({}, trace0);
158+
traceIn[letter] = [];
159+
supplyDefaults(traceIn, traceOut, defaultColor, layout);
160+
expect(traceOut.visible).toBe(false, letter);
161+
});
162+
});
163+
149164
it('should include \'name\' in \'hoverinfo\' default if multi trace graph', function() {
150165
traceIn = {
151166
type: 'scatterternary',

0 commit comments

Comments
 (0)