Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlatList - Extreme scrolling breaks touches with focused TextInput #12976

Closed
iggyfisk opened this issue Mar 16, 2017 · 13 comments
Closed

FlatList - Extreme scrolling breaks touches with focused TextInput #12976

iggyfisk opened this issue Mar 16, 2017 · 13 comments
Assignees
Labels
JavaScript Resolution: Locked This issue was locked by the bot.

Comments

@iggyfisk
Copy link

Description

With a FlatList that has a fairly large number of items, sometimes you can scroll far/fast enough to permanently break something around TextInputs and touches. After "extreme" scrolling inside the FlatList, and after something is typed into a TextInput, the first touch outside the TextInput is swallowed. The second touch works as normal. It's hard to describe so I made a test project to reproduce and recorded a video to show every step and the resulting error.

The video is recorded using the iOS simulator, but the bug occurs on every iOS system we've tried so far, regardless of hardware or OS version. We have yet to reproduce it on Android.

We're not sure of the relevance of scroll speed, Y position, or number of items. Our live application has much fewer items than the test application in the video, but it's still easy to reproduce there. Probably because those items are more taxing on the hardware (images, different opacity etc). But that's all conjecture.

Either way this happens organically in our application and is preventing us from releasing our move away from ListView to production, which is a shame because overall the performance improvements with FlatList have been fantastic.

Reproduction

This repository contains the test application from the video above. scrolltest.js has all the relevant code.

Clone the repository, npm install, react-native run-ios, quickly scroll through the list, type something in the text input and try the "Clear" button. I highly recommend watching the video so you'll know what I'm talking about.

Additional Information

  • React Native version: Current RN master d5dda1b with React 16.0.0-alpha.3
  • Platform: iOS simulator and device. Android seems unaffected.
@ptomasroos
Copy link
Contributor

Please take a look at this whenever you have the time @sahrens since its holding us back from moving FlatList to production. Our feeling is that its not just happening on extremely fast scroll, but thats a very easy way to reproduce it. It happens otherwise too just not deterministically. Cheers!

@sahrens
Copy link
Contributor

sahrens commented Mar 16, 2017

iOS only? And you don't see the same problem with ListView and the same level of "extreme scrolling"? Very odd. I wonder if it could be related to event coalescing? @shergin - maybe you can take a look?

@iggyfisk
Copy link
Author

I gave it another shot now that my scrolling hand is Usain Scrollt and could not make it happen with ListView nor Android.

On Android I would sometimes run into #12884, but that's temporary and delays events, so different in nature from this permanent event skip.

@shergin
Copy link
Contributor

shergin commented Mar 16, 2017

OMG, this bug report is so amazing! Thank you, @iggyfisk!
I will try to debug it soon.

@CheragV
Copy link
Contributor

CheragV commented Mar 17, 2017

I am facing a similar issue with my project. While the text input is focused 2 things happen :

1 => scrolling works as usual, so this is great
2=> when I touch the list to select an option the first touch doesn't work, I checked onBlur is called at this point, and then the second press works just fine

I am not sure if it is the same problem you are facing, If not do you have a solution to this @iggyfisk ?
Thanks 👍

@iggyfisk
Copy link
Author

That doesn't sound like the exact same thing @CheragV, once I break things by monster scrolling a FlatList, the onBlur handler on TextInputs still works as expected. It doesn't fire on the first touch after typing since that touch gets eaten by the bug.

Either way my solution so far is to spend an hour doing retakes of a bug report video, then wait for the California boys to sort it out.

@shergin
Copy link
Contributor

shergin commented Mar 17, 2017

I found an actual problem. The fix is coming! 🎉

@ptomasroos
Copy link
Contributor

What a hero @shergin! Looking forward to it!

@grin
Copy link

grin commented Mar 17, 2017

I'm also running into a similar issue where extreme scrolling of a FlatList breaks touches. In my app I have a list of 100-200 items where each item has two touchable areas. After a bit of scrolling elements stop responding to touches, sometimes it takes a second try for it to work but sometimes it's 3-4 touches. I don't use TextInputs.

@RichardLindhout
Copy link

#11809 (comment)

shergin added a commit to shergin/react that referenced this issue Mar 19, 2017
…tter

This PR fixes the problem originally introduced in facebook#6590
The problem is that `ResponderEventPlugin` (and `ResponderTouchHistoryStore`) relies on that fact that touch events are balanced.
So if one `startish` event happened, should be coming `endish` event. Otherwise there is no way to maintain internal `trackedTouchCount` counter. So, if we drop some events, we break this logic.

Moreover, that optimization clearly contradict with this statement from `ResponderEventPlugin`:
```
We must be resilient to `targetInst` being `null` on `touchMove` or
`touchEnd`. On certain platforms, this means that a native scroll has
assumed control and the original touch targets are destroyed.
```

This issue causes several major problems in React Native, and one of them (finally!) is easy to reproduce: facebook/react-native#12976 .

The test also illustrates this problem.
shergin added a commit to shergin/react that referenced this issue Mar 19, 2017
…tter

This PR fixes the problem originally introduced in facebook#6590
The problem is that `ResponderEventPlugin` (and `ResponderTouchHistoryStore`) relies on that fact that touch events are balanced.
So if one `startish` event happened, should be coming `endish` event. Otherwise there is no way to maintain internal `trackedTouchCount` counter. So, if we drop some events, we break this logic.

Moreover, that optimization clearly contradict with this statement from `ResponderEventPlugin`:
```
We must be resilient to `targetInst` being `null` on `touchMove` or
`touchEnd`. On certain platforms, this means that a native scroll has
assumed control and the original touch targets are destroyed.
```

This issue causes several major problems in React Native, and one of them (finally!) is easy to reproduce: facebook/react-native#12976 .

The test also illustrates this problem.
@shergin
Copy link
Contributor

shergin commented Mar 20, 2017

Was fixed in 23c2a6c. 🎉

@ptomasroos
Copy link
Contributor

ptomasroos commented Mar 20, 2017 via email

@iggyfisk
Copy link
Author

Fixed as far as I could test, good hustle team!

shergin added a commit to shergin/react that referenced this issue Mar 20, 2017
…tter

This PR fixes the problem originally introduced in facebook#6590
The problem is that `ResponderEventPlugin` (and `ResponderTouchHistoryStore`) relies on that fact that touch events are balanced.
So if one `startish` event happened, should be coming `endish` event. Otherwise there is no way to maintain internal `trackedTouchCount` counter. So, if we drop some events, we break this logic.

Moreover, that optimization clearly contradict with this statement from `ResponderEventPlugin`:
```
We must be resilient to `targetInst` being `null` on `touchMove` or
`touchEnd`. On certain platforms, this means that a native scroll has
assumed control and the original touch targets are destroyed.
```

This issue causes several major problems in React Native, and one of them (finally!) is easy to reproduce: facebook/react-native#12976 .

The test also illustrates this problem.
shergin added a commit to shergin/react that referenced this issue Mar 20, 2017
…tter

This PR fixes the problem originally introduced in facebook#6590
The problem is that `ResponderEventPlugin` (and `ResponderTouchHistoryStore`) relies on that fact that touch events are balanced.
So if one `startish` event happened, should be coming `endish` event. Otherwise there is no way to maintain internal `trackedTouchCount` counter. So, if we drop some events, we break this logic.

Moreover, that optimization clearly contradict with this statement from `ResponderEventPlugin`:
```
We must be resilient to `targetInst` being `null` on `touchMove` or
`touchEnd`. On certain platforms, this means that a native scroll has
assumed control and the original touch targets are destroyed.
```

This issue causes several major problems in React Native, and one of them (finally!) is easy to reproduce: facebook/react-native#12976 .

The test also illustrates this problem.
sophiebits pushed a commit to facebook/react that referenced this issue Mar 21, 2017
…tter (#9219)

* Removed optimization for events without target in ReactNativeEventEmitter

This PR fixes the problem originally introduced in #6590
The problem is that `ResponderEventPlugin` (and `ResponderTouchHistoryStore`) relies on that fact that touch events are balanced.
So if one `startish` event happened, should be coming `endish` event. Otherwise there is no way to maintain internal `trackedTouchCount` counter. So, if we drop some events, we break this logic.

Moreover, that optimization clearly contradict with this statement from `ResponderEventPlugin`:
```
We must be resilient to `targetInst` being `null` on `touchMove` or
`touchEnd`. On certain platforms, this means that a native scroll has
assumed control and the original touch targets are destroyed.
```

This issue causes several major problems in React Native, and one of them (finally!) is easy to reproduce: facebook/react-native#12976 .

The test also illustrates this problem.

* Prettier
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
JavaScript Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

8 participants