Leaflet Draw update compatibility to L1.0.0+ #607
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Leaflet 1.0 release included a number of incompatible API changes that
broke some features of Leaflet.draw (notably the edit features).
These include:
1. Polylines and Polygons may now use nested Arrays of LatLng objects.
These correspond to deprecated MultiPolygons containing multiple "rings"
where the first Array is the outer ring and all subsequent rings are
subtracted inner rings (e.g. donut).
2. The Polygon.getLatLngs() method now always returns a nested array of rings.
3. Removed Polyline/Polygon `spliceLatLngs` method (use `setLatLngs` instead).
4. Removed LatLng `RAD_TO_DEG`, `DEG_TO_RAD`, and `MAX_MARGIN` constants.
Other relevant internal API changes include:
5. Polyline._projectLatLngs() no longer sets `this._originalPoints` array.
Instead, it expects for array to be passed in (by reference) for Point
objects to be assigned to.
6. L.Marker._initInteraction() was rewritten (copied verbatim in
src/ext/TouchEvents.js file).
In this commit I refactored the draw/edit classes to use feature-detection to
determine if the Polygon/Polyline models follow the new format (1) and attempt
to handle the LatLng arrays properly. There were also a number of event
issues related to editing Markers that I fixed, possibly due to bad merges.
With these changes the plugin should now be fully compatible with both the
Leaflet 0.7.x and 1.0+ branches. I tested using the existing 0.7.2
examples and the latest 1.0.0-rc3 release.
To help verify 1.0 compatibility (and establish it as baseline going forward) I copied the existing examples/ to examples/0.7.x/ and brought in the current Leaflet 1.0.0-rc3 release candidate to use in the main code examples. I also copied in the remote 'leaflet.geometryutil.js' and 'leaflet.snap.js' dependencies for the snapping.html example, since those remote links no longer work.
Changed '**' usage to Math.pow(), because the world isn't ready for ES7.
There was a bug introduced in Leaflet 1.0.0-rc2 where the map would lock in
a "dragging" state after editing a polygon with an invalid point (when using
allowIntersection=false). This seemed to be caused by the marker getting
removed from the Poly object while still in the 'dragging' state, and the
Draggable event was never released causing the map to hang. This was evident
from the 'dragging' mouse cursor on the map, and the 'leaflet-dragging' class
assigned to the <body>.
My workaround is to directly call the private API method to release 'drag':
marker.dragging._draggable._onUp(e);
There is a related ticket #4484 that has a similar issue where 'drag'
listeners are lost when the Marker is modified (specifically, a setIcon()
call), so I believe this to be an upstream issue. This issue does not
occur in the 0.7.x branch, so I have added some rudimentary version checks
to avoid running this code on that version.
Unrelated to the 'drag' bug, I reordered some of the lines in that function
to ensure the tooltip error message is actually rendered properly.
The tooltip contents were likely rerendered as part of the onMarkerClick()
call.
Included small fix to create L.Draw namespace in Tooltip.js if not yet defined (as this broke the examples).
|
Great to hear this PR was accepted! Most of the issues referenced look like they were addressed in my commits (based on the errors), but I will look into open issues to see if I can resolve them further |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks to @chugcup for this awesome work towards 1.0.0 support while retaining 0.7.0. There are breaking changes for custom toolbars.
Pull Request
Issues
NOTE: Linked issues do not imply issues have been patched or fixed.