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.

Sign up for GitHub

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

Jump to bottom

Move events #1374

Merged
merged 3 commits into from
Feb 14, 2013
Merged

Move events #1374

merged 3 commits into from
Feb 14, 2013

Conversation

oslek
Copy link
Contributor

@oslek oslek commented Feb 14, 2013

I created this fiddle http://jsfiddle.net/cxZRM/ to view the behavior of move-related events. To use the fiddle, open your browser console and watch event behavior with various map moves. Resize the result pane to see moveend events. Click the actions below the map. Drag the map with and without inertia. “Log Line” can be used to add a console line/tag for clarity.

Leaflet v0.5 results in following output:

LOG: _______________________________ zoom 
LOG: < movestart 
LOG:     ( zoomstart 
LOG:     ) zoomend 
LOG: > moveend 
LOG: _______________________________ drag 
LOG: < movestart 
LOG:     [ dragstart 
LOG: > moveend 
LOG:     ] dragend 
LOG: _______________________________ drag with inertia 
LOG: < movestart 
LOG:     [ dragstart 
LOG:     ] dragend 
LOG: < movestart 
LOG: > moveend 
LOG: _______________________________ vertical resize
LOG: > moveend

There are three commits in this branch to address three separate behaviors: false move events on resize, event order on drag, and extra movestart on drag with inertia. The output becomes:

LOG: _______________________________ zoom 
LOG: < movestart 
LOG:     ( zoomstart 
LOG:     ) zoomend 
LOG: > moveend 
LOG: _______________________________ drag 
LOG: < movestart 
LOG:     [ dragstart 
LOG:     ] dragend 
LOG: > moveend 
LOG: _______________________________ drag with inertia 
LOG: < movestart 
LOG:     [ dragstart 
LOG:     ] dragend 
LOG: > moveend 
LOG: _______________________________ vertical resize

(no change to zoom, just shown for comparison)

@mourner
Copy link
Member

mourner commented Feb 14, 2013

That's great, thanks for taking the time to fix this!

I'm merging this, but it would be completely awesome if you could have a go at writing tests for the events so that there are no regressions for them in future (some of the things you fixed were working correctly in older versions and probably broke somewhere along the way).

mourner added a commit that referenced this pull request Feb 14, 2013
@mourner
Merge pull request #1374 from oslek/move-events
d8ef52b 
Move events
@mourner mourner merged commit d8ef52b into Leaflet:master Feb 14, 2013
@oslek oslek deleted the move-events branch February 14, 2013 21:58
@oslek
Copy link
Contributor Author

oslek commented Feb 15, 2013

Excellent! Yes, I will have a go at writing the tests. It will be new territory for me, but I’m looking forward to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@oslek @mourner