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

Base L.Path has no _updatePath function #23

Open
tuspet opened this issue Oct 25, 2016 · 2 comments
Open

Base L.Path has no _updatePath function #23

tuspet opened this issue Oct 25, 2016 · 2 comments

Comments

@tuspet
Copy link

tuspet commented Oct 25, 2016

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.

@w8r
Copy link
Owner

w8r commented Nov 8, 2016

@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

@tuspet
Copy link
Author

tuspet commented Nov 10, 2016

Dear @w8r,

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

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

2 participants