After upgrading Leaflet library to 1.2.0, I get the error: "L.MultiPolygon is not a constructor", when trying to add MultiPolygons to the map.
Apparently, since Leaflet 1.x, L.MultiPolygon has been replaced by L.polygon (http://leafletjs.com/reference-1.0.0.html#polygon).
Added support for more complex geometries, work based on the related #2799311: Support geometry collections
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | fix_multipolygon_const-2905407.patch | 1.18 KB | dchatry |
Comments
Comment #2
rremigius commentedCreated a patch using the new L.polygon function instead of the old L.Multipolygon.
Comment #3
rremigius commentedComment #4
rremigius commentedPrevious file contained too much.
Comment #5
drshearer commentedHi rremigius, thanks for your patch... I'm getting the same error (using Leaflet 8.x-1.0-beta5 bundled with v1.0.3 of the library). Applying your patch has no effect. Is it still working on your end?
Comment #6
andileco commentedThis patch (#4) works for me on Leaflet 8.x-1.0-beta13.
Comment #7
dchatryPatch works fine on the last beta (14) for me, however the "fit bounds" didn't work like it did for the other types of geometries because the
this.boundsarray wasn't populated, the attached patch corrects the behavior.Comment #8
itamair commentedsorry folks, just to make sure ... should we commit the #7 patch into dev?
Any regression?
I haven't got time and even proper local dev to test and review it.
Just confirm that that is well tested and reviewed by the community.
Comment #9
itamair commentedComment #10
viappidu commentedFound this issue (and patch) late.
Based mine on #2799311: Support geometry collections so I added support for GeometryCollection and MultiPoint too. Uploading it here as it seems the other issue is a bit stale.
Tested on beta16
Comment #11
viappidu commentedSorry forgot to change state to Needs review
Comment #12
itamair commentedThanks @@viappidu, but ... this is becoming messy.
We shouldn't mix different Patches in the same issue, also named with different patterns, without clearly explaining what would update and override what ...
So, @viappidu would your patch overcome and overrides all the previous???
May you add properly the mentioned parallel/related issue also in the "Related issues" field?
Thanks ...
Comment #13
itamair commented@viappidu your patch is really heavier and much more invasive ... than @dchatry's one.
In particular it looks a greater refactoring of the leaflet.drupal.js file, that handles all the Leaflet mapping logics.
May you kindly explain what is your one doing and mainly why it should be preferred to the @dchatry's one?
Comment #14
viappidu commentedItamair, you are very right, sorry for messing things a up.
I'm not actually a professional developer and I am mainly trying to solve my issues...
My main objective was to display a GeometryCollection (eg "GEOMETRYCOLLECTION (POLYGON ((6.9204068 43.7516656, 9.5351529 40.057485, 10.5019498 41.8986093, 6.9204068 43.7516656)), LINESTRING (7.4038053 45.6720389, 11.9741178 42.8240245), POINT (12.4355436 45.0079348), POINT (14.0615201 43.6086488))")
I found a 2 years old patch at the issue mentioned above, obviously very outdated. I rewrote that to fix my problem and meanwhile I added functionality also for MultiPoint (eg "MULTIPOINT ((8.76611 45.0079348), (8.4365201 43.1935748), (11.3369108 42.5008699))") (guess I forgot to mention that...).
When testing everything was working I found this issue (I actually found the solution looking at leaflet documentation) and thought it would have been better to post my patch here other than at a stale issue.
So, just to recap...
My patch (other than fixing this problem too) added support for:
GeometryCollection
MultiPoint
MultiPolyline
MultiPolygon
Tested everything (including single point), with kml files exported from Google maps and I did not see any error on Drupal 8.7.
The real difference is not actually on the .js file but in the LeafletService.php file where I added an helper function to loop through complex objects. As mentioned I'm not a pro so I pretty much copied and fixed the patch at #2799311. I do believe the code can be 'prettyfied' much...
Comment #15
itamair commentedOk ... but why did you post your patch here (that seem to solve mainly your specific use cases) without clarifying why (and where) the previous one was not working???
Did you check the @dchatry's one, that seemed already well tested and reviewed ... before overriding everything with your one?
Please reload (anyway) this last patch of you in its original issue, describe well it and reopen the issue itself to make your patch ready for review.
(and let's keep the relation among the issues ...)
Comment #16
viappidu commentedI didn't say @dchatry patch was not working, I simply didn't test it. Solved the problem before finding his patch.
Mine (seems to) solve this problem too and more. The exactly same issue happens if you have MultyPolyline and (now reading his code) it doesn't look like his patch is solving this problem. Do you want to open a new issue and make another patch?
If you think it's worth I will open an issue 'Support complex geometries' to include everything.
Comment #17
itamair commented@viappidu ... it seems you are more getting the point now.
If you overlap another's work with your one you should explain why.
If you even don't review, asses and review previous other work you shouldn't post your one.
If you post something here it means that you want to help,
and you can help just if you align to others work and baseline, and explain your one (over the others).
Otherwise it helps just the confusion ...
That's it.
Please, do whatever come more logic after this discussion and what you think might help the community.
Clearly assess and explain your progresses over the others'
Comment #18
itamair commentedYes please, open a brand new issue, properly related with these existing ones, where you post your patch (for review) and clearly explain what it is achieving over this issue ones (#7) ... thanks!
Comment #19
viappidu commentedCreated #3017492: Support complex geometries
Sorry for the mess and thanks @itamair for the support :)
Comment #21
itamair commentedPatch #7 tested and reviewed and committed into dev and last leaflet 8.x-1.0-beta17 release.
Comment #22
itamair commented