Merge pull request #584 from n-riesco/issue-578-do-not-reset-scrollbar-position-after-toggle

Issue 578 (keep legend scrollbar position after toggle)

Author: etpinard

src/components/legend/draw.js

@@ -43,9 +43,6 @@ module.exports = function draw(gd) {
         return;
     }
 
-    if(typeof gd.firstRender === 'undefined') gd.firstRender = true;
-    else if(gd.firstRender) gd.firstRender = false;
-
     var legend = fullLayout._infolayer.selectAll('g.legend')
         .data([0]);
 
@@ -122,7 +119,8 @@ module.exports = function draw(gd) {
                 .call(setupTraceToggle, gd);
         });
 
-    if(gd.firstRender) {
+    var firstRender = legend.enter().size() !== 0;
+    if(firstRender) {
         computeLegendDimensions(gd, groups, traces);
         expandMargin(gd);
     }
@@ -198,60 +196,64 @@ module.exports = function draw(gd) {
     // legend, background and border, scroll box and scroll bar
     Lib.setTranslate(legend, lx, ly);
 
-    bg.attr({
-        width: legendWidth - opts.borderwidth,
-        height: legendHeight - opts.borderwidth,
-        x: opts.borderwidth / 2,
-        y: opts.borderwidth / 2
-    });
+    var scrollBarYMax = legendHeight -
+            constants.scrollBarHeight -
+            2 * constants.scrollBarMargin,
+        scrollBoxYMax = opts.height - legendHeight,
+        scrollBarY,
+        scrollBoxY;
 
-    var scrollPosition = scrollBox.attr('data-scroll') || 0;
+    if(opts.height <= legendHeight || gd._context.staticPlot) {
+        // if scrollbar should not be shown.
+        bg.attr({
+            width: legendWidth - opts.borderwidth,
+            height: legendHeight - opts.borderwidth,
+            x: opts.borderwidth / 2,
+            y: opts.borderwidth / 2
+        });
 
-    Lib.setTranslate(scrollBox, 0, scrollPosition);
+        Lib.setTranslate(scrollBox, 0, 0);
 
-    clipPath.select('rect').attr({
-        width: legendWidth - 2 * opts.borderwidth,
-        height: legendHeight - 2 * opts.borderwidth,
-        x: opts.borderwidth - scrollPosition,
-        y: opts.borderwidth
-    });
-
-    scrollBox.call(Drawing.setClipUrl, clipId);
+        clipPath.select('rect').attr({
+            width: legendWidth - 2 * opts.borderwidth,
+            height: legendHeight - 2 * opts.borderwidth,
+            x: opts.borderwidth,
+            y: opts.borderwidth
+        });
 
-    // If scrollbar should be shown.
-    if(opts.height - legendHeight > 0 && !gd._context.staticPlot) {
+        scrollBox.call(Drawing.setClipUrl, clipId);
+    }
+    else {
+        scrollBarY = constants.scrollBarMargin,
+        scrollBoxY = scrollBox.attr('data-scroll') || 0;
 
         // increase the background and clip-path width
         // by the scrollbar width and margin
         bg.attr({
             width: legendWidth -
                 2 * opts.borderwidth +
                 constants.scrollBarWidth +
-                constants.scrollBarMargin
+                constants.scrollBarMargin,
+            height: legendHeight - opts.borderwidth,
+            x: opts.borderwidth / 2,
+            y: opts.borderwidth / 2
         });
 
         clipPath.select('rect').attr({
             width: legendWidth -
                 2 * opts.borderwidth +
                 constants.scrollBarWidth +
-                constants.scrollBarMargin
+                constants.scrollBarMargin,
+            height: legendHeight - 2 * opts.borderwidth,
+            x: opts.borderwidth,
+            y: opts.borderwidth - scrollBoxY
         });
 
-        if(gd.firstRender) {
-            // Move scrollbar to starting position
-            scrollHandler(constants.scrollBarMargin, 0);
-        }
-
-        var scrollBarYMax = legendHeight -
-                constants.scrollBarHeight -
-                2 * constants.scrollBarMargin,
-            scrollBoxYMax = opts.height - legendHeight,
-            scrollBarY = constants.scrollBarMargin,
-            scrollBoxY = 0;
+        scrollBox.call(Drawing.setClipUrl, clipId);
 
-        scrollHandler(scrollBarY, scrollBoxY);
+        if(firstRender) scrollHandler(scrollBarY, scrollBoxY);
 
-        legend.on('wheel', null);
+        legend.on('wheel', null);  // to be safe, remove previous listeners
         legend.on('wheel', function() {
             scrollBoxY = Lib.constrain(
                 scrollBox.attr('data-scroll') -
@@ -263,8 +265,10 @@ module.exports = function draw(gd) {
             d3.event.preventDefault();
         });
 
+        // to be safe, remove previous listeners
         scrollBar.on('.drag', null);
         scrollBox.on('.drag', null);
+
         var drag = d3.behavior.drag().on('drag', function() {
             scrollBarY = Lib.constrain(
                 d3.event.y - constants.scrollBarHeight / 2,
@@ -277,7 +281,6 @@ module.exports = function draw(gd) {
 
         scrollBar.call(drag);
         scrollBox.call(drag);
-
     }
 
 

test/jasmine/tests/legend_scroll_test.js

@@ -79,6 +79,57 @@ describe('The legend', function() {
                 'translate(0, ' + finalDataScroll + ')');
         });
 
+        it('should keep the scrollbar position after a toggle event', function() {
+            var scrollBox = legend.getElementsByClassName('scrollbox')[0],
+                toggle = legend.getElementsByClassName('legendtoggle')[0],
+                wheelDeltaY = 100;
+
+            legend.dispatchEvent(scrollTo(wheelDeltaY));
+
+            var dataScroll = scrollBox.getAttribute('data-scroll');
+            toggle.dispatchEvent(new MouseEvent('click'));
+            expect(+toggle.parentNode.style.opacity).toBeLessThan(1);
+            expect(scrollBox.getAttribute('data-scroll')).toBe(dataScroll);
+            expect(scrollBox.getAttribute('transform')).toBe(
+                'translate(0, ' + dataScroll + ')');
+        });
+
+        it('should be restored and functional after relayout', function() {
+            var wheelDeltaY = 100,
+                legend = document.getElementsByClassName('legend')[0],
+                scrollBox,
+                scrollBar,
+                scrollBarX,
+                scrollBarY,
+                toggle;
+
+            legend.dispatchEvent(scrollTo(wheelDeltaY));
+            scrollBar = legend.getElementsByClassName('scrollbar')[0];
+            scrollBarX = scrollBar.getAttribute('x'),
+            scrollBarY = scrollBar.getAttribute('y');
+
+            Plotly.relayout(gd, 'showlegend', false);
+            Plotly.relayout(gd, 'showlegend', true);
+
+            legend = document.getElementsByClassName('legend')[0];
+            scrollBox = legend.getElementsByClassName('scrollbox')[0];
+            scrollBar = legend.getElementsByClassName('scrollbar')[0];
+            toggle = legend.getElementsByClassName('legendtoggle')[0];
+
+            legend.dispatchEvent(scrollTo(wheelDeltaY));
+            expect(scrollBar.getAttribute('x')).toBe(scrollBarX);
+            expect(scrollBar.getAttribute('y')).toBe(scrollBarY);
+
+            var dataScroll = scrollBox.getAttribute('data-scroll');
+            toggle.dispatchEvent(new MouseEvent('click'));
+            expect(+toggle.parentNode.style.opacity).toBeLessThan(1);
+            expect(scrollBox.getAttribute('data-scroll')).toBe(dataScroll);
+            expect(scrollBox.getAttribute('transform')).toBe(
+                'translate(0, ' + dataScroll + ')');
+            expect(scrollBar.getAttribute('width')).toBeGreaterThan(0);
+            expect(scrollBar.getAttribute('height')).toBeGreaterThan(0);
+        });
+
         it('should constrain scrolling to the contents', function() {
             var scrollBox = legend.getElementsByClassName('scrollbox')[0];