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

Improve heuristic for what event targets are considered markers #5885

Merged
merged 1 commit into from
Oct 30, 2017

Conversation

perliedman
Copy link
Member

As mentioned in #5635, the current logic for determining what is a marker has flaws (if you add an option called icon, it will be considered a marker).

As realized by @ghybs, it's not possible to just replace this with an instanceof check for technical reasons. Also, my opinion is that this is not the desired effect: what this code tries to accomplish is that layers that denote a specific location, like markers and circle markers, should always get their location as event location; other layers, that show areas (circles, polygons, lines) should get the exact clicked location.

This PR addresses this by considering if the layer has a getLatLng method (layer has an exact point location) and is missing radius or has a small radius (measured in pixels).

As any heuristic, this will have edge cases, but I think it will cover the codes intention better with less risk of doing the wrong thing.

Copy link
Member

@IvanSanchez IvanSanchez left a comment

Choose a reason for hiding this comment

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

LGTM

@mourner mourner merged commit 0bcccb0 into master Oct 30, 2017
@mourner mourner deleted the improve-event-marker-detection-heuristic branch October 30, 2017 17:39
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