Skip to content
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

Merged
merged 4 commits into from
Aug 8, 2016
Merged

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Jul 27, 2016

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.

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.
@codecov-io
Copy link

codecov-io commented Jul 27, 2016

Current coverage is 81.57% (diff: 100%)

Merging #604 into master will increase coverage by 0.01%

@@             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          

Powered by Codecov. Last update ccef540...1ea42af

@manthey
Copy link
Contributor Author

manthey commented Jul 27, 2016

To see the problem, load the tile example page from http://opengeoscience.github.io/geojs/examples/tiles/, open a javascript console and type tileDebug.map.geoOn('geo_mousemove', function (evt) { console.log(evt.map.x, evt.map.y); }). Now when you move the mouse, you'll see screen coordinates in the console. Focus the map window, then right click. The screen coordinates will stop updating until you left click.

manthey added 2 commits August 3, 2016 09:50
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};
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

@aashish24
Copy link
Member

LGTM (but didn't run it yet) 👍

@manthey manthey merged commit fde443f into master Aug 8, 2016
@manthey manthey deleted the fix-mousemove-events branch August 8, 2016 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants