Merge pull request #2829 from plotly/legend-horiz-edit-position-fixups

Position-editable horizontal legend toggle fixups

Author: etpinard

src/components/legend/draw.js

@@ -341,12 +341,13 @@ module.exports = function draw(gd) {
                 }
             },
             clickFn: function(numClicks, e) {
-                var clickedTrace =
-                    fullLayout._infolayer.selectAll('g.traces').filter(function() {
-                        var bbox = this.getBoundingClientRect();
-                        return (e.clientX >= bbox.left && e.clientX <= bbox.right &&
-                            e.clientY >= bbox.top && e.clientY <= bbox.bottom);
-                    });
+                var clickedTrace = fullLayout._infolayer.selectAll('g.traces').filter(function() {
+                    var bbox = this.getBoundingClientRect();
+                    return (
+                        e.clientX >= bbox.left && e.clientX <= bbox.right &&
+                        e.clientY >= bbox.top && e.clientY <= bbox.bottom
+                    );
+                });
                 if(clickedTrace.size() > 0) {
                     clickOrDoubleClick(gd, legend, clickedTrace, numClicks, e);
                 }
@@ -672,14 +673,19 @@ function computeLegendDimensions(gd, groups, traces) {
     opts._width = Math.ceil(opts._width);
     opts._height = Math.ceil(opts._height);
 
+    var isEditable = (
+        gd._context.edits.legendText ||
+        gd._context.edits.legendPosition
+    );
+
     traces.each(function(d) {
-        var legendItem = d[0],
-            bg = d3.select(this).select('.legendtoggle');
+        var legendItem = d[0];
+        var bg = d3.select(this).select('.legendtoggle');
 
         Drawing.setRect(bg,
             0,
             -legendItem.height / 2,
-            (gd._context.edits.legendText ? 0 : opts._width) + extraWidth,
+            (isEditable ? 0 : opts._width) + extraWidth,
             legendItem.height
         );
     });

test/jasmine/tests/legend_test.js

@@ -10,6 +10,7 @@ var anchorUtils = require('@src/components/legend/anchor_utils');
 
 var d3 = require('d3');
 var failTest = require('../assets/fail_test');
+var mouseEvent = require('../assets/mouse_event');
 var delay = require('../assets/delay');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
@@ -929,6 +930,7 @@ describe('legend interaction', function() {
 
     describe('editable mode interactions', function() {
         var gd;
+
         var mock = {
             data: [{
                 x: [1, 2, 3],
@@ -1027,6 +1029,81 @@ describe('legend interaction', function() {
         });
     });
 
+    describe('visible toggle', function() {
+        var gd;
+
+        beforeEach(function() {
+            gd = createGraphDiv();
+        });
+
+        afterEach(destroyGraphDiv);
+
+        var data = [
+            {y: [1, 2, 1]},
+            {y: [2, 1, 2]},
+            {y: [2, 3, 4]}
+        ];
+
+        // we need to click on the drag cover to truly test this,
+        function clickAt(p) {
+            return function() {
+                return new Promise(function(resolve) {
+                    var el = d3.select('g.legend').node();
+                    var opts = {element: el};
+                    mouseEvent('mousedown', p[0], p[1], opts);
+                    mouseEvent('mouseup', p[0], p[1], opts);
+                    setTimeout(resolve, DBLCLICKDELAY + 20);
+                });
+            };
+        }
+
+        function assertVisible(expectation) {
+            return function() {
+                var actual = gd._fullData.map(function(t) { return t.visible; });
+                expect(actual).toEqual(expectation);
+            };
+        }
+
+        var specs = [{
+            orientation: 'h',
+            edits: {legendPosition: true},
+            clickPos: [[118, 469], [212, 469], [295, 469]]
+        }, {
+            orientation: 'h',
+            edits: {legendPosition: true, legendText: true},
+            clickPos: [[118, 469], [212, 469], [295, 469]]
+        }, {
+            orientation: 'v',
+            edits: {legendPosition: true},
+            clickPos: [[430, 114], [430, 131], [430, 153]]
+        }, {
+            orientation: 'v',
+            edits: {legendPosition: true, legendText: true},
+            clickPos: [[430, 114], [430, 131], [430, 153]]
+        }];
+
+        specs.forEach(function(s) {
+            var msg = s.orientation + ' - ' + JSON.stringify(s.edits);
+
+            it('should find correct bounding box (case ' + msg + ')', function(done) {
+                Plotly.plot(gd,
+                    Lib.extendDeep([], data),
+                    {legend: {orientation: s.orientation}, width: 500, height: 500},
+                    {edits: s.edits}
+                )
+                .then(assertVisible([true, true, true]))
+                .then(clickAt(s.clickPos[0]))
+                .then(assertVisible(['legendonly', true, true]))
+                .then(clickAt(s.clickPos[1]))
+                .then(assertVisible(['legendonly', 'legendonly', true]))
+                .then(clickAt(s.clickPos[2]))
+                .then(assertVisible(['legendonly', 'legendonly', 'legendonly']))
+                .catch(failTest)
+                .then(done);
+            });
+        });
+    });
+
     describe('legend visibility interactions', function() {
         var gd;