stamen / modestmaps-js

Modest Maps javascript port
http://modestmaps.com
566 stars 152 forks source link

Remove 640x480 default #60

Closed tmcw closed 12 years ago

tmcw commented 12 years ago

Address Issue #59 by removing default size.

The default sizing of Modest Maps is proving to be more trouble than it's worth. Mainly because maps that are sized with CSS but in display:none containers on load will acquire a fixed width that can't change when window.onresize fires. In display: none, offsetWidth and Height are both 0, so trigger the default. I don't think that the Google Maps API does such a default, and it makes the sizing API more complex in a way that I don't think is very worthwhile.

I'll pull together a branch that simply removes this default.

RandomEtc commented 12 years ago

Am I right in thinking that if you've set your map to display:none and then subsequently to display:block then all you need to do is call setSize(map.parent.offsetWidth, map.parent.offsetHeight) to reveal it? Could you add an example showing this behavior?

Before merging this needs a patch version bump I think - or maybe a minor version bump since it's not really a bug fix, rather it's just more predictable behavior.

tmcw commented 12 years ago

Sure, here's a demo:

http://macwright.org/demo/mmnone/

auto-size removed, display:none, and then on toggle run setSize with the parentWidth as you suggest

http://macwright.org/demo/mmnone/index_old.html

existing code - sets specific size, so parent doesn't change size. 0x0 offsets from display:none triggers strict size.

RandomEtc commented 12 years ago

Cool, thanks for that. The new version appears at the correct full window size, as desired.

However it doesn't work for me in Firefox (toggle needs to be getElementById('toggle')?) and in Safari it doesn't automatically resize after it is shown, which would be the desired behavior if no dimensions are supplied in the map constructor. Any thoughts? Does it work for you?

tmcw commented 12 years ago

Heh, sorry about that. For quick testing, I tend to use non-cross-browser oddities, like the availability of elements by id. I'll do a quick update.

tmcw commented 12 years ago

Fixed in FF. I don't think that this really can work after setSize, because that locks down the map size as well. Down the line setSize might mean 'set dimensions based on what is' rather than 'resize the map and set dimensions', but I think at this point this is the expected behavior, that after setSize, size is fixed.

RandomEtc commented 12 years ago

Ah right, I see how setSize breaks the window resize behavior. It was always my intention that making a map without dimensions meant "fit to parent" - I don't want to break that if possible. Could we come up with a built-in way to show and hide maps?

With dimensions 0x0 your map isn't going to pre-load many tiles, what's the advantage to creating it upfront rather than on demand?

Or, more constructively (and less combatively), what about a new value for the initial map size (e.g. 'auto') that would be the default? If we store that value internally then setSize can be more intelligent and not modify the parent style if it is set. Thoughts?

tmcw commented 12 years ago

With dimensions 0x0 your map isn't going to pre-load many tiles, what's the advantage to creating it upfront rather than on demand?

There's none - and when we build sites, that's definitely the way to do it. But, what I'm dealing with currently is the integration of maps elsewhere - here with embeds with hosting, in which people are cut-and-pasting map HTML - iframes or javascript - into divs, and building a probably-jQuery-powered slideshow as they would any other content. This is what some rather big sites (erm, like this one) use, and I think it's reasonable as an approach - the slight cost of loading tiles on-demand versus reworking and understanding a new stack is worth it.

Or, more constructively (and less combatively), what about a new value for the initial map size (e.g. 'auto') that would be the default? If we store that value internally then setSize can be more intelligent and not modify the parent style if it is set. Thoughts?

Hmm, not sure - if you want to code up a demo, go for it. I'm not sure that there's a good way to really do this that can jive with all size/positioning/parent combinations.

RandomEtc commented 12 years ago

So, the old behavior is:

The new proposed behavior is:

I think we can improve on both these scenarios by storing the developer's intent at map creation time. If dimensions are not specified (undefined or null) or are specified as 'auto' then resize the map to fit the parent element. If dimensions are specified then always use those dimensions and apply them to the parent if necessary. Rather than calling setSize when the map is revealed, it should be sufficient to request a redraw. When drawing, if dimensions are zero and auto-size is set then we can check against the parent element and see if they need updating.

I'll code this up and make some test cases too. Please hold off on merging this pull request til then, hassle me if I'm too slow (it'll be a weekend thing, but shouldn't take long).

RandomEtc commented 12 years ago

Added a new autoSize property to Map that should iron out the wrinkles, and a show-hide demo to toggle the map. A call to map.draw() should suffice to display the map at the correct size and maintain size-to-fit on window resize after display is toggled on an element.

Does this cover the corner cases from #59 well enough? I updated the version numbers and changelog, so please merge if it looks good, but let's keep pushing at the issue if not.

tmcw commented 12 years ago

Awesome, I think this is pretty understandable and consistent. Mergin.