Fix #47 by preventing click events

[erpheus]

Closes #47

Attached touchstart event to clock to allow cancelation and thus prevent click event triggering.

[erpheus]

Just wanted to leave the process and the reasoning behind the solution documented here:

As can be seen in the related issue #47, mobile safari was triggering click events after the touch events had already fired. This is common in mobile browsers to allow websites to just implement click handlers if they don't need anything specific to touch events. As the clock allows dragging the finger around the clock, this is not the case.

A solution was already in place which consisted on a react state that would prevent click handlers from doing anything for 10 milliseconds after a touch event was detected. Increasing those 10 milliseconds to a 100 already seemed sufficient to make the solution also work on my devices, but it didn't feel like the best solution.

Usually it is enough with just calling e.preventDefault() on the touchstart event, which signals the browser that the touch will be handled and there is no need for click events. But it didn't work here since the events that were being delivered to the onTouchStart handler were not cancelable. Apparently this happens when the event listener is not set to the element itself but to the document, and it turns out that is exatly what react does with its onEvent props (for now).

As suggested in the first link, the only option for now is to use the addEventListener function manually. I did it using an approach similar to the one mentioned in this post. At first I tried to go for having both the mousedown and touchstart events in the same way but that made the testing super complex since enzyme's simulate method relies on the onEvent way of adding listeners in react, and it seems quite quite complex to change it to use the handlers added by addEventListener (I tried for a while). So that also explains why I didn't add any more tests, as this is also quite a browser-specific behavior to properly simulate in the tests.

[erpheus]

ping @catc ;)

Let me know if you want any help / screen recordings for testing this.

[catc]

Hey @erpheus I'm going to try to dedicate some time to this closer to Christmas when I have more time. I did briefly look at it and I think it changed some of the original user interaction - I need investigate further. I'll definitely ping you if I need some help testing on ios.

[erpheus]

Thanks! That sounds great! I have no rush but didn’t want it to get forgotten either :)

[jBenes]

Hi, @catc, are you still going to work on this issue? I can help with iOS. Thanks

[jBenes]

Btw what about adding optional parameter which would set disableMouse timeout or as mentioned above, raise it to 100 or 150ms? Even though it's not the best solution, it should work 99% times and it's still better than nothing :)

[catc]

Will take a look at this in dec when I have some more time.

[catc]

Released fix as v2.2.1 - thanks for driving this @erpheus!