diff --git a/src/components/Search/FilterComponents/BankAccountSelector.tsx b/src/components/Search/FilterComponents/BankAccountSelector.tsx index e62f7892095a..11862fc15af3 100644 --- a/src/components/Search/FilterComponents/BankAccountSelector.tsx +++ b/src/components/Search/FilterComponents/BankAccountSelector.tsx @@ -7,6 +7,7 @@ import SelectionListWithSections from '@components/SelectionList/SelectionListWi import type {TextInputOptions} from '@components/SelectionList/types'; import useDebouncedState from '@hooks/useDebouncedState'; +import useInitialValue from '@hooks/useInitialValue'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; @@ -97,12 +98,21 @@ function BankAccountSelector({value = [], selectionListTextInputStyle, selection const shouldShowSearchInput = openItems.length + closedItems.length >= CONST.STANDARD_LIST_ITEM_LIMIT; + // Snapshot the accounts selected when the filter first opened so they stay floated in the top section on first render + // without repinning rows that are toggled afterwards. Section membership keys on this snapshot while each row's + // checkbox still reflects the live selection, so selecting/deselecting an account no longer makes it jump between sections. + // Only float the initial selection when the list is long enough to warrant it (>= STANDARD_LIST_ITEM_LIMIT), mirroring + // the shared moveInitialSelectionToTop gate; for short lists items stay in their natural order so nothing is pinned. + const initialSelectedValues = useInitialValue(() => value); + const wasInitiallySelected = (item: BankAccountFilterItem) => initialSelectedValues.includes(item.value); + const shouldPinInitialSelection = shouldShowSearchInput; + const searchFunction = (item: BankAccountFilterItem) => item.text.toLocaleLowerCase().includes(debouncedSearchTerm.toLocaleLowerCase()) || item.lastFour.toLocaleLowerCase().includes(debouncedSearchTerm.toLocaleLowerCase()); - const selectedData = [...openItems, ...closedItems].filter((item) => item.isSelected && searchFunction(item)); - const unselectedOpenData = openItems.filter((item) => !item.isSelected && searchFunction(item)); - const unselectedClosedData = closedItems.filter((item) => !item.isSelected && searchFunction(item)); + const selectedData = shouldPinInitialSelection ? [...openItems, ...closedItems].filter((item) => wasInitiallySelected(item) && searchFunction(item)) : []; + const unselectedOpenData = openItems.filter((item) => (!shouldPinInitialSelection || !wasInitiallySelected(item)) && searchFunction(item)); + const unselectedClosedData = closedItems.filter((item) => (!shouldPinInitialSelection || !wasInitiallySelected(item)) && searchFunction(item)); const itemCount = selectedData.length + unselectedOpenData.length + unselectedClosedData.length; const sectionHeaderCount = unselectedClosedData.length > 0 ? 1 : 0; @@ -176,6 +186,9 @@ function BankAccountSelector({value = [], selectionListTextInputStyle, selection textInputOptions={textInputOptions} shouldStopPropagation canSelectMultiple + shouldClearInputOnSelect={false} + shouldUpdateFocusedIndex + shouldPreventAutoScrollOnSelect style={selectionListStyle} footerContent={footer} /> diff --git a/src/components/Search/FilterComponents/CardSelector.tsx b/src/components/Search/FilterComponents/CardSelector.tsx index 92023ca768db..2c84675bb5d7 100644 --- a/src/components/Search/FilterComponents/CardSelector.tsx +++ b/src/components/Search/FilterComponents/CardSelector.tsx @@ -7,6 +7,7 @@ import type {TextInputOptions} from '@components/SelectionList/types'; import {useCompanyCardFeedIcons} from '@hooks/useCompanyCardIcons'; import useDebouncedState from '@hooks/useDebouncedState'; +import useInitialValue from '@hooks/useInitialValue'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; @@ -67,7 +68,16 @@ function CardSelector({value = [], selectionListTextInputStyle, selectionListSty const closedCardsSectionData = buildCardsData(workspaceCardFeeds ?? {}, userCardList ?? {}, personalDetails ?? {}, value, illustrations, companyCardFeedIcons, true, customCardNames); - const shouldShowSearchInput = individualCardsSectionData.length >= CONST.STANDARD_LIST_ITEM_LIMIT; + const shouldShowSearchInput = individualCardsSectionData.length + closedCardsSectionData.length >= CONST.STANDARD_LIST_ITEM_LIMIT; + + // Snapshot the cards selected when the filter first opened so they stay floated in the top section on first render + // without repinning rows that are toggled afterwards. Section membership keys on this snapshot while each row's + // checkbox still reflects the live selection, so selecting/deselecting a card no longer makes it jump between sections. + // Only float the initial selection when the list is long enough to warrant it (>= STANDARD_LIST_ITEM_LIMIT), mirroring + // the shared moveInitialSelectionToTop gate; for short lists items stay in their natural order so nothing is pinned. + const initialSelectedValues = useInitialValue(() => value); + const wasInitiallySelected = (item: CardFilterItem) => !!item.keyForList && initialSelectedValues.includes(item.keyForList); + const shouldPinInitialSelection = shouldShowSearchInput; const searchFunction = (item: CardFilterItem) => !!item.text?.toLocaleLowerCase().includes(debouncedSearchTerm.toLocaleLowerCase()) || @@ -75,9 +85,9 @@ function CardSelector({value = [], selectionListTextInputStyle, selectionListSty !!item.cardName?.toLocaleLowerCase().includes(debouncedSearchTerm.toLocaleLowerCase()) || (item.isVirtual && translate('workspace.expensifyCard.virtual').toLocaleLowerCase().includes(debouncedSearchTerm.toLocaleLowerCase())); - const selectedData = [...individualCardsSectionData, ...closedCardsSectionData].filter((item) => item.isSelected && searchFunction(item)); - const unselectedIndividualCardsData = individualCardsSectionData.filter((item) => !item.isSelected && searchFunction(item)); - const unselectedClosedCardsData = closedCardsSectionData.filter((item) => !item.isSelected && searchFunction(item)); + const selectedData = shouldPinInitialSelection ? [...individualCardsSectionData, ...closedCardsSectionData].filter((item) => wasInitiallySelected(item) && searchFunction(item)) : []; + const unselectedIndividualCardsData = individualCardsSectionData.filter((item) => (!shouldPinInitialSelection || !wasInitiallySelected(item)) && searchFunction(item)); + const unselectedClosedCardsData = closedCardsSectionData.filter((item) => (!shouldPinInitialSelection || !wasInitiallySelected(item)) && searchFunction(item)); const itemCount = selectedData.length + unselectedIndividualCardsData.length + unselectedClosedCardsData.length; const sectionHeaderCount = unselectedClosedCardsData.length > 0 ? 1 : 0; @@ -155,6 +165,9 @@ function CardSelector({value = [], selectionListTextInputStyle, selectionListSty textInputOptions={textInputOptions} shouldStopPropagation canSelectMultiple + shouldClearInputOnSelect={false} + shouldUpdateFocusedIndex + shouldPreventAutoScrollOnSelect style={selectionListStyle} footerContent={footer} /> diff --git a/tests/ui/BankAccountSelectorTest.tsx b/tests/ui/BankAccountSelectorTest.tsx new file mode 100644 index 000000000000..6df33101d8fe --- /dev/null +++ b/tests/ui/BankAccountSelectorTest.tsx @@ -0,0 +1,164 @@ +import {render} from '@testing-library/react-native'; + +import BankAccountSelector from '@components/Search/FilterComponents/BankAccountSelector'; +import SelectionListWithSections from '@components/SelectionList/SelectionListWithSections'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +// The bank accounts returned by the (mocked) Onyx `BANK_ACCOUNT_LIST` key. Tests mutate this to switch between a +// long list (pinning enabled) and a short list (pinning disabled). Each account's `text` drives the search filter. +let mockBankAccountList: Record = {}; +let mockSearchTerm = ''; + +jest.mock('@components/SelectionList/SelectionListWithSections', () => jest.fn(() => null)); +jest.mock('@components/SelectionList/ListItem/MultiSelectListItem', () => jest.fn(() => null)); +jest.mock('@components/Icon', () => jest.fn(() => null)); +jest.mock('@components/Icon/BankIcons', () => jest.fn(() => ({icon: jest.fn(() => null), iconSize: 0, iconStyles: []}))); +jest.mock('@components/Search/FilterComponents/ListFilterViewWrapper', () => jest.fn(({children}: {children: React.ReactNode}) => children)); + +jest.mock('@hooks/useOnyx', () => jest.fn(() => [mockBankAccountList])); +jest.mock('@hooks/useDebouncedState', () => jest.fn(() => [mockSearchTerm, mockSearchTerm, jest.fn()])); +jest.mock('@hooks/useResponsiveLayout', () => jest.fn(() => ({isLargeScreenWidth: true}))); +jest.mock('@hooks/useTheme', () => jest.fn(() => ({}))); +jest.mock('@hooks/useThemeStyles', () => jest.fn(() => ({}))); +jest.mock('@hooks/useLocalize', () => + jest.fn(() => ({ + translate: (key: string) => key, + })), +); +jest.mock('@src/types/utils/isLoadingOnyxValue', () => jest.fn(() => false)); +jest.mock('@libs/BankAccountUtils', () => ({ + // The mocked account carries its display label directly on `text`. + getBankAccountSearchLabel: (bankAccount: {text: string}) => bankAccount.text, + isFilterableBankAccount: () => true, +})); + +describe('BankAccountSelector', () => { + const mockedSelectionList = jest.mocked(SelectionListWithSections); + + // Build `count` open bank accounts keyed "1".."count" (IDs must be truthy, so they start at 1). + // `matcher` optionally decides which accounts' labels contain the word "match" so the search test can target them. + const buildBankAccounts = (count: number, matcher?: (key: string) => boolean) => { + const list: typeof mockBankAccountList = {}; + for (let index = 1; index <= count; index++) { + const key = index.toString(); + list[key] = { + accountData: { + bankAccountID: index, + accountNumber: `0000${index}`, + additionalData: {bankName: 'Chase'}, + state: CONST.BANK_ACCOUNT.STATE.OPEN, + }, + text: matcher?.(key) ? `match ${key}` : `Bank ${key}`, + }; + } + return list; + }; + + const getSections = () => mockedSelectionList.mock.lastCall?.[0].sections ?? []; + + beforeEach(() => { + mockedSelectionList.mockClear(); + mockSearchTerm = ''; + // A long list so the "move selected to top" behavior is enabled. + mockBankAccountList = buildBankAccounts(CONST.STANDARD_LIST_ITEM_LIMIT + 2); + }); + + it('floats the initially-selected bank account to the top of a long list', () => { + render( + , + ); + + const sections = getSections(); + // Top section holds only the pre-selected account. + expect(sections.at(0)?.data.map((item) => item.keyForList)).toEqual(['11']); + // Main section keeps every other account, with the pinned one removed. + expect(sections.at(1)?.data.map((item) => item.keyForList)).not.toContain('11'); + expect(sections.at(1)?.data).toHaveLength(CONST.STANDARD_LIST_ITEM_LIMIT + 1); + }); + + it('keeps a bank account in place when it is toggled after first render (does not jump to the top)', () => { + const {rerender} = render( + , + ); + + // Simulate the user toggling another account on: the parent re-renders the filter with the updated value. + rerender( + , + ); + + const sections = getSections(); + // Only the originally pre-selected account stays pinned at the top. + expect(sections.at(0)?.data.map((item) => item.keyForList)).toEqual(['11']); + + // The newly toggled account is marked selected but stays in its natural position (keys "1".."14" minus "11", + // so "4" is at index 3) instead of jumping up. + const mainSection = sections.at(1)?.data ?? []; + expect(mainSection.findIndex((item) => item.keyForList === '4')).toBe(3); + expect(mainSection.find((item) => item.keyForList === '4')?.isSelected).toBe(true); + }); + + it('keeps the initially-selected account pinned at the top while searching, and drops non-matching pinned accounts', () => { + // Only accounts "3", "11", and "12" have labels containing the search term. "11" is pinned and matches -> it + // stays at the top; "5" is pinned but does not match -> it is dropped. + mockSearchTerm = 'match'; + mockBankAccountList = buildBankAccounts(CONST.STANDARD_LIST_ITEM_LIMIT + 2, (key) => key === '3' || key === '11' || key === '12'); + + render( + , + ); + + const sections = getSections(); + // The pinned account that still matches the search stays at the top; the non-matching pinned account is dropped. + expect(sections.at(0)?.data.map((item) => item.keyForList)).toEqual(['11']); + // The main section shows the remaining matches with the pinned one removed (no duplicate). + expect(sections.at(1)?.data.map((item) => item.keyForList)).toEqual(['3', '12']); + }); + + it('does not pin selected accounts to the top of a short list', () => { + mockBankAccountList = buildBankAccounts(5); + + render( + , + ); + + const sections = getSections(); + // No pinned section for a short list. + expect(sections.at(0)?.data).toHaveLength(0); + // Every account stays in place; the selected one is simply marked in position. + const mainSection = sections.at(1)?.data ?? []; + expect(mainSection.map((item) => item.keyForList)).toEqual(['1', '2', '3', '4', '5']); + expect(mainSection.find((item) => item.keyForList === '2')?.isSelected).toBe(true); + }); + + it('passes the expected selection behavior props to the list so a selected row does not clear the search, lose focus, or auto-scroll', () => { + render( + , + ); + + const props = mockedSelectionList.mock.lastCall?.[0]; + expect(props?.shouldClearInputOnSelect).toBe(false); + expect(props?.shouldUpdateFocusedIndex).toBe(true); + expect(props?.shouldPreventAutoScrollOnSelect).toBe(true); + }); +}); diff --git a/tests/ui/CardSelectorTest.tsx b/tests/ui/CardSelectorTest.tsx new file mode 100644 index 000000000000..7b7fb6bbaa35 --- /dev/null +++ b/tests/ui/CardSelectorTest.tsx @@ -0,0 +1,163 @@ +import {render} from '@testing-library/react-native'; + +import CardSelector from '@components/Search/FilterComponents/CardSelector'; +import SelectionListWithSections from '@components/SelectionList/SelectionListWithSections'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +// The number of individual cards `buildCardsData` returns. Tests mutate this to switch between a long list +// (pinning enabled) and a short list (pinning disabled). `mockMatcher` optionally decides which cards' labels +// contain the search term so the search test can target them. +let mockCardCount = 0; +let mockSearchTerm = ''; +let mockMatcher: ((key: string) => boolean) | undefined; + +jest.mock('@components/SelectionList/SelectionListWithSections', () => jest.fn(() => null)); +jest.mock('@components/SelectionList/ListItem/CardListItem', () => jest.fn(() => null)); +jest.mock('@components/OnyxListItemProvider', () => ({usePersonalDetails: jest.fn(() => ({}))})); +jest.mock('@components/Search/FilterComponents/ListFilterViewWrapper', () => jest.fn(({children}: {children: React.ReactNode}) => children)); + +jest.mock('@hooks/useOnyx', () => jest.fn(() => [undefined])); +jest.mock('@hooks/useDebouncedState', () => jest.fn(() => [mockSearchTerm, mockSearchTerm, jest.fn()])); +jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: true}))); +jest.mock('@hooks/useCompanyCardIcons', () => ({useCompanyCardFeedIcons: jest.fn(() => ({}))})); +jest.mock('@hooks/useTheme', () => jest.fn(() => ({}))); +jest.mock('@hooks/useThemeIllustrations', () => jest.fn(() => ({}))); +jest.mock('@hooks/useThemeStyles', () => jest.fn(() => ({}))); +jest.mock('@hooks/useLocalize', () => + jest.fn(() => ({ + translate: (key: string) => key, + })), +); +jest.mock('@src/types/utils/isLoadingOnyxValue', () => jest.fn(() => false)); +jest.mock('@libs/actions/Search', () => ({openSearchCardFiltersPage: jest.fn()})); +jest.mock('@libs/CardFeedUtils', () => ({ + // Build the individual-card section from `mockCardCount`; the closed-card section is always empty here. + // `isSelected` mirrors the real helper, which derives it from the passed-in `value`. + buildCardsData: jest.fn((workspaceCardFeeds, userCardList, personalDetails, value: string[], illustrations, icons, isClosed: boolean) => { + if (isClosed) { + return []; + } + return Array.from({length: mockCardCount}, (_, index) => { + const keyForList = `c${index}`; + return { + keyForList, + text: mockMatcher?.(keyForList) ? `match ${keyForList}` : `Card ${keyForList}`, + isSelected: (value ?? []).includes(keyForList), + lastFourPAN: '', + cardName: '', + }; + }); + }), +})); + +describe('CardSelector', () => { + const mockedSelectionList = jest.mocked(SelectionListWithSections); + + const getSections = () => mockedSelectionList.mock.lastCall?.[0].sections ?? []; + + beforeEach(() => { + mockedSelectionList.mockClear(); + mockSearchTerm = ''; + mockMatcher = undefined; + // A long list so the "move selected to top" behavior is enabled. + mockCardCount = CONST.STANDARD_LIST_ITEM_LIMIT + 2; + }); + + it('floats the initially-selected card to the top of a long list', () => { + render( + , + ); + + const sections = getSections(); + // Top section holds only the pre-selected card. + expect(sections.at(0)?.data.map((item) => item.keyForList)).toEqual(['c10']); + // Main section keeps every other card, with the pinned one removed. + expect(sections.at(1)?.data.map((item) => item.keyForList)).not.toContain('c10'); + expect(sections.at(1)?.data).toHaveLength(CONST.STANDARD_LIST_ITEM_LIMIT + 1); + }); + + it('keeps a card in place when it is toggled after first render (does not jump to the top)', () => { + const {rerender} = render( + , + ); + + // Simulate the user toggling another card on: the parent re-renders the filter with the updated value. + rerender( + , + ); + + const sections = getSections(); + // Only the originally pre-selected card stays pinned at the top. + expect(sections.at(0)?.data.map((item) => item.keyForList)).toEqual(['c10']); + + // The newly toggled card is marked selected but stays in its natural position (index 3) instead of jumping up. + const mainSection = sections.at(1)?.data ?? []; + expect(mainSection.findIndex((item) => item.keyForList === 'c3')).toBe(3); + expect(mainSection.find((item) => item.keyForList === 'c3')?.isSelected).toBe(true); + }); + + it('keeps the initially-selected card pinned at the top while searching, and drops non-matching pinned cards', () => { + // Only cards "c3", "c10", and "c11" have labels containing the search term. "c10" is pinned and matches -> it + // stays at the top; "c5" is pinned but does not match -> it is dropped. + mockSearchTerm = 'match'; + mockMatcher = (key) => key === 'c3' || key === 'c10' || key === 'c11'; + + render( + , + ); + + const sections = getSections(); + // The pinned card that still matches the search stays at the top; the non-matching pinned card is dropped. + expect(sections.at(0)?.data.map((item) => item.keyForList)).toEqual(['c10']); + // The main section shows the remaining matches with the pinned one removed (no duplicate). + expect(sections.at(1)?.data.map((item) => item.keyForList)).toEqual(['c3', 'c11']); + }); + + it('does not pin selected cards to the top of a short list', () => { + mockCardCount = 5; + + render( + , + ); + + const sections = getSections(); + // No pinned section for a short list. + expect(sections.at(0)?.data).toHaveLength(0); + // Every card stays in place; the selected one is simply marked in position. + const mainSection = sections.at(1)?.data ?? []; + expect(mainSection.map((item) => item.keyForList)).toEqual(['c0', 'c1', 'c2', 'c3', 'c4']); + expect(mainSection.find((item) => item.keyForList === 'c2')?.isSelected).toBe(true); + }); + + it('passes the expected selection behavior props to the list so a selected row does not clear the search, lose focus, or auto-scroll', () => { + render( + , + ); + + const props = mockedSelectionList.mock.lastCall?.[0]; + expect(props?.shouldClearInputOnSelect).toBe(false); + expect(props?.shouldUpdateFocusedIndex).toBe(true); + expect(props?.shouldPreventAutoScrollOnSelect).toBe(true); + }); +});