bug: [#192] Fixed broken map event unmounting#193
Open
robinmolen wants to merge 1 commit intoalex3165:masterfrom
Open
bug: [#192] Fixed broken map event unmounting#193robinmolen wants to merge 1 commit intoalex3165:masterfrom
robinmolen wants to merge 1 commit intoalex3165:masterfrom
Conversation
When unmounting a specific map event handler (`map.off(eventType, func)`), the exact same mounting function (`map.on(eventType, func)`) has to be used. Otherwise the map unmounting doesn't know how which function should be removed, and nothing happens (resulting in more and more duplicate handlers for the same event). See https://leafletjs.com/reference.html#evented-off By using a custom function on the `map.on()` (which in turn *might* call the correct `props[handler]`), the `props[handler]` is actually never used as event handler. Meaning that the `map.off()`, in the current setup, will never work. By making sure that the `map.on()` and `map.off()` use the same function, the unmounting works as expected and old event handlers are removed.
Collaborator
|
if someone with more time than me can approve this I will merge it and deploy a version |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #192
To correctly unmount a event handler the
map.on()andmap.off()have to use the exact same props, otherwise themap.off()cannot correctly identify which handler to unmount, resulting in multiple (read duplicate) event handlers for the same event. See https://leafletjs.com/reference.html#evented-off for more informationBy using a custom function on the
map.on()(which in turn might call the correctprops[handler]), theprops[handler]is actually never used as event handler. Meaning that themap.off(), in the current setup, will never work.By making sure that the
map.on()andmap.off()use the same function, the unmounting works as expected and old event handlers are removed.I couldn't find a reason why this "function checking its own type, and then maybe calling the correct
props[handler]" behaviour was implemented. If someone can tell me why (and preferably a bug report that shows an actual bug), i can look into a solution that uses the old setup.