diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx index d51b7847d508..29dd908d45f6 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx @@ -1,4 +1,4 @@ -import React, {useEffect} from 'react'; +import React, {useEffect, useState} from 'react'; import {View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import PrevNextButtons from '@components/PrevNextButtons'; @@ -31,6 +31,21 @@ const selectIsExpenseReportSearch = (lastSearchQuery: OnyxEntry): number | undefined => lastSearchQuery?.queryJSON?.hash; +const isSameReportList = (a: Array, b: Array | null): boolean => { + if (a === b) { + return true; + } + if (b === null || a.length !== b.length) { + return false; + } + for (let i = 0; i < a.length; i++) { + if (a.at(i) !== b.at(i)) { + return false; + } + } + return true; +}; + const buildSnapshotGuardSelector = (reportID: string | undefined) => (snapshot: OnyxEntry): SnapshotGuard => { @@ -60,11 +75,27 @@ function MoneyRequestReportNavigationInner({reportID, shouldDisplayNarrowVersion const {allReports, isSearchLoading, lastSearchQuery} = useSearchSections(); const styles = useThemeStyles(); - const currentIndex = allReports.indexOf(reportID); + const liveCurrentIndex = allReports.indexOf(reportID); + + // Cache the last list where the current report was still present. When the search snapshot + // is refreshed and the current report drops out (e.g. after approving from Spend > Needs + // Approval), keep using the cached list so the carousel stays populated and the user can + // navigate to the next report instead of the arrows disappearing. setState during render is + // the React-recommended pattern for storing information from previous renders. We compare + // by content rather than reference because useSearchSections rebuilds allReports via + // filter/map each render, so identity comparison would refire the setState every render and + // trigger an infinite update loop. + const [lastValidReports, setLastValidReports] = useState | null>(null); + if (liveCurrentIndex !== -1 && !isSameReportList(allReports, lastValidReports)) { + setLastValidReports(allReports); + } + const effectiveAllReports = liveCurrentIndex === -1 && lastValidReports ? lastValidReports : allReports; + const currentIndex = effectiveAllReports.indexOf(reportID); + const allReportsCount = lastSearchQuery?.previousLengthOfResults ?? 0; - const hideNextButton = !lastSearchQuery?.hasMoreResults && currentIndex === allReports.length - 1; + const hideNextButton = !lastSearchQuery?.hasMoreResults && currentIndex === effectiveAllReports.length - 1; const hidePrevButton = currentIndex === 0; - const shouldDisplayNavigationArrows = allReports.length > 1 && currentIndex !== -1 && !!lastSearchQuery?.queryJSON; + const shouldDisplayNavigationArrows = effectiveAllReports.length > 1 && currentIndex !== -1 && !!lastSearchQuery?.queryJSON; useEffect(() => { if (!lastSearchQuery?.queryJSON) { @@ -75,16 +106,16 @@ function MoneyRequestReportNavigationInner({reportID, shouldDisplayNarrowVersion saveLastSearchParams({ ...lastSearchQuery, allowPostSearchRecount: false, - previousLengthOfResults: allReports.length, + previousLengthOfResults: effectiveAllReports.length, }); return; } // Update count when reports are added or removed (e.g., created offline) - if (allReports.length !== allReportsCount) { + if (effectiveAllReports.length !== allReportsCount) { saveLastSearchParams({ ...lastSearchQuery, - previousLengthOfResults: allReports.length, + previousLengthOfResults: effectiveAllReports.length, }); return; } @@ -95,9 +126,9 @@ function MoneyRequestReportNavigationInner({reportID, shouldDisplayNarrowVersion saveLastSearchParams({ ...lastSearchQuery, - previousLengthOfResults: allReports.length, + previousLengthOfResults: effectiveAllReports.length, }); - }, [currentIndex, allReportsCount, allReports.length, lastSearchQuery?.queryJSON, lastSearchQuery]); + }, [currentIndex, allReportsCount, effectiveAllReports.length, lastSearchQuery?.queryJSON, lastSearchQuery]); const goToReportId = (reportId?: string) => { if (!reportId) { @@ -113,34 +144,34 @@ function MoneyRequestReportNavigationInner({reportID, shouldDisplayNarrowVersion }; const goToNextReport = () => { - if (currentIndex === -1 || allReports.length === 0 || !lastSearchQuery?.queryJSON) { + if (currentIndex === -1 || effectiveAllReports.length === 0 || !lastSearchQuery?.queryJSON) { return; } - const threshold = Math.min(allReports.length * 0.75, allReports.length - 2); + const threshold = Math.min(effectiveAllReports.length * 0.75, effectiveAllReports.length - 2); if (currentIndex + 1 >= threshold && lastSearchQuery?.hasMoreResults) { const newOffset = (lastSearchQuery.offset ?? 0) + CONST.SEARCH.RESULTS_PAGE_SIZE; search({ queryJSON: lastSearchQuery.queryJSON, offset: newOffset, - prevReportsLength: allReports.length, + prevReportsLength: effectiveAllReports.length, shouldCalculateTotals: false, searchKey: lastSearchQuery.searchKey, isLoading: isSearchLoading, }); } - const nextIndex = (currentIndex + 1) % allReports.length; - goToReportId(allReports.at(nextIndex)); + const nextIndex = (currentIndex + 1) % effectiveAllReports.length; + goToReportId(effectiveAllReports.at(nextIndex)); }; const goToPrevReport = () => { - if (currentIndex === -1 || allReports.length === 0) { + if (currentIndex === -1 || effectiveAllReports.length === 0) { return; } - const prevIndex = (currentIndex - 1) % allReports.length; - goToReportId(allReports.at(prevIndex)); + const prevIndex = (currentIndex - 1) % effectiveAllReports.length; + goToReportId(effectiveAllReports.at(prevIndex)); }; if (!shouldDisplayNavigationArrows) { @@ -166,9 +197,19 @@ function MoneyRequestReportNavigation({reportID, shouldDisplayNarrowVersion}: Mo const snapshotGuardSelector = buildSnapshotGuardSelector(reportID); const [snapshotGuard = EMPTY_GUARD] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`, {selector: snapshotGuardSelector}); - const shouldMount = isExpenseReportSearch && snapshotGuard.hasMultiple && snapshotGuard.includesReport; + const isLiveGuardSatisfied = isExpenseReportSearch && snapshotGuard.hasMultiple && snapshotGuard.includesReport; + + // Once the live snapshot has satisfied the guard during this mount, keep the inner component + // mounted even if the guard later flips false (e.g. the current report is removed from the + // snapshot after approving it). The inner component falls back to a cached list so the + // carousel stays visible for continued navigation. setState during render is the React- + // recommended pattern for storing information from previous renders. + const [shouldKeepMounted, setShouldKeepMounted] = useState(false); + if (isLiveGuardSatisfied && !shouldKeepMounted) { + setShouldKeepMounted(true); + } - if (!shouldMount) { + if (!shouldKeepMounted) { return null; }