Merge pull request #365 from plotly/legend-fixup

Legend fixups [fixes #348]

Author: etpinard

src/components/legend/draw.js

@@ -56,8 +56,9 @@ module.exports = function draw(gd) {
         });
 
     var clipPath = fullLayout._topdefs.selectAll('#' + clipId)
-        .data([0])
-      .enter().append('clipPath')
+        .data([0]);
+
+    clipPath.enter().append('clipPath')
         .attr('id', clipId)
         .append('rect');
 
@@ -210,7 +211,7 @@ module.exports = function draw(gd) {
 
     legend.attr('transform', 'translate(' + lx + ',' + ly + ')');
 
-    clipPath.attr({
+    clipPath.select('rect').attr({
         width: opts.width,
         height: scrollheight,
         x: 0,
@@ -220,7 +221,8 @@ module.exports = function draw(gd) {
     legend.call(Drawing.setClipUrl, clipId);
 
     // If scrollbar should be shown.
-    if(gd.firstRender && opts.height - scrollheight > 0 && !gd._context.staticPlot) {
+    if(opts.height - scrollheight > 0 && !gd._context.staticPlot) {
+
         bg.attr({
             width: opts.width - 2 * opts.borderwidth + constants.scrollBarWidth
         });
@@ -229,44 +231,46 @@ module.exports = function draw(gd) {
             width: opts.width + constants.scrollBarWidth
         });
 
-        legend.node().addEventListener('wheel', function(e) {
-            e.preventDefault();
-            scrollHandler(e.deltaY / 20);
-        });
-
-        scrollBar.node().addEventListener('mousedown', function(e) {
-            e.preventDefault();
+        if(gd.firstRender) {
+            // Move scrollbar to starting position
+            scrollBar.call(
+                Drawing.setRect,
+                opts.width - (constants.scrollBarWidth + constants.scrollBarMargin),
+                constants.scrollBarMargin,
+                constants.scrollBarWidth,
+                constants.scrollBarHeight
+            );
+            scrollBox.attr('data-scroll',0);
+        }
 
-            function mMove(e) {
-                if(e.buttons === 1) {
-                    scrollHandler(e.movementY);
-                }
-            }
+        scrollHandler(0,scrollheight);
 
-            function mUp() {
-                scrollBar.node().removeEventListener('mousemove', mMove);
-                window.removeEventListener('mouseup', mUp);
-            }
+        legend.on('wheel',null);
 
-            window.addEventListener('mousemove', mMove);
-            window.addEventListener('mouseup', mUp);
+        legend.on('wheel', function() {
+            var e = d3.event;
+            e.preventDefault();
+            scrollHandler(e.deltaY / 20, scrollheight);
         });
 
-        // Move scrollbar to starting position on the first render
-        scrollBar.call(
-            Drawing.setRect,
-            opts.width - (constants.scrollBarWidth + constants.scrollBarMargin),
-            constants.scrollBarMargin,
-            constants.scrollBarWidth,
-            constants.scrollBarHeight
-        );
+        scrollBar.on('.drag',null);
+        scrollBox.on('.drag',null);
+        var drag = d3.behavior.drag()
+            .on('drag', function() {
+                scrollHandler(d3.event.dy, scrollheight);
+            });
+
+        scrollBar.call(drag);
+        scrollBox.call(drag);
+
     }
 
-    function scrollHandler(delta) {
+
+    function scrollHandler(delta, scrollheight) {
 
         var scrollBarTrack = scrollheight - constants.scrollBarHeight - 2 * constants.scrollBarMargin,
             translateY = scrollBox.attr('data-scroll'),
-            scrollBoxY = Lib.constrain(translateY - delta, Math.min(scrollheight - opts.height, 0), 0),
+            scrollBoxY = Lib.constrain(translateY - delta, scrollheight-opts.height, 0),
             scrollBarY = -scrollBoxY / (opts.height - scrollheight) * scrollBarTrack + constants.scrollBarMargin;
 
         scrollBox.attr('data-scroll', scrollBoxY);

test/jasmine/tests/legend_scroll_test.js

@@ -1,4 +1,6 @@
 var Plotly = require('@lib/index');
+var Lib = require('@src/lib');
+
 var createGraph = require('../assets/create_graph_div');
 var destroyGraph = require('../assets/destroy_graph_div');
 var getBBox = require('../assets/get_bbox');
@@ -20,20 +22,29 @@ describe('The legend', function() {
         return gd._fullLayout._topdefs.selectAll('#legend' + uid).size();
     }
 
+    function getPlotHeight(gd) {
+        return gd._fullLayout.height - gd._fullLayout.margin.t - gd._fullLayout.margin.b;
+    }
+
+
     describe('when plotted with many traces', function() {
-        beforeEach(function() {
+        beforeEach(function(done) {
             gd = createGraph();
-            Plotly.plot(gd, mock.data, mock.layout);
-            legend = document.getElementsByClassName('legend')[0];
+
+            var mockCopy = Lib.extendDeep({}, mock);
+
+            Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
+                legend = document.getElementsByClassName('legend')[0];
+                done();
+            });
         });
 
         afterEach(destroyGraph);
 
         it('should not exceed plot height', function() {
-            var legendHeight = getBBox(legend).height,
-                plotHeight = gd._fullLayout.height - gd._fullLayout.margin.t - gd._fullLayout.margin.b;
+            var legendHeight = getBBox(legend).height;
 
-            expect(+legendHeight).toBe(plotHeight);
+            expect(+legendHeight).toBe(getPlotHeight(gd));
         });
 
         it('should insert a scrollbar', function() {
@@ -88,8 +99,27 @@ describe('The legend', function() {
                 done();
             });
         });
+
+        it('should resize when relayout\'ed with new height', function(done) {
+            var origLegendHeight = getBBox(legend).height;
+
+            Plotly.relayout(gd, 'height', gd._fullLayout.height / 2).then(function() {
+                var legendHeight = getBBox(legend).height;
+
+                //legend still exists and not duplicated
+                expect(countLegendGroups(gd)).toBe(1);
+                expect(countLegendClipPaths(gd)).toBe(1);
+
+                // clippath resized to new height less than new plot height
+                expect(+legendHeight).toBe(getPlotHeight(gd));
+                expect(+legendHeight).toBeLessThan(+origLegendHeight);
+
+                done();
+            });
+        });
     });
 
+
     describe('when plotted with few traces', function() {
         var gd;
 
@@ -122,6 +152,22 @@ describe('The legend', function() {
                 done();
             });
         });
+
+        it('should resize when traces added', function(done) {
+            var origLegend = document.getElementsByClassName('legend')[0];
+            var origLegendHeight = getBBox(origLegend).height;
+
+            Plotly.addTraces(gd, { x: [1,2,3], y: [4,3,2], name: 'Test2' }).then(function() {
+                var legend = document.getElementsByClassName('legend')[0];
+                var legendHeight = getBBox(legend).height;
+
+                // clippath resized to show new trace
+                expect(+legendHeight).toBeCloseTo(+origLegendHeight + 19, 0);
+
+                done();
+            });
+
+        });
     });
 });