-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #2598 - sloppy click on cartesian zoom #2649
Conversation
and they shouldn't - it's the mouseDown and mouseMove events that actually matter to the action. So update the test to reflect that.
else zoomMode = (ew ? 'x' : '') + (ns ? 'y' : ''); | ||
|
||
dragTail(zoomMode); | ||
dragTail(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, dragTail
no longer takes arguments after 5b6a1fa
@@ -266,6 +269,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { | |||
path0 = 'M0,0H' + pw + 'V' + ph + 'H0V0'; | |||
dimmed = false; | |||
zoomMode = 'xy'; | |||
zoomDragged = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one is in zoomPrep
@@ -341,6 +345,9 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { | |||
box.w = box.r - box.l; | |||
box.h = box.b - box.t; | |||
|
|||
if(zoomMode) zoomDragged = true; | |||
gd._dragged = zoomDragged; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this one move in zoomMove
|
||
mouseEvent('mousemove', x, y, moveOpts); | ||
mouseEvent('mousedown', x, y, downOpts); | ||
if(callContext) mouseEvent('contextmenu', x, y, downOpts); | ||
if(slopX || slopY) mouseEvent('mousemove', x + slopX, y + slopY, slopOpts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, slop
is only used for testing? This is fine, but perhaps we could add a comment about this in the jsDoc above so that we don't feel like 🔪 down the road.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure -> c56f223
Thanks! Looks good 💃 |
As mentioned in a comment, zoom takes over
minDrag
, so it also has to take overgd._dragged
Also removed a little obsolete behavior while I was at it.
cc @etpinard