Uh oh!
There was an error while loading. Please reload this page.
Legend scroll fix - #2426
Conversation
alexcjohnson
commented
Feb 28, 2018
Since this doesn't show up in test images, here's what the new scrollbars look like - a little wider and a lot taller: |
etpinard
commented
Feb 28, 2018
| return finalDataScroll; | ||
| } | ||
| it('should scroll on dragging the scrollbar', function() { |
There was a problem hiding this comment.
Yes, these do test #2387, if I'm interpreting it correctly. What I think was happening in #2387 is that somehow the scrollBox drag was being triggered - and that's what I decided (with no dissent) had no reason ever to happen. So this test shows that regular drag works, then the next one shows that the scrollBox won't drag even if it does somehow get the event, and then the third new test shows that right-click (the only form of #2387 I could reproduce on my own computer) even on the scrollBar will not trigger drag.
etpinard
commented
Feb 28, 2018
Probably related to #808. The new variable-height scroll bar doesn't update. For example, from
No need to fix this in this PR though, just confirming that scrollable legends don't update well as first reported in #808 |
| var scrollBarBB = scrollBar.getBoundingClientRect(); | ||
| var y0 = scrollBarBB.top + scrollBarBB.height / 2; | ||
| var y0 = scrollBarBB.top + scrollBarBB.height / 5; |
There was a problem hiding this comment.
notice that I moved the drag point away from the center of the scrollBar, thereby testing that it drags from where you grab it rather than snapping to the center.
and when `this` isn't important - ie in all of our helper functions but we still need `call` for `d3.behavior.drag()`
too confusing with it always being negative
alexcjohnson
commented
Mar 1, 2018
Fixed #808 (and #2426 (comment)) in 569b378 |
| var scrollRatio = scrollBarYMax / scrollBoxYMax; | ||
| var scrollBarY = constants.scrollBarMargin; | ||
| var scrollBoxY = scrollBox.attr('data-scroll') || 0; |
There was a problem hiding this comment.
Yes. Way better to use _ then a DOM attr 🎉
| // scrollBoxY is 0 or a negative number | ||
| var scrollBoxY = Math.max(opts._scrollY || 0, -scrollBoxYMax); | ||
| var scrollBoxY = Math.min(opts._scrollY || 0, scrollBoxYMax); |
etpinard
commented
Mar 1, 2018
Amazing PR! 💃 |
Fixes#2387 - don't allow dragging from the scroll box of a legend, only from the scroll bar itself, and even then not on right clicks, only left click. I also made the scroll bar handle a little bit wider so it's a bit easier to grab. And another part of the issue is that the scrollbar would jump to be centered on the mouse cursor, rather than simply moving with it from wherever you grabbed it. I fixed that too.
Fixes the legend part of #1859 - makes the scrollbar scale to show the fraction of the legend that's visible.
cc @etpinard