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

Resize tracking is not working for multiple maps #1980

Closed
aparshin opened this issue Aug 16, 2013 · 5 comments
Closed

Resize tracking is not working for multiple maps #1980

aparshin opened this issue Aug 16, 2013 · 5 comments
Assignees
Labels
Milestone

Comments

@aparshin
Copy link
Contributor

Test case: http://jsfiddle.net/cF2zZ/

If you will change page width, the top map updates property while the bottom one doesn't.

Probably, the proplem is that L.DomEvent doesn't allow to add listener multiple times to the same object (window in this case) for the same event type (resize).

@mourner
Copy link
Member

mourner commented Aug 27, 2013

You're right, confirmed. Hmm... A tricky problem. Should we stamp contexts and include them in the key too?

@ghost ghost assigned mourner Aug 27, 2013
@fastrde
Copy link
Contributor

fastrde commented Aug 27, 2013

sometimes it could be a desired behavior that a listener overwrites the previous added listener.
Is this behavior used in leaflet (perhaps unintentionally)?

The removeListener function has no context yet, so you can't remove a specific listener for a event type anymore.
Is the context available everywhere we have to/want to remove a listener?

@aparshin
Copy link
Contributor Author

More solution variants:

  • Add another optional parameter separateByContext (unfortunately, it increase number of addListener parameters)
  • Add optional postfix to event type to separate different contexts (something like "resize:map1").
  • Implement singleton, that will track window events and deligate them to all created maps (rather complex way)

@mourner
Copy link
Member

mourner commented Aug 28, 2013

I tried implementing a solution with context stamping if it is specified, but then found out that there is a lot of existing code (not only Leaflet but many plugins) in which DomEvent.removeListener doesn't have a context passed, and that means that this change will break a lot of plugins and will require all of them to update, which is not very desirable.

Considering this issue really only affects window resize, I'll try to wrap the passed listener so that it gets different ids.

@mourner
Copy link
Member

mourner commented Aug 28, 2013

Yep, works well with a very simple fix :)

jfirebaugh added a commit to Leaflet/Leaflet.fullscreen that referenced this issue Jun 29, 2014
First work around the Leaflet bug described here:
Leaflet/Leaflet#1980

Then only respond to fullscreenchange events for the
appropriate target.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants