Changes for Leaflet update to >1.* #6
Conversation
|
This PR breaks the demo in this repo and I can see why (the code is depending on 2 coords and you just pick the first one and continue from there) Btw: TIP:
|
|
Idk, but it didn't just work for me, there's is a breaking change in a method, as you can see in the diff. That results in a null exception and, this change fixes that... at least in how it's used in AppShaping... let's talk more about it when you're back... So if you check this link, you'll see what it fixes. I thought the commit description was clear/simple enough... I'll add more thorough descriptions... Also, running the demo with this update seems to work (i.e. I can produce straight lines with any JS errors), although I do get some other errors (404 for map PNGs)... |
|
if you get latest master, the demo will work with leaflet 1.2.0 (npm install first) I think what the problem is: there is a difference between creating straightlines with polylines and a polygons, both are possible! the top one creates a polyline, the other a polygon your change probably only works with the polygon, so I think you have to do some check with what you are dealing with R |
|
Just an observation... leaflet is not installable/upgradable via npm, it's just committed to the repository in example/js/leaflet.js... ?!???? so i have to copy the new version manually... |
|
Good point, indeed it's not working with polygons... The thing is that in AppShaping I don't have this error, because I've made a generic method (getLatLngsOuter) which does have proper checks, but obviously I couldn't reference AppShaping scripts in leaflet.straightlines repository... |
afirte
force-pushed
the
leaflet-update
branch
from
be6f096
to
58006aa
Compare
|
Hey @rvdkooy , does it work alright now? |
src/leaflet.draw.straightlines.js
Outdated
| @@ -53,6 +53,10 @@ | |||
| var currentLine = getLayerOfType(L.Polyline); | |||
| if (currentLine) { | |||
| var latLngs = currentLine.getLatLngs(); | |||
|
|
|||
| if (latLngs != undefined && latLngs.length === 1 && latLngs[0].constructor === Array) //then it's a nested array | |||
There was a problem hiding this comment.
does Array.isArray not work?
it is supported from IE9 onward
this should work:
if (latLngs && latLngs.length === 1 && Array.isArray(latLngs[0]))
There was a problem hiding this comment.
it would, but .constructor seemed like the most common way to test if the type is array... is it not good enough?
There was a problem hiding this comment.
I could use jQuery's $.isArray... is that better?
There was a problem hiding this comment.
Array.isArray is part of the ecmascript 5 spec, is safe to depend on that
jQuery is wrong because leaflet is not depending on jQuery, now I need to add jQuery as a depencency to make this work.
There was a problem hiding this comment.
ok, so should I just use Array.isArray?
|
version 2.0.0 is published on npm |
@rvdkooy check https://github.com/eVisionSoftware/AppShaping/issues/407