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

Ensure drag handlers request render frames on move #6068

Merged
merged 1 commit into from Jan 27, 2018
Merged

Conversation

anandthakker
Copy link
Contributor

@anandthakker anandthakker commented Jan 26, 2018

Closes #6063

Caused by #6005 because, after that change, the drag handlers relied on the move event to trigger a map rerender to pick up the next batch of mouse movements.

This worked fine while dragging was in progress, but after the user paused and the render frame triggered by the last move event was complete, there was nothing to re-initiate the render loop once the mouse moved again.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page

Closes #6063

Caused by #6005 because,
after that change, the drag handlers relied on the `move` event to
trigger a map rerender to pick up the next batch of mouse movements.

This worked fine while dragging was in progress, but after the user
paused and the render frame triggered by the last `move` event was
complete, there was nothing to re-initiate the render loop once the
mouse moved again.
Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚡️ thanks for the quick fix + test coverage! 🏆

// https://github.com/mapbox/mapbox-gl-js/issues/6063
update.reset();
simulate.mousemove(map.getCanvas(), {bubbles: true, buttons: 2});
t.equal(update.callCount, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can do t.ok(update.calledOnce()) as well but this works too!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like t.equal(update.callCount, 1) because if it fails, you see the actual/incorrect value in the test log

@pathmapper
Copy link
Contributor

Sorry if it's a stupid question, but I wonder why it has been merged into 0.44 instead of master

@anandthakker
Copy link
Contributor Author

We're using 0.44 as a release branch for putting out patch releases (which we'll do next week). Backported to master #6073

anandthakker added a commit that referenced this pull request Jan 29, 2018
Closes #6063

Caused by #6005 because,
after that change, the drag handlers relied on the `move` event to
trigger a map rerender to pick up the next batch of mouse movements.

This worked fine while dragging was in progress, but after the user
paused and the render frame triggered by the last `move` event was
complete, there was nothing to re-initiate the render loop once the
mouse moved again.
pathmapper pushed a commit to pathmapper/mapbox-gl-js that referenced this pull request Feb 13, 2018
…box#6073)

Closes mapbox#6063

Caused by mapbox#6005 because,
after that change, the drag handlers relied on the `move` event to
trigger a map rerender to pick up the next batch of mouse movements.

This worked fine while dragging was in progress, but after the user
paused and the render frame triggered by the last `move` event was
complete, there was nothing to re-initiate the render loop once the
mouse moved again.
pathmapper pushed a commit to pathmapper/mapbox-gl-js that referenced this pull request Feb 13, 2018
…box#6073)

Closes mapbox#6063

Caused by mapbox#6005 because,
after that change, the drag handlers relied on the `move` event to
trigger a map rerender to pick up the next batch of mouse movements.

This worked fine while dragging was in progress, but after the user
paused and the render frame triggered by the last `move` event was
complete, there was nothing to re-initiate the render loop once the
mouse moved again.
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.

None yet

3 participants