Skip to content

Commit e0d485d

Browse files
cristipavalcristipaval
authored
Revert "feat: Reduce amount of items displayed per page of SelectionList"
1 parent c593f40 commit e0d485d

3 files changed

Lines changed: 13 additions & 197 deletions

File tree

‎src/CONST/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5204,7 +5204,7 @@ const CONST = {
52045204
* The maximum count of items per page for SelectionList.
52055205
* When paginate, it multiplies by page number.
52065206
*/
5207-
MAX_SELECTION_LIST_PAGE_LENGTH: 50,
5207+
MAX_SELECTION_LIST_PAGE_LENGTH: 500,
52085208

52095209
/**
52105210
* Bank account names

‎src/components/SelectionList/BaseSelectionList.tsx‎

Lines changed: 11 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -167,34 +167,10 @@ function BaseSelectionList<TItem extends ListItem>(
167167
const {isKeyboardShown} = useKeyboardState();
168168
const [itemsToHighlight, setItemsToHighlight] = useState<Set<string> | null>(null);
169169
const itemFocusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
170-
const isItemSelected = useCallback(
171-
(item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple),
172-
[isSelected, selectedItems, canSelectMultiple],
173-
);
174-
/** Calculates on which page is selected item so we can scroll to it on first render */
175-
const calculateInitialCurrentPage = useCallback(() => {
176-
if (canSelectMultiple || sections.length === 0) {
177-
return 1;
178-
}
179-
180-
let currentIndex = 0;
181-
for (const section of sections) {
182-
if (section.data) {
183-
for (const item of section.data) {
184-
if (isItemSelected(item)) {
185-
return Math.floor(currentIndex / CONST.MAX_SELECTION_LIST_PAGE_LENGTH) + 1;
186-
}
187-
currentIndex++;
188-
}
189-
}
190-
}
191-
return 1;
192-
}, [canSelectMultiple, isItemSelected, sections]);
193-
const [currentPage, setCurrentPage] = useState(() => calculateInitialCurrentPage());
170+
const [currentPage, setCurrentPage] = useState(1);
194171
const isTextInputFocusedRef = useRef<boolean>(false);
195172
const {singleExecution} = useSingleExecution();
196173
const [itemHeights, setItemHeights] = useState<Record<string, number>>({});
197-
const pendingScrollIndexRef = useRef<number | null>(null);
198174

199175
const onItemLayout = (event: LayoutChangeEvent, itemKey: string | null | undefined) => {
200176
if (!itemKey) {
@@ -211,6 +187,11 @@ function BaseSelectionList<TItem extends ListItem>(
211187

212188
const incrementPage = () => setCurrentPage((prev) => prev + 1);
213189

190+
const isItemSelected = useCallback(
191+
(item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple),
192+
[isSelected, selectedItems, canSelectMultiple],
193+
);
194+
214195
const canShowProductTrainingTooltipMemo = useMemo(() => {
215196
return canShowProductTrainingTooltip && isFocused;
216197
}, [canShowProductTrainingTooltip, isFocused]);
@@ -343,17 +324,6 @@ function BaseSelectionList<TItem extends ListItem>(
343324
return;
344325
}
345326

346-
// Calculate which page is needed to show this index
347-
const requiredPage = Math.ceil((index + 1) / CONST.MAX_SELECTION_LIST_PAGE_LENGTH);
348-
349-
// If the required page is beyond the current page, load all pages up to it,
350-
// then return early and let the scroll happen after the page update
351-
if (requiredPage > currentPage) {
352-
pendingScrollIndexRef.current = index;
353-
setCurrentPage(requiredPage);
354-
return;
355-
}
356-
357327
const itemIndex = item.index ?? -1;
358328
const sectionIndex = item.sectionIndex ?? -1;
359329
let viewOffsetToKeepFocusedItemAtTopOfViewableArea = 0;
@@ -370,11 +340,10 @@ function BaseSelectionList<TItem extends ListItem>(
370340
}
371341

372342
listRef.current.scrollToLocation({sectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight - viewOffsetToKeepFocusedItemAtTopOfViewableArea});
373-
pendingScrollIndexRef.current = null;
374343
},
375344

376345
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
377-
[flattenedSections.allOptions, currentPage],
346+
[flattenedSections.allOptions],
378347
);
379348

380349
const [disabledArrowKeyIndexes, setDisabledArrowKeyIndexes] = useState(flattenedSections.disabledArrowKeyOptionsIndexes);
@@ -387,21 +356,6 @@ function BaseSelectionList<TItem extends ListItem>(
387356
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
388357
}, [flattenedSections.disabledArrowKeyOptionsIndexes]);
389358

390-
/** Check whether there is a need to scroll to an item and if all items are loaded */
391-
useEffect(() => {
392-
if (pendingScrollIndexRef.current === null) {
393-
return;
394-
}
395-
396-
const indexToScroll = pendingScrollIndexRef.current;
397-
const targetItem = flattenedSections.allOptions.at(indexToScroll);
398-
399-
if (targetItem && indexToScroll < CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage) {
400-
pendingScrollIndexRef.current = null;
401-
scrollToIndex(indexToScroll, true);
402-
}
403-
}, [currentPage, scrollToIndex, flattenedSections.allOptions]);
404-
405359
const debouncedScrollToIndex = useMemo(() => lodashDebounce(scrollToIndex, CONST.TIMING.LIST_SCROLLING_DEBOUNCE_TIME, {leading: true, trailing: true}), [scrollToIndex]);
406360

407361
const setHasKeyBeenPressed = useCallback(() => {
@@ -816,28 +770,17 @@ function BaseSelectionList<TItem extends ListItem>(
816770
) {
817771
return;
818772
}
819-
// Reset the current page to 1 when the user types something
820-
if (prevTextInputValue !== textInputValue) {
821-
setCurrentPage(1);
822-
}
823-
824-
// When clearing the search, scroll to the selected item if one exists
825-
if (prevTextInputValue !== '' && textInputValue === '') {
826-
const foundSelectedItemIndex = flattenedSections.allOptions.findIndex(isItemSelected);
827-
if (foundSelectedItemIndex !== -1) {
828-
updateAndScrollToFocusedIndex(foundSelectedItemIndex);
829-
return;
830-
}
831-
}
832-
833773
// 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)
834774
// else focus on the first non disabled item
835775
const newSelectedIndex =
836-
(prevTextInputValue !== '' && textInputValue === '') ||
776+
(isEmpty(prevTextInputValue) && textInputValue === '') ||
837777
(flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length)
838778
? -1
839779
: 0;
840780

781+
// Reset the current page to 1 when the user types something
782+
setCurrentPage(1);
783+
841784
updateAndScrollToFocusedIndex(newSelectedIndex);
842785
}, [
843786
canSelectMultiple,
@@ -849,8 +792,6 @@ function BaseSelectionList<TItem extends ListItem>(
849792
prevSelectedOptionsLength,
850793
prevAllOptionsLength,
851794
shouldUpdateFocusedIndex,
852-
flattenedSections.allOptions,
853-
isItemSelected,
854795
]);
855796

856797
useEffect(

‎tests/unit/BaseSelectionListTest.tsx‎

Lines changed: 1 addition & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as NativeNavigation from '@react-navigation/native';
22
import {fireEvent, render, screen} from '@testing-library/react-native';
3-
import {useState} from 'react';
43
import {SectionList} from 'react-native';
54
import BaseSelectionList from '@components/SelectionList/BaseSelectionList';
65
import RadioListItem from '@components/SelectionList/RadioListItem';
@@ -11,9 +10,6 @@ import CONST from '@src/CONST';
1110
type BaseSelectionListSections<TItem extends ListItem> = {
1211
sections: SelectionListProps<TItem>['sections'];
1312
canSelectMultiple?: boolean;
14-
initialNumToRender?: number;
15-
searchText?: string;
16-
setSearchText?: (searchText: string) => void;
1713
};
1814

1915
const mockSections = Array.from({length: 10}, (_, index) => ({
@@ -22,18 +18,6 @@ const mockSections = Array.from({length: 10}, (_, index) => ({
2218
isSelected: index === 1,
2319
}));
2420

25-
const largeMockSections = Array.from({length: 100}, (_, index) => ({
26-
text: `Item ${index}`,
27-
keyForList: `${index}`,
28-
isSelected: index === 1,
29-
}));
30-
31-
const largeMockSectionsWithSelectedItemFromSecondPage = Array.from({length: 100}, (_, index) => ({
32-
text: `Item ${index}`,
33-
keyForList: `${index}`,
34-
isSelected: index === 70,
35-
}));
36-
3721
jest.mock('@src/components/ConfirmedRoute.tsx');
3822
jest.mock('@react-navigation/native', () => {
3923
const actualNav = jest.requireActual<typeof Navigation>('@react-navigation/native');
@@ -44,31 +28,20 @@ jest.mock('@react-navigation/native', () => {
4428
};
4529
});
4630

47-
jest.mock('@hooks/useLocalize', () =>
48-
jest.fn(() => ({
49-
translate: jest.fn((key: string) => key),
50-
numberFormat: jest.fn((num: number) => num.toString()),
51-
})),
52-
);
53-
5431
describe('BaseSelectionList', () => {
5532
const onSelectRowMock = jest.fn();
5633

5734
function BaseListItemRenderer<TItem extends ListItem>(props: BaseSelectionListSections<TItem>) {
58-
const {sections, canSelectMultiple, initialNumToRender, setSearchText, searchText} = props;
35+
const {sections, canSelectMultiple} = props;
5936
const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList;
6037
return (
6138
<BaseSelectionList
6239
sections={sections}
63-
textInputLabel="common.search"
6440
ListItem={RadioListItem}
6541
onSelectRow={onSelectRowMock}
6642
shouldSingleExecuteRowSelect
6743
canSelectMultiple={canSelectMultiple}
6844
initiallyFocusedOptionKey={focusedKey}
69-
initialNumToRender={initialNumToRender}
70-
onChangeText={setSearchText}
71-
textInputValue={searchText}
7245
/>
7346
);
7447
}
@@ -114,102 +87,4 @@ describe('BaseSelectionList', () => {
11487
fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`));
11588
expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0}));
11689
});
117-
118-
it('should show only elements from first page and Show More button when items exceed page limit', () => {
119-
render(
120-
<BaseListItemRenderer
121-
sections={[{data: largeMockSections}]}
122-
canSelectMultiple={false}
123-
initialNumToRender={60}
124-
/>,
125-
);
126-
127-
// Should render exactly first page (50 items)
128-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
129-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}49`)).toBeTruthy();
130-
131-
// Should NOT render items from second page
132-
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeFalsy();
133-
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy();
134-
135-
expect(screen.getByText('common.showMore')).toBeTruthy();
136-
expect(screen.getByText('50')).toBeTruthy();
137-
expect(screen.getByText('100')).toBeTruthy();
138-
});
139-
140-
it('should hide Show More button when items fit on one page', () => {
141-
render(
142-
<BaseListItemRenderer
143-
sections={[{data: mockSections}]}
144-
canSelectMultiple={false}
145-
initialNumToRender={60}
146-
/>,
147-
);
148-
149-
expect(screen.queryByText('common.showMore')).toBeFalsy();
150-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
151-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}9`)).toBeTruthy();
152-
});
153-
154-
it('should load more items when Show More button is clicked', () => {
155-
render(
156-
<BaseListItemRenderer
157-
sections={[{data: largeMockSections}]}
158-
canSelectMultiple={false}
159-
initialNumToRender={110}
160-
/>,
161-
);
162-
163-
// Click Show More button
164-
fireEvent.press(screen.getByText('common.showMore'));
165-
166-
// Should now show items from second page
167-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeTruthy();
168-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy();
169-
170-
// Should not show, Show more button as we rendered whole list
171-
expect(screen.queryByText('common.showMore')).toBeFalsy();
172-
});
173-
174-
it('should search for first item then scroll back to preselected item when search is cleared', () => {
175-
function SearchableListWrapper() {
176-
const [searchText, setSearchText] = useState('');
177-
178-
// Filter sections based on search text
179-
const filteredSections = searchText
180-
? largeMockSectionsWithSelectedItemFromSecondPage.filter((item) => item.text.toLowerCase().includes(searchText.toLowerCase()))
181-
: largeMockSectionsWithSelectedItemFromSecondPage;
182-
183-
return (
184-
<BaseListItemRenderer
185-
sections={[{data: filteredSections}]}
186-
searchText={searchText}
187-
setSearchText={setSearchText}
188-
canSelectMultiple={false}
189-
initialNumToRender={110}
190-
/>
191-
);
192-
}
193-
194-
render(<SearchableListWrapper />);
195-
196-
// Initially should show item 70 as selected and visible
197-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy();
198-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected();
199-
200-
// Search for "Item 0"
201-
fireEvent.changeText(screen.getByTestId('selection-list-text-input'), 'Item 0');
202-
203-
// Should show only the first item (Item 0) in search results
204-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
205-
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeFalsy();
206-
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}1`)).toBeFalsy();
207-
208-
// Clear the search text
209-
fireEvent.changeText(screen.getByTestId('selection-list-text-input'), '');
210-
211-
// Should scroll back to and show the preselected item 70
212-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy();
213-
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected();
214-
});
21590
});

0 commit comments

Comments
 (0)