Skip to content

Commit 023b652

Browse files
authored
Merge pull request Expensify#90361 from callstack-internal/VickyStash/bugfix/90206-update-SelectionListWithModal
Move debounce from `SelectionListWithModal` to `useSearchResults`
2 parents 1b89b63 + d8e86e0 commit 023b652

5 files changed

Lines changed: 178 additions & 54 deletions

File tree

src/components/SelectionListWithModal/index.tsx

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,18 @@
11
import {useIsFocused} from '@react-navigation/native';
22
import type {ForwardedRef} from 'react';
3-
import React, {useEffect, useMemo, useState} from 'react';
3+
import React, {useMemo, useState} from 'react';
44
import MenuItem from '@components/MenuItem';
55
import Modal from '@components/Modal';
66
import SelectionList from '@components/SelectionList';
77
import type {ListItem, SelectionListHandle, SelectionListProps} from '@components/SelectionList/types';
8-
import useDebouncedState from '@hooks/useDebouncedState';
98
import useHandleSelectionMode from '@hooks/useHandleSelectionMode';
109
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
1110
import useLocalize from '@hooks/useLocalize';
1211
import useMobileSelectionMode from '@hooks/useMobileSelectionMode';
13-
import useNetwork from '@hooks/useNetwork';
1412
import useResponsiveLayout from '@hooks/useResponsiveLayout';
1513
import useThemeStyles from '@hooks/useThemeStyles';
1614
import {turnOnMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
1715
import CONST from '@src/CONST';
18-
import {isEmptyValueObject} from '@src/types/utils/EmptyObject';
1916

2017
type SelectionListWithModalProps<TItem extends ListItem> = SelectionListProps<TItem> & {
2118
turnOnSelectionModeOnLongPress?: boolean;
@@ -37,7 +34,6 @@ function SelectionListWithModal<TItem extends ListItem>({
3734
const [isModalVisible, setIsModalVisible] = useState(false);
3835
const [longPressedItem, setLongPressedItem] = useState<TItem | null>(null);
3936
const {translate} = useLocalize();
40-
const {isOffline} = useNetwork();
4137
const styles = useThemeStyles();
4238
// We need to use isSmallScreenWidth instead of shouldUseNarrowLayout here because there is a race condition that causes shouldUseNarrowLayout to change indefinitely in this component
4339
// See https://github.com/Expensify/App/issues/48675 for more details
@@ -46,37 +42,19 @@ function SelectionListWithModal<TItem extends ListItem>({
4642
const isFocused = useIsFocused();
4743
const icons = useMemoizedLazyExpensifyIcons(['CheckSquare']);
4844

49-
// Filter out the pending delete item without errors when online to prevent making multiple updates to debouncedData which causes the deleted item is shown again
50-
const filteredData = useMemo(() => {
51-
return data.filter((item) => item.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || isOffline || !isEmptyValueObject(item?.errors));
52-
}, [data, isOffline]);
53-
5445
const isMobileSelectionModeEnabled = useMobileSelectionMode();
5546

56-
// Debounce the data prop to prevent rapid updates that cause FlashList layout errors
57-
// This gives FlashList time to properly update its layout cache when searching/filtering
58-
const [, debouncedData, setDataState] = useDebouncedState<TItem[]>(filteredData, CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
59-
60-
// Determine if this is changed by filtering (to limit multiple rerenders)
61-
const isFiltering = filteredData.length < debouncedData.length;
62-
63-
useEffect(() => {
64-
setDataState(filteredData);
65-
}, [filteredData, setDataState]);
66-
67-
const displayData = isFiltering ? debouncedData : filteredData;
68-
6947
const selectedItems = useMemo(
7048
() =>
7149
selectedItemsProp ??
72-
displayData.filter((item) => {
50+
data.filter((item) => {
7351
if (isSelected) {
7452
return isSelected(item);
7553
}
7654
return !!item.isSelected;
7755
}) ??
7856
[],
79-
[isSelected, displayData, selectedItemsProp],
57+
[isSelected, data, selectedItemsProp],
8058
);
8159

8260
useHandleSelectionMode(selectedItems);
@@ -116,7 +94,7 @@ function SelectionListWithModal<TItem extends ListItem>({
11694
<>
11795
<SelectionList
11896
ref={ref}
119-
data={displayData}
97+
data={data}
12098
addBottomSafeAreaPadding
12199
selectedItems={selectedItemsProp}
122100
onLongPressRow={handleLongPressRow}

src/hooks/useSearchResults.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
import {useDeferredValue, useEffect, useState} from 'react';
1+
import {useEffect} from 'react';
22
import CONST from '@src/CONST';
3+
import useDebouncedState from './useDebouncedState';
34
import usePrevious from './usePrevious';
45

56
const stableSortDataDefault = <TValue>(data: TValue[]) => data;
67

78
/**
89
* This hook filters (and optionally sorts) a dataset based on a search parameter.
9-
* It utilizes `useDeferredValue` to allow the searchQuery to change rapidly, while more expensive renders that occur using
10-
* the result of the filtering and sorting are de-prioritized, allowing them to happen in the background.
10+
* The search input updates immediately for instant UI feedback, while the value used for
11+
* filtering is debounced so expensive filter/sort work runs at most once per debounce window.
1112
*
1213
* @param data - The dataset to filter and sort.
1314
* @param filterData - Predicate that decides whether a datum matches the current search input.
@@ -20,11 +21,10 @@ function useSearchResults<TValue>(
2021
sortData: (data: TValue[]) => TValue[] = stableSortDataDefault,
2122
preFilter?: (datum: TValue) => boolean,
2223
) {
23-
const [inputValue, setInputValue] = useState('');
24-
const deferredInput = useDeferredValue(inputValue);
24+
const [inputValue, debouncedInput, setInputValue] = useDebouncedState('', CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
2525
const prevData = usePrevious(data);
2626

27-
const searchQuery = inputValue.trim().length ? deferredInput : '';
27+
const searchQuery = inputValue.trim().length ? debouncedInput : '';
2828

2929
const base = preFilter ? data.filter(preFilter) : data;
3030
const normalizedSearchQuery = searchQuery.trim().toLowerCase();
@@ -36,7 +36,7 @@ function useSearchResults<TValue>(
3636
return;
3737
}
3838
setInputValue('');
39-
}, [data.length, prevData.length]);
39+
}, [data.length, prevData.length, setInputValue]);
4040

4141
return [inputValue, setInputValue, result] as const;
4242
}

src/pages/ReportParticipantsPage.tsx

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {useIsFocused} from '@react-navigation/native';
2-
import React, {useEffect, useRef, useState} from 'react';
2+
import React, {useEffect, useRef} from 'react';
33
// eslint-disable-next-line no-restricted-imports
44
import {InteractionManager, View} from 'react-native';
55
import type {TupleToUnion, ValueOf} from 'type-fest';
@@ -27,6 +27,7 @@ import useReportAttributes from '@hooks/useReportAttributes';
2727
import useReportIsArchived from '@hooks/useReportIsArchived';
2828
import useResponsiveLayout from '@hooks/useResponsiveLayout';
2929
import useSearchBackPress from '@hooks/useSearchBackPress';
30+
import useSearchResults from '@hooks/useSearchResults';
3031
import useStyleUtils from '@hooks/useStyleUtils';
3132
import useThemeStyles from '@hooks/useThemeStyles';
3233
import {turnOffMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
@@ -90,7 +91,6 @@ function ReportParticipantsPage({report, route}: ReportParticipantsPageProps) {
9091
const isFocused = useIsFocused();
9192
const {isOffline} = useNetwork();
9293
const canSelectMultiple = isGroupChat && isCurrentUserAdmin && (isSmallScreenWidth ? isMobileSelectionModeEnabled : true);
93-
const [searchValue, setSearchValue] = useState('');
9494

9595
const {personalDetailsParticipants, participantsForDisplay} = getReportPersonalDetailsParticipants(report, personalDetails, reportMetadata);
9696
const participantsForDisplayMap = participantsForDisplay.reduce<Record<number, TupleToUnion<typeof participantsForDisplay>>>((acc, participant) => {
@@ -107,6 +107,13 @@ function ReportParticipantsPage({report, route}: ReportParticipantsPageProps) {
107107

108108
const [selectedMembers, setSelectedMembers] = useFilteredSelection(personalDetailsParticipants, filterParticipants);
109109

110+
const [searchValue, setSearchValue, searchFilteredParticipants] = useSearchResults(
111+
participantsForDisplay,
112+
(participant, search) => isSearchStringMatchUserDetails(participant.details, search),
113+
undefined,
114+
(participant) => isOffline || !participant.isPendingDelete,
115+
);
116+
110117
// Get the active chat members by filtering out the pending members with delete action
111118
const activeParticipants = participantsForDisplay.filter((participant) => isOffline || !participant.isPendingDelete);
112119

@@ -215,16 +222,8 @@ function ReportParticipantsPage({report, route}: ReportParticipantsPageProps) {
215222

216223
// Build participants list
217224
let participants: MemberOption[] = [];
218-
for (const participantForDisplay of participantsForDisplay) {
219-
const {accountID, details, isDisabled, isPendingDelete, pendingAction, role} = participantForDisplay;
220-
221-
if (searchValue.trim() && !isSearchStringMatchUserDetails(details, searchValue)) {
222-
continue;
223-
}
224-
225-
if (!isOffline && isPendingDelete) {
226-
continue;
227-
}
225+
for (const participantForDisplay of searchFilteredParticipants) {
226+
const {accountID, details, isDisabled, pendingAction, role} = participantForDisplay;
228227

229228
const isAdmin = role === CONST.REPORT.ROLE.ADMIN;
230229

src/pages/RoomMembersPage.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import useReportAttributes from '@hooks/useReportAttributes';
2727
import useReportIsArchived from '@hooks/useReportIsArchived';
2828
import useResponsiveLayout from '@hooks/useResponsiveLayout';
2929
import useSearchBackPress from '@hooks/useSearchBackPress';
30+
import useSearchResults from '@hooks/useSearchResults';
3031
import useThemeStyles from '@hooks/useThemeStyles';
3132
import {turnOffMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
3233
import {clearUserSearchPhrase, updateUserSearchPhrase} from '@libs/actions/RoomMembersUserSearchPhrase';
@@ -75,7 +76,6 @@ function RoomMembersPage({report, policy}: RoomMembersPageProps) {
7576
const {formatPhoneNumber, translate, localeCompare} = useLocalize();
7677
const {showConfirmModal} = useConfirmModal();
7778
const [userSearchPhrase] = useOnyx(ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE);
78-
const [searchValue, setSearchValue] = useState('');
7979
const [didLoadRoomMembers, setDidLoadRoomMembers] = useState(false);
8080
const personalDetails = usePersonalDetails();
8181
const isPolicyExpenseChat = useMemo(() => isPolicyExpenseChatUtils(report), [report]);
@@ -88,6 +88,11 @@ function RoomMembersPage({report, policy}: RoomMembersPageProps) {
8888
[report, personalDetails, reportMetadata],
8989
);
9090

91+
const [searchValue, setSearchValue, searchFilteredAccountIDs] = useSearchResults(participants, (accountID, search) => {
92+
const details = personalDetails?.[accountID];
93+
return !!details && isSearchStringMatchUserDetails(details, search);
94+
});
95+
9196
const shouldIncludeMember = useCallback(
9297
(participant?: PersonalDetails) => {
9398
if (!participant) {
@@ -249,7 +254,7 @@ function RoomMembersPage({report, policy}: RoomMembersPageProps) {
249254
return;
250255
}
251256
setSearchValue(userSearchPhrase ?? '');
252-
}, [isFocusedScreen, shouldShowTextInput, userSearchPhrase]);
257+
}, [isFocusedScreen, setSearchValue, shouldShowTextInput, userSearchPhrase]);
253258

254259
useEffect(() => {
255260
updateUserSearchPhrase(searchValue);
@@ -278,11 +283,9 @@ function RoomMembersPage({report, policy}: RoomMembersPageProps) {
278283
const data = useMemo((): ListItem[] => {
279284
let result: ListItem[] = [];
280285

281-
for (const accountID of participants) {
286+
for (const accountID of searchFilteredAccountIDs) {
282287
const details = personalDetails?.[accountID];
283-
284-
// If search value is provided, filter out members that don't match the search value
285-
if (!details || (searchValue.trim() && !isSearchStringMatchUserDetails(details, searchValue))) {
288+
if (!details) {
286289
continue;
287290
}
288291
const pendingChatMember = reportMetadata?.pendingChatMembers?.findLast((member) => member.accountID === accountID.toString());
@@ -322,12 +325,11 @@ function RoomMembersPage({report, policy}: RoomMembersPageProps) {
322325
formatPhoneNumber,
323326
localeCompare,
324327
isPolicyExpenseChat,
325-
participants,
328+
searchFilteredAccountIDs,
326329
personalDetails,
327330
policy,
328331
report.ownerAccountID,
329332
reportMetadata?.pendingChatMembers,
330-
searchValue,
331333
selectedMembers,
332334
session?.accountID,
333335
icons.FallbackAvatar,
@@ -419,7 +421,7 @@ function RoomMembersPage({report, policy}: RoomMembersPageProps) {
419421
onChangeText: setSearchValue,
420422
headerMessage: searchValue.trim() && !data.length ? `${translate('roomMembersPage.memberNotFound')} ${translate('roomMembersPage.useInviteButton')}` : '',
421423
}),
422-
[data.length, searchValue, translate],
424+
[data.length, searchValue, setSearchValue, translate],
423425
);
424426

425427
let subtitleKey: '' | TranslationPaths | undefined;
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import {act, renderHook} from '@testing-library/react-native';
2+
import useSearchResults from '@hooks/useSearchResults';
3+
import CONST from '@src/CONST';
4+
5+
type Item = {id: number; name: string};
6+
7+
const buildItems = (count: number, prefix = 'item'): Item[] => Array.from({length: count}, (_, i) => ({id: i, name: `${prefix}-${i}`}));
8+
9+
const filterByName = (item: Item, query: string) => item.name.toLowerCase().includes(query);
10+
11+
describe('useSearchResults', () => {
12+
beforeEach(() => {
13+
jest.useFakeTimers();
14+
});
15+
16+
afterEach(() => {
17+
jest.clearAllTimers();
18+
jest.useRealTimers();
19+
});
20+
21+
it('returns the full list when the input is empty', () => {
22+
const data = buildItems(5);
23+
const {result} = renderHook(() => useSearchResults(data, filterByName));
24+
25+
expect(result.current[0]).toBe('');
26+
expect(result.current[2]).toEqual(data);
27+
});
28+
29+
it('updates the input value immediately but defers filtering until the debounce settles', () => {
30+
const data = buildItems(5);
31+
const {result} = renderHook(() => useSearchResults(data, filterByName));
32+
33+
act(() => {
34+
result.current[1]('item-2');
35+
});
36+
37+
// The typed value is visible right away but the filtered result still reflects the full list.
38+
expect(result.current[0]).toBe('item-2');
39+
expect(result.current[2]).toEqual(data);
40+
41+
act(() => {
42+
jest.advanceTimersByTime(CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
43+
});
44+
45+
expect(result.current[2]).toEqual([{id: 2, name: 'item-2'}]);
46+
});
47+
48+
it('restores the full list instantly when the input is cleared, without waiting for the debounce', () => {
49+
const data = buildItems(5);
50+
const {result} = renderHook(() => useSearchResults(data, filterByName));
51+
52+
act(() => {
53+
result.current[1]('item-2');
54+
jest.advanceTimersByTime(CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
55+
});
56+
expect(result.current[2]).toHaveLength(1);
57+
58+
act(() => {
59+
result.current[1]('');
60+
});
61+
expect(result.current[2]).toEqual(data);
62+
});
63+
64+
it('applies the sort comparator to the filtered results', () => {
65+
const data: Item[] = [
66+
{id: 1, name: 'banana'},
67+
{id: 2, name: 'apple'},
68+
{id: 3, name: 'cherry'},
69+
];
70+
const sortAlpha = (items: Item[]) => [...items].sort((a, b) => a.name.localeCompare(b.name));
71+
const {result} = renderHook(() => useSearchResults(data, filterByName, sortAlpha));
72+
73+
expect(result.current[2].map((i) => i.name)).toEqual(['apple', 'banana', 'cherry']);
74+
});
75+
76+
it('applies the preFilter before the text filter', () => {
77+
const data: Item[] = [
78+
{id: 1, name: 'apple-good'},
79+
{id: 2, name: 'apple-bad'},
80+
{id: 3, name: 'banana-good'},
81+
];
82+
const preFilter = (item: Item) => item.name.endsWith('-good');
83+
const {result} = renderHook(() => useSearchResults(data, filterByName, undefined, preFilter));
84+
85+
expect(result.current[2].map((i) => i.id)).toEqual([1, 3]);
86+
87+
act(() => {
88+
result.current[1]('apple');
89+
jest.advanceTimersByTime(CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
90+
});
91+
expect(result.current[2].map((i) => i.id)).toEqual([1]);
92+
});
93+
94+
describe('auto-clear when data crosses SEARCH_ITEM_LIMIT', () => {
95+
const aboveLimit = CONST.SEARCH_ITEM_LIMIT + 5;
96+
const belowLimit = CONST.SEARCH_ITEM_LIMIT - 5;
97+
98+
it('clears the input when the dataset shrinks from above the limit to at or below the limit', () => {
99+
const {result, rerender} = renderHook(({data}) => useSearchResults(data, filterByName), {
100+
initialProps: {data: buildItems(aboveLimit)},
101+
});
102+
103+
act(() => {
104+
result.current[1]('item');
105+
jest.advanceTimersByTime(CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
106+
});
107+
expect(result.current[0]).toBe('item');
108+
109+
rerender({data: buildItems(belowLimit)});
110+
111+
// The search bar is no longer shown for small datasets, so any leftover query must be wiped.
112+
expect(result.current[0]).toBe('');
113+
});
114+
115+
it('does not clear the input when the dataset stays above the limit', () => {
116+
const {result, rerender} = renderHook(({data}) => useSearchResults(data, filterByName), {
117+
initialProps: {data: buildItems(aboveLimit)},
118+
});
119+
120+
act(() => {
121+
result.current[1]('item');
122+
jest.advanceTimersByTime(CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
123+
});
124+
125+
rerender({data: buildItems(aboveLimit + 1)});
126+
127+
expect(result.current[0]).toBe('item');
128+
});
129+
130+
it('does not clear the input when the dataset stays at or below the limit', () => {
131+
const {result, rerender} = renderHook(({data}) => useSearchResults(data, filterByName), {
132+
initialProps: {data: buildItems(belowLimit)},
133+
});
134+
135+
act(() => {
136+
result.current[1]('item');
137+
jest.advanceTimersByTime(CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
138+
});
139+
140+
rerender({data: buildItems(belowLimit - 1)});
141+
142+
expect(result.current[0]).toBe('item');
143+
});
144+
});
145+
});

0 commit comments

Comments
 (0)