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

trigger event on scroll wheel #1182

Merged
merged 1 commit into from
Mar 20, 2022
Merged

Conversation

a14n
Copy link
Contributor

@a14n a14n commented Mar 10, 2022

This PR adds a MapEventScrollWheelZoom event that is fired when a scroll wheel occurs. Currently no event is triggered on scroll wheel (and zoom change)

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM - haven't tested, but only small changes and doesn't appear to have any breaking consequences.

Copy link
Collaborator

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

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

I think this looks good looking at the code, but I won't have chance to test it in the flesh until probably next weekend, but I'm happy to merge if someone else has.

@JaffaKetchup
Copy link
Member

@a14n @pmjobin How will #1182 and #1191 work together? Will they interfere?

@a14n
Copy link
Contributor Author

a14n commented Mar 20, 2022

I don't expect any issue between the 2 PR (except the merge conflict because there's a line that is changed on the 2 PR)

@ibrierley
Copy link
Collaborator

happy to merge this I think, have tested it doesn't seem to break anything obvious...noting the question about PR 1191, not so familiar with merge conflicts (it currently says there aren't any...), does it matter which order they are merged in or anything ?

@JaffaKetchup
Copy link
Member

@ibrierley It does depend on whichever gets merged first will cause a conflict in the other.

@JaffaKetchup
Copy link
Member

Testing now...

@JaffaKetchup JaffaKetchup self-requested a review March 20, 2022 17:10
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM, works as expected when tested. Will merge this before #1191.

@JaffaKetchup JaffaKetchup merged commit 17c1298 into fleaflet:master Mar 20, 2022
@JaffaKetchup
Copy link
Member

Many thanks for your contribution @a14n.

@a14n a14n deleted the scroll-wheel-event branch March 20, 2022 17:12
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