Leaflet.Path.Drag icon indicating copy to clipboard operation
Leaflet.Path.Drag copied to clipboard

Base L.Path has no _updatePath function

Open tuspet opened this issue 9 years ago • 2 comments
trafficstars

Dear @w8r,

I found an issue in the path drag plugin which should be checked by you. In our software we need to draw ellipses on the leaflet map. I searched for a plugin and found a forked version of the "officially suggested" jdfergason/Leaflet.Ellipse plugin (https://github.com/kapcsandi/Leaflet.Ellipse/) which already works correctly with the new leaflet 1.0.

But when I added the draggable option to the ellipse I got an error that the _updatePath function does not exist. I checked the code and I found the following reason: In the first part of the _onDragEnd function there is an _updatePath call:

if (moved) {
  this._transformPoints(this._matrix);
  this._path._updatePath();
  this._path._project();
  this._path._transform(null);

  L.DomEvent.stop(evt);
}

As I see the problem is that the original L.Path class has no _updatePath method, only the derived classes: L.PolyLine, L.Polygon and the L.CircleMarker. So if the type of the dragged object is an instance of one of them no problem occurs. But when I try to use the Ellipse plugin which derives directly from the L.Path (and has no _updatePath method), the this._path._updatePath row fails, (although the ellipse can be moved correctly without this call).

Of course I can create a hack to add an empty function to the L.Ellipse, but it is not a proper fix: L.Ellipse.include({ _updatePath: function () { } });

I suggest to wrap the _updatePath method in an if condition: if (this._path._updatePath) {...} to be compatible with this ellipse plugin as this is officially a suggested plugin on the http://leafletjs.com/plugins.html page.

if (moved) {
  this._transformPoints(this._matrix);
  if (this._path._updatePath) {
    this._path._updatePath();
  }
  this._path._project();
  this._path._transform(null);

  L.DomEvent.stop(evt);
}

Unfortunately this problem can happen with other 'private' functions as well, but this small 'if' condition for the _updatePath method can help us to have compatibility at least with this 'official' ellipse plugin.

Please feel free to comment this issue.

tuspet avatar Oct 25 '16 14:10 tuspet

@tuspet Hi and thanks for the request! I ended up re-writing L.Ellipse library completely, it's going to be compatible when I am done

https://github.com/w8r/Leaflet.Ellipse/tree/leaflet-1.0

w8r avatar Nov 08 '16 16:11 w8r

Dear @w8r,

Wow! You are awesome. :-) Thank you very much for your contribution!

tuspet avatar Nov 10 '16 08:11 tuspet