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: [Search v1] Search page scroll is not smooth #42170

Merged
merged 8 commits into from
May 27, 2024
11 changes: 11 additions & 0 deletions src/components/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ function Search({query, policyIDs}: SearchProps) {
return (
<SelectionList
customListHeader={<SearchTableHeader data={searchResults?.data} />}
// To enhance the smoothness of scrolling and minimize the risk of encountering blank spaces during scrolling,
// we have configured a larger windowSize and a longer delay between batch renders.
// The windowSize determines the number of items rendered before and after the currently visible items.
// A larger windowSize helps pre-render more items, reducing the likelihood of blank spaces appearing.
// The updateCellsBatchingPeriod sets the delay (in milliseconds) between rendering batches of cells.
// A longer delay allows the UI to handle rendering in smaller increments, which can improve performance and smoothness.
// For more information, refer to the React Native documentation:
// https://reactnative.dev/docs/0.73/optimizing-flatlist-configuration#windowsize
// https://reactnative.dev/docs/0.73/optimizing-flatlist-configuration#updatecellsbatchingperiod
windowSize={111}
updateCellsBatchingPeriod={200}
Comment on lines +93 to +94
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain the rationale for choosing these values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The windowSize is set to 111 because each page can contain a maximum of 500 items, and this size is sufficiently large to ensure that the entire page will not be recycled.

The choice of 200 for updateCellsBatchingPeriod is because 200ms out of 1000ms allows it to be slightly larger, preventing continuous loading while not causing too much delay.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use different values for desktop and mobile devices? As their screens size are different

Copy link
Contributor Author

@charles-liang charles-liang May 16, 2024

Choose a reason for hiding this comment

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

Recycling of cells is only necessary in cases such as phones with low memory. I tested in iPhone 14 pro is very smooth and not need recycling.

why I set it to 111. This size ensures that in both portrait mode and landscape mode, is large enough not to be recycled.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we leave a comment explaining that please?

ListItem={ListItem}
sections={[{data, isDisabled: false}]}
onSelectRow={(item) => {
Expand Down
8 changes: 6 additions & 2 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ function BaseSelectionList<TItem extends ListItem>(
listHeaderContent,
onEndReached = () => {},
onEndReachedThreshold,
windowSize = 5,
updateCellsBatchingPeriod = 50,
}: BaseSelectionListProps<TItem>,
ref: ForwardedRef<SelectionListHandle>,
) {
Expand Down Expand Up @@ -403,7 +405,8 @@ function BaseSelectionList<TItem extends ListItem>(
shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow}
// We're already handling the Enter key press in the useKeyboardShortcut hook, so we don't want the list item to submit the form
shouldPreventEnterKeySubmit
rightHandSideComponent={rightHandSideComponent}
// Change this because of lint
rightHandSideComponent={rightHandSideComponent && (typeof rightHandSideComponent === 'function' ? rightHandSideComponent({} as TItem) : rightHandSideComponent)}
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
keyForList={item.keyForList ?? ''}
isMultilineSupported={isRowMultilineSupported}
onFocus={() => {
Expand Down Expand Up @@ -642,7 +645,8 @@ function BaseSelectionList<TItem extends ListItem>(
showsVerticalScrollIndicator={showScrollIndicator}
initialNumToRender={12}
maxToRenderPerBatch={maxToRenderPerBatch}
windowSize={5}
windowSize={windowSize}
updateCellsBatchingPeriod={updateCellsBatchingPeriod}
viewabilityConfig={{viewAreaCoveragePercentThreshold: 95}}
testID="selection-list"
onLayout={onSectionListLayout}
Expand Down
Loading
Loading