Merge pull request Expensify#83917 from callstack-internal/callstack-internal/szymonzalarski/search/move-skeleton-to-top-level-search-page

Refactor search functionality to improve loading state handling

Author: luacmartins

src/components/Search/SearchLoadingSkeleton.tsx

@@ -0,0 +1,36 @@
+import React from 'react';
+import type {StyleProp, ViewStyle} from 'react-native';
+import Animated, {FadeIn, FadeOut} from 'react-native-reanimated';
+import SearchRowSkeleton from '@components/Skeletons/SearchRowSkeleton';
+import useThemeStyles from '@hooks/useThemeStyles';
+import {endSpanWithAttributes} from '@libs/telemetry/activeSpans';
+import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan';
+import CONST from '@src/CONST';
+
+type SearchLoadingSkeletonProps = {
+    containerStyle?: StyleProp<ViewStyle>;
+    reasonAttributes?: SkeletonSpanReasonAttributes;
+};
+
+function SearchLoadingSkeleton({containerStyle, reasonAttributes}: SearchLoadingSkeletonProps) {
+    const styles = useThemeStyles();
+
+    return (
+        <Animated.View
+            entering={FadeIn.duration(CONST.SEARCH.ANIMATION.FADE_DURATION)}
+            exiting={FadeOut.duration(CONST.SEARCH.ANIMATION.FADE_DURATION)}
+            style={[styles.flex1]}
+            onLayout={() => {
+                endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: false});
+            }}
+        >
+            <SearchRowSkeleton
+                shouldAnimate
+                containerStyle={containerStyle}
+                reasonAttributes={reasonAttributes}
+            />
+        </Animated.View>
+    );
+}
+
+export default SearchLoadingSkeleton;

src/components/Search/index.tsx

@@ -4,7 +4,7 @@ import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import type {NativeScrollEvent, NativeSyntheticEvent, StyleProp, ViewStyle} from 'react-native';
 import {View} from 'react-native';
 import type {OnyxEntry} from 'react-native-onyx';
-import Animated, {FadeIn, FadeOut, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
+import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
 import FullPageErrorView from '@components/BlockingViews/FullPageErrorView';
 import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
 import {ModalActions} from '@components/Modal/Global/ModalContext';
@@ -36,7 +36,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
 import {openOldDotLink} from '@libs/actions/Link';
 import {turnOffMobileSelectionMode, turnOnMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
 import type {TransactionPreviewData} from '@libs/actions/Search';
-import {openSearch, setOptimisticDataForTransactionThreadPreview} from '@libs/actions/Search';
+import {setOptimisticDataForTransactionThreadPreview} from '@libs/actions/Search';
 import {canUseTouchScreen} from '@libs/DeviceCapabilities';
 import Log from '@libs/Log';
 import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
@@ -51,6 +51,7 @@ import {
     getListItem,
     getSections,
     getSortedSections,
+    getValidGroupBy,
     getWideAmountIndicators,
     isGroupedItemArray,
     isReportActionListItemType,
@@ -229,7 +230,6 @@ function Search({
     const navigation = useNavigation<PlatformStackNavigationProp<SearchFullscreenNavigatorParamList>>();
     const isFocused = useIsFocused();
     const {markReportIDAsExpense} = useWideRHPActions();
-
     const {
         currentSearchHash,
         currentSearchKey,
@@ -241,7 +241,6 @@ function Search({
         shouldUseLiveData,
         suggestedSearches,
     } = useSearchStateContext();
-
     const {setSelectedTransactions, clearSelectedTransactions, setShouldShowFiltersBarLoading, setShouldShowSelectAllMatchingItems, selectAllMatchingItems, setShouldResetSearchQuery} =
         useSearchActionsContext();
     const [offset, setOffset] = useState(0);
@@ -306,7 +305,7 @@ function Search({
         }
     }, [onDEWModalOpen, showConfirmModal, translate]);
 
-    const validGroupBy = groupBy && Object.values(CONST.SEARCH.GROUP_BY).includes(groupBy) ? groupBy : undefined;
+    const validGroupBy = getValidGroupBy(groupBy);
     const prevValidGroupBy = usePrevious(validGroupBy);
     const isSearchResultsEmpty = !searchResults?.data || isSearchResultsEmptyUtil(searchResults, validGroupBy);
 
@@ -354,17 +353,6 @@ function Search({
         // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [isSmallScreenWidth]);
 
-    useEffect(() => {
-        openSearch({includePartiallySetupBankAccounts: true});
-    }, []);
-
-    useEffect(() => {
-        if (!prevIsOffline || isOffline) {
-            return;
-        }
-        openSearch({includePartiallySetupBankAccounts: true});
-    }, [isOffline, prevIsOffline]);
-
     const {newSearchResultKeys, handleSelectionListScroll, newTransactions} = useSearchHighlightAndScroll({
         searchResults,
         transactions,
@@ -449,21 +437,6 @@ function Search({
 
     const shouldShowLoadingMoreItems = !shouldShowLoadingState && searchResults?.search?.isLoading && searchResults?.search?.offset > 0;
 
-    const loadingSkeletonReasonAttributes = useMemo<SkeletonSpanReasonAttributes>(
-        () => ({
-            context: 'Search',
-            isOffline,
-            isDataLoaded,
-            isCardFeedsLoading,
-            isSearchLoading: !!searchResults?.search?.isLoading,
-            hasEmptyData: Array.isArray(searchResults?.data) && searchResults?.data.length === 0,
-            hasErrors,
-            hasPendingResponse: searchRequestResponseStatusCode === null,
-            shouldUseLiveData,
-        }),
-        [isOffline, isDataLoaded, isCardFeedsLoading, searchResults?.search?.isLoading, searchResults?.data, hasErrors, searchRequestResponseStatusCode, shouldUseLiveData],
-    );
-
     const loadMoreSkeletonReasonAttributes = useMemo<SkeletonSpanReasonAttributes>(
         () => ({
             context: 'Search.ListFooter',
@@ -1333,11 +1306,6 @@ function Search({
         spanExistedOnMount.current = false;
     }, []);
 
-    const onLayoutSkeleton = useCallback(() => {
-        hasHadFirstLayout.current = true;
-        endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: false});
-    }, []);
-
     const onLayoutChart = useCallback(() => {
         hasHadFirstLayout.current = true;
         endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: true});
@@ -1362,23 +1330,6 @@ function Search({
         }, [shouldShowLoadingState]),
     );
 
-    if (shouldShowLoadingState) {
-        return (
-            <Animated.View
-                entering={FadeIn.duration(CONST.SEARCH.ANIMATION.FADE_DURATION)}
-                exiting={FadeOut.duration(CONST.SEARCH.ANIMATION.FADE_DURATION)}
-                style={[styles.flex1]}
-                onLayout={onLayoutSkeleton}
-            >
-                <SearchRowSkeleton
-                    shouldAnimate
-                    containerStyle={shouldUseNarrowLayout ? styles.searchListContentContainerStyles : styles.mt3}
-                    reasonAttributes={loadingSkeletonReasonAttributes}
-                />
-            </Animated.View>
-        );
-    }
-
     if (searchResults === undefined) {
         Log.alert('[Search] Undefined search type');
         cancelNavigationSpans();

src/hooks/useSearchLoadingState.ts

@@ -0,0 +1,35 @@
+import {useSearchStateContext} from '@components/Search/SearchContext';
+import type {SearchQueryJSON} from '@components/Search/types';
+import {getValidGroupBy} from '@libs/SearchUIUtils';
+import CONST from '@src/CONST';
+import ONYXKEYS from '@src/ONYXKEYS';
+import type {SearchResults} from '@src/types/onyx';
+import useNetwork from './useNetwork';
+import useOnyx from './useOnyx';
+
+/**
+ * Computes whether the search page should show a loading skeleton.
+ * Accepts searchResults from the caller (which may include a sorting fallback)
+ * rather than reading raw context data, so that sorting doesn't trigger a skeleton flash.
+ */
+function useSearchLoadingState(queryJSON: SearchQueryJSON | undefined, searchResults: SearchResults | undefined): boolean {
+    const {isOffline} = useNetwork();
+    const {shouldUseLiveData} = useSearchStateContext();
+    const [, cardFeedsResult] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER);
+
+    if (shouldUseLiveData || isOffline || !queryJSON) {
+        return false;
+    }
+
+    const hasNoData = searchResults?.data === undefined;
+    const validGroupBy = getValidGroupBy(queryJSON.groupBy);
+    const isCardFeedsLoading = validGroupBy === CONST.SEARCH.GROUP_BY.CARD && cardFeedsResult?.status === 'loading';
+
+    // Show page-level skeleton when no data has ever arrived for this query,
+    // or when card feeds are still loading for card-grouped searches.
+    // Once data arrives (even empty []), Search mounts and handles its own
+    // loading/empty states internally via shouldShowLoadingState.
+    return hasNoData || isCardFeedsLoading;
+}
+
+export default useSearchLoadingState;

src/hooks/useSearchPageSetup.ts

@@ -0,0 +1,67 @@
+import {useFocusEffect} from '@react-navigation/native';
+import {useCallback, useEffect} from 'react';
+import {useSearchActionsContext, useSearchStateContext} from '@components/Search/SearchContext';
+import type {SearchQueryJSON} from '@components/Search/types';
+import {openSearch, search} from '@libs/actions/Search';
+import {isSearchDataLoaded} from '@libs/SearchUIUtils';
+import useNetwork from './useNetwork';
+import usePrevious from './usePrevious';
+import useSearchShouldCalculateTotals from './useSearchShouldCalculateTotals';
+
+/**
+ * Handles page-level setup for Search that must happen before the Search component mounts:
+ * - Clears selected transactions when the query changes
+ * - Fires the search() API call so data starts loading alongside the skeleton
+ * - Fires openSearch() to load bank account data
+ * - Re-fires openSearch() when coming back online
+ */
+function useSearchPageSetup(queryJSON: SearchQueryJSON | undefined) {
+    const {isOffline} = useNetwork();
+    const prevIsOffline = usePrevious(isOffline);
+    const {clearSelectedTransactions} = useSearchActionsContext();
+    const {shouldUseLiveData, currentSearchResults, currentSearchKey} = useSearchStateContext();
+
+    const hash = queryJSON?.hash;
+    const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, hash, true);
+
+    // Clear selected transactions when navigating to a different search query
+    const clearOnHashChange = useCallback(() => {
+        if (hash === undefined) {
+            return;
+        }
+        clearSelectedTransactions(hash);
+    }, [hash, clearSelectedTransactions]);
+
+    useFocusEffect(clearOnHashChange);
+
+    // useEffect supplements useFocusEffect: it handles both the initial mount
+    // and cases where route params change without a navigation event (e.g. sorting).
+    useEffect(clearOnHashChange, [clearOnHashChange]);
+
+    // Fire search() when the query changes (hash). This runs at the page level so the
+    // API request starts in parallel with the skeleton, before Search mounts its 14+ useOnyx hooks.
+    // currentSearchResults is intentionally read but not in deps — search should fire once per
+    // query change, not re-trigger on every data update from Onyx.
+    useEffect(() => {
+        if (!queryJSON || hash === undefined || shouldUseLiveData || isOffline) {
+            return;
+        }
+        if (isSearchDataLoaded(currentSearchResults, queryJSON) || currentSearchResults?.search?.isLoading) {
+            return;
+        }
+        search({queryJSON, searchKey: currentSearchKey, offset: 0, shouldCalculateTotals, isLoading: false});
+    }, [hash, isOffline, shouldUseLiveData, queryJSON]);
+
+    useEffect(() => {
+        openSearch({includePartiallySetupBankAccounts: true});
+    }, []);
+
+    useEffect(() => {
+        if (!prevIsOffline || isOffline) {
+            return;
+        }
+        openSearch({includePartiallySetupBankAccounts: true});
+    }, [isOffline, prevIsOffline]);
+}
+
+export default useSearchPageSetup;

src/libs/SearchUIUtils.ts

@@ -3946,6 +3946,10 @@ function isSearchDataLoaded(searchResults: SearchResults | undefined, queryJSON:
     return isDataLoaded;
 }
 
+function getValidGroupBy(groupBy: string | undefined): ValueOf<typeof CONST.SEARCH.GROUP_BY> | undefined {
+    return groupBy && Object.values(CONST.SEARCH.GROUP_BY).includes(groupBy as ValueOf<typeof CONST.SEARCH.GROUP_BY>) ? (groupBy as ValueOf<typeof CONST.SEARCH.GROUP_BY>) : undefined;
+}
+
 function getStatusOptions(translate: LocalizedTranslate, type: SearchDataTypes) {
     switch (type) {
         case CONST.SEARCH.DATA_TYPES.INVOICE:
@@ -4787,6 +4791,7 @@ export {
     shouldShowEmptyState,
     compareValues,
     isSearchDataLoaded,
+    getValidGroupBy,
     getStatusOptions,
     getTypeOptions,
     getGroupByOptions,

src/libs/actions/Search.ts

@@ -444,6 +444,11 @@ function openSearchPage({includePartiallySetupBankAccounts}: OpenSearchPageParam
     API.read(READ_COMMANDS.OPEN_SEARCH_PAGE, {includePartiallySetupBankAccounts});
 }
 
+// Tracks in-flight search requests by hash+offset to prevent duplicate API calls
+// when both page-level (useSearchPageSetup) and Search-internal (handleSearch) effects
+// fire for the same query. Cleared when the request completes.
+const inFlightSearchRequests = new Set<string>();
+
 let shouldPreventSearchAPI = false;
 function handlePreventSearchAPI(hash: number | undefined) {
     if (typeof hash === 'undefined') {
@@ -487,6 +492,12 @@ function search({
         return;
     }
 
+    const dedupeKey = `${queryJSON.hash}_${offset ?? 0}`;
+    if (inFlightSearchRequests.has(dedupeKey)) {
+        return;
+    }
+    inFlightSearchRequests.add(dedupeKey);
+
     const {optimisticData, finallyData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals);
     const {flatFilters, limit, ...queryJSONWithoutFlatFilters} = queryJSON;
     const query = {
@@ -508,36 +519,40 @@ function search({
 
     return waitForWrites(READ_COMMANDS.SEARCH).then(() => {
         // eslint-disable-next-line rulesdir/no-api-side-effects-method
-        return API.makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, finallyData, failureData}).then((result) => {
-            const response = result?.onyxData?.[0]?.value as OnyxSearchResponse;
-            const reports = Object.keys(response?.data ?? {})
-                .filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT))
-                .map((key) => key.replace(ONYXKEYS.COLLECTION.REPORT, ''));
-            if (response?.search?.offset) {
-                // Indicates that search results are extended from the Report view (with navigation between reports),
-                // using previous results to enable correct counter behavior.
-                if (prevReportsLength) {
+        return API.makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, finallyData, failureData})
+            .then((result) => {
+                const response = result?.onyxData?.[0]?.value as OnyxSearchResponse;
+                const reports = Object.keys(response?.data ?? {})
+                    .filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT))
+                    .map((key) => key.replace(ONYXKEYS.COLLECTION.REPORT, ''));
+                if (response?.search?.offset) {
+                    // Indicates that search results are extended from the Report view (with navigation between reports),
+                    // using previous results to enable correct counter behavior.
+                    if (prevReportsLength) {
+                        saveLastSearchParams({
+                            queryJSON,
+                            offset,
+                            hasMoreResults: !!response?.search?.hasMoreResults,
+                            previousLengthOfResults: prevReportsLength,
+                            allowPostSearchRecount: false,
+                        });
+                    }
+                } else {
+                    // Applies to all searches from the Search View
                     saveLastSearchParams({
                         queryJSON,
                         offset,
                         hasMoreResults: !!response?.search?.hasMoreResults,
-                        previousLengthOfResults: prevReportsLength,
-                        allowPostSearchRecount: false,
+                        previousLengthOfResults: reports.length,
+                        allowPostSearchRecount: true,
                     });
                 }
-            } else {
-                // Applies to all searches from the Search View
-                saveLastSearchParams({
-                    queryJSON,
-                    offset,
-                    hasMoreResults: !!response?.search?.hasMoreResults,
-                    previousLengthOfResults: reports.length,
-                    allowPostSearchRecount: true,
-                });
-            }
 
-            return result?.jsonCode;
-        });
+                return result?.jsonCode;
+            })
+            .finally(() => {
+                inFlightSearchRequests.delete(dedupeKey);
+            });
     });
 }