JedWatson / react-tappable

Tappable component for React
http://jedwatson.github.io/react-tappable/
MIT License
862 stars 90 forks source link

`detectScroll()` didn't work properly on Android #56

Closed riophae closed 8 years ago

riophae commented 9 years ago

detectScroll() didn't work properly on Android

I had been working on a Hybrid Mobile App recently. react-tappable was powerful and easy-to-use, saving us lots of time from re-inventing the wheel... Thanks for your outstanding work! :D

It worked pretty well both on Chrome for OS X and Safari for iOS. However, in the last days before releasing our app, we found a fetal bug when testing on Android. Links of video re-producing the bug follow:

As demonstrated in the videos, the bug is Android-only. Every time I tried scrolling the Tappable element, it turns to .Tappable-active status and won't go back. It stays blue until another tap.

After hours of googling, I finally figured out that it's due to the lack of triggering of touchend, when scrolling is not stopped by preventDefault() in touchstart or the first touchmove. Please check out this article for detailed explanation. It's a long-existed bug since Android 4.0 (or 2.3, maybe).

Having some hacking on the code, I found that detectScroll() couldn't work properly when user scrolling the Tappable element, since touchmove only triggered once somehow (twice in some rare cases). But detectScroll() needs at least two triggerings of touchmove to diff out the change of scrolling positions! I did some further tests. These screenshots show why detectScroll() returns false all the time on Android.

Android: http://i.imgur.com/cq357Cq.png iOS: http://i.imgur.com/N74jgei.png Chrome (OS X): http://i.imgur.com/XLlUIni.png

So we have to work around the bug. Since it only triggered once on Android and multiple times on iOS frequently & quickly, we could count the times of touchmove after the first triggering. If another triggering not occured in a very short time (I have set it to 64ms, period of ~4 frames in 60fps), we assume something is buggy and touchend should be triggered manually.

The final result seems fine for me. The demo videos follow:

The codes for demo are placed in:

JedWatson commented 8 years ago

Thanks for that comprehensive explanation and fix, @riophae, and sorry this took so long to merge in!