Merge pull request Expensify#89525 from software-mansion-labs/borys3kk-fix-only-first-expense-is-highlighted

Reports - Expense created while on "Reports", gets highlighted again when opened.

Author: mountiny

src/components/Search/index.tsx

@@ -450,7 +450,7 @@ function Search({
         // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [isSmallScreenWidth]);
 
-    const {newSearchResultKeys, handleSelectionListScroll, newTransactions} = useSearchHighlightAndScroll({
+    const {newSearchResultKeys, handleSelectionListScroll, newTransactions, hasQueuedHighlights} = useSearchHighlightAndScroll({
         searchResults,
         transactions,
         previousTransactions,
@@ -463,6 +463,16 @@ function Search({
         shouldUseLiveData,
     });
 
+    // Mirror `hasQueuedHighlights` into a ref so the post-create-flow `useFocusEffect`
+    // (which has empty deps) can read the latest value without re-creating its callback.
+    // Used to skip the deferral that would otherwise hide the freshly-added row from
+    // FlashList during the RHP dismiss transition, which would prevent the highlight
+    // animation from ever firing on it.
+    const hasQueuedHighlightsRef = useRef(hasQueuedHighlights);
+    useEffect(() => {
+        hasQueuedHighlightsRef.current = hasQueuedHighlights;
+    }, [hasQueuedHighlights]);
+
     // 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
     // we also need to check that the searchResults matches the type and status of the current search
     const isDataLoaded = shouldUseLiveData || isSearchDataLoaded(searchResults, queryJSON);
@@ -518,6 +528,14 @@ function Search({
                 return;
             }
 
+            // If the highlight hook already queued rows for the post-create animation,
+            // skip the skeleton-during-transition defer. Otherwise FlashList stays empty
+            // for ~1s while the RHP dismiss transition runs, the row never mounts inside
+            // the 300ms highlight window, and `useAnimatedHighlightStyle` never fires.
+            if (hasQueuedHighlightsRef.current) {
+                return;
+            }
+
             // Show skeleton while the RHP dismiss animation plays. The transition
             // hasn't started yet when useFocusEffect fires (it begins after paint),
             // so waitForUpcomingTransition defers until the animation actually ends.

src/hooks/useSearchHighlightAndScroll.ts

@@ -14,7 +14,6 @@ import type {SearchKey} from '@libs/SearchUIUtils';
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import type {ReportActions, SearchResults, Transaction} from '@src/types/onyx';
-import {isEmptyObject} from '@src/types/utils/EmptyObject';
 import useNetwork from './useNetwork';
 import useOnyx from './useOnyx';
 import usePrevious from './usePrevious';
@@ -227,40 +226,23 @@ function useSearchHighlightAndScroll({
             }
 
             const newKeys = new Set<string>();
+            const consumedManualIDs: string[] = [];
             for (const id of newTransactionIDs) {
                 const newTransactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${id}`;
                 highlightedIDs.current.add(newTransactionKey);
                 newKeys.add(newTransactionKey);
+                if (manualHighlightTransactionIDs.has(id)) {
+                    consumedManualIDs.push(id);
+                }
             }
             setNewSearchResultKeys(newKeys);
-        }
-    }, [searchResults?.data, previousSearchResults, isChat, transactionIDsToHighlight]);
 
-    // Reset transactionIDsToHighlight after they have been highlighted
-    useEffect(() => {
-        if (isEmptyObject(transactionIDsToHighlight) || newSearchResultKeys === null) {
-            return;
+            // Clear consumed manual highlight flags so subsequent detect runs don't re-highlight the same IDs.
+            if (consumedManualIDs.length > 0) {
+                mergeTransactionIdsHighlightOnSearchRoute(queryJSON.type, Object.fromEntries(consumedManualIDs.map((id) => [id, false])));
+            }
         }
-
-        const highlightedTransactionIDs = Object.keys(transactionIDsToHighlight).filter(
-            (id) => transactionIDsToHighlight[id] && newSearchResultKeys?.has(`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`),
-        );
-
-        // We need to use requestAnimationFrame here to ensure that setTimeout actually starts
-        // only after the user has navigated to the "Reports > Expenses" page.
-        // Otherwise, there is still a chance we might miss the timing because setTimeout runs too early,
-        // causing the highlight not to appear.
-        let timer: NodeJS.Timeout;
-        const animation = requestAnimationFrame(() => {
-            timer = setTimeout(() => {
-                mergeTransactionIdsHighlightOnSearchRoute(queryJSON.type, Object.fromEntries(highlightedTransactionIDs.map((id) => [id, false])));
-            }, CONST.ANIMATED_HIGHLIGHT_START_DURATION);
-        });
-        return () => {
-            clearTimeout(timer);
-            cancelAnimationFrame(animation);
-        };
-    }, [transactionIDsToHighlight, queryJSON.type, newSearchResultKeys]);
+    }, [searchResults?.data, previousSearchResults, isChat, transactionIDsToHighlight, queryJSON.type]);
 
     // Remove transactionIDsToHighlight when the user leaves the current search type
     useEffect(
@@ -329,7 +311,9 @@ function useSearchHighlightAndScroll({
         triggeredByHookRef.current = false;
     };
 
-    return {newSearchResultKeys, handleSelectionListScroll, newTransactions};
+    const hasQueuedHighlights = newSearchResultKeys !== null && newSearchResultKeys.size > 0;
+
+    return {newSearchResultKeys, handleSelectionListScroll, newTransactions, hasQueuedHighlights};
 }
 
 /**

tests/unit/useSearchHighlightAndScrollTest.ts

@@ -332,12 +332,6 @@ describe('useSearchHighlightAndScroll', () => {
         expect(result.current.newSearchResultKeys?.size).toBe(1);
         expect([...(result.current.newSearchResultKeys ?? new Set())]).toContain('transactions_3');
 
-        // Wait 1s for the timer in useSearchHighlightAndScroll to complete.
-        await new Promise((resolve) => {
-            setTimeout(resolve, 1000);
-        });
-
-        expect(spyOnMergeTransactionIdsHighlightOnSearchRoute).toHaveBeenCalledTimes(1);
         expect(spyOnMergeTransactionIdsHighlightOnSearchRoute).toHaveBeenCalledWith(baseProps.queryJSON.type, {'3': false});
     });