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

Fix searchbar with large lists with VirtualizedList optimizations #6038

Merged
merged 4 commits into from
May 7, 2022

Conversation

drewvolz
Copy link
Member

@drewvolz drewvolz commented May 7, 2022

Issue

When using pull-to-refresh in Dictionary or Student Orgs with the new searchbar, the SectionList behaves strangely. The navigation header with the embedded searchbar will become jittery and it appears as if performance is taking a hit.

Investigation

I initially thought that the list rendering issues might be related to calculating the heights of components. While this did not produce a solution, it pointed me further towards what may have been happening.

My test was to take the contentContainerStyle of the list and modify the style away from flexGrow to see if we were having a height calculation issue.

student-orgs/list.tsx

contentContainer: {
-   flexGrow: 1,
+   flex: 1,
},

Testing this out, you'll see that this test trades one problem for another. While this fixes our issue with pull-to-refresh, it makes the height of the list equal to the screen's height which is not a useful change in practice.

So maybe we're dealing with a list performance issue since we can get both the searchbar and the list to play nice with a smaller sized list.

Solution

I stumbled upon a github issue from 2018 in the react-native repo titled 'sticky headers scrolls jumpy when virtualization is enabled'. This tied together several things we were seeing –– a virtualized sectionlist, sticky headers, and jumpy behavior when scrolling a large list.

I thought it seemed familiar. It turns out that we've been here before! #3348 in the all about olaf repo centered around issues with large lists, jumpiness, and section headers in the course search view (to which we applied an upstream patch to the list virualizer and called it done).

This solution comes from this comment:

Hey, I was running into this too and found a workaround. I ended up setting a few additional properties, these in particular:

     initialNumToRender={50}
     maxToRenderPerBatch={50}
     windowSize={41}

I opted out of RN's defaults (because I am dealing with a large list) and this seems to fix the headers from jumping all over when scrolling. Your mileage may vary.

Looking at this with fresh eyes, we can optimize our larger virtualized lists with props (not inclusive) such as initialNumToRender, maxToRenderPerBatch, and windowSize. See optimizing flatlist configuration for the full documentation.

While the suggested increases from the react native defaults are a heuristic, these are not specially selected numbers for each of our virtualized lists... yet. These appear to work for larger lists such as the orgs and dictionary lists, but will it work with course search? Unknown, but this solution seems extensible and inline-able if we find sharing this pattern across other lists needs tweaking.

Documentation

initialNumToRender

default: 10
override: 50

How many items to render in the initial batch. This should be enough to fill the screen but not much more. Note these items will never be unmounted as part of the windowed rendering in order to improve perceived performance of scroll-to-top actions.

maxToRenderPerBatch

default: 10
override: 50

It is a VirtualizedList prop that can be passed through FlatList. This controls the amount of items rendered per batch, which is the next chunk of items rendered on every scroll.

Pros: Setting a bigger number means less visual blank areas when scrolling (increases the fill rate).

Cons: More items per batch means longer periods of JavaScript execution potentially blocking other event processing, like presses, hurting responsiveness.

windowSize

default: 21
override: 41

The number passed here is a measurement unit where 1 is equivalent to your viewport height. The default value is 21 (10 viewports above, 10 below, and one in between).

Pros: Bigger windowSize will result in less chance of seeing blank space while scrolling. On the other hand, smaller windowSize will result in fewer items mounted simultaneously, saving memory.

Cons: For a bigger windowSize, you will have more memory consumption. For a lower windowSize, you will have a bigger chance of seeing blank areas.

drewvolz added 3 commits May 7, 2022 01:27
this opts out of RN's defaults (because we are dealing with a large
list)  and this seems to fix the headers from jumping all over when
scrolling.

overrides: initialNumToRender, maxToRenderPerBatch, and windowSize

facebook/react-native#21468 (comment)
@drewvolz drewvolz requested review from hawkrives and rye as code owners May 7, 2022 08:41
Copy link
Member

@rye rye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice write-up!

@drewvolz drewvolz merged commit da1f31f into master May 7, 2022
@drewvolz drewvolz deleted the drew/large-list-props branch May 7, 2022 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate searchbar jitteriness on PTR
3 participants