Skip to content

Commit b407543

Browse files
arosiclairOSBotify
authored andcommitted
Merge pull request #61348 from Expensify/revert-60896-optimize-search-calls
Revert "Optimize /Search call in useSearchHighlightAndScroll hook" (cherry picked from commit da18c2c) (cherry-picked to staging by AndrewGable)
1 parent 960fb26 commit b407543

6 files changed

Lines changed: 240 additions & 394 deletions

File tree

src/components/Search/SearchList.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {PressableWithFeedback} from '@components/Pressable';
1414
import type ChatListItem from '@components/SelectionList/ChatListItem';
1515
import type ReportListItem from '@components/SelectionList/Search/ReportListItem';
1616
import type TransactionListItem from '@components/SelectionList/Search/TransactionListItem';
17-
import type {ExtendedTargetedEvent, ReportListItemType, SearchListItem} from '@components/SelectionList/types';
17+
import type {ExtendedTargetedEvent, ReportActionListItemType, ReportListItemType, TransactionListItemType} from '@components/SelectionList/types';
1818
import Text from '@components/Text';
1919
import useArrowKeyFocusManager from '@hooks/useArrowKeyFocusManager';
2020
import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
@@ -31,6 +31,7 @@ import variables from '@styles/variables';
3131
import CONST from '@src/CONST';
3232
import ONYXKEYS from '@src/ONYXKEYS';
3333

34+
type SearchListItem = TransactionListItemType | ReportListItemType | ReportActionListItemType;
3435
type SearchListItemComponentType = typeof TransactionListItem | typeof ChatListItem | typeof ReportListItem;
3536

3637
type SearchListHandle = {

src/components/Search/index.tsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import usePermissions from '@hooks/usePermissions';
1515
import usePrevious from '@hooks/usePrevious';
1616
import useResponsiveLayout from '@hooks/useResponsiveLayout';
1717
import useSearchHighlightAndScroll from '@hooks/useSearchHighlightAndScroll';
18-
import useSearchPusherUpdates from '@hooks/useSearchPusherUpdates';
1918
import useThemeStyles from '@hooks/useThemeStyles';
2019
import {turnOffMobileSelectionMode, turnOnMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
2120
import {search, updateSearchResultsWithTransactionThreadReportID} from '@libs/actions/Search';
@@ -152,7 +151,9 @@ function Search({queryJSON, currentSearchResults, lastNonEmptySearchResults, onS
152151
const {type, status, sortBy, sortOrder, hash, groupBy} = queryJSON;
153152

154153
const [transactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, {canBeMissing: true});
154+
const previousTransactions = usePrevious(transactions);
155155
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {canBeMissing: true});
156+
const previousReportActions = usePrevious(reportActions);
156157
const {translate} = useLocalize();
157158
const shouldGroupByReports = groupBy === CONST.SEARCH.GROUP_BY.REPORTS;
158159

@@ -199,17 +200,14 @@ function Search({queryJSON, currentSearchResults, lastNonEmptySearchResults, onS
199200
search({queryJSON, offset});
200201
}, [isOffline, offset, queryJSON]);
201202

202-
// Use custom hook to detect and handle Pusher updates
203-
useSearchPusherUpdates({
204-
isOffline,
205-
queryJSON,
206-
transactions,
207-
reportActions,
208-
});
209-
210203
const {newSearchResultKey, handleSelectionListScroll} = useSearchHighlightAndScroll({
211204
searchResults,
205+
transactions,
206+
previousTransactions,
212207
queryJSON,
208+
offset,
209+
reportActions,
210+
previousReportActions,
213211
});
214212

215213
// There's a race condition in Onyx which makes it return data from the previous Search, so in addition to checking that the data is loaded

src/components/SelectionList/types.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -726,8 +726,6 @@ type SectionListDataType<TItem extends ListItem> = ExtendedSectionListData<TItem
726726

727727
type SortableColumnName = SearchColumnType | typeof CONST.REPORT.TRANSACTION_LIST.COLUMNS.COMMENTS;
728728

729-
type SearchListItem = TransactionListItemType | ReportListItemType | ReportActionListItemType;
730-
731729
export type {
732730
BaseListItemProps,
733731
SelectionListProps,
@@ -750,6 +748,5 @@ export type {
750748
UserListItemProps,
751749
ReportActionListItemType,
752750
ChatListItemProps,
753-
SearchListItem,
754751
SortableColumnName,
755752
};

src/hooks/useSearchHighlightAndScroll.ts

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,80 @@
1+
import isEqual from 'lodash/isEqual';
12
import {useCallback, useEffect, useRef, useState} from 'react';
2-
import type {OnyxEntry} from 'react-native-onyx';
3+
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
34
import type {SearchQueryJSON} from '@components/Search/types';
4-
import type {ReportListItemType, SearchListItem, SelectionListHandle, TransactionListItemType} from '@components/SelectionList/types';
5+
import type {ReportActionListItemType, ReportListItemType, SelectionListHandle, TransactionListItemType} from '@components/SelectionList/types';
6+
import {search} from '@libs/actions/Search';
57
import {isReportActionEntry} from '@libs/SearchUIUtils';
68
import CONST from '@src/CONST';
79
import ONYXKEYS from '@src/ONYXKEYS';
8-
import type {SearchResults} from '@src/types/onyx';
10+
import type {ReportActions, SearchResults, Transaction} from '@src/types/onyx';
911
import usePrevious from './usePrevious';
1012

1113
type UseSearchHighlightAndScroll = {
1214
searchResults: OnyxEntry<SearchResults>;
15+
transactions: OnyxCollection<Transaction>;
16+
previousTransactions: OnyxCollection<Transaction>;
17+
reportActions: OnyxCollection<ReportActions>;
18+
previousReportActions: OnyxCollection<ReportActions>;
1319
queryJSON: SearchQueryJSON;
20+
offset: number;
1421
};
1522

1623
/**
17-
* Hook used to handle highlighting and scrolling for new search results.
24+
* Hook used to trigger a search when a new transaction or report action is added and handle highlighting and scrolling.
1825
*/
19-
function useSearchHighlightAndScroll({searchResults, queryJSON}: UseSearchHighlightAndScroll) {
26+
function useSearchHighlightAndScroll({searchResults, transactions, previousTransactions, reportActions, previousReportActions, queryJSON, offset}: UseSearchHighlightAndScroll) {
27+
// Ref to track if the search was triggered by this hook
28+
const triggeredByHookRef = useRef(false);
29+
const searchTriggeredRef = useRef(false);
30+
const hasNewItemsRef = useRef(false);
31+
const previousSearchResults = usePrevious(searchResults?.data);
2032
const [newSearchResultKey, setNewSearchResultKey] = useState<string | null>(null);
2133
const highlightedIDs = useRef<Set<string>>(new Set());
2234
const initializedRef = useRef(false);
23-
const previousSearchResults = usePrevious(searchResults?.data);
2435
const isChat = queryJSON.type === CONST.SEARCH.DATA_TYPES.CHAT;
2536

37+
// Trigger search when a new report action is added while on chat or when a new transaction is added for the other search types.
38+
useEffect(() => {
39+
const previousTransactionsIDs = Object.keys(previousTransactions ?? {});
40+
const transactionsIDs = Object.keys(transactions ?? {});
41+
42+
const reportActionsIDs = Object.values(reportActions ?? {})
43+
.map((actions) => Object.keys(actions ?? {}))
44+
.flat();
45+
const previousReportActionsIDs = Object.values(previousReportActions ?? {})
46+
.map((actions) => Object.keys(actions ?? {}))
47+
.flat();
48+
49+
if (searchTriggeredRef.current) {
50+
return;
51+
}
52+
const hasTransactionsIDsChange = !isEqual(transactionsIDs, previousTransactionsIDs);
53+
const hasReportActionsIDsChange = !isEqual(reportActionsIDs, previousReportActionsIDs);
54+
55+
// Check if there is a change in the transactions or report actions list
56+
if ((!isChat && hasTransactionsIDsChange) || hasReportActionsIDsChange) {
57+
// We only want to highlight new items if the addition of transactions or report actions triggered the search.
58+
// This is because, on deletion of items, the backend sometimes returns old items in place of the deleted ones.
59+
// We don't want to highlight these old items, even if they appear new in the current search results.
60+
hasNewItemsRef.current = isChat ? reportActionsIDs.length > previousReportActionsIDs.length : transactionsIDs.length > previousTransactionsIDs.length;
61+
62+
// Set the flag indicating the search is triggered by the hook
63+
triggeredByHookRef.current = true;
64+
65+
// Trigger the search
66+
search({queryJSON, offset});
67+
68+
// Set the ref to prevent further triggers until reset
69+
searchTriggeredRef.current = true;
70+
}
71+
72+
// Reset the ref when transactions or report actions in chat search type are updated
73+
return () => {
74+
searchTriggeredRef.current = false;
75+
};
76+
}, [transactions, previousTransactions, queryJSON, offset, reportActions, previousReportActions, isChat]);
77+
2678
// Initialize the set with existing IDs only once
2779
useEffect(() => {
2880
if (initializedRef.current || !searchResults?.data) {
@@ -34,7 +86,7 @@ function useSearchHighlightAndScroll({searchResults, queryJSON}: UseSearchHighli
3486
initializedRef.current = true;
3587
}, [searchResults?.data, isChat]);
3688

37-
// Detect new items in search results after a change in transactions or report actions
89+
// Detect new items (transactions or report actions)
3890
useEffect(() => {
3991
if (!previousSearchResults || !searchResults?.data) {
4092
return;
@@ -46,7 +98,7 @@ function useSearchHighlightAndScroll({searchResults, queryJSON}: UseSearchHighli
4698
// Find new report action IDs that are not in the previousReportActionIDs and not already highlighted
4799
const newReportActionIDs = currentReportActionIDs.filter((id) => !previousReportActionIDs.includes(id) && !highlightedIDs.current.has(id));
48100

49-
if (newReportActionIDs.length === 0) {
101+
if (!triggeredByHookRef.current || newReportActionIDs.length === 0 || !hasNewItemsRef.current) {
50102
return;
51103
}
52104

@@ -55,25 +107,23 @@ function useSearchHighlightAndScroll({searchResults, queryJSON}: UseSearchHighli
55107

56108
setNewSearchResultKey(newReportActionKey);
57109
highlightedIDs.current.add(newReportActionID);
58-
return;
59-
}
60-
61-
// For expenses/transactions
62-
const previousTransactionIDs = extractTransactionIDsFromSearchResults(previousSearchResults);
63-
const currentTransactionIDs = extractTransactionIDsFromSearchResults(searchResults.data);
110+
} else {
111+
const previousTransactionIDs = extractTransactionIDsFromSearchResults(previousSearchResults);
112+
const currentTransactionIDs = extractTransactionIDsFromSearchResults(searchResults.data);
64113

65-
// Find new transaction IDs not in previous search results and not already highlighted
66-
const newTransactionIDs = currentTransactionIDs.filter((id) => !previousTransactionIDs.includes(id) && !highlightedIDs.current.has(id));
114+
// Find new transaction IDs that are not in the previousTransactionIDs and not already highlighted
115+
const newTransactionIDs = currentTransactionIDs.filter((id) => !previousTransactionIDs.includes(id) && !highlightedIDs.current.has(id));
67116

68-
if (newTransactionIDs.length === 0) {
69-
return;
70-
}
117+
if (!triggeredByHookRef.current || newTransactionIDs.length === 0 || !hasNewItemsRef.current) {
118+
return;
119+
}
71120

72-
const newTransactionID = newTransactionIDs.at(0) ?? '';
73-
const newTransactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${newTransactionID}`;
121+
const newTransactionID = newTransactionIDs.at(0) ?? '';
122+
const newTransactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${newTransactionID}`;
74123

75-
setNewSearchResultKey(newTransactionKey);
76-
highlightedIDs.current.add(newTransactionID);
124+
setNewSearchResultKey(newTransactionKey);
125+
highlightedIDs.current.add(newTransactionID);
126+
}
77127
}, [searchResults?.data, previousSearchResults, isChat]);
78128

79129
// Reset newSearchResultKey after it's been used
@@ -93,10 +143,10 @@ function useSearchHighlightAndScroll({searchResults, queryJSON}: UseSearchHighli
93143
* Callback to handle scrolling to the new search result.
94144
*/
95145
const handleSelectionListScroll = useCallback(
96-
(data: SearchListItem[]) => (ref: SelectionListHandle | null) => {
146+
(data: Array<TransactionListItemType | ReportActionListItemType | ReportListItemType>) => (ref: SelectionListHandle | null) => {
97147
// Early return if there's no ref, new transaction wasn't brought in by this hook
98148
// or there's no new search result key
99-
if (!ref || newSearchResultKey === null) {
149+
if (!ref || !triggeredByHookRef.current || newSearchResultKey === null) {
100150
return;
101151
}
102152

@@ -131,6 +181,8 @@ function useSearchHighlightAndScroll({searchResults, queryJSON}: UseSearchHighli
131181

132182
// Perform the scrolling action
133183
ref.scrollToIndex(indexOfNewItem);
184+
// Reset the trigger flag to prevent unintended future scrolls and highlights
185+
triggeredByHookRef.current = false;
134186
},
135187
[newSearchResultKey, isChat],
136188
);

src/hooks/useSearchPusherUpdates.ts

Lines changed: 0 additions & 71 deletions
This file was deleted.

0 commit comments

Comments
 (0)