Skip to content

Commit 4b8b10d

Browse files
alexcjohnsonalexcjohnson
authored
Merge pull request #2127 from plotly/separators-fix
extend zhoverformat to 2d histogram types
2 parents aa67de2 + 3ac223c commit 4b8b10d

File tree

11 files changed

+195
-38
lines changed

11 files changed

+195
-38
lines changed

‎src/traces/contour/defaults.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,4 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
3434

3535
handleContoursDefaults(traceIn, traceOut, coerce);
3636
handleStyleDefaults(traceIn, traceOut, coerce, layout);
37-
38-
coerce('zhoverformat');
39-
// Needed for formatting of hoverlabel if format is not explicitly specified
40-
traceOut._separators = layout.separators;
4137
};

‎src/traces/contour/style_defaults.js‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,17 @@ var colorscaleDefaults = require('../../components/colorscale/defaults');
1313
var Lib = require('../../lib');
1414

1515

16-
module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout, defaultColor, defaultWidth) {
16+
module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout, opts) {
17+
if(!opts) opts = {};
1718
var coloring = coerce('contours.coloring');
1819

1920
var showLines;
2021
var lineColor = '';
2122
if(coloring === 'fill') showLines = coerce('contours.showlines');
2223

2324
if(showLines !== false) {
24-
if(coloring !== 'lines') lineColor = coerce('line.color', defaultColor || '#000');
25-
coerce('line.width', defaultWidth === undefined ? 0.5 : defaultWidth);
25+
if(coloring !== 'lines') lineColor = coerce('line.color', opts.defaultColor || '#000');
26+
coerce('line.width', opts.defaultWidth === undefined ? 0.5 : opts.defaultWidth);
2627
coerce('line.dash');
2728
}
2829

@@ -44,4 +45,10 @@ module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout,
4445
});
4546
coerce('contours.labelformat');
4647
}
48+
49+
if(opts.hasHover !== false) {
50+
coerce('zhoverformat');
51+
// Needed for formatting of hoverlabel if format is not explicitly specified
52+
traceOut._separators = layout.separators;
53+
}
4754
};

‎src/traces/contourcarpet/defaults.js‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
8080
// If there's a fill color, use it at full opacity for the line color
8181
var lineDfltColor = traceOut.fillcolor ? addOpacity(traceOut.fillcolor, 1) : defaultColor;
8282

83-
handleStyleDefaults(traceIn, traceOut, coerce, layout, lineDfltColor, 2);
83+
handleStyleDefaults(traceIn, traceOut, coerce, layout, {
84+
hasHover: false,
85+
defaultColor: lineDfltColor,
86+
defaultWidth: 2
87+
});
8488

8589
if(contours.operation === '=') {
8690
coerce('line.color', defaultColor);
@@ -108,8 +112,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
108112
delete traceOut.line.maxcolor;
109113
}
110114

111-
// TODO: These shouldb e deleted in accordance with toolpanel convention, but
112-
// we can't becuase we require them so that it magically makes the contour
115+
// TODO: These should be deleted in accordance with toolpanel convention, but
116+
// we can't because we require them so that it magically makes the contour
113117
// parts of the code happy:
114118
// delete traceOut.contours.start;
115119
// delete traceOut.contours.end;
@@ -141,7 +145,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
141145
coerce('ncontours');
142146
}
143147

144-
handleStyleDefaults(traceIn, traceOut, coerce, layout);
148+
handleStyleDefaults(traceIn, traceOut, coerce, layout, {hasHover: false});
145149

146150
delete traceOut.value;
147151
delete traceOut.operation;

‎src/traces/heatmap/defaults.js‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ var Lib = require('../../lib');
1313

1414
var hasColumns = require('./has_columns');
1515
var handleXYZDefaults = require('./xyz_defaults');
16+
var handleStyleDefaults = require('./style_defaults');
1617
var colorscaleDefaults = require('../../components/colorscale/defaults');
1718
var attributes = require('./attributes');
1819

@@ -30,17 +31,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
3031

3132
coerce('text');
3233

33-
var zsmooth = coerce('zsmooth');
34-
if(zsmooth === false) {
35-
// ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
36-
coerce('xgap');
37-
coerce('ygap');
38-
}
34+
handleStyleDefaults(traceIn, traceOut, coerce, layout);
3935

4036
coerce('connectgaps', hasColumns(traceOut) && (traceOut.zsmooth !== false));
4137

4238
colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'});
43-
44-
coerce('zhoverformat');
45-
traceOut._separators = layout.separators; // Needed for formatting of hoverlabel if format is not explicitly specified
4639
};

‎src/traces/heatmap/style_defaults.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* Copyright 2012-2017, Plotly, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
10+
'use strict';
11+
12+
module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout) {
13+
var zsmooth = coerce('zsmooth');
14+
if(zsmooth === false) {
15+
// ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
16+
coerce('xgap');
17+
coerce('ygap');
18+
}
19+
20+
coerce('zhoverformat');
21+
// Needed for formatting of hoverlabel if format is not explicitly specified
22+
traceOut._separators = layout.separators;
23+
};

‎src/traces/histogram2d/attributes.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ module.exports = extendFlat({},
4545

4646
xgap: heatmapAttrs.xgap,
4747
ygap: heatmapAttrs.ygap,
48-
zsmooth: heatmapAttrs.zsmooth
48+
zsmooth: heatmapAttrs.zsmooth,
49+
zhoverformat: heatmapAttrs.zhoverformat
4950
},
5051
colorscaleAttrs,
5152
{ autocolorscale: extendFlat({}, colorscaleAttrs.autocolorscale, {dflt: false}) },

‎src/traces/histogram2d/defaults.js‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
var Lib = require('../../lib');
1313

1414
var handleSampleDefaults = require('./sample_defaults');
15+
var handleStyleDefaults = require('../heatmap/style_defaults');
1516
var colorscaleDefaults = require('../../components/colorscale/defaults');
1617
var attributes = require('./attributes');
1718

@@ -22,14 +23,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
2223
}
2324

2425
handleSampleDefaults(traceIn, traceOut, coerce, layout);
26+
if(traceOut.visible === false) return;
2527

26-
var zsmooth = coerce('zsmooth');
27-
if(zsmooth === false) {
28-
// ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
29-
coerce('xgap');
30-
coerce('ygap');
31-
}
32-
28+
handleStyleDefaults(traceIn, traceOut, coerce, layout);
3329
colorscaleDefaults(
3430
traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'}
3531
);

‎src/traces/histogram2dcontour/attributes.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ module.exports = extendFlat({
3333
autocontour: contourAttrs.autocontour,
3434
ncontours: contourAttrs.ncontours,
3535
contours: contourAttrs.contours,
36-
line: contourAttrs.line
36+
line: contourAttrs.line,
37+
zhoverformat: histogram2dAttrs.zhoverformat
3738
},
3839
colorscaleAttrs, {
3940
zmin: extendFlat({}, colorscaleAttrs.zmin, {editType: 'calc'}),

‎src/traces/histogram2dcontour/defaults.js‎

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

2525
handleSampleDefaults(traceIn, traceOut, coerce, layout);
26+
if(traceOut.visible === false) return;
27+
2628
handleContoursDefaults(traceIn, traceOut, coerce);
2729
handleStyleDefaults(traceIn, traceOut, coerce, layout);
2830
};

‎test/jasmine/tests/histogram2d_test.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,43 @@ describe('Test histogram2d', function() {
2020
traceOut = {};
2121
});
2222

23-
it('should set zsmooth to false when zsmooth is empty', function() {
23+
it('should quit early if there is no data', function() {
2424
traceIn = {};
2525
supplyDefaults(traceIn, traceOut, '', {});
26+
expect(traceOut.visible).toBe(false);
27+
['zsmooth', 'xgap', 'ygap', 'calendar'].forEach(function(v) {
28+
expect(traceOut[v]).toBeUndefined(v);
29+
});
30+
});
31+
32+
it('should set zsmooth to false when zsmooth is empty', function() {
33+
traceIn = {x: [1, 2], y: [1, 2]};
34+
supplyDefaults(traceIn, traceOut, '', {});
35+
expect(traceOut.visible).not.toBe(false);
2636
expect(traceOut.zsmooth).toBe(false);
2737
});
2838

2939
it('doesnt step on zsmooth when zsmooth is set', function() {
3040
traceIn = {
41+
x: [1, 2],
42+
y: [1, 2],
3143
zsmooth: 'fast'
3244
};
3345
supplyDefaults(traceIn, traceOut, '', {});
3446
expect(traceOut.zsmooth).toBe('fast');
3547
});
3648

3749
it('should set xgap and ygap to 0 when xgap and ygap are empty', function() {
38-
traceIn = {};
50+
traceIn = {x: [1, 2], y: [1, 2]};
3951
supplyDefaults(traceIn, traceOut, '', {});
4052
expect(traceOut.xgap).toBe(0);
4153
expect(traceOut.ygap).toBe(0);
4254
});
4355

4456
it('shouldnt step on xgap and ygap when xgap and ygap are set', function() {
4557
traceIn = {
58+
x: [1, 2],
59+
y: [1, 2],
4660
xgap: 10,
4761
ygap: 5
4862
};
@@ -53,6 +67,8 @@ describe('Test histogram2d', function() {
5367

5468
it('shouldnt coerce gap when zsmooth is set', function() {
5569
traceIn = {
70+
x: [1, 2],
71+
y: [1, 2],
5672
xgap: 10,
5773
ygap: 5,
5874
zsmooth: 'best'

0 commit comments

Comments
 (0)