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

Avoid error if _snakeForward if polyline is not on the map #9

Merged
merged 2 commits into from Dec 28, 2018

Conversation

YuanxiangFranck
Copy link
Contributor

When Polyline is removed before the animation is finished the following error is thrown:
Uncaught TypeError: Cannot read property 'latLngToContainerPoint' of null

@@ -75,6 +75,8 @@ L.Polyline.include({

_snakeForward: function(forward) {

// If polyline has been removed from the map stop _snakeForward
if (this._map == null) return;
Copy link
Owner

Choose a reason for hiding this comment

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

Actually, this should be if (!this._map), for consistency with other Leaflet code. You cannot be sure that this._map will be either null or undefined when the layer is removed, just that it will be falsy.

@YuanxiangFranck
Copy link
Contributor Author

Well this._map == null handle both null and undefined
In general it's safer to use this because it can return false if this._map = false

But here I don't think this._map can be either 0 or false, so if you prefer to keep consistency I can change it.

@IvanSanchez
Copy link
Owner

I know that == will work, but in Leaflet we try to avoid using == and using === instead.

For the style that is currently used in Leaflet, see https://github.com/Leaflet/Leaflet/blob/5cc9b79c97be3df653474a774f3b06af252a44bc/src/layer/ImageOverlay.js#L96 and https://github.com/Leaflet/Leaflet/blob/faeb46b27ccba0f3691ad2a074a46ae3be55538c/src/control/Control.js#L81 , for example. The code just checks for truthy/falsy.

So I would prefer you to change it. It's not a big problem, but just a small coding style issue.

@IvanSanchez IvanSanchez merged commit 86d5547 into IvanSanchez:master Dec 28, 2018
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.

None yet

2 participants