Add a small tolerance when testing pointer event positions #6778
Conversation
|
@ischas can you test https://codepen.io/anon/pen/QvOPzp ? |
|
@simonseyock ping |
|
I am sorry, i can't test this. The problem does not occur on my phone. |
src/ol/mapbrowsereventhandler.js
Outdated
| return pointerEvent.clientX != this.down_.clientX || | ||
| pointerEvent.clientY != this.down_.clientY; | ||
| return Math.abs(pointerEvent.clientX - this.down_.clientX) > 1 || | ||
| Math.abs(pointerEvent.clientY != this.down_.clientY) > 1; |
There was a problem hiding this comment.
Typo: the second condition should be pointerEvent.clientY - this.down_.clientY.
One question: Any chance the threshold 1 may be too large? The real offset that are causing the bug seems well below 1. May be 0.5 or even 0.9 is a better alternative?
There was a problem hiding this comment.
I have problems handling the two fingers in order to make a pinch zoom...
|
For me it's nearly impossible to select a country in this demo: The linked CodePen works for me (but I guess @mustaqahmed is right with its review comment). |
fredj
force-pushed
the
isMoving_tolerance
branch
2 times, most recently
from
7895301
to
7b719cd
Compare
|
I changed the threshold to 0.75 as suggested and updated the CodePen. |
|
Now selecting is better than with master, but less easy as with the first commit. |
|
@ischas back to 1px, CodePen updated |
|
@simonseyock and I tested on my Samsung S5 with Android 6.0.1 and Chrome 58. While with the select feature demo only round about every fifth to tenth touch is registered as touch, with the CodePen maybe every second touch is registered as touch. That's much better, but not good. Please consider that the first commit behaved "better" in context of selection because of the bug and |
|
I tested on a Samsung S6 and it's not working. I've added the |
|
CodePen now works fine for me. |
|
@ischas and are the pan, pinch zoom and rotate working as well? |
|
@simonseyock can you also test on the phone that doesn't have the issue if everything continues to work as expected? |
|
I Tested tap, double tap, pan, pinch zoom, rotate, everything's fine. Did I miss anything else? @simonseyock is out of office until Friday, but maybe he'll find some time for testing in its spare time. ;-) |
|
I'm tested with a Nexus 5x (doesn't have the issue) and it works as expected |
|
Tested Nexus 4, Android 5.1.1, Chrome 58.0.3029.83 - previously had the issue now working fine. |
|
@openlayers/core thanks for any review |
src/ol/mapbrowsereventhandler.js
Outdated
| * @type {number} | ||
| * @private | ||
| */ | ||
| this.moveTolerance_ = 1 * ol.has.DEVICE_PIXEL_RATIO; |
|
Any chance for a patch release? |
|
I have tested the CodePen with a Sony Z3 Compact, Android 6.0.1, Chrome 58.0.3029.83, and I still have problems selecting countries. It works slightly better than http://openlayers.org/en/latest/examples/select-features.html, but not good. I have also tested with a Sony Z4 Tablet, Android 7.0, Chrome 58.0.3029.83, and there the CodePen works fine, and the select-features example does not work. |
|
What's the device pixel ratio of these devices ? see http://devicepixelratio.com/ |
|
On the Z3 compact it reports "2".
On the Z4 tablet it also reports "2".
|
|
Update: when I applied your patch to my 4.1.1-based application, my application now works as it should on the phone (using 1.5 * ol.has.DEVICE_PIXEL_RATIO) gives even better result (i.e., with a factor of 1 some taps are still missed)). But the codepen above still doesn't work; I have tried it several times. |
|
Hello. |
|
Need to make a correction because I was only checking in codepen and ol samples: after properly make @fredj update in library js, it began working as supposed. So, this has worked for me. |
|
Nice PR, thanks @fredj ! |
|
We'll be releasing v4.2.0 in the next couple of days anyway. |
fixes #6776
demo: https://codepen.io/anon/pen/QvOPzp