Merge pull request #2127 from plotly/separators-fix

alexcjohnson · web-flow · commit 4b8b10df27c5 · 2017-10-27T10:23:25.000-04:00
extend zhoverformat to 2d histogram types
diff --git a/src/traces/contour/defaults.js b/src/traces/contour/defaults.js
@@ -34,8 +34,4 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
     handleContoursDefaults(traceIn, traceOut, coerce);
     handleStyleDefaults(traceIn, traceOut, coerce, layout);
-
-    coerce('zhoverformat');
-    // Needed for formatting of hoverlabel if format is not explicitly specified
-    traceOut._separators = layout.separators;
 };
diff --git a/src/traces/contour/style_defaults.js b/src/traces/contour/style_defaults.js
@@ -13,16 +13,17 @@ var colorscaleDefaults = require('../../components/colorscale/defaults');
 var Lib = require('../../lib');
 
 
-module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout, defaultColor, defaultWidth) {
+module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout, opts) {
+    if(!opts) opts = {};
     var coloring = coerce('contours.coloring');
 
     var showLines;
     var lineColor = '';
     if(coloring === 'fill') showLines = coerce('contours.showlines');
 
     if(showLines !== false) {
-        if(coloring !== 'lines') lineColor = coerce('line.color', defaultColor || '#000');
-        coerce('line.width', defaultWidth === undefined ? 0.5 : defaultWidth);
+        if(coloring !== 'lines') lineColor = coerce('line.color', opts.defaultColor || '#000');
+        coerce('line.width', opts.defaultWidth === undefined ? 0.5 : opts.defaultWidth);
         coerce('line.dash');
     }
 
@@ -44,4 +45,10 @@ module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout,
         });
         coerce('contours.labelformat');
     }
+
+    if(opts.hasHover !== false) {
+        coerce('zhoverformat');
+        // Needed for formatting of hoverlabel if format is not explicitly specified
+        traceOut._separators = layout.separators;
+    }
 };
diff --git a/src/traces/contourcarpet/defaults.js b/src/traces/contourcarpet/defaults.js
@@ -80,7 +80,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
             // If there's a fill color, use it at full opacity for the line color
             var lineDfltColor = traceOut.fillcolor ? addOpacity(traceOut.fillcolor, 1) : defaultColor;
 
-            handleStyleDefaults(traceIn, traceOut, coerce, layout, lineDfltColor, 2);
+            handleStyleDefaults(traceIn, traceOut, coerce, layout, {
+                hasHover: false,
+                defaultColor: lineDfltColor,
+                defaultWidth: 2
+            });
 
             if(contours.operation === '=') {
                 coerce('line.color', defaultColor);
@@ -108,8 +112,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
                 delete traceOut.line.maxcolor;
             }
 
-            // TODO: These shouldb e deleted in accordance with toolpanel convention, but
-            // we can't becuase we require them so that it magically makes the contour
+            // TODO: These should be deleted in accordance with toolpanel convention, but
+            // we can't because we require them so that it magically makes the contour
             // parts of the code happy:
             // delete traceOut.contours.start;
             // delete traceOut.contours.end;
@@ -141,7 +145,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
                 coerce('ncontours');
             }
 
-            handleStyleDefaults(traceIn, traceOut, coerce, layout);
+            handleStyleDefaults(traceIn, traceOut, coerce, layout, {hasHover: false});
 
             delete traceOut.value;
             delete traceOut.operation;
diff --git a/src/traces/heatmap/defaults.js b/src/traces/heatmap/defaults.js
@@ -13,6 +13,7 @@ var Lib = require('../../lib');
 
 var hasColumns = require('./has_columns');
 var handleXYZDefaults = require('./xyz_defaults');
+var handleStyleDefaults = require('./style_defaults');
 var colorscaleDefaults = require('../../components/colorscale/defaults');
 var attributes = require('./attributes');
 
@@ -30,17 +31,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
     coerce('text');
 
-    var zsmooth = coerce('zsmooth');
-    if(zsmooth === false) {
-        // ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
-        coerce('xgap');
-        coerce('ygap');
-    }
+    handleStyleDefaults(traceIn, traceOut, coerce, layout);
 
     coerce('connectgaps', hasColumns(traceOut) && (traceOut.zsmooth !== false));
 
     colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'});
-
-    coerce('zhoverformat');
-    traceOut._separators = layout.separators; // Needed for formatting of hoverlabel if format is not explicitly specified
 };
diff --git a/src/traces/heatmap/style_defaults.js b/src/traces/heatmap/style_defaults.js
@@ -0,0 +1,23 @@
+/**
+* Copyright 2012-2017, Plotly, Inc.
+* All rights reserved.
+*
+* This source code is licensed under the MIT license found in the
+* LICENSE file in the root directory of this source tree.
+*/
+
+
+'use strict';
+
+module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout) {
+    var zsmooth = coerce('zsmooth');
+    if(zsmooth === false) {
+        // ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
+        coerce('xgap');
+        coerce('ygap');
+    }
+
+    coerce('zhoverformat');
+    // Needed for formatting of hoverlabel if format is not explicitly specified
+    traceOut._separators = layout.separators;
+};
diff --git a/src/traces/histogram2d/attributes.js b/src/traces/histogram2d/attributes.js
@@ -45,7 +45,8 @@ module.exports = extendFlat({},
 
         xgap: heatmapAttrs.xgap,
         ygap: heatmapAttrs.ygap,
-        zsmooth: heatmapAttrs.zsmooth
+        zsmooth: heatmapAttrs.zsmooth,
+        zhoverformat: heatmapAttrs.zhoverformat
     },
     colorscaleAttrs,
     { autocolorscale: extendFlat({}, colorscaleAttrs.autocolorscale, {dflt: false}) },
diff --git a/src/traces/histogram2d/defaults.js b/src/traces/histogram2d/defaults.js
@@ -12,6 +12,7 @@
 var Lib = require('../../lib');
 
 var handleSampleDefaults = require('./sample_defaults');
+var handleStyleDefaults = require('../heatmap/style_defaults');
 var colorscaleDefaults = require('../../components/colorscale/defaults');
 var attributes = require('./attributes');
 
@@ -22,14 +23,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     }
 
     handleSampleDefaults(traceIn, traceOut, coerce, layout);
+    if(traceOut.visible === false) return;
 
-    var zsmooth = coerce('zsmooth');
-    if(zsmooth === false) {
-        // ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
-        coerce('xgap');
-        coerce('ygap');
-    }
-
+    handleStyleDefaults(traceIn, traceOut, coerce, layout);
     colorscaleDefaults(
         traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'}
     );
diff --git a/src/traces/histogram2dcontour/attributes.js b/src/traces/histogram2dcontour/attributes.js
@@ -33,7 +33,8 @@ module.exports = extendFlat({
     autocontour: contourAttrs.autocontour,
     ncontours: contourAttrs.ncontours,
     contours: contourAttrs.contours,
-    line: contourAttrs.line
+    line: contourAttrs.line,
+    zhoverformat: histogram2dAttrs.zhoverformat
 },
     colorscaleAttrs, {
         zmin: extendFlat({}, colorscaleAttrs.zmin, {editType: 'calc'}),
diff --git a/src/traces/histogram2dcontour/defaults.js b/src/traces/histogram2dcontour/defaults.js
@@ -23,6 +23,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     }
 
     handleSampleDefaults(traceIn, traceOut, coerce, layout);
+    if(traceOut.visible === false) return;
+
     handleContoursDefaults(traceIn, traceOut, coerce);
     handleStyleDefaults(traceIn, traceOut, coerce, layout);
 };
diff --git a/test/jasmine/tests/histogram2d_test.js b/test/jasmine/tests/histogram2d_test.js
@@ -20,29 +20,43 @@ describe('Test histogram2d', function() {
             traceOut = {};
         });
 
-        it('should set zsmooth to false when zsmooth is empty', function() {
+        it('should quit early if there is no data', function() {
             traceIn = {};
             supplyDefaults(traceIn, traceOut, '', {});
+            expect(traceOut.visible).toBe(false);
+            ['zsmooth', 'xgap', 'ygap', 'calendar'].forEach(function(v) {
+                expect(traceOut[v]).toBeUndefined(v);
+            });
+        });
+
+        it('should set zsmooth to false when zsmooth is empty', function() {
+            traceIn = {x: [1, 2], y: [1, 2]};
+            supplyDefaults(traceIn, traceOut, '', {});
+            expect(traceOut.visible).not.toBe(false);
             expect(traceOut.zsmooth).toBe(false);
         });
 
         it('doesnt step on zsmooth when zsmooth is set', function() {
             traceIn = {
+                x: [1, 2],
+                y: [1, 2],
                 zsmooth: 'fast'
             };
             supplyDefaults(traceIn, traceOut, '', {});
             expect(traceOut.zsmooth).toBe('fast');
         });
 
         it('should set xgap and ygap to 0 when xgap and ygap are empty', function() {
-            traceIn = {};
+            traceIn = {x: [1, 2], y: [1, 2]};
             supplyDefaults(traceIn, traceOut, '', {});
             expect(traceOut.xgap).toBe(0);
             expect(traceOut.ygap).toBe(0);
         });
 
         it('shouldnt step on xgap and ygap when xgap and ygap are set', function() {
             traceIn = {
+                x: [1, 2],
+                y: [1, 2],
                 xgap: 10,
                 ygap: 5
             };
@@ -53,6 +67,8 @@ describe('Test histogram2d', function() {
 
         it('shouldnt coerce gap when zsmooth is set', function() {
             traceIn = {
+                x: [1, 2],
+                y: [1, 2],
                 xgap: 10,
                 ygap: 5,
                 zsmooth: 'best'
diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js
@@ -474,9 +474,12 @@ describe('hover info', function() {
     }
 
     describe('\'hover info for x/y/z traces', function() {
-        it('should display correct label content', function(done) {
-            var gd = createGraphDiv();
+        var gd;
+        beforeEach(function() {
+            gd = createGraphDiv();
+        });
 
+        it('should display correct label content', function(done) {
             Plotly.plot(gd, [{
                 type: 'heatmap',
                 y: [0, 1],
@@ -510,20 +513,20 @@ describe('hover info', function() {
             .then(done);
         });
 
-        it('should display correct label content with specified format', function(done) {
-            var gd = createGraphDiv();
-
+        it('should display correct label content with specified format - heatmap', function(done) {
             Plotly.plot(gd, [{
                 type: 'heatmap',
                 y: [0, 1],
                 z: [[1.11111, 2.2222, 3.33333], [4.44444, 5.55555, 6.66666]],
                 name: 'one',
-                zhoverformat: '.2f'
+                zhoverformat: '.2f',
+                showscale: false
             }, {
                 type: 'heatmap',
                 y: [2, 3],
                 z: [[1, 2, 3], [2, 2, 1]],
-                name: 'two'
+                name: 'two',
+                showscale: false
             }], {
                 width: 500,
                 height: 400,
@@ -546,6 +549,121 @@ describe('hover info', function() {
             .catch(fail)
             .then(done);
         });
+
+        it('should display correct label content with specified format - contour', function(done) {
+            Plotly.plot(gd, [{
+                type: 'contour',
+                y: [0, 1],
+                z: [[1.11111, 2.2222, 3.33333], [4.44444, 5.55555, 6.66666]],
+                name: 'one',
+                zhoverformat: '.2f',
+                showscale: false
+            }, {
+                type: 'contour',
+                y: [2, 3],
+                z: [[1, 2, 3], [2, 2, 1]],
+                name: 'two',
+                showscale: false
+            }], {
+                width: 500,
+                height: 400,
+                margin: {l: 0, t: 0, r: 0, b: 0}
+            })
+            .then(function() {
+                _hover(gd, 250, 50);
+                assertHoverLabelContent({
+                    nums: 'x: 1\ny: 3\nz: 2',
+                    name: 'two'
+                });
+            })
+            .then(function() {
+                _hover(gd, 250, 300);
+                assertHoverLabelContent({
+                    nums: 'x: 1\ny: 1\nz: 5.56',
+                    name: 'one'
+                });
+            })
+            .catch(fail)
+            .then(done);
+        });
+
+        it('should display correct label content with specified format - histogram2d', function(done) {
+            Plotly.plot(gd, [{
+                type: 'histogram2d',
+                x: [0, 1, 2, 0, 1, 2, 1],
+                y: [0, 0, 0, 1, 1, 1, 1],
+                z: [1.11111, 2.2222, 3.3333, 4.4444, 4.4444, 6.6666, 1.1111],
+                histfunc: 'sum',
+                name: 'one',
+                zhoverformat: '.2f',
+                showscale: false
+            }, {
+                type: 'histogram2d',
+                x: [0, 1, 2, 0, 1, 2, 1, 2, 0, 1, 2],
+                y: [2, 2, 2, 3, 3, 3, 2, 2, 3, 3, 2],
+                name: 'two',
+                showscale: false
+            }], {
+                width: 500,
+                height: 400,
+                margin: {l: 0, t: 0, r: 0, b: 0}
+            })
+            .then(function() {
+                _hover(gd, 250, 100);
+                assertHoverLabelContent({
+                    nums: 'x: 1\ny: 3\nz: 2',
+                    name: 'two'
+                });
+            })
+            .then(function() {
+                _hover(gd, 250, 300);
+                assertHoverLabelContent({
+                    nums: 'x: 1\ny: 1\nz: 5.56',
+                    name: 'one'
+                });
+            })
+            .catch(fail)
+            .then(done);
+        });
+
+        it('should display correct label content with specified format - histogram2dcontour', function(done) {
+            Plotly.plot(gd, [{
+                type: 'histogram2dcontour',
+                x: [0, 1, 2, 0, 1, 2, 1],
+                y: [0, 0, 0, 1, 1, 1, 1],
+                z: [1.11111, 2.2222, 3.3333, 4.4444, 4.4444, 6.6666, 1.1111],
+                histfunc: 'sum',
+                name: 'one',
+                zhoverformat: '.2f',
+                showscale: false
+            }, {
+                type: 'histogram2dcontour',
+                x: [0, 1, 2, 0, 1, 2, 1, 2, 0, 1, 2],
+                y: [2, 2, 2, 3, 3, 3, 2, 2, 3, 3, 2],
+                name: 'two',
+                showscale: false
+            }], {
+                width: 500,
+                height: 400,
+                margin: {l: 0, t: 0, r: 0, b: 0}
+            })
+            .then(function() {
+                _hover(gd, 250, 50);
+                assertHoverLabelContent({
+                    nums: 'x: 1\ny: 3\nz: 2',
+                    name: 'two'
+                });
+            })
+            .then(function() {
+                _hover(gd, 250, 270);
+                assertHoverLabelContent({
+                    nums: 'x: 1\ny: 1\nz: 5.56',
+                    name: 'one'
+                });
+            })
+            .catch(fail)
+            .then(done);
+        });
     });
 
     describe('hover info for negative data on a log axis', function() {

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`handleSampleDefaults(traceIn, traceOut, coerce, layout);`
	`26`	`+ if(traceOut.visible === false) return;`
	`27`	`+`
`26`	`28`	`handleContoursDefaults(traceIn, traceOut, coerce);`
`27`	`29`	`handleStyleDefaults(traceIn, traceOut, coerce, layout);`
`28`	`30`	`};`