conflict if I use Leaflet.DistortableImage with Leaflet.Draw

[Charmatzis]

Hi, I want to use Leaflet.DistortableImage tool with Leaflet.Draw.
If I load first Leaflet.Draw I get an error "Polyline" is undefined.....

If I load first Leaflet.DistortableImage it works but if I click on an image overlay the popup doesn't work and I get an error at leaflet.toolbar-src.js line 300 L.Toolbar.prototype.onAdd.call(this, map); where the function "onAdd" is unidentified.

Very strange.....!!!!!

[jywarren]

Hi, sorry for the late response, but this could be related to the v1.0 release of Leaflet, as #64 also shows a v1.0 related issue. Can you confirm the Leaflet version is 1.0? Thanks for reporting!

[triat]

Hi, I do have the error also with the version 0.7.7 but using mapbox (I also tried with the mapbox standalone just to be sure that this is the good version of leaflet).
I'm trying right now to load first distortableimage and then draw just to see if it makes a difference.

[Charmatzis]

Hi, the version was 0.7.7 . Sorry, I am late!!!

[triat]

I just tested to load draw after distorableimage and it doesn't change anything. I'll try to take a look on the code to see if I can do anything for that

[jywarren]

Hmm, that would be great. The demo is running on v0.7.3, for reference. I have not had time to look at a potential port for 1.0, and it'd be good to know if we have to at least peg version to something prior to 0.7.7... appreciated!

[jywarren]

Ah, but apologies, we're not saying here that it doesn't work, just that it may not be compatible with Leaflet.Draw? Thanks in any case!

[triat]

So, alone it is working, just that when you have both of the plugins, it does until you click on the image witch trigger the error TypeError: L.Toolbar.prototype.onAdd is undefined

[batiste]

Here is what I found:

https://github.com/Leaflet/Leaflet.draw/blob/272ebe022553a8eaf5c536a5df01e0e539524100/src/Toolbar.js#L1

https://github.com/Leaflet/Leaflet.toolbar/blob/master/dist/leaflet.toolbar-src.js#L5

My guess is that those 2 plugins define both L.Toolbar and potentially are stepping on each other foots... Does that make sense?

[jywarren]

Yes, indeed -- perhaps we can do a namespace disambiguation in one of them?
@justinmanley has worked on both this lib and Toolbar, which he's the
author of, so maybe we could request a renaming? It also seems like we
shouldn't need to overwrite L.Toolbar namespace if we have good scope
control. Maybe we need to use npm-style packaging more strictly and then we
can just assign different namespaces when we include.

[batiste]

Here is my fix on Leaflet.draw, it seems to work Leaflet/Leaflet.draw#593

[justinmanley]

Yes, this is an unfortunate issue. Leaflet.draw was intended to migrate from its own custom toolbar to using Leaflet.toolbar (Leaflet/Leaflet.draw#354), but the migration was never completed (see e.g. this issue: Leaflet/Leaflet.toolbar#8).

@batiste - since your change in Leaflet/Leaflet.draw#593 is a breaking change, I would recommend guarding it behind a compatibility flag, similar to the way that Leaflet's noConflict method: https://github.com/Leaflet/Leaflet/blob/master/src/Leaflet.js#L9 is implemented. That is, Leaflet.draw should store the value of L.Toolbar, and Leaflet.Draw.noConflict() should restore the original value of L.Toolbar and return an object containing Leaflet.draw's version of L.Toolbar. This will prevent your change from breaking existing Leaflet.draw users who are not Leaflet.toolbar users.

[batiste]

@justinmanley how is my change a breaking change? Are other plugins expecting L.Toolbar to be created by Leaflet.draw?

I find this noConflict trick dirty. Who is responsible to call this noConflict method? At what point? I strongly discourage such practice as it is non obvious for the end user.

[jywarren]

OK - looks like this may be addressed in https://github.com/justinmanley/leaflet-draw-toolbar, just released.

Related, #78 highlights some issues with the new Leaflet.Toolbar, also just released. Any help there appreciated to bring this lib up to full v1.0 Leaflet compatibility.

[jywarren]

leaflet-draw-toolbar augments Leaflet.Toolbar. Previously, you needed to patch Leaflet/Leaflet.draw#354 in order to use Leaflet.draw with Leaflet.toolbar. Now, Leaflet.toolbar + Leaflet.draw should work together out-of-the-box if you use leaflet-draw-toolbar.

via justinmanley!

[jywarren]

I believe this may be fixed as of v1.0.0 -- can someone confirm? We're now pegged to leaflet-toolbar v0.3.0 and leaflet v1.x.

[sashadev-sky]

Hi guys, just double checked version 0.20.7 of our package, [email protected] version of leaflet-toolbar, [email protected] version of leaflet and [email protected] version of leaflet-draw and confirm there is full compatibility. Closing this -- will reopen if @MerionLial double checks and finds an issue, but I don't think he should.

(Note our package has probably worked with leaflet-draw going back many many versions)

@jywarren @ebarry @sagarpreet-chadha @cesswairimu note here! If you wanted Leaflet.Draw in MK this is now an option!!