Can only draw two-point polygons in Chrome w/ Touch

stuporglue commented 7 years ago

[ x ] I'm reporting a bug, not asking for help
[ x ] I've looked at the documentation to make sure the behaviour is documented and expected
[ x ] I'm sure this is a Leaflet Draw code issue, not an issue with my own code nor with the framework I'm using (Cordova, Ionic, Angular, React…)
[ x? ] I've searched through the issues to make sure it's not yet reported

I don't think that this is the same as the other touch issues that have been reported, but I could be wrong.

How to reproduce

Environment

Leaflet version I'm using: 1.0.1+ffcfcc1, 1.2.0
Leaflet Draw version I'm using: 0.4.12+26a654a, 0.4.12+4f26385 (master, at the moment)
Browser (with version) I'm using: Chrome 61.0.3163.100 on OSX, Chrome 61.0.3163.98 on Android
OS/Platform (with version) I'm using: OSX, Android

Steps

On OSX, in Chrome

Go to http://leaflet.github.io/Leaflet.draw/docs/examples/full.html
Open the developer tools
Click the icon for "Toggle Device Toolbar" to view the page as you would on a mobile device. (This also sends touch events).
Start drawing a polyline
If you click very quickly you can get 3 or 4 points, but clicking at a normal speed completes the polyline after the 2nd point.

The steps to reproduce this on Android are the same, except you don't have to enable dev tools since you're already in a touch enabled browser.

What behaviour I'm expecting and which behaviour I'm seeing

I expect to be able to continue to add points.

Instead, the polyline completes after the second click.

Minimal example reproducing the issue

[ x ] this example is as simple as possible
[ x ] this example does not rely on any third party code

http://leaflet.github.io/Leaflet.draw/docs/examples/full.html

stuporglue commented 7 years ago

It seems that the newly added vertext is immediately given the same event that was used to create the vertex.

The following patch allows drawing to work, but doesn't feel like it's the real fix.

Instead of immediately setting up the click handler for the last marker in the Polyline, this patch uses setTimeout to delay that. It seems that the timeout needs to be greater than 50ms to work at all, and higher values seem more reliable.

index fbb7877..c988d85 100644
--- a/src/draw/handler/Draw.Polyline.js
+++ b/src/draw/handler/Draw.Polyline.js
@@ -368,7 +368,13 @@ L.Draw.Polyline = L.Draw.Feature.extend({
                var markerCount = this._markers.length;
                // The last marker should have a click handler to close the polyline
                if (markerCount > 1) {
-                       this._markers[markerCount - 1].on('click', this._finishShape, this);
+                       setTimeout(function(){
+                               if ( this._markers === undefined ) {
+                                       return;
+                               }
+
+                               this._markers[this._markers.length - 1].on('click', this._finishShape, this);
+                       }.bind(this), 75); 
                }

                // Remove the old marker click handler (as only the last point should close the polyline)

timthelion commented 6 years ago

This bug is worse than simply ending after two lines. If you click "wrong" the first time, you'll end up with a point rather than a polyline, which is an invalid geometry.

johnd0e commented 5 years ago

More observations:

When placing 1st point it's often shown non-zero distance.
- Perhaps similar issue is with polygon: when placing first point we get red tooltip Error: shape edges cannot cross! (but after second point all get right).
It's possible to draw multi-point polyline if instead of tap we use taphold (longtap)
We can get rid of polyline issue if we disable TouchExtend handler (create map with touchExtend: false option).
- But then we lost possibility to draw Circle and Rectangle (related: #923)
- And completely brake another case: android mobile with attached USB mouse (https://github.com/IITC-CE/ingress-intel-total-conversion/issues/162)
- It is still can be workarounded with some tricks:
  
```js map.on('draw:drawstart', function (e) { var mouseActive = L.Browser.touch && matchMedia('(hover:hover)').matches; // workaround for https://github.com/IITC-CE/ingress-intel-total-conversion/issues/162 if (mouseActive || e.layerType === 'circle' || e.layerType === 'rectangle') { e.target.touchExtend.enable() } else { e.target.touchExtend.disable() }; }); ```

Edit: this workaround is not good for iOS

johnd0e commented 5 years ago

Workaround: #926

johnd0e commented 4 years ago

It seems related to #695 (fix included)

ghost commented 4 years ago

Hi @johnd0e I am also experiencing below issue

When placing 1st point it's often shown non-zero distance. Perhaps similar issue is with polygon: when placing first point we get red tooltip Error: shape edges cannot cross! (but after second point all get right).

were you able to resolve this ?

johnd0e commented 4 years ago

Sure, read from this: https://github.com/Leaflet/Leaflet.draw/issues/695#issuecomment-577151966

Leaflet / Leaflet.draw