fix violin box/meanline/pts removal

etpinard · etpinard · commit 4e48cd795323 · 2018-07-05T17:40:06.000-04:00
diff --git a/src/traces/box/plot.js b/src/traces/box/plot.js
@@ -73,18 +73,9 @@ function plot(gd, plotinfo, cdbox, boxLayer) {
         // always split the distance to the closest box
         t.wHover = t.dPos * (group ? groupFraction / numBoxes : 1);
 
-        // boxes and whiskers
         plotBoxAndWhiskers(sel, {pos: posAxis, val: valAxis}, trace, t);
-
-        // draw points, if desired
-        if(trace.boxpoints) {
-            plotPoints(sel, {x: xa, y: ya}, trace, t);
-        }
-
-        // draw mean (and stdev diamond) if desired
-        if(trace.boxmean) {
-            plotBoxMean(sel, {pos: posAxis, val: valAxis}, trace, t);
-        }
+        plotPoints(sel, {x: xa, y: ya}, trace, t);
+        plotBoxMean(sel, {pos: posAxis, val: valAxis}, trace, t);
     });
 }
 
@@ -109,7 +100,14 @@ function plotBoxAndWhiskers(sel, axes, trace, t) {
         bdPos1 = t.bdPos;
     }
 
-    var paths = sel.selectAll('path.box').data(Lib.identity);
+    var fn;
+    if(trace.type === 'box' ||
+        (trace.type === 'violin' && (trace.box || {}).visible)
+    ) {
+        fn = Lib.identity;
+    }
+
+    var paths = sel.selectAll('path.box').data(fn || []);
 
     paths.enter().append('path')
         .style('vector-effect', 'non-scaling-stroke')
@@ -187,16 +185,18 @@ function plotPoints(sel, axes, trace, t) {
     // repeatable pseudo-random number generator
     Lib.seedPseudoRandom();
 
-    var gPoints = sel.selectAll('g.points')
-        // since box plot points get an extra level of nesting, each
-        // box needs the trace styling info
-        .data(function(d) {
-            d.forEach(function(v) {
-                v.t = t;
-                v.trace = trace;
-            });
-            return d;
+    // since box plot points get an extra level of nesting, each
+    // box needs the trace styling info
+    var fn = function(d) {
+        d.forEach(function(v) {
+            v.t = t;
+            v.trace = trace;
         });
+        return d;
+    };
+
+    var gPoints = sel.selectAll('g.points')
+        .data(mode ? fn : []);
 
     gPoints.enter().append('g')
         .attr('class', 'points');
@@ -306,7 +306,14 @@ function plotBoxMean(sel, axes, trace, t) {
         bdPos1 = t.bdPos;
     }
 
-    var paths = sel.selectAll('path.mean').data(Lib.identity);
+    var fn;
+    if(trace.type === 'box' && trace.boxmean ||
+        (trace.type === 'violin' && (trace.box || {}).visible && (trace.meanline || {}).visible)
+    ) {
+        fn = Lib.identity;
+    }
+
+    var paths = sel.selectAll('path.mean').data(fn || []);
 
     paths.enter().append('path')
         .attr('class', 'mean')
diff --git a/src/traces/violin/plot.js b/src/traces/violin/plot.js
@@ -69,8 +69,6 @@ module.exports = function plot(gd, plotinfo, cd, violinLayer) {
         var hasBothSides = trace.side === 'both';
         var hasPositiveSide = hasBothSides || trace.side === 'positive';
         var hasNegativeSide = hasBothSides || trace.side === 'negative';
-        var hasBox = trace.box && trace.box.visible;
-        var hasMeanLine = trace.meanline && trace.meanline.visible;
         var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];
 
         var violins = sel.selectAll('path.violin').data(Lib.identity);
@@ -149,66 +147,61 @@ module.exports = function plot(gd, plotinfo, cd, violinLayer) {
             d.pathLength = d.path.getTotalLength() / (hasBothSides ? 2 : 1);
         });
 
-        if(hasBox) {
-            var boxWidth = trace.box.width;
-            var boxLineWidth = trace.box.line.width;
-            var bdPosScaled;
-            var bPosPxOffset;
+        var boxAttrs = trace.box || {};
+        var boxWidth = boxAttrs.width;
+        var boxLineWidth = (boxAttrs.line || {}).width;
+        var bdPosScaled;
+        var bPosPxOffset;
+
+        if(hasBothSides) {
+            bdPosScaled = bdPos * boxWidth;
+            bPosPxOffset = 0;
+        } else if(hasPositiveSide) {
+            bdPosScaled = [0, bdPos * boxWidth / 2];
+            bPosPxOffset = -boxLineWidth;
+        } else {
+            bdPosScaled = [bdPos * boxWidth / 2, 0];
+            bPosPxOffset = boxLineWidth;
+        }
 
-            if(hasBothSides) {
-                bdPosScaled = bdPos * boxWidth;
-                bPosPxOffset = 0;
-            } else if(hasPositiveSide) {
-                bdPosScaled = [0, bdPos * boxWidth / 2];
-                bPosPxOffset = -boxLineWidth;
-            } else {
-                bdPosScaled = [bdPos * boxWidth / 2, 0];
-                bPosPxOffset = boxLineWidth;
-            }
+        // inner box
+        boxPlot.plotBoxAndWhiskers(sel, {pos: posAxis, val: valAxis}, trace, {
+            bPos: bPos,
+            bdPos: bdPosScaled,
+            bPosPxOffset: bPosPxOffset
+        });
 
-            boxPlot.plotBoxAndWhiskers(sel, {pos: posAxis, val: valAxis}, trace, {
-                bPos: bPos,
-                bdPos: bdPosScaled,
-                bPosPxOffset: bPosPxOffset
-            });
-
-            // if both box and meanline are visible, show mean line inside box
-            if(hasMeanLine) {
-                boxPlot.plotBoxMean(sel, {pos: posAxis, val: valAxis}, trace, {
-                    bPos: bPos,
-                    bdPos: bdPosScaled,
-                    bPosPxOffset: bPosPxOffset
-                });
-            }
-        }
-        else {
-            if(hasMeanLine) {
-                var meanPaths = sel.selectAll('path.mean').data(Lib.identity);
-
-                meanPaths.enter().append('path')
-                    .attr('class', 'mean')
-                    .style({
-                        fill: 'none',
-                        'vector-effect': 'non-scaling-stroke'
-                    });
-
-                meanPaths.exit().remove();
-
-                meanPaths.each(function(d) {
-                    var v = valAxis.c2p(d.mean, true);
-                    var p = helpers.getPositionOnKdePath(d, trace, v);
-
-                    d3.select(this).attr('d',
-                        trace.orientation === 'h' ?
-                            'M' + v + ',' + p[0] + 'V' + p[1] :
-                            'M' + p[0] + ',' + v + 'H' + p[1]
-                    );
-                });
-            }
-        }
+        // meanline insider box
+        boxPlot.plotBoxMean(sel, {pos: posAxis, val: valAxis}, trace, {
+            bPos: bPos,
+            bdPos: bdPosScaled,
+            bPosPxOffset: bPosPxOffset
+        });
 
-        if(trace.points) {
-            boxPlot.plotPoints(sel, {x: xa, y: ya}, trace, t);
+        var fn;
+        if(!(trace.box || {}).visible && (trace.meanline || {}).visible) {
+            fn = Lib.identity;
         }
+
+        // N.B. use different class name than boxPlot.plotBoxMean,
+        // to avoid selectAll conflict
+        var meanPaths = sel.selectAll('path.meanline').data(fn || []);
+        meanPaths.enter().append('path')
+            .attr('class', 'meanline')
+            .style('fill', 'none')
+            .style('vector-effect', 'non-scaling-stroke');
+        meanPaths.exit().remove();
+        meanPaths.each(function(d) {
+            var v = valAxis.c2p(d.mean, true);
+            var p = helpers.getPositionOnKdePath(d, trace, v);
+
+            d3.select(this).attr('d',
+                trace.orientation === 'h' ?
+                    'M' + v + ',' + p[0] + 'V' + p[1] :
+                    'M' + p[0] + ',' + v + 'H' + p[1]
+            );
+        });
+
+        boxPlot.plotPoints(sel, {x: xa, y: ya}, trace, t);
     });
 };
diff --git a/src/traces/violin/style.js b/src/traces/violin/style.js
@@ -35,11 +35,17 @@ module.exports = function style(gd, cd) {
             .call(Color.stroke, boxLine.color)
             .call(Color.fill, box.fillcolor);
 
+        var meanLineStyle = {
+            'stroke-width': meanLineWidth + 'px',
+            'stroke-dasharray': (2 * meanLineWidth) + 'px,' + meanLineWidth + 'px'
+        };
+
         sel.selectAll('path.mean')
-            .style({
-                'stroke-width': meanLineWidth + 'px',
-                'stroke-dasharray': (2 * meanLineWidth) + 'px,' + meanLineWidth + 'px'
-            })
+            .style(meanLineStyle)
+            .call(Color.stroke, meanline.color);
+
+        sel.selectAll('path.meanline')
+            .style(meanLineStyle)
             .call(Color.stroke, meanline.color);
 
         stylePoints(sel, trace, gd);
diff --git a/test/jasmine/tests/violin_test.js b/test/jasmine/tests/violin_test.js
@@ -493,3 +493,61 @@ describe('Test violin hover:', function() {
         });
     });
 });
+
+describe('Test violin restyle:', function() {
+    var gd;
+
+    beforeEach(function() {
+        gd = createGraphDiv();
+    });
+
+    afterEach(destroyGraphDiv);
+
+    it('should be able to add/remove innner parts', function(done) {
+        var fig = Lib.extendDeep({}, require('@mocks/violin_old-faithful.json'));
+        // start with just 1 violin
+        delete fig.data[0].meanline;
+        delete fig.data[0].points;
+
+        function _assertOne(msg, exp, trace3, k, query) {
+            expect(trace3.selectAll(query).size())
+                .toBe(exp[k] || 0, k + ' - ' + msg);
+        }
+
+        function _assert(msg, exp) {
+            var trace3 = d3.select(gd).select('.violinlayer > .trace');
+            _assertOne(msg, exp, trace3, 'violinCnt', 'path.violin');
+            _assertOne(msg, exp, trace3, 'boxCnt', 'path.box');
+            _assertOne(msg, exp, trace3, 'meanlineInBoxCnt', 'path.mean');
+            _assertOne(msg, exp, trace3, 'meanlineOutOfBoxCnt', 'path.meanline');
+            _assertOne(msg, exp, trace3, 'ptsCnt', 'path.point');
+        }
+
+        Plotly.plot(gd, fig)
+        .then(function() {
+            _assert('base', {violinCnt: 1});
+        })
+        .then(function() { return Plotly.restyle(gd, 'box.visible', true); })
+        .then(function() {
+            _assert('with inner box', {violinCnt: 1, boxCnt: 1});
+        })
+        .then(function() { return Plotly.restyle(gd, 'meanline.visible', true); })
+        .then(function() {
+            _assert('with inner box & meanline', {violinCnt: 1, boxCnt: 1, meanlineInBoxCnt: 1});
+        })
+        .then(function() { return Plotly.restyle(gd, 'box.visible', false); })
+        .then(function() {
+            _assert('with meanline', {violinCnt: 1, meanlineOutOfBoxCnt: 1});
+        })
+        .then(function() { return Plotly.restyle(gd, 'points', 'all'); })
+        .then(function() {
+            _assert('with meanline & pts', {violinCnt: 1, meanlineOutOfBoxCnt: 1, ptsCnt: 272});
+        })
+        .then(function() { return Plotly.restyle(gd, 'meanline.visible', false); })
+        .then(function() {
+            _assert('with pts', {violinCnt: 1, ptsCnt: 272});
+        })
+        .catch(failTest)
+        .then(done);
+    });
+});
