From 8f053164a643ebf49e1e6887f05cd48c194b3f9b Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Mon, 30 May 2016 18:19:42 +0100 Subject: [PATCH 1/7] legend: keep scrollbar position after toggle * Only set the scrollbar position during first render. * Only register the wheel and drag listeners during first render. * If the scrollbar is to be shown, ensure only the expanded scrollbox is drawn. Fixes #578 --- src/components/legend/draw.js | 119 ++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 57 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 9c6dd4e1bb7..46bbc823739 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -198,28 +198,36 @@ 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 scrollPosition = scrollBox.attr('data-scroll') || 0; - - Lib.setTranslate(scrollBox, 0, scrollPosition); + var scrollBarYMax = legendHeight - + constants.scrollBarHeight - + 2 * constants.scrollBarMargin, + scrollBoxYMax = opts.height - legendHeight, + scrollBarY, + scrollBoxY; + + 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 + }); - clipPath.select('rect').attr({ - width: legendWidth - 2 * opts.borderwidth, - height: legendHeight - 2 * opts.borderwidth, - x: opts.borderwidth - scrollPosition, - y: opts.borderwidth - }); + Lib.setTranslate(scrollBox, 0, 0); - 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 @@ -227,57 +235,54 @@ module.exports = function draw(gd) { 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); - } + scrollBox.call(Drawing.setClipUrl, clipId); - var scrollBarYMax = legendHeight - - constants.scrollBarHeight - - 2 * constants.scrollBarMargin, - scrollBoxYMax = opts.height - legendHeight, - scrollBarY = constants.scrollBarMargin, - scrollBoxY = 0; - - scrollHandler(scrollBarY, scrollBoxY); - - legend.on('wheel', null); - legend.on('wheel', function() { - scrollBoxY = Lib.constrain( - scrollBox.attr('data-scroll') - - d3.event.deltaY / scrollBarYMax * scrollBoxYMax, - -scrollBoxYMax, 0); - scrollBarY = constants.scrollBarMargin - - scrollBoxY / scrollBoxYMax * scrollBarYMax; + if(gd.firstRender) { scrollHandler(scrollBarY, scrollBoxY); - d3.event.preventDefault(); - }); - 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, - constants.scrollBarMargin, - constants.scrollBarMargin + scrollBarYMax); - scrollBoxY = - (scrollBarY - constants.scrollBarMargin) / - scrollBarYMax * scrollBoxYMax; - scrollHandler(scrollBarY, scrollBoxY); - }); + legend.on('wheel', null); + legend.on('wheel', function() { + scrollBoxY = Lib.constrain( + scrollBox.attr('data-scroll') - + d3.event.deltaY / scrollBarYMax * scrollBoxYMax, + -scrollBoxYMax, 0); + scrollBarY = constants.scrollBarMargin - + scrollBoxY / scrollBoxYMax * scrollBarYMax; + scrollHandler(scrollBarY, scrollBoxY); + d3.event.preventDefault(); + }); - scrollBar.call(drag); - scrollBox.call(drag); + 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, + constants.scrollBarMargin, + constants.scrollBarMargin + scrollBarYMax); + scrollBoxY = - (scrollBarY - constants.scrollBarMargin) / + scrollBarYMax * scrollBoxYMax; + scrollHandler(scrollBarY, scrollBoxY); + }); + scrollBar.call(drag); + scrollBox.call(drag); + } } From 9b6baa82a9eeafbe69994eea861e567d5cd6df5a Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Mon, 30 May 2016 18:57:04 +0100 Subject: [PATCH 2/7] test: legend scrollbar keeps position after toggle --- test/jasmine/tests/legend_scroll_test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/jasmine/tests/legend_scroll_test.js b/test/jasmine/tests/legend_scroll_test.js index e7e0103909d..5aa1696e577 100644 --- a/test/jasmine/tests/legend_scroll_test.js +++ b/test/jasmine/tests/legend_scroll_test.js @@ -79,6 +79,21 @@ 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 constrain scrolling to the contents', function() { var scrollBox = legend.getElementsByClassName('scrollbox')[0]; From 1021272d6dd714626cc43f3ded6f1210f5984797 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 1 Jun 2016 20:40:45 +0100 Subject: [PATCH 3/7] test: hiding and showing back a legend * Tested that event listeners in a legend work after hiding and showing back the legend. --- test/jasmine/tests/legend_scroll_test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/jasmine/tests/legend_scroll_test.js b/test/jasmine/tests/legend_scroll_test.js index 5aa1696e577..b414efc06fb 100644 --- a/test/jasmine/tests/legend_scroll_test.js +++ b/test/jasmine/tests/legend_scroll_test.js @@ -85,8 +85,27 @@ describe('The legend', function() { 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 keep toggle listeners after relayout', function() { + Plotly.relayout(gd, 'showlegend', false); + Plotly.relayout(gd, 'showlegend', true); + + var legend = document.getElementsByClassName('legend')[0], + 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); From 17d07c003ef71ee58e867461520e33beec5e4c8c Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 1 Jun 2016 20:49:27 +0100 Subject: [PATCH 4/7] legend: ensure event listeners are registered --- src/components/legend/draw.js | 52 +++++++++++++++++------------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 46bbc823739..153b0a31c25 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -253,36 +253,36 @@ module.exports = function draw(gd) { scrollBox.call(Drawing.setClipUrl, clipId); - if(gd.firstRender) { + if(gd.firstRender) scrollHandler(scrollBarY, scrollBoxY); + + legend.on('wheel', null); // to be safe, remove previous listeners + legend.on('wheel', function() { + scrollBoxY = Lib.constrain( + scrollBox.attr('data-scroll') - + d3.event.deltaY / scrollBarYMax * scrollBoxYMax, + -scrollBoxYMax, 0); + scrollBarY = constants.scrollBarMargin - + scrollBoxY / scrollBoxYMax * scrollBarYMax; scrollHandler(scrollBarY, scrollBoxY); + d3.event.preventDefault(); + }); - legend.on('wheel', null); - legend.on('wheel', function() { - scrollBoxY = Lib.constrain( - scrollBox.attr('data-scroll') - - d3.event.deltaY / scrollBarYMax * scrollBoxYMax, - -scrollBoxYMax, 0); - scrollBarY = constants.scrollBarMargin - - scrollBoxY / scrollBoxYMax * scrollBarYMax; - scrollHandler(scrollBarY, scrollBoxY); - d3.event.preventDefault(); - }); + // to be safe, remove previous listeners + scrollBar.on('.drag', null); + scrollBox.on('.drag', null); - 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, - constants.scrollBarMargin, - constants.scrollBarMargin + scrollBarYMax); - scrollBoxY = - (scrollBarY - constants.scrollBarMargin) / - scrollBarYMax * scrollBoxYMax; - scrollHandler(scrollBarY, scrollBoxY); - }); + var drag = d3.behavior.drag().on('drag', function() { + scrollBarY = Lib.constrain( + d3.event.y - constants.scrollBarHeight / 2, + constants.scrollBarMargin, + constants.scrollBarMargin + scrollBarYMax); + scrollBoxY = - (scrollBarY - constants.scrollBarMargin) / + scrollBarYMax * scrollBoxYMax; + scrollHandler(scrollBarY, scrollBoxY); + }); - scrollBar.call(drag); - scrollBox.call(drag); - } + scrollBar.call(drag); + scrollBox.call(drag); } From 12c760460598b9f1dc90c601664a7d8abd80299d Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Fri, 3 Jun 2016 00:01:48 +0100 Subject: [PATCH 5/7] test: legend scrollbar is restored after relayout --- test/jasmine/tests/legend_scroll_test.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/jasmine/tests/legend_scroll_test.js b/test/jasmine/tests/legend_scroll_test.js index b414efc06fb..bd949ab80f8 100644 --- a/test/jasmine/tests/legend_scroll_test.js +++ b/test/jasmine/tests/legend_scroll_test.js @@ -94,14 +94,22 @@ describe('The legend', function() { 'translate(0, ' + dataScroll + ')'); }); - it('should keep toggle listeners after relayout', function() { + it('should be restored and functional after relayout', function() { + var wheelDeltaY = 100, + legend = document.getElementsByClassName('legend')[0], + scrollBox, + scrollBar, + toggle; + + legend.dispatchEvent(scrollTo(wheelDeltaY)); + Plotly.relayout(gd, 'showlegend', false); Plotly.relayout(gd, 'showlegend', true); - var legend = document.getElementsByClassName('legend')[0], - scrollBox = legend.getElementsByClassName('scrollbox')[0], - toggle = legend.getElementsByClassName('legendtoggle')[0], - wheelDeltaY = 100; + legend = document.getElementsByClassName('legend')[0]; + scrollBox = legend.getElementsByClassName('scrollbox')[0]; + scrollBar = legend.getElementsByClassName('scrollbar')[0]; + toggle = legend.getElementsByClassName('legendtoggle')[0]; legend.dispatchEvent(scrollTo(wheelDeltaY)); @@ -111,6 +119,8 @@ describe('The legend', function() { 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() { From ce7f8d89705749fd4331384d19f56fa6c16900a0 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Fri, 3 Jun 2016 00:02:55 +0100 Subject: [PATCH 6/7] legend: restore scrollbar after relayout --- src/components/legend/draw.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 153b0a31c25..17d386288fb 100644 --- a/src/components/legend/draw.js +++ b/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); } @@ -253,7 +251,7 @@ module.exports = function draw(gd) { scrollBox.call(Drawing.setClipUrl, clipId); - if(gd.firstRender) scrollHandler(scrollBarY, scrollBoxY); + if(firstRender) scrollHandler(scrollBarY, scrollBoxY); legend.on('wheel', null); // to be safe, remove previous listeners legend.on('wheel', function() { From 3ebb324852b34bee32dce3d1012b4e5b8382f4d0 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Fri, 3 Jun 2016 11:24:30 +0100 Subject: [PATCH 7/7] test: legend scrollbox after relayout --- test/jasmine/tests/legend_scroll_test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/jasmine/tests/legend_scroll_test.js b/test/jasmine/tests/legend_scroll_test.js index bd949ab80f8..3e09f0f91da 100644 --- a/test/jasmine/tests/legend_scroll_test.js +++ b/test/jasmine/tests/legend_scroll_test.js @@ -99,9 +99,14 @@ describe('The legend', function() { 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); @@ -112,6 +117,8 @@ describe('The legend', function() { 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'));