From 655fdcac06dbcef96d385f96a37b685a9c4fe235 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Mon, 21 Jul 2025 12:20:09 +0530 Subject: [PATCH 01/11] =?UTF-8?q?fix:=20Tags=20-=20List=20jumps=20to=20?= =?UTF-8?q?=E2=80=9CShow=20More=E2=80=9D=20when=20selecting=20tag=20after?= =?UTF-8?q?=20500.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: krishna2323 --- .../SelectionList/BaseSelectionList.tsx | 16 ++- src/components/SelectionList/types.ts | 6 + src/pages/TeachersUnite/ImTeacherPage.tsx | 2 +- tests/unit/BaseSelectionListTest.tsx | 128 +++++++++++++++++- 4 files changed, 147 insertions(+), 5 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 1ea9636fec34..bdcc2c61a927 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -149,6 +149,8 @@ function BaseSelectionList( selectedItems = [], isSelected, canShowProductTrainingTooltip, + onCurrentPageChange, + onFocusReset, }: SelectionListProps, ref: ForwardedRef, ) { @@ -761,6 +763,15 @@ function BaseSelectionList( const prevSelectedOptionsLength = usePrevious(flattenedSections.selectedOptions.length); const prevAllOptionsLength = usePrevious(flattenedSections.allOptions.length); + useEffect(() => { + if (prevTextInputValue === textInputValue) { + return; + } + // Reset the current page to 1 when the user types something + setCurrentPage(1); + onCurrentPageChange?.(); + }, [textInputValue, prevTextInputValue, onCurrentPageChange]); + useEffect(() => { // Avoid changing focus if the textInputValue remains unchanged. if ( @@ -778,9 +789,7 @@ function BaseSelectionList( ? -1 : 0; - // Reset the current page to 1 when the user types something - setCurrentPage(1); - + onFocusReset?.(); updateAndScrollToFocusedIndex(newSelectedIndex); }, [ canSelectMultiple, @@ -792,6 +801,7 @@ function BaseSelectionList( prevSelectedOptionsLength, prevAllOptionsLength, shouldUpdateFocusedIndex, + onFocusReset, ]); useEffect( diff --git a/src/components/SelectionList/types.ts b/src/components/SelectionList/types.ts index 8c2e66dbc04b..e0951c8f2116 100644 --- a/src/components/SelectionList/types.ts +++ b/src/components/SelectionList/types.ts @@ -826,6 +826,12 @@ type SelectionListProps = Partial & { /** Whether product training tooltips can be displayed */ canShowProductTrainingTooltip?: boolean; + + /** Called when the current page is changed */ + onCurrentPageChange?: () => void; + + /** Called when the focus is changed if user has typed somehting */ + onFocusReset?: () => void; } & TRightHandSideComponent; type SelectionListHandle = { diff --git a/src/pages/TeachersUnite/ImTeacherPage.tsx b/src/pages/TeachersUnite/ImTeacherPage.tsx index dee5eb0f18ac..330bd5dd1cbf 100644 --- a/src/pages/TeachersUnite/ImTeacherPage.tsx +++ b/src/pages/TeachersUnite/ImTeacherPage.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import {useSession} from '@components/OnyxProvider'; +import {useSession} from '@components/OnyxListItemProvider'; import {isEmailPublicDomain} from '@libs/LoginUtils'; import ImTeacherUpdateEmailPage from './ImTeacherUpdateEmailPage'; import IntroSchoolPrincipalPage from './IntroSchoolPrincipalPage'; diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 575476b354c7..7ba76aefe920 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -10,6 +10,8 @@ import CONST from '@src/CONST'; type BaseSelectionListSections = { sections: SelectionListProps['sections']; canSelectMultiple?: boolean; + textInputValue?: string; + shouldUpdateFocusedIndex?: boolean; }; const mockSections = Array.from({length: 10}, (_, index) => ({ @@ -29,10 +31,16 @@ jest.mock('@react-navigation/native', () => { }); describe('BaseSelectionList', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + const onCurrentPageChange = jest.fn(); + const onFocusReset = jest.fn(); const onSelectRowMock = jest.fn(); function BaseListItemRenderer(props: BaseSelectionListSections) { - const {sections, canSelectMultiple} = props; + const {sections, canSelectMultiple, textInputValue = '', shouldUpdateFocusedIndex} = props; const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList; return ( { shouldSingleExecuteRowSelect canSelectMultiple={canSelectMultiple} initiallyFocusedOptionKey={focusedKey} + onCurrentPageChange={onCurrentPageChange} + onFocusReset={onFocusReset} + textInputValue={textInputValue} + shouldUpdateFocusedIndex={shouldUpdateFocusedIndex} /> ); } @@ -87,4 +99,118 @@ describe('BaseSelectionList', () => { fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)); expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0})); }); + + it('does not reset page when only selectedOptions changes', () => { + const {rerender} = render(); + expect(onCurrentPageChange).not.toHaveBeenCalled(); + expect(onFocusReset).not.toHaveBeenCalled(); + + // simulate selection-only change + const updated = mockSections.map((item, i) => ({ + ...item, + isSelected: i === 2, + })); + + rerender(); + expect(onFocusReset).not.toHaveBeenCalled(); + expect(onCurrentPageChange).not.toHaveBeenCalled(); + + rerender( + , + ); + expect(onFocusReset).toHaveBeenCalled(); + expect(onCurrentPageChange).toHaveBeenCalledTimes(1); + }); + + it('should reset current page and call onCurrentPageChange when text input changes', () => { + const {rerender} = render( + , + ); + + rerender( + , + ); + expect(onCurrentPageChange).toHaveBeenCalledTimes(1); + }); + + it('should reset focus when text input becomes empty after having a value', () => { + const {rerender} = render( + , + ); + + rerender( + , + ); + expect(onFocusReset).toHaveBeenCalled(); + }); + + it('should reset focus when selected options change but all options remain same', () => { + const initialSections = mockSections.map((item, i) => ({ + ...item, + isSelected: i === 1, + })); + const updatedSections = mockSections.map((item, i) => ({ + ...item, + isSelected: i === 2 || i === 3, + })); + + const {rerender} = render( + , + ); + + rerender( + , + ); + expect(onFocusReset).toHaveBeenCalled(); + }); + + it('should not reset focus when shouldUpdateFocusedIndex is true', () => { + const initialSections = mockSections.map((item, i) => ({ + ...item, + isSelected: i === 1, + })); + const updatedSections = mockSections.map((item, i) => ({ + ...item, + isSelected: i === 2 || i === 3, + })); + + const {rerender} = render( + , + ); + onFocusReset.mockClear(); + + rerender( + , + ); + expect(onFocusReset).not.toHaveBeenCalled(); + }); }); From e186b1a017b748bfa45896377f8b23f094138464 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 23 Jul 2025 21:22:30 +0530 Subject: [PATCH 02/11] resolve merge conflicts. Signed-off-by: krishna2323 --- .../SelectionList/BaseSelectionList.tsx | 7 +- src/components/SelectionList/types.ts | 6 - tests/unit/BaseSelectionListTest.tsx | 118 ------------------ 3 files changed, 1 insertion(+), 130 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index bdcc2c61a927..8c578ebef405 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -149,8 +149,6 @@ function BaseSelectionList( selectedItems = [], isSelected, canShowProductTrainingTooltip, - onCurrentPageChange, - onFocusReset, }: SelectionListProps, ref: ForwardedRef, ) { @@ -769,8 +767,7 @@ function BaseSelectionList( } // Reset the current page to 1 when the user types something setCurrentPage(1); - onCurrentPageChange?.(); - }, [textInputValue, prevTextInputValue, onCurrentPageChange]); + }, [textInputValue, prevTextInputValue]); useEffect(() => { // Avoid changing focus if the textInputValue remains unchanged. @@ -789,7 +786,6 @@ function BaseSelectionList( ? -1 : 0; - onFocusReset?.(); updateAndScrollToFocusedIndex(newSelectedIndex); }, [ canSelectMultiple, @@ -801,7 +797,6 @@ function BaseSelectionList( prevSelectedOptionsLength, prevAllOptionsLength, shouldUpdateFocusedIndex, - onFocusReset, ]); useEffect( diff --git a/src/components/SelectionList/types.ts b/src/components/SelectionList/types.ts index e0951c8f2116..8c2e66dbc04b 100644 --- a/src/components/SelectionList/types.ts +++ b/src/components/SelectionList/types.ts @@ -826,12 +826,6 @@ type SelectionListProps = Partial & { /** Whether product training tooltips can be displayed */ canShowProductTrainingTooltip?: boolean; - - /** Called when the current page is changed */ - onCurrentPageChange?: () => void; - - /** Called when the focus is changed if user has typed somehting */ - onFocusReset?: () => void; } & TRightHandSideComponent; type SelectionListHandle = { diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 7ba76aefe920..99d1d64b8fc8 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -35,8 +35,6 @@ describe('BaseSelectionList', () => { jest.clearAllMocks(); }); - const onCurrentPageChange = jest.fn(); - const onFocusReset = jest.fn(); const onSelectRowMock = jest.fn(); function BaseListItemRenderer(props: BaseSelectionListSections) { @@ -50,8 +48,6 @@ describe('BaseSelectionList', () => { shouldSingleExecuteRowSelect canSelectMultiple={canSelectMultiple} initiallyFocusedOptionKey={focusedKey} - onCurrentPageChange={onCurrentPageChange} - onFocusReset={onFocusReset} textInputValue={textInputValue} shouldUpdateFocusedIndex={shouldUpdateFocusedIndex} /> @@ -99,118 +95,4 @@ describe('BaseSelectionList', () => { fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)); expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0})); }); - - it('does not reset page when only selectedOptions changes', () => { - const {rerender} = render(); - expect(onCurrentPageChange).not.toHaveBeenCalled(); - expect(onFocusReset).not.toHaveBeenCalled(); - - // simulate selection-only change - const updated = mockSections.map((item, i) => ({ - ...item, - isSelected: i === 2, - })); - - rerender(); - expect(onFocusReset).not.toHaveBeenCalled(); - expect(onCurrentPageChange).not.toHaveBeenCalled(); - - rerender( - , - ); - expect(onFocusReset).toHaveBeenCalled(); - expect(onCurrentPageChange).toHaveBeenCalledTimes(1); - }); - - it('should reset current page and call onCurrentPageChange when text input changes', () => { - const {rerender} = render( - , - ); - - rerender( - , - ); - expect(onCurrentPageChange).toHaveBeenCalledTimes(1); - }); - - it('should reset focus when text input becomes empty after having a value', () => { - const {rerender} = render( - , - ); - - rerender( - , - ); - expect(onFocusReset).toHaveBeenCalled(); - }); - - it('should reset focus when selected options change but all options remain same', () => { - const initialSections = mockSections.map((item, i) => ({ - ...item, - isSelected: i === 1, - })); - const updatedSections = mockSections.map((item, i) => ({ - ...item, - isSelected: i === 2 || i === 3, - })); - - const {rerender} = render( - , - ); - - rerender( - , - ); - expect(onFocusReset).toHaveBeenCalled(); - }); - - it('should not reset focus when shouldUpdateFocusedIndex is true', () => { - const initialSections = mockSections.map((item, i) => ({ - ...item, - isSelected: i === 1, - })); - const updatedSections = mockSections.map((item, i) => ({ - ...item, - isSelected: i === 2 || i === 3, - })); - - const {rerender} = render( - , - ); - onFocusReset.mockClear(); - - rerender( - , - ); - expect(onFocusReset).not.toHaveBeenCalled(); - }); }); From 17b4e2eefbc368b3af5588168b6aeb25e6bcfcc8 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 23 Jul 2025 21:23:59 +0530 Subject: [PATCH 03/11] resolve merge conflicts. Signed-off-by: krishna2323 --- tests/unit/BaseSelectionListTest.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 99d1d64b8fc8..b5fd618177c8 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -10,8 +10,6 @@ import CONST from '@src/CONST'; type BaseSelectionListSections = { sections: SelectionListProps['sections']; canSelectMultiple?: boolean; - textInputValue?: string; - shouldUpdateFocusedIndex?: boolean; }; const mockSections = Array.from({length: 10}, (_, index) => ({ @@ -38,7 +36,7 @@ describe('BaseSelectionList', () => { const onSelectRowMock = jest.fn(); function BaseListItemRenderer(props: BaseSelectionListSections) { - const {sections, canSelectMultiple, textInputValue = '', shouldUpdateFocusedIndex} = props; + const {sections, canSelectMultiple} = props; const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList; return ( { shouldSingleExecuteRowSelect canSelectMultiple={canSelectMultiple} initiallyFocusedOptionKey={focusedKey} - textInputValue={textInputValue} - shouldUpdateFocusedIndex={shouldUpdateFocusedIndex} /> ); } From 0dbcc61704a46fb3c94303bf50500d32e26fdbdf Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 23 Jul 2025 22:09:48 +0530 Subject: [PATCH 04/11] update unit tests. Signed-off-by: krishna2323 --- .../SelectionList/BaseSelectionList.tsx | 4 -- tests/unit/BaseSelectionListTest.tsx | 58 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 64c8b4af191d..cd333ac0fccd 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -824,10 +824,6 @@ function BaseSelectionList( ) { 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 === '') { diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index a4e1acd26986..3fedd09888b4 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -216,4 +216,62 @@ describe('BaseSelectionList', () => { expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy(); expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected(); }); + + it('does not reset page when only selectedOptions changes', () => { + const {rerender} = render( + , + ); + + // Click Show More button + fireEvent.press(screen.getByText('common.showMore')); + + rerender( + ({...item, isSelected: index === 3}))}]} + canSelectMultiple={false} + initialNumToRender={110} + />, + ); + + // 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 reset current page when text input changes', () => { + const {rerender} = render( + , + ); + + // Click Show More button + fireEvent.press(screen.getByText('common.showMore')); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy(); + + // Rerender with changed `searchText` to trigger `setCurrentPage(1)` + rerender( + ({...item, isSelected: index === 3}))}]} + canSelectMultiple={false} + initialNumToRender={110} + searchText="Item" + />, + ); + + // Should now show items from second page + expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy(); + + // Should show, Show more button as current page is reset + expect(screen.getByText('common.showMore')).toBeOnTheScreen(); + }); }); From 3e8a6aed51e0fd2caf41851c63e4c8f0791959d8 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 23 Jul 2025 22:10:27 +0530 Subject: [PATCH 05/11] comment update Signed-off-by: krishna2323 --- tests/unit/BaseSelectionListTest.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 3fedd09888b4..71fc72906ebb 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -241,7 +241,7 @@ describe('BaseSelectionList', () => { 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 + // Should not show, Show more button as we rendered whole list and search text was not changed expect(screen.queryByText('common.showMore')).toBeFalsy(); }); From 364ac0ca7ae9b2b13bd6370779e170f10ca471b0 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Thu, 24 Jul 2025 00:32:24 +0530 Subject: [PATCH 06/11] update comment. Signed-off-by: krishna2323 --- tests/unit/BaseSelectionListTest.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 71fc72906ebb..5a795f09dc85 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -268,7 +268,7 @@ describe('BaseSelectionList', () => { />, ); - // Should now show items from second page + // Should not show the items from second page expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy(); // Should show, Show more button as current page is reset From daa7675c644161fbd60c98f86e0288a5c7a62c80 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Sat, 26 Jul 2025 02:17:06 +0530 Subject: [PATCH 07/11] fix unit tests. Signed-off-by: krishna2323 --- .../SelectionList/BaseSelectionList.tsx | 3 -- tests/unit/BaseSelectionListTest.tsx | 34 ++++++++++++++----- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index fc551484b6e0..8c578ebef405 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -786,9 +786,6 @@ function BaseSelectionList( ? -1 : 0; - // Reset the current page to 1 when the user types something - setCurrentPage(1); - updateAndScrollToFocusedIndex(newSelectedIndex); }, [ canSelectMultiple, diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 8bc930c3ede7..cb76b70dbf58 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -10,6 +10,8 @@ import CONST from '@src/CONST'; type BaseSelectionListSections = { sections: SelectionListProps['sections']; canSelectMultiple?: boolean; + textInputValue?: string; + initialNumToRender?: number; }; const mockSections = Array.from({length: 10}, (_, index) => ({ @@ -18,6 +20,12 @@ const mockSections = Array.from({length: 10}, (_, index) => ({ isSelected: index === 1, })); +const largeMockSections = Array.from({length: CONST.MAX_SELECTION_LIST_PAGE_LENGTH * 2}, (_, index) => ({ + text: `Item ${index}`, + keyForList: `${index}`, + isSelected: index === 1, +})); + jest.mock('@src/components/ConfirmedRoute.tsx'); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); @@ -27,6 +35,12 @@ jest.mock('@react-navigation/native', () => { useFocusEffect: jest.fn(), }; }); +jest.mock('@hooks/useLocalize', () => + jest.fn(() => ({ + translate: jest.fn((key: string) => key), + numberFormat: jest.fn((num: number) => num.toString()), + })), +); describe('BaseSelectionList', () => { beforeEach(() => { @@ -36,7 +50,7 @@ describe('BaseSelectionList', () => { const onSelectRowMock = jest.fn(); function BaseListItemRenderer(props: BaseSelectionListSections) { - const {sections, canSelectMultiple} = props; + const {sections, canSelectMultiple, textInputValue = '', initialNumToRender} = props; const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList; return ( { shouldSingleExecuteRowSelect canSelectMultiple={canSelectMultiple} initiallyFocusedOptionKey={focusedKey} + textInputValue={textInputValue} + initialNumToRender={initialNumToRender} /> ); } @@ -97,7 +113,7 @@ describe('BaseSelectionList', () => { , ); @@ -108,13 +124,13 @@ describe('BaseSelectionList', () => { ({...item, isSelected: index === 3}))}]} canSelectMultiple={false} - initialNumToRender={110} + initialNumToRender={600} />, ); // 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(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}500`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}503`)).toBeTruthy(); // Should not show, Show more button as we rendered whole list and search text was not changed expect(screen.queryByText('common.showMore')).toBeFalsy(); @@ -125,7 +141,7 @@ describe('BaseSelectionList', () => { , ); @@ -138,13 +154,13 @@ describe('BaseSelectionList', () => { ({...item, isSelected: index === 3}))}]} canSelectMultiple={false} - initialNumToRender={110} - searchText="Item" + textInputValue="Item" + initialNumToRender={600} />, ); // Should not show the items from second page - expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy(); + expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}502`)).toBeFalsy(); // Should show, Show more button as current page is reset expect(screen.getByText('common.showMore')).toBeOnTheScreen(); From ce1a6229eb5bcde0ba6c7f3e5cf86bb5f15c938c Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Mon, 28 Jul 2025 09:54:21 +0530 Subject: [PATCH 08/11] comment update. Signed-off-by: krishna2323 --- tests/unit/BaseSelectionListTest.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index cb76b70dbf58..b5387ec66657 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -132,7 +132,7 @@ describe('BaseSelectionList', () => { expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}500`)).toBeTruthy(); expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}503`)).toBeTruthy(); - // Should not show, Show more button as we rendered whole list and search text was not changed + // Should not show, "Show more" button as we rendered whole list and search text was not changed expect(screen.queryByText('common.showMore')).toBeFalsy(); }); @@ -162,7 +162,7 @@ describe('BaseSelectionList', () => { // Should not show the items from second page expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}502`)).toBeFalsy(); - // Should show, Show more button as current page is reset + // Should show, "Show more" button as current page is reset expect(screen.getByText('common.showMore')).toBeOnTheScreen(); }); }); From 334b4996ef7ebd9944234fd751dbd4063d63a7d6 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 5 Aug 2025 13:57:13 +0530 Subject: [PATCH 09/11] fix merge conflicts. Signed-off-by: krishna2323 --- tests/unit/BaseSelectionListTest.tsx | 80 +--------------------------- 1 file changed, 1 insertion(+), 79 deletions(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index b5387ec66657..575476b354c7 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -10,8 +10,6 @@ import CONST from '@src/CONST'; type BaseSelectionListSections = { sections: SelectionListProps['sections']; canSelectMultiple?: boolean; - textInputValue?: string; - initialNumToRender?: number; }; const mockSections = Array.from({length: 10}, (_, index) => ({ @@ -20,12 +18,6 @@ const mockSections = Array.from({length: 10}, (_, index) => ({ isSelected: index === 1, })); -const largeMockSections = Array.from({length: CONST.MAX_SELECTION_LIST_PAGE_LENGTH * 2}, (_, index) => ({ - text: `Item ${index}`, - keyForList: `${index}`, - isSelected: index === 1, -})); - jest.mock('@src/components/ConfirmedRoute.tsx'); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); @@ -35,22 +27,12 @@ jest.mock('@react-navigation/native', () => { useFocusEffect: jest.fn(), }; }); -jest.mock('@hooks/useLocalize', () => - jest.fn(() => ({ - translate: jest.fn((key: string) => key), - numberFormat: jest.fn((num: number) => num.toString()), - })), -); describe('BaseSelectionList', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - const onSelectRowMock = jest.fn(); function BaseListItemRenderer(props: BaseSelectionListSections) { - const {sections, canSelectMultiple, textInputValue = '', initialNumToRender} = props; + const {sections, canSelectMultiple} = props; const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList; return ( { shouldSingleExecuteRowSelect canSelectMultiple={canSelectMultiple} initiallyFocusedOptionKey={focusedKey} - textInputValue={textInputValue} - initialNumToRender={initialNumToRender} /> ); } @@ -107,62 +87,4 @@ describe('BaseSelectionList', () => { fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)); expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0})); }); - - it('does not reset page when only selectedOptions changes', () => { - const {rerender} = render( - , - ); - - // Click Show More button - fireEvent.press(screen.getByText('common.showMore')); - - rerender( - ({...item, isSelected: index === 3}))}]} - canSelectMultiple={false} - initialNumToRender={600} - />, - ); - - // Should now show items from second page - expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}500`)).toBeTruthy(); - expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}503`)).toBeTruthy(); - - // Should not show, "Show more" button as we rendered whole list and search text was not changed - expect(screen.queryByText('common.showMore')).toBeFalsy(); - }); - - it('should reset current page when text input changes', () => { - const {rerender} = render( - , - ); - - // Click Show More button - fireEvent.press(screen.getByText('common.showMore')); - expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy(); - - // Rerender with changed `searchText` to trigger `setCurrentPage(1)` - rerender( - ({...item, isSelected: index === 3}))}]} - canSelectMultiple={false} - textInputValue="Item" - initialNumToRender={600} - />, - ); - - // Should not show the items from second page - expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}502`)).toBeFalsy(); - - // Should show, "Show more" button as current page is reset - expect(screen.getByText('common.showMore')).toBeOnTheScreen(); - }); }); From cac3654bb0934e94bd616ef8b0e632765adb3d00 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 5 Aug 2025 13:59:35 +0530 Subject: [PATCH 10/11] re-implement tests. Signed-off-by: krishna2323 --- tests/unit/BaseSelectionListTest.tsx | 58 ++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index faf5c7395dd1..183757456808 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -212,4 +212,62 @@ describe('BaseSelectionList', () => { expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy(); expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected(); }); + + it('does not reset page when only selectedOptions changes', () => { + const {rerender} = render( + , + ); + + // Click Show More button + fireEvent.press(screen.getByText('common.showMore')); + + rerender( + ({...item, isSelected: index === 3}))}]} + canSelectMultiple={false} + initialNumToRender={110} + />, + ); + + // Should now show items from second page + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}51`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}62`)).toBeTruthy(); + + // Should not show, "Show more" button as we rendered whole list and search text was not changed + expect(screen.queryByText('common.showMore')).toBeFalsy(); + }); + + it('should reset current page when text input changes', () => { + const {rerender} = render( + , + ); + + // Click Show More button + fireEvent.press(screen.getByText('common.showMore')); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy(); + + // Rerender with changed `searchText` to trigger `setCurrentPage(1)` + rerender( + ({...item, isSelected: index === 3}))}]} + canSelectMultiple={false} + searchText="Item" + initialNumToRender={110} + />, + ); + + // Should not show the items from second page + expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}52`)).toBeFalsy(); + + // Should show, "Show more" button as current page is reset + expect(screen.getByText('common.showMore')).toBeOnTheScreen(); + }); }); From 0291827b45aa3d8384195ae14d744b301787a50a Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 5 Aug 2025 14:01:13 +0530 Subject: [PATCH 11/11] remove redundant code. Signed-off-by: krishna2323 --- src/components/SelectionList/BaseSelectionList.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 4776896293be..5018215c9b72 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -837,11 +837,6 @@ function BaseSelectionList( } } - // Reset the current page to 1 when the user types something - if (prevTextInputValue !== textInputValue) { - setCurrentPage(1); - } - // 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 =