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

Fixed zoomToShowLayer() markers disappearing bug #747

Merged
merged 3 commits into from
Jan 25, 2017
Merged

Fixed zoomToShowLayer() markers disappearing bug #747

merged 3 commits into from
Jan 25, 2017

Conversation

z3ut
Copy link
Contributor

@z3ut z3ut commented Jan 4, 2017

Hello,
this will fix #739

Timeout allows to handle map movestart event before checking it execution, so layer parent won't spiderfy when move start.

Fixed example from issue http://codepen.io/anon/pen/wgBBJV

@danzel
Copy link
Member

danzel commented Jan 5, 2017

I'm not sure this is the right way to solve this issue.
Is there a tidier way to detect this?

@z3ut
Copy link
Contributor Author

z3ut commented Jan 8, 2017

What use case for movestart? Without movestart event it seems to work fine.

Original issue occurs when marker can be displayed on next zoom level and his parent is in map view. It reproduced here http://codepen.io/anon/pen/rjVgVw

I added some tests for zoomToShowLayer function (Well, at least I tried). It covers the issue, but it looks like the problem has different behaviour in PhantomJS and real browsers (Chrome 55, Firefox 50). After zoom in browser marker._icon == null, but not for tests. I checked with 200px*200px map in browser (like for tests), same result. More likely it's my fault, but maybe someone saw similar before.

Am I missing the function logics and some use cases, or movestart event can be removed?

@danzel
Copy link
Member

danzel commented Jan 25, 2017

I've retested the original issue, it looks fine with this fix (#286).
Going to merge and do a bit more testing. Thanks!

@danzel danzel merged commit 37d5143 into Leaflet:master Jan 25, 2017
@z3ut z3ut deleted the fix-zoom-to-show-layer branch January 26, 2017 05:40
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.

Issue with .zoomToShowLayer()
2 participants