Leaflet / Leaflet.draw

Vector drawing and editing plugin for Leaflet
https://leaflet.github.io/Leaflet.draw/docs/leaflet-draw-latest.html
MIT License
1.97k stars 993 forks source link

Tap on Polygon point on touch device cause creating new point #548

Open y-fedorov opened 8 years ago

y-fedorov commented 8 years ago

Start draw polygon and click to on of the just created point. In Google Chrome on non touch device ignores click. On device with touch support or in touch support mode instead of ignoring click creates new point. Also double-click on the last point does not lead to closing the polygon.

Creating new point on L.Draw.Polyline on non touch devece pseudocode _onMouseDown (fill this._mouseDownOrigin) _onMouseUp --> (this._mouseDownOrigin not empty) -> make null _onTouch -> do nothing --> Fine.

And tap on already created point: _onMouseUp --> this._mouseDownOrigin empty _onTouch -> exit. Fine.

Same with touch support environment: _onMouseDown (fill this._mouseDownOrigin) _onTouch _onMouseDown (fill this._mouseDownOrigin) _onMouseUp --> (this._mouseDownOrigin not empty) -> make null)

and tap on already created point: _onTouch _onMouseDown (fill this._mouseDownOrigin) - we don't need to fill this._mouseDownOrigin _onMouseUp--> this._mouseDownOrigin :not empty value -> addVertex (Mistake)

It can be reproduced by example files full.html

tiagogm commented 8 years ago

Hi, im having the same issue, What vesion of leaflet are you using @indy43333 ?

I'm using leaflet@0.7.7 It's not possible to close a polygon shape, because there are two vertex markers being created for every touch event.

As far I can tell from debugging the code, this is caused because the addVertex method is being called twice. Once from leaflet mouseup event and another from leaflet-draw _onTouch event.

By looking at the leaflet-draw _onTouch I see it might not be necessary anymore, and is instead causing the problem.

https://github.com/Leaflet/Leaflet.draw/blob/master/src/draw/handler/Draw.Polyline.js#L266

    _onTouch: function (e) {
        // #TODO: use touchstart and touchend vs using click(touch start & end).
        if (L.Browser.touch) { // #TODO: get rid of this once leaflet fixes their click/touch.
            this._onMouseDown(e);
            this._onMouseUp(e);
        }
    }`

If I comment out those two event calls, it works fine, as addVertex is called only once.

ddproxy commented 8 years ago

@TGMorais Can you confirm in other browsers?

tiagogm commented 8 years ago

@ddproxy I think this will happen on any browser that L.Browser.touch is true. I've tested on a chrome and edge on a windows laptop with touch screen.

y-fedorov commented 8 years ago

@TGMorais I'm using leaflet@0.7.7 version also.

tdltdl commented 8 years ago

Would it be an option to add a check before doing "mouse Down/Up" to check if the touch is "close" to the first point of the polygon. If it is close enough (a few pixels), instead of triggering the mouse down/up events, we should trigger the closure of the polygon in every other case we call mouse down/up.

jasonbaik commented 8 years ago

bump 👍