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

Removing maps? #55

Closed
dhcole opened this issue Sep 15, 2011 · 7 comments
Closed

Removing maps? #55

dhcole opened this issue Sep 15, 2011 · 7 comments

Comments

@dhcole
Copy link

dhcole commented Sep 15, 2011

What's the best way to remove a map? Is it just to remove the maps container? I'm working on a page that will have about 10 maps, but doesn't need to have more than 3 at a time.

@RandomEtc
Copy link
Contributor

This is something that isn't supported properly yet, but definitely should be.

If your Map is initialized without dimensions then it will listen for resize events on the window. This needs cleaning up if the Map is removed. The MouseHandler object also listens for events on window but doesn't (yet) have a way remove them.

A map.destroy() function would need to handle these things and also clear any in-flight requests for tiles (otherwise the unload/error handlers for the images might cause errors when they return). That sounds scary but I'm pretty sure the request manager does the right thing already if you set a new provider, so it's probably just a matter of piggy-backing on that logic.

Hopefully we can look at this soon if it's not something you want to tackle yourself. In the meantime, honestly, 10 maps should be fine because there's not a lot of overhead to a map once the images have loaded.

But if they really aren't all visible at once (e.g. in a slide show) and you want to conserve memory/resources, consider a creating a pool of maps that you recycle by resetting the provider (if needed) and moving the parent element around in the DOM. The only caveat I can think of is that you might need to keep the maps in the DOM somewhere for some of the loading events to work, but you should be able to put a map in an element hidden with display:none without trouble.

@dhcole
Copy link
Author

dhcole commented Sep 15, 2011

Thanks @RandomEtc. Yes, the layout is similar to a slideshow. I'm not seeing any real performance issues at this point but wanted to play it safe. Thanks for this information.

@tmcw
Copy link
Contributor

tmcw commented Sep 27, 2011

First shot at this work in https://github.com/stamen/modestmaps-js/tree/mapdestroy

This

  • clears the request queue
  • unbinds window.resize

I'm uncertain as far as whether map.destroy should kill all of the DOM elements the map creates, the map parent, or none of the above, so haven't pushed on that work yet.

@RandomEtc
Copy link
Contributor

Looks good so far. Removing the mouse/touch event handlers would be the next main thing, and I'd say just go for it and clear all the DOM elements too. If that's not what you want, don't call destroy :)

@tmcw
Copy link
Contributor

tmcw commented Sep 28, 2011

Yeah, I'll fold #45 into this ticket then to simplify the branching.

@tmcw
Copy link
Contributor

tmcw commented Sep 28, 2011

Cool, in b70e476 and 756950d , handlers are now removable and removed in mapdestroy and layerParent is removed from the page. I think this branch is ready to roll.

@tmcw
Copy link
Contributor

tmcw commented Oct 25, 2011

Fixed in 2fd8e7d

@tmcw tmcw closed this as completed Oct 25, 2011
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

No branches or pull requests

3 participants