Skip to content

Commit

Permalink
Improve SelectionList item rendering performance
Browse files Browse the repository at this point in the history
  • Loading branch information
QichenZhu committed Oct 19, 2024
1 parent 1531141 commit 166363d
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 69 deletions.
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,7 @@ const CONST = {
SEARCH_BUILD_TREE: 'search_build_tree',
SEARCH_FILTER_OPTIONS: 'search_filter_options',
USE_DEBOUNCED_STATE_DELAY: 300,
LIST_SCROLLING_DEBOUNCE_TIME: 200,
},
PRIORITY_MODE: {
GSD: 'gsd',
Expand Down
138 changes: 69 additions & 69 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {useFocusEffect, useIsFocused} from '@react-navigation/native';
import lodashDebounce from 'lodash/debounce';
import isEmpty from 'lodash/isEmpty';
import type {ForwardedRef} from 'react';
import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
Expand All @@ -24,11 +25,11 @@ import useSingleExecution from '@hooks/useSingleExecution';
import useThemeStyles from '@hooks/useThemeStyles';
import getSectionsWithIndexOffset from '@libs/getSectionsWithIndexOffset';
import Log from '@libs/Log';
import * as SearchUtils from '@libs/SearchUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import arraysEqual from '@src/utils/arraysEqual';
import BaseSelectionListItemRenderer from './BaseSelectionListItemRenderer';
import FocusAwareCellRendererComponent from './FocusAwareCellRendererComponent';
import type {BaseSelectionListProps, ButtonOrCheckBoxRoles, FlattenedSectionsReturn, ListItem, SectionListDataType, SectionWithIndexOffset, SelectionListHandle} from './types';

Expand Down Expand Up @@ -106,6 +107,7 @@ function BaseSelectionList<TItem extends ListItem>(
shouldIgnoreFocus = false,
scrollEventThrottle,
contentContainerStyle,
shouldDebounceScrolling = false,
shouldPreventActiveCellVirtualization = false,
}: BaseSelectionListProps<TItem>,
ref: ForwardedRef<SelectionListHandle>,
Expand Down Expand Up @@ -275,14 +277,16 @@ function BaseSelectionList<TItem extends ListItem>(
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [flattenedSections.disabledArrowKeyOptionsIndexes]);

const debouncedScrollToIndex = useMemo(() => lodashDebounce(scrollToIndex, CONST.TIMING.LIST_SCROLLING_DEBOUNCE_TIME, {leading: true, trailing: true}), [scrollToIndex]);

// If `initiallyFocusedOptionKey` is not passed, we fall back to `-1`, to avoid showing the highlight on the first member
const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
initialFocusedIndex: flattenedSections.allOptions.findIndex((option) => option.keyForList === initiallyFocusedOptionKey),
maxIndex: Math.min(flattenedSections.allOptions.length - 1, CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage - 1),
disabledIndexes: disabledArrowKeyIndexes,
isActive: isFocused,
onFocusedIndexChange: (index: number) => {
scrollToIndex(index, true);
(shouldDebounceScrolling ? debouncedScrollToIndex : scrollToIndex)(index, true);
},
isFocused,
});
Expand All @@ -297,36 +301,50 @@ function BaseSelectionList<TItem extends ListItem>(
* @param item - the list item
* @param indexToFocus - the list item index to focus
*/
const selectRow = (item: TItem, indexToFocus?: number) => {
// In single-selection lists we don't care about updating the focused index, because the list is closed after selecting an item
if (canSelectMultiple) {
if (sections.length > 1) {
// If the list has only 1 section (e.g. Workspace Members list), we do nothing.
// If the list has multiple sections (e.g. Workspace Invite list), and `shouldUnfocusRow` is false,
// we focus the first one after all the selected (selected items are always at the top).
const selectedOptionsCount = item.isSelected ? flattenedSections.selectedOptions.length - 1 : flattenedSections.selectedOptions.length + 1;

if (!item.isSelected) {
// If we're selecting an item, scroll to it's position at the top, so we can see it
scrollToIndex(Math.max(selectedOptionsCount - 1, 0), true);
const selectRow = useCallback(
(item: TItem, indexToFocus?: number) => {
// In single-selection lists we don't care about updating the focused index, because the list is closed after selecting an item
if (canSelectMultiple) {
if (sections.length > 1) {
// If the list has only 1 section (e.g. Workspace Members list), we do nothing.
// If the list has multiple sections (e.g. Workspace Invite list), and `shouldUnfocusRow` is false,
// we focus the first one after all the selected (selected items are always at the top).
const selectedOptionsCount = item.isSelected ? flattenedSections.selectedOptions.length - 1 : flattenedSections.selectedOptions.length + 1;

if (!item.isSelected) {
// If we're selecting an item, scroll to it's position at the top, so we can see it
scrollToIndex(Math.max(selectedOptionsCount - 1, 0), true);
}
}
}

if (shouldShowTextInput) {
clearInputAfterSelect();
if (shouldShowTextInput) {
clearInputAfterSelect();
}
}
}

if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') {
setFocusedIndex(indexToFocus);
}
if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') {
setFocusedIndex(indexToFocus);
}

onSelectRow(item);
onSelectRow(item);

if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
innerTextInputRef.current.focus();
}
};
if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
innerTextInputRef.current.focus();
}
},
[
canSelectMultiple,
sections.length,
flattenedSections.selectedOptions.length,
scrollToIndex,
shouldShowTextInput,
clearInputAfterSelect,
shouldUpdateFocusedIndex,
setFocusedIndex,
onSelectRow,
shouldPreventDefaultFocusOnSelectRow,
],
);

const selectAllRow = () => {
onSelectAll?.();
Expand Down Expand Up @@ -438,50 +456,32 @@ function BaseSelectionList<TItem extends ListItem>(
// We only create tooltips for the first 10 users or so since some reports have hundreds of users, causing performance to degrade.
const showTooltip = shouldShowTooltips && normalizedIndex < 10;

const handleOnCheckboxPress = () => {
if (SearchUtils.isReportListItemType(item)) {
return onCheckboxPress;
}
return onCheckboxPress ? () => onCheckboxPress(item) : undefined;
};

return (
<>
<ListItem
item={item}
isFocused={isItemFocused}
isDisabled={isDisabled}
showTooltip={showTooltip}
canSelectMultiple={canSelectMultiple}
onLongPressRow={onLongPressRow}
onSelectRow={() => {
if (shouldSingleExecuteRowSelect) {
singleExecution(() => selectRow(item, index))();
} else {
selectRow(item, index);
}
}}
onCheckboxPress={handleOnCheckboxPress()}
onDismissError={() => onDismissError?.(item)}
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}
keyForList={item.keyForList ?? ''}
isMultilineSupported={isRowMultilineSupported}
isAlternateTextMultilineSupported={isAlternateTextMultilineSupported}
alternateTextNumberOfLines={alternateTextNumberOfLines}
onFocus={() => {
if (shouldIgnoreFocus || isDisabled) {
return;
}
setFocusedIndex(normalizedIndex);
}}
shouldSyncFocus={!isTextInputFocusedRef.current}
wrapperStyle={listItemWrapperStyle}
/>
{item.footerContent && item.footerContent}
</>
<BaseSelectionListItemRenderer
ListItem={ListItem}
item={item}
index={index}
isFocused={isItemFocused}
isDisabled={isDisabled}
showTooltip={showTooltip}
canSelectMultiple={canSelectMultiple}
onLongPressRow={onLongPressRow}
shouldSingleExecuteRowSelect={shouldSingleExecuteRowSelect}
selectRow={selectRow}
onCheckboxPress={onCheckboxPress}
onDismissError={onDismissError}
shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow}
rightHandSideComponent={rightHandSideComponent}
isMultilineSupported={isRowMultilineSupported}
isAlternateTextMultilineSupported={isAlternateTextMultilineSupported}
alternateTextNumberOfLines={alternateTextNumberOfLines}
shouldIgnoreFocus={shouldIgnoreFocus}
setFocusedIndex={setFocusedIndex}
normalizedIndex={normalizedIndex}
shouldSyncFocus={!isTextInputFocusedRef.current}
wrapperStyle={listItemWrapperStyle}
singleExecution={singleExecution}
/>
);
};

Expand Down
89 changes: 89 additions & 0 deletions src/components/SelectionList/BaseSelectionListItemRenderer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import React from 'react';
import type useArrowKeyFocusManager from '@hooks/useArrowKeyFocusManager';
import type useSingleExecution from '@hooks/useSingleExecution';
import * as SearchUtils from '@libs/SearchUtils';
import type {BaseListItemProps, BaseSelectionListProps, ListItem} from './types';

function BaseSelectionListItemRenderer<TItem extends ListItem>({
ListItem,
item,
index,
isFocused,
isDisabled,
showTooltip,
canSelectMultiple,
onLongPressRow,
shouldSingleExecuteRowSelect,
selectRow,
onCheckboxPress,
onDismissError,
shouldPreventDefaultFocusOnSelectRow,
rightHandSideComponent,
isMultilineSupported,
isAlternateTextMultilineSupported,
alternateTextNumberOfLines,
shouldIgnoreFocus,
setFocusedIndex,
normalizedIndex,
shouldSyncFocus,
wrapperStyle,
singleExecution,
}: Omit<BaseListItemProps<TItem>, 'onSelectRow'> &
Pick<BaseSelectionListProps<TItem>, 'ListItem' | 'shouldIgnoreFocus' | 'shouldSingleExecuteRowSelect'> & {
index: number;
selectRow: (item: TItem, indexToFocus?: number) => void;
setFocusedIndex: ReturnType<typeof useArrowKeyFocusManager>[1];
normalizedIndex: number;
singleExecution: ReturnType<typeof useSingleExecution>['singleExecution'];
}) {
const handleOnCheckboxPress = () => {
if (SearchUtils.isReportListItemType(item)) {
return onCheckboxPress;
}
return onCheckboxPress ? () => onCheckboxPress(item) : undefined;
};

return (
<>
<ListItem
item={item}
isFocused={isFocused}
isDisabled={isDisabled}
showTooltip={showTooltip}
canSelectMultiple={canSelectMultiple}
onLongPressRow={onLongPressRow}
onSelectRow={() => {
if (shouldSingleExecuteRowSelect) {
singleExecution(() => selectRow(item, index))();
} else {
selectRow(item, index);
}
}}
onCheckboxPress={handleOnCheckboxPress()}
onDismissError={() => onDismissError?.(item)}
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}
keyForList={item.keyForList ?? ''}
isMultilineSupported={isMultilineSupported}
isAlternateTextMultilineSupported={isAlternateTextMultilineSupported}
alternateTextNumberOfLines={alternateTextNumberOfLines}
onFocus={() => {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (shouldIgnoreFocus || isDisabled) {
return;
}
setFocusedIndex(normalizedIndex);
}}
shouldSyncFocus={shouldSyncFocus}
wrapperStyle={wrapperStyle}
/>
{item.footerContent && item.footerContent}
</>
);
}

BaseSelectionListItemRenderer.displayName = 'BaseSelectionListItemRenderer';

export default BaseSelectionListItemRenderer;
29 changes: 29 additions & 0 deletions src/components/SelectionList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {ForwardedRef} from 'react';
import {Keyboard} from 'react-native';
import * as Browser from '@libs/Browser';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import CONST from '@src/CONST';
import BaseSelectionList from './BaseSelectionList';
import type {BaseSelectionListProps, ListItem, SelectionListHandle} from './types';

Expand All @@ -28,6 +29,33 @@ function SelectionList<TItem extends ListItem>({onScroll, ...props}: BaseSelecti
};
}, []);

const [shouldDebounceScrolling, setShouldDebounceScrolling] = useState(false);

const checkShouldDebounceScrolling = (event: KeyboardEvent) => {
if (!event) {
return;
}

// Moving through items using the keyboard triggers scrolling by the browser, so we debounce programmatic scrolling to prevent jittering.
if (
event.key === CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN.shortcutKey ||
event.key === CONST.KEYBOARD_SHORTCUTS.ARROW_UP.shortcutKey ||
event.key === CONST.KEYBOARD_SHORTCUTS.TAB.shortcutKey
) {
setShouldDebounceScrolling(event.type === 'keydown');
}
};

useEffect(() => {
document.addEventListener('keydown', checkShouldDebounceScrolling, {passive: true});
document.addEventListener('keyup', checkShouldDebounceScrolling, {passive: true});

return () => {
document.removeEventListener('keydown', checkShouldDebounceScrolling);
document.removeEventListener('keyup', checkShouldDebounceScrolling);
};
}, []);

// In SearchPageBottomTab we use useAnimatedScrollHandler from reanimated(for performance reasons) and it returns object instead of function. In that case we cannot change it to a function call, that's why we have to choose between onScroll and defaultOnScroll.
const defaultOnScroll = () => {
// Only dismiss the keyboard whenever the user scrolls the screen
Expand All @@ -46,6 +74,7 @@ function SelectionList<TItem extends ListItem>({onScroll, ...props}: BaseSelecti
// Ignore the focus if it's caused by a touch event on mobile chrome.
// For example, a long press will trigger a focus event on mobile chrome.
shouldIgnoreFocus={Browser.isMobileChrome() && isScreenTouched}
shouldDebounceScrolling={shouldDebounceScrolling}
/>
);
}
Expand Down
3 changes: 3 additions & 0 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,9 @@ type BaseSelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {
/** Additional styles to apply to scrollable content */
contentContainerStyle?: StyleProp<ViewStyle>;

/** Whether to debounce scrolling on focused index change */
shouldDebounceScrolling?: boolean;

/** Whether to prevent the active cell from being virtualized and losing focus in browsers */
shouldPreventActiveCellVirtualization?: boolean;
} & TRightHandSideComponent<TItem>;
Expand Down

0 comments on commit 166363d

Please sign in to comment.