-
Notifications
You must be signed in to change notification settings - Fork 75
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 an interactor bug with momentum. #604
Conversation
Using the default map interactor options, if you right click or right drag on the map, mousemove event cease getting generated. The action state was marked as zoom or momentum after the mouse up (since it was performing that action), and this is never cleared as clearing is done on mouse up. This blocked generating mousemove events. Also, tweak one of the polygon tests to try to eliminate occasional test failures.
Current coverage is 81.57% (diff: 100%)@@ master #604 diff @@
==========================================
Files 82 82
Lines 7436 7442 +6
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 6065 6071 +6
Misses 1371 1371
Partials 0 0
|
To see the problem, load the tile example page from http://opengeoscience.github.io/geojs/examples/tiles/, open a javascript console and type |
We were calculating momentum using some of the mouse properties, even after the mouse was released. This results is odd behavior if you pan quickly, release the mouse button, then move the mouse in a non-linear way. The solution is to use a separate momentum record instead of modifying the mouse record for momentum. This bug was only exposed after fixing that we weren't reporting mouse movement during momentum.
Sometimes a browser (Chrome 51) will send a mouse move event after a mouse down event with the same coordinates as the mouse down. This should not cancel a click.
@@ -636,7 +636,7 @@ var mapInteractor = function (args) { | |||
(!m_mouse.buttons.left || m_options.click.buttons.left) && | |||
(!m_mouse.buttons.right || m_options.click.buttons.right) && | |||
(!m_mouse.buttons.middle || m_options.click.buttons.middle)) { | |||
m_clickMaybe = true; | |||
m_clickMaybe = {x: m_mouse.page.x, y: m_mouse.page.y}; |
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.
Looks like we
Are using clickMaybe in many ways : boolean and object? Wondering why.
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.
We can receive a mousemove event from some browsers where there is no movement. We need to record where the mouse down occurred, but we only care until we decide that a click is impossible. At present, a click only gets passed through if there is no movement. We could make it so that there is a small threshold of movement that would not trigger moving events, but would still allow clicks.
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.
Sounds good. I was thinking from consistency point of view.
LGTM (but didn't run it yet) 👍 |
Using the default map interactor options, if you right click or right drag on the map, mousemove event cease getting generated. The action state was marked as zoom or momentum after the mouse up (since it was performing that action), and this is never cleared as clearing is done on mouse up. This blocked generating mousemove events.
Also, tweak one of the polygon tests to try to eliminate occasional test failures.