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

How to implement Leaflet.curve to LeafletDraw plugin #32

Open
osper66 opened this issue Sep 15, 2020 · 9 comments
Open

How to implement Leaflet.curve to LeafletDraw plugin #32

osper66 opened this issue Sep 15, 2020 · 9 comments

Comments

@osper66
Copy link

osper66 commented Sep 15, 2020

Any plan?

@elfalem
Copy link
Owner

elfalem commented Sep 17, 2020

Hello,
It will require significant effort to implement that integration. Although I would like it to happen, unfortunately there are no current plans. I do welcome contributions though.

@elfalem
Copy link
Owner

elfalem commented Nov 24, 2020

@qpincon
Copy link

qpincon commented Mar 11, 2021

I plan to submit a pull request to the Leaflet.draw project with the ability to draw curves.
Since this Leaflet.draw does not have any dependency, it would involve integrating the source code of Leaflet.curve to Leaflet.draw (with attribution of course).
An option would also be to tell users to import Leaflet.curve before Leaflet.draw to be able to draw curves. Maybe it's better, but it will of course involve a bit of more work on my end.
What do you think ?

@elfalem
Copy link
Owner

elfalem commented Mar 11, 2021

@pinconquentin That would be great!
In my opinion, I think it would be better to tell users to import Leaflet.curve. I think Leaflet.draw already depends on the main leaflet.js code, so this could be another dependency (optional one). Decoupling it in this way would mean users not interested in drawing curves won't have unnecessary code. And for those interested, any updates to Leaflet.curve would be immediately available to them (instead of having to wait for those changes to be integrated into Leaflet.draw source code).

Whichever option is taken, I appreciate the effort and I'll be glad to assist time permitting. Thanks!

@qpincon
Copy link

qpincon commented Mar 12, 2021

@elfalem Yes, I leaned towards that option as well. Thank you for your answer !
I will probably submit the PR around next week.

@elfalem
Copy link
Owner

elfalem commented Mar 22, 2021

PR for reference - Leaflet/Leaflet.draw#1008
thanks for your work on this @pinconquentin , hopefully it gets merged soon.

@qpincon
Copy link

qpincon commented Mar 22, 2021

With pleasure ! The repo seems pretty dead unfortunatly, I hope somebody will check it out. Meanwhile, if someone wants to use this feature, install this branch from the forked repo.

@AndrejGajdos
Copy link

@qpincon I checked your branch, but it seems like it's not working because there are some errors in console.log

Screenshot 2023-06-01 at 16 38 46

@antoinelouis
Copy link

I successfully added that feature from @qpincon branch to my project (using the leaflet.draw.js file under dist).
I only had to manually add the icon to the control panel (as it was missing) and there are a few errors in the console but it does the trick if you plan to generate curves out of production.
Screenshot 2023-11-24 at 11 40 07 am

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

5 participants