Merge pull request #2280 from plotly/ghost-fill

fix ghost fill-tonext when either trace is emptied out

Author: alexcjohnson

src/traces/scatter/link_traces.js

@@ -9,12 +9,11 @@
 'use strict';
 
 module.exports = function linkTraces(gd, plotinfo, cdscatter) {
-    var cd, trace;
+    var trace, i;
     var prevtrace = null;
 
-    for(var i = 0; i < cdscatter.length; ++i) {
-        cd = cdscatter[i];
-        trace = cd[0].trace;
+    for(i = 0; i < cdscatter.length; ++i) {
+        trace = cdscatter[i][0].trace;
 
         // Note: The check which ensures all cdscatter here are for the same axis and
         // are either cartesian or scatterternary has been removed. This code assumes

src/traces/scatter/plot.js

@@ -323,6 +323,10 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
 
     Drawing.setClipUrl(lineJoin, plotinfo.layerClipId);
 
+    function clearFill(selection) {
+        transition(selection).attr('d', 'M0,0Z');
+    }
+
     if(segments.length) {
         if(ownFillEl3) {
             if(pt0 && pt1) {
@@ -348,30 +352,41 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 }
             }
         }
-        else if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevRevpath) {
-            // fill to next: full trace path, plus the previous path reversed
-            if(trace.fill === 'tonext') {
-                // tonext: for use by concentric shapes, like manually constructed
-                // contours, we just add the two paths closed on themselves.
-                // This makes strange results if one path is *not* entirely
-                // inside the other, but then that is a strange usage.
-                transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
-                    .call(Drawing.singleFillStyle);
+        else if(tonext) {
+            if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevRevpath) {
+                // fill to next: full trace path, plus the previous path reversed
+                if(trace.fill === 'tonext') {
+                    // tonext: for use by concentric shapes, like manually constructed
+                    // contours, we just add the two paths closed on themselves.
+                    // This makes strange results if one path is *not* entirely
+                    // inside the other, but then that is a strange usage.
+                    transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
+                        .call(Drawing.singleFillStyle);
+                }
+                else {
+                    // tonextx/y: for now just connect endpoints with lines. This is
+                    // the correct behavior if the endpoints are at the same value of
+                    // y/x, but if they *aren't*, we should ideally do more complicated
+                    // things depending on whether the new endpoint projects onto the
+                    // existing curve or off the end of it
+                    transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z')
+                        .call(Drawing.singleFillStyle);
+                }
+                trace._polygons = trace._polygons.concat(prevPolygons);
             }
             else {
-                // tonextx/y: for now just connect endpoints with lines. This is
-                // the correct behavior if the endpoints are at the same value of
-                // y/x, but if they *aren't*, we should ideally do more complicated
-                // things depending on whether the new endpoint projects onto the
-                // existing curve or off the end of it
-                transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z')
-                    .call(Drawing.singleFillStyle);
+                clearFill(tonext);
+                trace._polygons = null;
             }
-            trace._polygons = trace._polygons.concat(prevPolygons);
         }
         trace._prevRevpath = revpath;
         trace._prevPolygons = thisPolygons;
     }
+    else {
+        if(ownFillEl3) clearFill(ownFillEl3);
+        else if(tonext) clearFill(tonext);
+        trace._polygons = trace._prevRevpath = trace._prevPolygons = null;
+    }
 
 
     function visFilter(d) {

test/jasmine/tests/scatter_test.js

@@ -713,6 +713,50 @@ describe('end-to-end scatter tests', function() {
             expect(fill()).toEqual('rgb(0, 0, 255)');
         }).catch(fail).then(done);
     });
+
+    it('clears fills tonext when either trace is emptied out', function(done) {
+        var trace0 = {y: [1, 3, 5, 7]};
+        var trace1 = {y: [1, 2, 3, 4], line: {width: 0}, mode: 'lines'};
+        var trace2 = {y: [3, 4, 5, 6], fill: 'tonexty', mode: 'none'};
+
+        function checkFill(visible, msg) {
+            var fillSelection = d3.select(gd).selectAll('.scatterlayer .js-fill');
+            expect(fillSelection.size()).toBe(1, msg);
+            expect(fillSelection.attr('d')).negateIf(visible).toBe('M0,0Z', msg);
+        }
+
+        Plotly.newPlot(gd, [trace0, trace1, trace2], {}, {scrollZoom: true})
+        .then(function() {
+            checkFill(true, 'initial');
+
+            return Plotly.restyle(gd, {y: [[null, null, null, null]]}, [1]);
+        })
+        .then(function() {
+            checkFill(false, 'null out trace 1');
+
+            return Plotly.restyle(gd, {y: [[1, 2, 3, 4]]}, [1]);
+        })
+        .then(function() {
+            checkFill(true, 'restore trace 1');
+
+            return Plotly.restyle(gd, {y: [[null, null, null, null]]}, [2]);
+        })
+        .then(function() {
+            checkFill(false, 'null out trace 2');
+
+            return Plotly.restyle(gd, {y: [[1, 2, 3, 4]]}, [2]);
+        })
+        .then(function() {
+            checkFill(true, 'restore trace 2');
+
+            return Plotly.restyle(gd, {y: [[null, null, null, null], [null, null, null, null]]}, [1, 2]);
+        })
+        .then(function() {
+            checkFill(false, 'null out both traces');
+        })
+        .catch(fail)
+        .then(done);
+    });
 });
 
 describe('scatter hoverPoints', function() {