mapbox / mapbox/leaflet-pip

Leaflet-pip is not working with Leaflet-draw plugin

Open
#27 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
202
Forks
44
PR merge metrics
No merged PRs in 30d

Description

Hello,
I've tried to update my app with leaflet 1.0.x and I'm facing up to an incompatibility between 2 plugins:
- [leaflet-draw](https://github.com/Leaflet/Leaflet.draw)
- leaflet-pip

Since [this commit in leaflet-pip](https://github.com/mapbox/leaflet-pip/commit/e76cfdc064cfe3d29ab01eebe2d4ad9b35564129#diff-168726dbe96b3ce427e7fedce31bb0bcR28), function `is_poly` is not anymore able to take into account polygons just created with leaflet-draw. I thought leaflet-draw created an incorrect polygon in [this issue](https://github.com/Leaflet/Leaflet.draw/issues/731) but I suspect now leaflet-pip is maybe not enough permissive.
I would suggest a PR which updates the `is_poly` function from your plugin:
```
function isPoly(l) {
if (l instanceof L.Polygon) return true;
return l.feature && l.feature.geometry && l.feature.geometry.type &&
['Polygon', 'MultiPolygon'].indexOf(l.feature.geometry.type) !== -1;
}
```
Indeed layer created by leaflet-draw has no `l.feature && l.feature.geometry && l.feature.geometry.type` attribute...

And maybe rollback entirely https://github.com/mapbox/leaflet-pip/commit/ff2b2f43df3d4a539afb587e7fe033046f6e3b7f for `is_poly`:
```
function isPoly(l) {
return L.MultiPolygon && l instanceof L.MultiPolygon ||
l instanceof L.Polygon ||
l.feature && l.feature.geometry && l.feature.geometry.type &&
['Polygon', 'MultiPolygon'].indexOf(l.feature.geometry.type) !== -1;
}
```

What do you thinking about it ?

Fabien

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the isPoly function in leaflet-pip and compare its current layer checks with the Leaflet-draw layers described here. Reproduce the incompatibility with Leaflet 1.0.x and leaflet-draw, then verify that newly drawn polygons are recognized without breaking GeoJSON Polygon or MultiPolygon handling.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.