From 3b7faf75d4339da03939f91d696b061883762e3a Mon Sep 17 00:00:00 2001
From: alexcjohnson <alex@plot.ly>
Date: Mon, 12 Feb 2018 15:41:04 -0500
Subject: [PATCH] fix errorbars for Plotly.react and for uneven data arrays

---
 src/components/errorbars/compute_error.js | 16 +++++--
 src/components/errorbars/defaults.js      |  6 +--
 src/components/errorbars/plot.js          |  8 ++--
 test/image/mocks/basic_error_bar.json     |  5 ++
 test/jasmine/tests/errorbars_test.js      | 56 +++++++++++++++++++++--
 test/jasmine/tests/plot_api_test.js       |  1 +
 6 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/src/components/errorbars/compute_error.js b/src/components/errorbars/compute_error.js
index a7b6b2ad2a9..87512817aed 100644
--- a/src/components/errorbars/compute_error.js
+++ b/src/components/errorbars/compute_error.js
@@ -30,18 +30,26 @@ module.exports = function makeComputeError(opts) {
         symmetric = opts.symmetric;
 
     if(type === 'data') {
-        var array = opts.array,
-            arrayminus = opts.arrayminus;
+        var array = opts.array || [];
 
-        if(symmetric || arrayminus === undefined) {
+        if(symmetric) {
             return function computeError(dataPt, index) {
                 var val = +(array[index]);
                 return [val, val];
             };
         }
         else {
+            var arrayminus = opts.arrayminus || [];
             return function computeError(dataPt, index) {
-                return [+arrayminus[index], +array[index]];
+                var val = +array[index];
+                var valMinus = +arrayminus[index];
+                // in case one is present and the other is missing, fill in 0
+                // so we still see the present one. Mostly useful during manual
+                // data entry.
+                if(!isNaN(val) || !isNaN(valMinus)) {
+                    return [valMinus || 0, val || 0];
+                }
+                return [NaN, NaN];
             };
         }
     }
diff --git a/src/components/errorbars/defaults.js b/src/components/errorbars/defaults.js
index 500b3af2520..0eb5a2f75b5 100644
--- a/src/components/errorbars/defaults.js
+++ b/src/components/errorbars/defaults.js
@@ -44,12 +44,10 @@ module.exports = function(traceIn, traceOut, defaultColor, opts) {
     }
 
     if(type === 'data') {
-        var array = coerce('array');
-        if(!array) containerOut.array = [];
+        coerce('array');
         coerce('traceref');
         if(!symmetric) {
-            var arrayminus = coerce('arrayminus');
-            if(!arrayminus) containerOut.arrayminus = [];
+            coerce('arrayminus');
             coerce('tracerefminus');
         }
     }
diff --git a/src/components/errorbars/plot.js b/src/components/errorbars/plot.js
index 4e540263b68..d03187cfcd2 100644
--- a/src/components/errorbars/plot.js
+++ b/src/components/errorbars/plot.js
@@ -76,6 +76,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {
 
             var path;
 
+            var yerror = errorbar.select('path.yerror');
             if(yObj.visible && isNumeric(coords.x) &&
                     isNumeric(coords.yh) &&
                     isNumeric(coords.ys)) {
@@ -88,8 +89,6 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {
 
                 if(!coords.noYS) path += 'm-' + yw + ',0h' + (2 * yw); // shoe
 
-                var yerror = errorbar.select('path.yerror');
-
                 isNew = !yerror.size();
 
                 if(isNew) {
@@ -105,7 +104,9 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {
 
                 yerror.attr('d', path);
             }
+            else yerror.remove();
 
+            var xerror = errorbar.select('path.xerror');
             if(xObj.visible && isNumeric(coords.y) &&
                     isNumeric(coords.xh) &&
                     isNumeric(coords.xs)) {
@@ -117,8 +118,6 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {
 
                 if(!coords.noXS) path += 'm0,-' + xw + 'v' + (2 * xw); // shoe
 
-                var xerror = errorbar.select('path.xerror');
-
                 isNew = !xerror.size();
 
                 if(isNew) {
@@ -134,6 +133,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {
 
                 xerror.attr('d', path);
             }
+            else xerror.remove();
         });
     });
 };
diff --git a/test/image/mocks/basic_error_bar.json b/test/image/mocks/basic_error_bar.json
index 76b8749532c..5b21661dfe4 100644
--- a/test/image/mocks/basic_error_bar.json
+++ b/test/image/mocks/basic_error_bar.json
@@ -20,6 +20,11 @@
         ],
         "visible": true
       },
+      "error_x": {
+        "type": "data",
+        "symmetric": false,
+        "visible": true
+      },
       "type": "scatter"
     }
   ]
diff --git a/test/jasmine/tests/errorbars_test.js b/test/jasmine/tests/errorbars_test.js
index 3f6f9bd49bf..6cf7a1ea900 100644
--- a/test/jasmine/tests/errorbars_test.js
+++ b/test/jasmine/tests/errorbars_test.js
@@ -15,15 +15,29 @@ describe('errorbar plotting', function() {
 
     afterEach(destroyGraphDiv);
 
+    function countBars(xCount, yCount) {
+        expect(d3.select(gd).selectAll('.xerror').size()).toBe(xCount);
+        expect(d3.select(gd).selectAll('.yerror').size()).toBe(yCount);
+    }
+
+    function checkCalcdata(cdTrace, errorBarData) {
+        cdTrace.forEach(function(di, i) {
+            var ebi = errorBarData[i] || {};
+            expect(di.xh).toBe(ebi.xh);
+            expect(di.xs).toBe(ebi.xs);
+            expect(di.yh).toBe(ebi.yh);
+            expect(di.ys).toBe(ebi.ys);
+        });
+    }
+
     it('should autorange to the visible bars and remove invisible bars', function(done) {
-        function check(xrange, yrange, xcount, ycount) {
+        function check(xrange, yrange, xCount, yCount) {
             var xa = gd._fullLayout.xaxis;
             var ya = gd._fullLayout.yaxis;
             expect(xa.range).toBeCloseToArray(xrange, 3);
             expect(ya.range).toBeCloseToArray(yrange, 3);
 
-            expect(d3.selectAll('.xerror').size()).toBe(xcount);
-            expect(d3.selectAll('.yerror').size()).toBe(ycount);
+            countBars(xCount, yCount);
         }
         Plotly.newPlot(gd, [{
             y: [1, 2, 3],
@@ -50,4 +64,40 @@ describe('errorbar plotting', function() {
         .catch(fail)
         .then(done);
     });
+
+    it('shows half errorbars and removes individual bars that disappear', function(done) {
+        Plotly.newPlot(gd, [{
+            x: [0, 10, 20],
+            y: [30, 40, 50],
+            error_x: {type: 'data', array: [2, 3], visible: true, symmetric: false},
+            error_y: {type: 'data', arrayminus: [4], visible: true, symmetric: false}
+        }])
+        .then(function() {
+            countBars(2, 1);
+            checkCalcdata(gd.calcdata[0], [
+                {xs: 0, xh: 2, ys: 26, yh: 30},
+                {xs: 10, xh: 13}
+            ]);
+
+            Plotly.restyle(gd, {'error_x.array': [[1]], 'error_y.arrayminus': [[5, 6]]});
+        })
+        .then(function() {
+            countBars(1, 2);
+            checkCalcdata(gd.calcdata[0], [
+                {xs: 0, xh: 1, ys: 25, yh: 30},
+                {ys: 34, yh: 40}
+            ]);
+
+            Plotly.restyle(gd, {'error_x.array': [[7, 8]], 'error_y.arrayminus': [[9]]});
+        })
+        .then(function() {
+            countBars(2, 1);
+            checkCalcdata(gd.calcdata[0], [
+                {xs: 0, xh: 7, ys: 21, yh: 30},
+                {xs: 10, xh: 18}
+            ]);
+        })
+        .catch(fail)
+        .then(done);
+    });
 });
diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js
index b783f3d89ad..0d52ac76d28 100644
--- a/test/jasmine/tests/plot_api_test.js
+++ b/test/jasmine/tests/plot_api_test.js
@@ -2577,6 +2577,7 @@ describe('Test plot api', function() {
             ['axes_enumerated_ticks', require('@mocks/axes_enumerated_ticks.json')],
             ['axes_visible-false', require('@mocks/axes_visible-false.json')],
             ['bar_and_histogram', require('@mocks/bar_and_histogram.json')],
+            ['basic_error_bar', require('@mocks/basic_error_bar.json')],
             ['binding', require('@mocks/binding.json')],
             ['cheater_smooth', require('@mocks/cheater_smooth.json')],
             ['finance_style', require('@mocks/finance_style.json')],