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.

Sign up for GitHub

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

Jump to bottom

Animated.Loop prevents VirtualizedList updates #16092

Closed
willdawsonme opened this issue Sep 26, 2017 · 5 comments
Closed

Animated.Loop prevents VirtualizedList updates #16092

willdawsonme opened this issue Sep 26, 2017 · 5 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@willdawsonme
Copy link

willdawsonme commented Sep 26, 2017 •
edited

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 6.9.1
Yarn: 0.17.9
npm: 3.10.10
Watchman: 4.7.0
Xcode: Xcode 8.3 Build version 8E162
Android Studio: 2.2 AI-145.3537739

Packages: (wanted => installed)
react: 16.0.0-alpha.12 => 16.0.0-alpha.12
react-native: 0.48.3 => 0.48.3

Steps to Reproduce

  1. Create any VirtualizedList and add a getItemLayout handler.
  2. Start an Animated.Loop animated.
  3. Scroll the list.

Expected Behavior

I would expect the new cells to render.

Actual Behavior

New cells do not render, only the cells that rendered initially are visible.

Reproducible Demo

https://snack.expo.io/H18xSNvi-

Comments

I believe part of the issue lies in _scheduleCellsToRenderUpdate:

react-native/Libraries/Lists/VirtualizedList.js

Lines 1090 to 1099 in 3649fce

if (hiPri && this._averageCellLength) {
// Don't worry about interactions when scrolling quickly; focus on filling content as fast
// as possible.
this._updateCellsToRenderBatcher.dispose({abort: true});
this._updateCellsToRender();
return;
} else {
this._updateCellsToRenderBatcher.schedule();
}
}
.
When scrolling, cell updates are scheduled and never called. Adding useNativeDriver: true to the child animation doesn't seem to help.

Removing the getItemLayout handler causes the list to calculate an averageCellLength. Now, the cell updates aren't scheduled but rendered immediately. Perhaps if a getItemLayout handler is provided an averageCellLength could be calculated which would let hiPri updates be rendered immediately.

Thanks
@rawrmaan
Copy link
Contributor

I've got the same issue on 0.47.1. I noticed the same thing as you--removing getItemLayout fixes it, but that's absolutely not something I'd want to do. Glad you found the actual source code of the issue.

@janicduplessis
Copy link
Contributor

VirtualizedList is probably using InteractionManager to schedule work but get's blocked by your looping animation. You can add isInteraction: false to your animation config and it should fix the issue.

@janicduplessis
Copy link
Contributor

Maybe we could set isInteraction to false automatically when using Animated.loop since the main use case is for long running animations and it should not block InteractionManager.

@willdawsonme
Copy link
Author

Ah perfect! Yeah that sounds like a sensible change, but at the very least a documentation update would be really helpful. Thank you!

What are your thoughts on the potential VirtualizedList averageCellLength improvement I mentioned at the end of the original issue? Happy to open up another ticket if you think it's worthwhile.

@rawrmaan rawrmaan mentioned this issue Sep 29, 2017
Closed
@rawrmaan
Copy link
Contributor

rawrmaan commented Sep 29, 2017 •
edited

@willdawsonme I submitted a doc update in #16136.

facebook-github-bot pushed a commit that referenced this issue Oct 14, 2017
@rawrmaan @facebook-github-bot
Update Animated docs to mention potential issues with VirtualizedList
a59d157 
Summary:
Doc update to clarify how to prevent `Animated.loop` and other animations from pre-empting `VirtualizedList` rendering as discussed in #16092.
Closes #16136

Differential Revision: D6057466

Pulled By: hramos

fbshipit-source-id: 946bcde97b364c623b48ddaeb643309630c072c9
@xqin xqin mentioned this issue Apr 11, 2018
Closed
@rhamag rhamag mentioned this issue Sep 14, 2018
Closed
@facebook facebook locked as resolved and limited conversation to collaborators Sep 29, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Sep 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rawrmaan @janicduplessis @willdawsonme @react-native-bot