Skip to content

Commit

Permalink
turn scrollBoxY into a positive number
Browse files Browse the repository at this point in the history
too confusing with it always being negative
  • Loading branch information
alexcjohnson committed Mar 1, 2018
1 parent 5660bf5 commit f2d8cfd
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 24 deletions.
21 changes: 10 additions & 11 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,7 @@ module.exports = function draw(gd) {
var scrollBoxYMax = opts._height - legendHeight;
var scrollRatio = scrollBarYMax / scrollBoxYMax;

// scrollBoxY is 0 or a negative number
var scrollBoxY = Math.max(opts._scrollY || 0, -scrollBoxYMax);
var scrollBoxY = Math.min(opts._scrollY || 0, scrollBoxYMax);

// increase the background and clip-path width
// by the scrollbar width and margin
Expand All @@ -265,7 +264,7 @@ module.exports = function draw(gd) {
constants.scrollBarMargin,
height: legendHeight - 2 * opts.borderwidth,
x: opts.borderwidth,
y: opts.borderwidth - scrollBoxY
y: opts.borderwidth + scrollBoxY
});

Drawing.setClipUrl(scrollBox, clipId);
Expand All @@ -274,11 +273,11 @@ module.exports = function draw(gd) {

legend.on('wheel', function() {
scrollBoxY = Lib.constrain(
opts._scrollY -
opts._scrollY +
d3.event.deltaY / scrollBarYMax * scrollBoxYMax,
-scrollBoxYMax, 0);
0, scrollBoxYMax);
scrollHandler(scrollBoxY, scrollBarHeight, scrollRatio);
if(scrollBoxY !== 0 && scrollBoxY !== -scrollBoxYMax) {
if(scrollBoxY !== 0 && scrollBoxY !== scrollBoxYMax) {
d3.event.preventDefault();
}
});
Expand All @@ -295,8 +294,8 @@ module.exports = function draw(gd) {
if(e.buttons === 2 || e.ctrlKey) return;

scrollBoxY = Lib.constrain(
(eventY0 - e.clientY) / scrollRatio + scrollBoxY0,
-scrollBoxYMax, 0);
(e.clientY - eventY0) / scrollRatio + scrollBoxY0,
0, scrollBoxYMax);
scrollHandler(scrollBoxY, scrollBarHeight, scrollRatio);
});

Expand All @@ -306,17 +305,17 @@ module.exports = function draw(gd) {

function scrollHandler(scrollBoxY, scrollBarHeight, scrollRatio) {
opts._scrollY = gd._fullLayout.legend._scrollY = scrollBoxY;
Drawing.setTranslate(scrollBox, 0, scrollBoxY);
Drawing.setTranslate(scrollBox, 0, -scrollBoxY);

Drawing.setRect(
scrollBar,
legendWidth,
constants.scrollBarMargin - scrollBoxY * scrollRatio,
constants.scrollBarMargin + scrollBoxY * scrollRatio,
constants.scrollBarWidth,
scrollBarHeight
);
clipPath.select('rect').attr({
y: opts.borderwidth - scrollBoxY
y: opts.borderwidth + scrollBoxY
});
}

Expand Down
26 changes: 13 additions & 13 deletions test/jasmine/tests/legend_scroll_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ describe('The legend', function() {
2 * constants.scrollBarMargin;
var initialDataScroll = getScroll(gd);
var wheelDeltaY = 100;
var finalDataScroll = Lib.constrain(initialDataScroll -
var finalDataScroll = Lib.constrain(initialDataScroll +
wheelDeltaY / scrollBarYMax * scrollBoxYMax,
-scrollBoxYMax, 0);
0, scrollBoxYMax);

legend.dispatchEvent(scrollTo(wheelDeltaY));

expect(getScroll(gd)).toBe(finalDataScroll);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + finalDataScroll + ')');
'translate(0, ' + -finalDataScroll + ')');
});

function dragScroll(element, rightClick) {
Expand All @@ -120,9 +120,9 @@ describe('The legend', function() {
2 * constants.scrollBarMargin;
var initialDataScroll = getScroll(gd);
var dy = 50;
var finalDataScroll = Lib.constrain(initialDataScroll -
var finalDataScroll = Lib.constrain(initialDataScroll +
dy / scrollBarYMax * scrollBoxYMax,
-scrollBoxYMax, 0);
0, scrollBoxYMax);

var y0 = scrollBarBB.top + scrollBarBB.height / 5;
var y1 = y0 + dy;
Expand Down Expand Up @@ -152,7 +152,7 @@ describe('The legend', function() {
var dataScroll = getScroll(gd);
expect(dataScroll).toBeCloseTo(finalDataScroll, 3);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + dataScroll + ')');
'translate(0, ' + -dataScroll + ')');
});

it('should not scroll on dragging the scrollbox', function() {
Expand All @@ -162,7 +162,7 @@ describe('The legend', function() {
var dataScroll = getScroll(gd);
expect(dataScroll).not.toBeCloseTo(finalDataScroll, 3);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + dataScroll + ')');
'translate(0, ' + -dataScroll + ')');
});

it('should not scroll on dragging the scrollbar with a right click', function() {
Expand All @@ -172,7 +172,7 @@ describe('The legend', function() {
var dataScroll = getScroll(gd);
expect(dataScroll).not.toBeCloseTo(finalDataScroll, 3);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + dataScroll + ')');
'translate(0, ' + -dataScroll + ')');
});

it('removes scroll bar and handlers when switching to horizontal', function(done) {
Expand Down Expand Up @@ -214,15 +214,15 @@ describe('The legend', function() {
expect(scrollBarHeight1).toBeGreaterThan(scrollBarHeight);

// we haven't quite removed the scrollbar, but we should have clipped the scroll value
return Plotly.deleteTraces(gd, [0, 1, 2, 3, 4, 5, 6, 7]);
return Plotly.deleteTraces(gd, [0, 1, 2, 3, 4, 5, 6]);
})
.then(function() {
expect(getScroll(gd)).toBeGreaterThan(dataScroll + 1);
expect(getScroll(gd)).toBeLessThan(dataScroll - 1);
var scrollBarHeight2 = getScrollBar().getBoundingClientRect().height;
expect(scrollBarHeight2).toBeGreaterThan(scrollBarHeight1);

// now no more scrollBar
return Plotly.deleteTraces(gd, [0, 1]);
return Plotly.deleteTraces(gd, [0, 1, 2]);
})
.then(function() {
expect(hasScrollBar()).toBe(false);
Expand Down Expand Up @@ -251,7 +251,7 @@ describe('The legend', function() {
expect(+toggle.parentNode.style.opacity).toBeLessThan(1);
expect(getScroll(gd)).toBe(dataScroll);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + dataScroll + ')');
'translate(0, ' + -dataScroll + ')');
done();
}, DBLCLICKDELAY * 2);
});
Expand Down Expand Up @@ -289,7 +289,7 @@ describe('The legend', function() {
expect(+toggle.parentNode.style.opacity).toBeLessThan(1);
expect(getScroll(gd)).toBe(dataScroll);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + dataScroll + ')');
'translate(0, ' + -dataScroll + ')');
expect(scrollBar.getAttribute('width')).toBeGreaterThan(0);
expect(scrollBar.getAttribute('height')).toBeGreaterThan(0);
done();
Expand Down

0 comments on commit f2d8cfd

Please sign in to comment.