Merge pull request #3005 from plotly/per-stack-first

Fix stacked area layering and deletion bugs

Author: alexcjohnson

Diff for: src/traces/scatter/attributes.js

@@ -85,7 +85,11 @@ module.exports = {
             'stacked. Stacking also turns `fill` on by default, using *tonexty*',
             '(*tonextx*) if `orientation` is *h* (*v*) and sets the default',
             '`mode` to *lines* irrespective of point count.',
-            'You can only stack on a numeric (linear or log) axis.'
+            'You can only stack on a numeric (linear or log) axis.',
+            'Traces in a `stackgroup` will only fill to (or be filled to) other',
+            'traces in the same group. With multiple `stackgroup`s or some',
+            'traces stacked and some not, if fill-linked traces are not already',
+            'consecutive, the later ones will be pushed down in the drawing order.'
         ].join(' ')
     },
     orientation: {
@@ -299,7 +303,11 @@ module.exports = {
             '*tonext* fills the space between two traces if one completely',
             'encloses the other (eg consecutive contour lines), and behaves like',
             '*toself* if there is no trace before it. *tonext* should not be',
-            'used if one trace does not enclose the other.'
+            'used if one trace does not enclose the other.',
+            'Traces in a `stackgroup` will only fill to (or be filled to) other',
+            'traces in the same group. With multiple `stackgroup`s or some',
+            'traces stacked and some not, if fill-linked traces are not already',
+            'consecutive, the later ones will be pushed down in the drawing order.'
         ].join(' ')
     },
     fillcolor: {

Diff for: src/traces/scatter/calc.js

@@ -153,7 +153,7 @@ function calcAxisExpansion(gd, trace, xa, ya, x, y, ppad) {
     var fullLayout = gd._fullLayout;
     var xId = xa._id;
     var yId = ya._id;
-    var firstScatter = fullLayout._firstScatter[xId + yId + trace.type] === trace.uid;
+    var firstScatter = fullLayout._firstScatter[firstScatterGroup(trace)] === trace.uid;
     var stackOrientation = (getStackOpts(trace, fullLayout, xa, ya) || {}).orientation;
     var fill = trace.fill;
 
@@ -257,9 +257,15 @@ function calcMarkerSize(trace, serieslen) {
  * per-trace calc this will get confused.
  */
 function setFirstScatter(fullLayout, trace) {
-    var subplotAndType = trace.xaxis + trace.yaxis + trace.type;
+    var group = firstScatterGroup(trace);
     var firstScatter = fullLayout._firstScatter;
-    if(!firstScatter[subplotAndType]) firstScatter[subplotAndType] = trace.uid;
+    if(!firstScatter[group]) firstScatter[group] = trace.uid;
+}
+
+function firstScatterGroup(trace) {
+    var stackGroup = trace.stackgroup;
+    return trace.xaxis + trace.yaxis + trace.type +
+        (stackGroup ? '-' + stackGroup : '');
 }
 
 function getStackOpts(trace, fullLayout, xa, ya) {

Diff for: src/traces/scatter/link_traces.js

@@ -8,12 +8,56 @@
 
 'use strict';
 
+var LINKEDFILLS = {tonextx: 1, tonexty: 1, tonext: 1};
+
 module.exports = function linkTraces(gd, plotinfo, cdscatter) {
-    var trace, i;
-    var prevtrace = null;
+    var trace, i, group, prevtrace, groupIndex;
 
-    for(i = 0; i < cdscatter.length; ++i) {
+    // first sort traces to keep stacks & filled-together groups together
+    var groupIndices = {};
+    var needsSort = false;
+    var prevGroupIndex = -1;
+    var nextGroupIndex = 0;
+    var prevUnstackedGroupIndex = -1;
+    for(i = 0; i < cdscatter.length; i++) {
         trace = cdscatter[i][0].trace;
+        group = trace.stackgroup || '';
+        if(group) {
+            if(group in groupIndices) {
+                groupIndex = groupIndices[group];
+            }
+            else {
+                groupIndex = groupIndices[group] = nextGroupIndex;
+                nextGroupIndex++;
+            }
+        }
+        else if(trace.fill in LINKEDFILLS && prevUnstackedGroupIndex >= 0) {
+            groupIndex = prevUnstackedGroupIndex;
+        }
+        else {
+            groupIndex = prevUnstackedGroupIndex = nextGroupIndex;
+            nextGroupIndex++;
+        }
+
+        if(groupIndex < prevGroupIndex) needsSort = true;
+        trace._groupIndex = prevGroupIndex = groupIndex;
+    }
+
+    var cdscatterSorted = cdscatter.slice();
+    if(needsSort) {
+        cdscatterSorted.sort(function(a, b) {
+            var traceA = a[0].trace;
+            var traceB = b[0].trace;
+            return (traceA._groupIndex - traceB._groupIndex) ||
+                (traceA.index - traceB.index);
+        });
+    }
+
+    // now link traces to each other
+    var prevtraces = {};
+    for(i = 0; i < cdscatterSorted.length; i++) {
+        trace = cdscatterSorted[i][0].trace;
+        group = trace.stackgroup || '';
 
         // 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
@@ -22,17 +66,20 @@ module.exports = function linkTraces(gd, plotinfo, cdscatter) {
         if(trace.visible === true) {
             trace._nexttrace = null;
 
-            if(['tonextx', 'tonexty', 'tonext'].indexOf(trace.fill) !== -1) {
-                trace._prevtrace = prevtrace;
+            if(trace.fill in LINKEDFILLS) {
+                prevtrace = prevtraces[group];
+                trace._prevtrace = prevtrace || null;
 
                 if(prevtrace) {
                     prevtrace._nexttrace = trace;
                 }
             }
 
-            prevtrace = trace;
+            prevtraces[group] = trace;
         } else {
             trace._prevtrace = trace._nexttrace = null;
         }
     }
+
+    return cdscatterSorted;
 };

Diff for: src/traces/scatter/plot.js

@@ -23,43 +23,29 @@ var linkTraces = require('./link_traces');
 var polygonTester = require('../../lib/polygon').tester;
 
 module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transitionOpts, makeOnCompleteCallback) {
-    var i, uids, join, onComplete;
+    var join, onComplete;
 
     // If transition config is provided, then it is only a partial replot and traces not
     // updated are removed.
     var isFullReplot = !transitionOpts;
     var hasTransition = !!transitionOpts && transitionOpts.duration > 0;
 
+    // Link traces so the z-order of fill layers is correct
+    var cdscatterSorted = linkTraces(gd, plotinfo, cdscatter);
+
     join = scatterLayer.selectAll('g.trace')
-        .data(cdscatter, function(d) { return d[0].trace.uid; });
+        .data(cdscatterSorted, function(d) { return d[0].trace.uid; });
 
     // Append new traces:
     join.enter().append('g')
         .attr('class', function(d) {
             return 'trace scatter trace' + d[0].trace.uid;
         })
         .style('stroke-miterlimit', 2);
-
-    // After the elements are created but before they've been draw, we have to perform
-    // this extra step of linking the traces. This allows appending of fill layers so that
-    // the z-order of fill layers is correct.
-    linkTraces(gd, plotinfo, cdscatter);
+    join.order();
 
     createFills(gd, join, plotinfo);
 
-    // Sort the traces, once created, so that the ordering is preserved even when traces
-    // are shown and hidden. This is needed since we're not just wiping everything out
-    // and recreating on every update.
-    for(i = 0, uids = {}; i < cdscatter.length; i++) {
-        uids[cdscatter[i][0].trace.uid] = i;
-    }
-
-    join.sort(function(a, b) {
-        var idx1 = uids[a[0].trace.uid];
-        var idx2 = uids[b[0].trace.uid];
-        return idx1 - idx2;
-    });
-
     if(hasTransition) {
         if(makeOnCompleteCallback) {
             // If it was passed a callback to register completion, make a callback. If
@@ -82,12 +68,12 @@ module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transition
             // Must run the selection again since otherwise enters/updates get grouped together
             // and these get executed out of order. Except we need them in order!
             scatterLayer.selectAll('g.trace').each(function(d, i) {
-                plotOne(gd, i, plotinfo, d, cdscatter, this, transitionOpts);
+                plotOne(gd, i, plotinfo, d, cdscatterSorted, this, transitionOpts);
             });
         });
     } else {
-        scatterLayer.selectAll('g.trace').each(function(d, i) {
-            plotOne(gd, i, plotinfo, d, cdscatter, this, transitionOpts);
+        join.each(function(d, i) {
+            plotOne(gd, i, plotinfo, d, cdscatterSorted, this, transitionOpts);
         });
     }
 

Diff for: test/image/baselines/stacked_area_groups.png

@@ -0,0 +1,48 @@
+{
+  "data": [
+    {
+        "x": [0, 2, 4], "y": [6, 1, 6], "line": {"color": "#000"},
+        "name": "bottom", "legendgroup": "l"
+    }, {
+        "x0": -0.5, "y": [1, 2, 3, 4, 5], "name": "a 1",
+        "stackgroup": "a", "legendgroup": "a",
+        "line": {"color": "#f00"}, "fillcolor": "rgba(255,0,0,0.8)"
+    }, {
+        "x0": 0.5, "y": [5, 4, 3, 2, 1], "name": "b 1",
+        "stackgroup": "b", "legendgroup": "b",
+        "line": {"color": "#00f"}, "fillcolor": "rgba(0,0,255,0.8)"
+    }, {
+        "x0": -0.5, "y": [1, 1, 1, 1, 1], "name": "a 2",
+        "stackgroup": "a", "legendgroup": "a",
+        "line": {"color": "#f80"}, "fillcolor": "rgba(255,136,0,0.8)"
+    }, {
+        "x0": 1, "y": [1, 2, 1], "name": "unstacked 1",
+        "fill": "tozeroy", "legendgroup": "u",
+        "line": {"color": "#888"}, "fillcolor": "rgba(136,136,136,0.8)"
+    }, {
+        "x0": -0.5, "y": [1, 1, 1, 1, 1], "name": "a 3",
+        "stackgroup": "a", "legendgroup": "a",
+        "line": {"color": "#ff0"}, "fillcolor": "rgba(255,255,0,0.8)"
+    }, {
+        "x0": 0.5, "y": [1, 1, 1, 1, 1], "name": "b 2",
+        "stackgroup": "b", "legendgroup": "b",
+        "line": {"color": "#80f"}, "fillcolor": "rgba(136,0,255,0.8)"
+    }, {
+        "x0": 1, "y": [5, 8, 5], "name": "unstacked 2",
+        "fill": "tonexty", "legendgroup": "u",
+        "line": {"color": "#ccc"}, "fillcolor": "rgba(204,204,204,0.8)"
+    }, {
+        "x": [0, 2, 4], "y": [7, 3, 7], "line": {"color": "#0c0"},
+        "name": "top", "legendgroup": "l"
+    }, {
+        "x0": 0.5, "y": [1, 1, 1, 1, 1], "name": "b 3",
+        "stackgroup": "b", "legendgroup": "b",
+        "line": {"color": "#f0f"}, "fillcolor": "rgba(255,0,255,0.8)"
+    }
+  ],
+  "layout": {
+    "width": 500,
+    "height": 450,
+    "title": "Stack groups and unstacked filled traces"
+  }
+}

Diff for: test/image/mocks/stacked_area_groups.json

@@ -1184,6 +1184,39 @@ describe('stacked area', function() {
         .then(done);
     });
 
+    it('can add/delete stack groups', function(done) {
+        var data01 = [
+            {mode: 'markers', y: [1, 2, -1, 2, 1], stackgroup: 'a'},
+            {mode: 'markers', y: [2, 3, 2, 3, 2], stackgroup: 'b'}
+        ];
+        var data0 = [Lib.extendDeep({}, data01[0])];
+        var data1 = [Lib.extendDeep({}, data01[1])];
+
+        function _assert(yRange, nTraces) {
+            expect(gd._fullLayout.yaxis.range).toBeCloseToArray(yRange, 2);
+            expect(gd.querySelectorAll('g.trace.scatter').length).toBe(nTraces);
+        }
+
+        Plotly.newPlot(gd, data01)
+        .then(function() {
+            _assert([-1.293, 3.293], 2);
+            return Plotly.react(gd, data0);
+        })
+        .then(function() {
+            _assert([-1.220, 2.220], 1);
+            return Plotly.react(gd, data01);
+        })
+        .then(function() {
+            _assert([-1.293, 3.293], 2);
+            return Plotly.react(gd, data1);
+        })
+        .then(function() {
+            _assert([0, 3.205], 1);
+        })
+        .catch(failTest)
+        .then(done);
+    });
+
     it('does not stack on date axes', function(done) {
         Plotly.newPlot(gd, [
             {y: ['2016-01-01', '2017-01-01'], stackgroup: 'a'},