Open justinmanley opened 10 years ago
Sounds pretty good to me :+1:
I'm working on this in manleyjster/Leaflet.Toolbar. (I figured it was general enough to merit its own plugin).
Will post an update when I have a working version which fully reproduces existing functionality.
I think this is a great idea. Once ready Leaflet.draw can replace it's toolbar implementation with your plugin.
Hey folks - Leaflet.Toolbar is ready - at the very least, it's ready for us to have a discussion about it. I submitted a PR (#354), interested to hear your thoughts!
:+1:
I'm already looking forward to rolling some custom toolbars in my projects with Leaflet.Toolbar, and will probably wait for the next Leaflet.Draw release that ships with it.
I'm also looking forward to Leaflet.Toolbar being used in Leaflet.draw, primarily for the popup controls that can be added to individual features. It's great to see all the work done to make that happen! Thanks @manleyjster! This will enable a much more intuitive editing experience in my application.
Great work @manleyjster , this is a great addition to the plugin!
A proposal for API changes that will bring the Leaflet.draw API more in line with the Leaflet API and make it more user-friendly.
addToolbar
andremoveToolbar
methods ofL.Toolbar
. InheritL.Toolbar
fromL.Control
, instead ofL.Class
, and move the code fromaddToolbar
andremoveToolbar
intoonAdd
andonRemove
methods (in line with the rest of the Leaflet API).control.addTo(map)
alias formap.addControl(control)
.L.Control.Draw
toL.ToolbarGroup
, since that's what it is - a group of toolbars. This will bring it in line with other leaflet aggregator classes (L.LayerGroup
,L.FeatureGroup
).L.Control.Draw
. The only way to do right now is to extendL.Control.Draw
and override (read: copy-and-paste with small modifications) theinitialize
method. Not very DRY. This could be fixed in two ways:L.ToolbarGroup
into smaller methods and provide a private method_addToolbars
so that folks can easily extendL.ToolbarGroup
with custom toolbars.L.ToolbarGroup
take an array ofL.Toolbar
objects.Chime in if you think Leaflet.draw would benefit from these changes! Also would love to hear if there are good reasons that I've missed for the oddities and contradictions of Leaflet.draw that I've outlined above.