Merge pull request Expensify#67064 from Expensify/revert-66237-feat/65634-reduce-amount-of-items-per-page-of-selection-list

[CP Staging] Revert "feat: Reduce amount of items displayed per page of SelectionList"

Author: MarioExpensify

src/CONST/index.ts

@@ -5204,7 +5204,7 @@ const CONST = {
      * The maximum count of items per page for SelectionList.
      * When paginate, it multiplies by page number.
      */
-    MAX_SELECTION_LIST_PAGE_LENGTH: 50,
+    MAX_SELECTION_LIST_PAGE_LENGTH: 500,
 
     /**
      * Bank account names

src/components/SelectionList/BaseSelectionList.tsx

@@ -167,34 +167,10 @@ function BaseSelectionList<TItem extends ListItem>(
     const {isKeyboardShown} = useKeyboardState();
     const [itemsToHighlight, setItemsToHighlight] = useState<Set<string> | null>(null);
     const itemFocusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
-    const isItemSelected = useCallback(
-        (item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple),
-        [isSelected, selectedItems, canSelectMultiple],
-    );
-    /** Calculates on which page is selected item so we can scroll to it on first render  */
-    const calculateInitialCurrentPage = useCallback(() => {
-        if (canSelectMultiple || sections.length === 0) {
-            return 1;
-        }
-
-        let currentIndex = 0;
-        for (const section of sections) {
-            if (section.data) {
-                for (const item of section.data) {
-                    if (isItemSelected(item)) {
-                        return Math.floor(currentIndex / CONST.MAX_SELECTION_LIST_PAGE_LENGTH) + 1;
-                    }
-                    currentIndex++;
-                }
-            }
-        }
-        return 1;
-    }, [canSelectMultiple, isItemSelected, sections]);
-    const [currentPage, setCurrentPage] = useState(() => calculateInitialCurrentPage());
+    const [currentPage, setCurrentPage] = useState(1);
     const isTextInputFocusedRef = useRef<boolean>(false);
     const {singleExecution} = useSingleExecution();
     const [itemHeights, setItemHeights] = useState<Record<string, number>>({});
-    const pendingScrollIndexRef = useRef<number | null>(null);
 
     const onItemLayout = (event: LayoutChangeEvent, itemKey: string | null | undefined) => {
         if (!itemKey) {
@@ -211,6 +187,11 @@ function BaseSelectionList<TItem extends ListItem>(
 
     const incrementPage = () => setCurrentPage((prev) => prev + 1);
 
+    const isItemSelected = useCallback(
+        (item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple),
+        [isSelected, selectedItems, canSelectMultiple],
+    );
+
     const canShowProductTrainingTooltipMemo = useMemo(() => {
         return canShowProductTrainingTooltip && isFocused;
     }, [canShowProductTrainingTooltip, isFocused]);
@@ -343,17 +324,6 @@ function BaseSelectionList<TItem extends ListItem>(
                 return;
             }
 
-            // Calculate which page is needed to show this index
-            const requiredPage = Math.ceil((index + 1) / CONST.MAX_SELECTION_LIST_PAGE_LENGTH);
-
-            // If the required page is beyond the current page, load all pages up to it,
-            // then return early and let the scroll happen after the page update
-            if (requiredPage > currentPage) {
-                pendingScrollIndexRef.current = index;
-                setCurrentPage(requiredPage);
-                return;
-            }
-
             const itemIndex = item.index ?? -1;
             const sectionIndex = item.sectionIndex ?? -1;
             let viewOffsetToKeepFocusedItemAtTopOfViewableArea = 0;
@@ -370,11 +340,10 @@ function BaseSelectionList<TItem extends ListItem>(
             }
 
             listRef.current.scrollToLocation({sectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight - viewOffsetToKeepFocusedItemAtTopOfViewableArea});
-            pendingScrollIndexRef.current = null;
         },
 
         // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
-        [flattenedSections.allOptions, currentPage],
+        [flattenedSections.allOptions],
     );
 
     const [disabledArrowKeyIndexes, setDisabledArrowKeyIndexes] = useState(flattenedSections.disabledArrowKeyOptionsIndexes);
@@ -387,21 +356,6 @@ function BaseSelectionList<TItem extends ListItem>(
         // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
     }, [flattenedSections.disabledArrowKeyOptionsIndexes]);
 
-    /** Check whether there is a need to scroll to an item and if all items are loaded */
-    useEffect(() => {
-        if (pendingScrollIndexRef.current === null) {
-            return;
-        }
-
-        const indexToScroll = pendingScrollIndexRef.current;
-        const targetItem = flattenedSections.allOptions.at(indexToScroll);
-
-        if (targetItem && indexToScroll < CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage) {
-            pendingScrollIndexRef.current = null;
-            scrollToIndex(indexToScroll, true);
-        }
-    }, [currentPage, scrollToIndex, flattenedSections.allOptions]);
-
     const debouncedScrollToIndex = useMemo(() => lodashDebounce(scrollToIndex, CONST.TIMING.LIST_SCROLLING_DEBOUNCE_TIME, {leading: true, trailing: true}), [scrollToIndex]);
 
     const setHasKeyBeenPressed = useCallback(() => {
@@ -816,28 +770,17 @@ function BaseSelectionList<TItem extends ListItem>(
         ) {
             return;
         }
-        // Reset the current page to 1 when the user types something
-        if (prevTextInputValue !== textInputValue) {
-            setCurrentPage(1);
-        }
-
-        // When clearing the search, scroll to the selected item if one exists
-        if (prevTextInputValue !== '' && textInputValue === '') {
-            const foundSelectedItemIndex = flattenedSections.allOptions.findIndex(isItemSelected);
-            if (foundSelectedItemIndex !== -1) {
-                updateAndScrollToFocusedIndex(foundSelectedItemIndex);
-                return;
-            }
-        }
-
         // Remove the focus if the search input is empty and prev search input not empty or selected options length is changed (and allOptions length remains the same)
         // else focus on the first non disabled item
         const newSelectedIndex =
-            (prevTextInputValue !== '' && textInputValue === '') ||
+            (isEmpty(prevTextInputValue) && textInputValue === '') ||
             (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length)
                 ? -1
                 : 0;
 
+        // Reset the current page to 1 when the user types something
+        setCurrentPage(1);
+
         updateAndScrollToFocusedIndex(newSelectedIndex);
     }, [
         canSelectMultiple,
@@ -849,8 +792,6 @@ function BaseSelectionList<TItem extends ListItem>(
         prevSelectedOptionsLength,
         prevAllOptionsLength,
         shouldUpdateFocusedIndex,
-        flattenedSections.allOptions,
-        isItemSelected,
     ]);
 
     useEffect(

tests/unit/BaseSelectionListTest.tsx

@@ -1,6 +1,5 @@
 import * as NativeNavigation from '@react-navigation/native';
 import {fireEvent, render, screen} from '@testing-library/react-native';
-import {useState} from 'react';
 import {SectionList} from 'react-native';
 import BaseSelectionList from '@components/SelectionList/BaseSelectionList';
 import RadioListItem from '@components/SelectionList/RadioListItem';
@@ -11,9 +10,6 @@ import CONST from '@src/CONST';
 type BaseSelectionListSections<TItem extends ListItem> = {
     sections: SelectionListProps<TItem>['sections'];
     canSelectMultiple?: boolean;
-    initialNumToRender?: number;
-    searchText?: string;
-    setSearchText?: (searchText: string) => void;
 };
 
 const mockSections = Array.from({length: 10}, (_, index) => ({
@@ -22,18 +18,6 @@ const mockSections = Array.from({length: 10}, (_, index) => ({
     isSelected: index === 1,
 }));
 
-const largeMockSections = Array.from({length: 100}, (_, index) => ({
-    text: `Item ${index}`,
-    keyForList: `${index}`,
-    isSelected: index === 1,
-}));
-
-const largeMockSectionsWithSelectedItemFromSecondPage = Array.from({length: 100}, (_, index) => ({
-    text: `Item ${index}`,
-    keyForList: `${index}`,
-    isSelected: index === 70,
-}));
-
 jest.mock('@src/components/ConfirmedRoute.tsx');
 jest.mock('@react-navigation/native', () => {
     const actualNav = jest.requireActual<typeof Navigation>('@react-navigation/native');
@@ -44,31 +28,20 @@ jest.mock('@react-navigation/native', () => {
     };
 });
 
-jest.mock('@hooks/useLocalize', () =>
-    jest.fn(() => ({
-        translate: jest.fn((key: string) => key),
-        numberFormat: jest.fn((num: number) => num.toString()),
-    })),
-);
-
 describe('BaseSelectionList', () => {
     const onSelectRowMock = jest.fn();
 
     function BaseListItemRenderer<TItem extends ListItem>(props: BaseSelectionListSections<TItem>) {
-        const {sections, canSelectMultiple, initialNumToRender, setSearchText, searchText} = props;
+        const {sections, canSelectMultiple} = props;
         const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList;
         return (
             <BaseSelectionList
                 sections={sections}
-                textInputLabel="common.search"
                 ListItem={RadioListItem}
                 onSelectRow={onSelectRowMock}
                 shouldSingleExecuteRowSelect
                 canSelectMultiple={canSelectMultiple}
                 initiallyFocusedOptionKey={focusedKey}
-                initialNumToRender={initialNumToRender}
-                onChangeText={setSearchText}
-                textInputValue={searchText}
             />
         );
     }
@@ -114,102 +87,4 @@ describe('BaseSelectionList', () => {
         fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`));
         expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0}));
     });
-
-    it('should show only elements from first page and Show More button when items exceed page limit', () => {
-        render(
-            <BaseListItemRenderer
-                sections={[{data: largeMockSections}]}
-                canSelectMultiple={false}
-                initialNumToRender={60}
-            />,
-        );
-
-        // Should render exactly first page (50 items)
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}49`)).toBeTruthy();
-
-        // Should NOT render items from second page
-        expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeFalsy();
-        expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy();
-
-        expect(screen.getByText('common.showMore')).toBeTruthy();
-        expect(screen.getByText('50')).toBeTruthy();
-        expect(screen.getByText('100')).toBeTruthy();
-    });
-
-    it('should hide Show More button when items fit on one page', () => {
-        render(
-            <BaseListItemRenderer
-                sections={[{data: mockSections}]}
-                canSelectMultiple={false}
-                initialNumToRender={60}
-            />,
-        );
-
-        expect(screen.queryByText('common.showMore')).toBeFalsy();
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}9`)).toBeTruthy();
-    });
-
-    it('should load more items when Show More button is clicked', () => {
-        render(
-            <BaseListItemRenderer
-                sections={[{data: largeMockSections}]}
-                canSelectMultiple={false}
-                initialNumToRender={110}
-            />,
-        );
-
-        // Click Show More button
-        fireEvent.press(screen.getByText('common.showMore'));
-
-        // Should now show items from second page
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeTruthy();
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy();
-
-        // Should not show, Show more button as we rendered whole list
-        expect(screen.queryByText('common.showMore')).toBeFalsy();
-    });
-
-    it('should search for first item then scroll back to preselected item when search is cleared', () => {
-        function SearchableListWrapper() {
-            const [searchText, setSearchText] = useState('');
-
-            // Filter sections based on search text
-            const filteredSections = searchText
-                ? largeMockSectionsWithSelectedItemFromSecondPage.filter((item) => item.text.toLowerCase().includes(searchText.toLowerCase()))
-                : largeMockSectionsWithSelectedItemFromSecondPage;
-
-            return (
-                <BaseListItemRenderer
-                    sections={[{data: filteredSections}]}
-                    searchText={searchText}
-                    setSearchText={setSearchText}
-                    canSelectMultiple={false}
-                    initialNumToRender={110}
-                />
-            );
-        }
-
-        render(<SearchableListWrapper />);
-
-        // Initially should show item 70 as selected and visible
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy();
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected();
-
-        // Search for "Item 0"
-        fireEvent.changeText(screen.getByTestId('selection-list-text-input'), 'Item 0');
-
-        // Should show only the first item (Item 0) in search results
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
-        expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeFalsy();
-        expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}1`)).toBeFalsy();
-
-        // Clear the search text
-        fireEvent.changeText(screen.getByTestId('selection-list-text-input'), '');
-
-        // Should scroll back to and show the preselected item 70
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy();
-        expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected();
-    });
 });