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

Mapbox event triggers w/o duplication #901

Merged
merged 5 commits into from Sep 7, 2016
Merged

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Sep 2, 2016

Mapbox map API methods such as map.setCenter and map.setZoom trigger move / movestart and moveend events on (see http://codepen.io/etpinard/pen/PGoyWO) - which in turn triggered plotly_relayout multiple events per update.

This PR cleans up the on-move logic:

  • 24a5ac8 : make the handler fire on 'moveend' only
  • 207bfe4 : adds a check ensuring that plotly_relayout is only trigger on moveend originating from mouse interactions.

- no need map mapbox opts to layout.mapbox and
  fire events at each step during the move,
  handling this at the end works just fine for our needs.
- 'moveend' (and 'movestart', 'move') gets triggered
   by map.setCenter, map.setZoom, map.setBearing and map.setPitch.
- make sure that 'plotly_relayout' is triggered here only
  when the 'moveend' originates from a mouse target.
@@ -658,6 +703,9 @@ describe('mapbox plots', function() {
}).then(function() {
return _mouseEvent('mousemove', p1, noop);
}).then(function() {
// repeat mousemove to simulate long dragging motion
return _mouseEvent('mousemove', p1, noop);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@mdtusz
Copy link
Contributor

mdtusz commented Sep 7, 2016

Lgtm! 💃

@etpinard etpinard merged commit 2eccfbd into master Sep 7, 2016
@etpinard etpinard deleted the mapbox-on-move-end branch September 7, 2016 21:38
etpinard added a commit that referenced this pull request Sep 8, 2016
Mapbox event triggers w/o duplication
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants