Merge pull request plotly#909 from plotly/legend-visibility-toggle

Fix legend trace toggle background on restyles

Author: etpinard

src/components/legend/draw.js

@@ -429,7 +429,6 @@ function setupTraceToggle(g, gd) {
 
 function computeTextDimensions(g, gd) {
     var legendItem = g.data()[0][0],
-        bg = g.selectAll('.legendtoggle'),
         mathjaxGroup = g.select('g[class*=math-group]'),
         opts = gd._fullLayout.legend,
         lineHeight = opts.font.size * 1.3,
@@ -466,8 +465,6 @@ function computeTextDimensions(g, gd) {
 
     height = Math.max(height, 16) + 3;
 
-    bg.attr({x: 0, y: -height / 2, height: height});
-
     legendItem.height = height;
     legendItem.width = width;
 }
@@ -508,12 +505,21 @@ function computeLegendDimensions(gd, groups, traces) {
             opts.height += (opts._lgroupsLength - 1) * opts.tracegroupgap;
         }
 
-        traces.selectAll('.legendtoggle')
-            .attr('width', (gd._context.editable ? 0 : opts.width) + 40);
-
         // make sure we're only getting full pixels
         opts.width = Math.ceil(opts.width);
         opts.height = Math.ceil(opts.height);
+
+        traces.each(function(d) {
+            var legendItem = d[0],
+                bg = d3.select(this).select('.legendtoggle');
+
+            bg.call(Drawing.setRect,
+                0,
+                -legendItem.height / 2,
+                (gd._context.editable ? 0 : opts.width) + 40,
+                legendItem.height
+            );
+        });
     }
     else if(isGrouped) {
         opts.width = 0;
@@ -564,8 +570,17 @@ function computeLegendDimensions(gd, groups, traces) {
         opts.width = Math.ceil(opts.width);
         opts.height = Math.ceil(opts.height);
 
-        traces.selectAll('.legendtoggle')
-            .attr('width', (gd._context.editable ? 0 : opts.width));
+        traces.each(function(d) {
+            var legendItem = d[0],
+                bg = d3.select(this).select('.legendtoggle');
+
+            bg.call(Drawing.setRect,
+                0,
+                -legendItem.height / 2,
+                (gd._context.editable ? 0 : opts.width),
+                legendItem.height
+            );
+        });
     }
     else {
         opts.width = 0;
@@ -581,7 +596,6 @@ function computeLegendDimensions(gd, groups, traces) {
         });
 
         traces.each(function(d) {
-
             var legendItem = d[0],
                 traceWidth = maxTraceWidth,
                 traceGap = opts.tracegroupgap || 5;
@@ -613,8 +627,17 @@ function computeLegendDimensions(gd, groups, traces) {
         opts.width = Math.ceil(opts.width);
         opts.height = Math.ceil(opts.height);
 
-        traces.selectAll('.legendtoggle')
-            .attr('width', (gd._context.editable ? 0 : opts.width));
+        traces.each(function(d) {
+            var legendItem = d[0],
+                bg = d3.select(this).select('.legendtoggle');
+
+            bg.call(Drawing.setRect,
+                0,
+                -legendItem.height / 2,
+                (gd._context.editable ? 0 : opts.width),
+                legendItem.height
+            );
+        });
     }
 }
 

src/traces/bar/arrays_to_calcdata.js

@@ -15,12 +15,16 @@ var mergeArray = require('../../lib').mergeArray;
 // arrayOk attributes, merge them into calcdata array
 module.exports = function arraysToCalcdata(cd) {
     var trace = cd[0].trace,
-        marker = trace.marker,
-        markerLine = marker.line;
+        marker = trace.marker;
 
     mergeArray(trace.text, cd, 'tx');
-    mergeArray(marker.opacity, cd, 'mo');
-    mergeArray(marker.color, cd, 'mc');
-    mergeArray(markerLine.color, cd, 'mlc');
-    mergeArray(markerLine.width, cd, 'mlw');
+
+    if(marker && marker.line) {
+        var markerLine = marker.line;
+
+        mergeArray(marker.opacity, cd, 'mo');
+        mergeArray(marker.color, cd, 'mc');
+        mergeArray(markerLine.color, cd, 'mlc');
+        mergeArray(markerLine.width, cd, 'mlw');
+    }
 };

test/jasmine/tests/legend_test.js

@@ -10,6 +10,7 @@ var anchorUtils = require('@src/components/legend/anchor_utils');
 var d3 = require('d3');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
+var customMatchers = require('../assets/custom_matchers');
 
 
 describe('legend defaults', function() {
@@ -563,3 +564,59 @@ describe('legend orientation change:', function() {
         });
     });
 });
+
+describe('legend restyle update', function() {
+    'use strict';
+
+    beforeAll(function() {
+        jasmine.addMatchers(customMatchers);
+    });
+
+    afterEach(destroyGraphDiv);
+
+    it('should update trace toggle background rectangle', function(done) {
+        var mock = require('@mocks/0.json'),
+            mockCopy = Lib.extendDeep({}, mock),
+            gd = createGraphDiv();
+
+        mockCopy.data[0].visible = false;
+        mockCopy.data[0].showlegend = false;
+        mockCopy.data[1].visible = false;
+        mockCopy.data[1].showlegend = false;
+
+        function countLegendItems() {
+            return d3.select(gd).selectAll('rect.legendtoggle').size();
+        }
+
+        function assertTraceToggleRect() {
+            var nodes = d3.selectAll('rect.legendtoggle');
+
+            nodes.each(function() {
+                var node = d3.select(this);
+
+                expect(node.attr('x')).toEqual('0');
+                expect(node.attr('y')).toEqual('-9.5');
+                expect(node.attr('height')).toEqual('19');
+
+                var w = +node.attr('width');
+                expect(Math.abs(w - 160)).toBeLessThan(10);
+            });
+        }
+
+        Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
+            expect(countLegendItems()).toEqual(1);
+            assertTraceToggleRect();
+
+            return Plotly.restyle(gd, 'visible', [true, false, false]);
+        }).then(function() {
+            expect(countLegendItems()).toEqual(0);
+
+            return Plotly.restyle(gd, 'showlegend', [true, false, false]);
+        }).then(function() {
+            expect(countLegendItems()).toEqual(1);
+            assertTraceToggleRect();
+
+            done();
+        });
+    });
+});