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

Added support for Leaflet 1.0+ #42

Merged
merged 1 commit into from Feb 20, 2017
Merged

Conversation

chugcup
Copy link
Contributor

@chugcup chugcup commented Nov 8, 2016

Starting in Leaflet 1.0, the L.Control class stores current layers as an
Array ([]) versus an id-mapped Object ({}). This caused issues with
the current codebase as it expected to look up layers by their ID
using a simple property lookup.

In this commit I use the new Array style for storing layers but added
a helper method _getLayer() that is used to retrieve a layer by its
ID. I replaced all layer accesses with this new method, which allows
the library to maintain both 0.7.x and 1.0+ compatibility.

To promote moving forward, I updated the examples to use the latest
Leaflet version (1.0.1). To help verify compatibility, I moved a copy
of the existing examples to a 0.7.x/ folder. I also updated the CDN
in the examples to use cdnjs.com (since the existing host did not
contain needed 1.0 libraries).

Starting in Leaflet 1.0, the L.Control class stores current layers as an
Array ([]) versus an id-mapped Object ({}).  This caused issues with
the current codebase as it expected to look up layers by their ID
using a simple property lookup.

In this commit I use the new Array style for storing layers but added
a helper method `_getLayer()` that is used to retrieve a layer by its
ID.  I replaced all layer accesses with this new method, which allows
the library to maintain both 0.7.x and 1.0+ compatibility.

To promote moving forward, I updated the examples to use the latest
Leaflet version (1.0.1).  To help verify compatiblity, I moved a copy
of the existing examples to a 0.7.x/ folder.  I also updated the CDN
in the examples to use cdnjs.com (since the existing host did not
contain needed 1.0 libraries).
@chugcup chugcup mentioned this pull request Nov 8, 2016
@grischard
Copy link

You've updated the cdn for 1.0, wouldn't this be the perfect opportunity, before it gets merged, to use https everywhere? That way, you'd also fix #40.

@ismyrnow
Copy link
Owner

This is great. Thanks for contributing this, including the https changes in the docs.

@ismyrnow ismyrnow merged commit d82696d into ismyrnow:gh-pages Feb 20, 2017
ismyrnow added a commit that referenced this pull request Feb 20, 2017
- #42 - Leaflet 1.x support added
- #39 - Fix typo in docs
@ismyrnow ismyrnow mentioned this pull request Mar 13, 2017
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

3 participants