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

Support Leaflet 1.0 #43

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from

Conversation

nrenner
Copy link
Contributor

@nrenner nrenner commented Jan 30, 2017

Suggesting to support Leaflet 1.0 by updating to current version 1.0.3 and fixing any breaking changes.

Obvious breaking changes so far (see also Changelog 1.0-beta1):

  • clickable > interactive
    • Renamed clickable option to interactive (by AndriiHeonia). #2838 #2499
  • L.Routing.Storage extends removed L.MultiPolyline
    • Removed MultiPolyline and MultiPolygon classes since multiple rings are now handled by Polyline and Polygon classes respectively. Layers with multiple rings now perform much better (since each is now physically a single path object instead of being a FeatureGroup of layers).

Not sure what to do with L.Routing.Storage: extend Polyline or FeatureGroup or copy over old MultiPoly implementation? It doesn't seem to be used anywhere and is not documented in the Readme, could we even remove this L.Routing.Storage class?

ie.css now in main css; remove libs folder with old Leaflet 0.6-dev
Copy link
Contributor

@Starefossen Starefossen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOW! Thanks a lot @nrenner 👍 Feel free to merge this!

@nrenner
Copy link
Contributor Author

nrenner commented Feb 11, 2017

@Starefossen What about the Storage class (see comments above)? I would tend to delete it?

The parent class L.MultiPolyline has been removed with Leaflet 1.0. But
Storage seems not to be used or documented anyway.
@nrenner nrenner changed the title [WIP] Support Leaflet 1.0 Support Leaflet 1.0 Jun 3, 2017
@nrenner
Copy link
Contributor Author

nrenner commented Jun 3, 2017

I decided to delete the Storage class as it would be a breaking change anyway and it doesn't seem to be used anywhere.

Created a separate issue #45 for MultiPolyline and MultiPolygon in LineUtil.Snapping.js, won't address this as we're not using snapping yet and is seems to work fine with just LineStrings.

Ready for review and merging now.

@Starefossen
Copy link
Contributor

Sorry for the lack of effort by my part. It looks like a solid effort has been made into this, @nrenner!

I would very much like you to take over as lead maintainer for this project with the merge of this PR if you would like that 🙂

@nrenner
Copy link
Contributor Author

nrenner commented Jun 17, 2017

Thanks for your trust. Not sure what to say, I would rather want to reduce the number of projects to maintain.

But am I right to assume that you're no longer with Turistforeningen and that leaflet-routing is not actually being used in production there? Then it might make sense as long as we're still using it with brouter-web.

@bagage
Copy link

bagage commented Oct 7, 2019

@nrenner any news on that?

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.

3 participants