From 5b7654a488f875dc7c282026bd85e8d205eccca7 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Thu, 30 Apr 2026 10:50:00 -0700 Subject: [PATCH 1/4] carrousel fixes --- .../MoneyRequestReportNavigation.tsx | 61 +++++++++++++------ 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx index d51b7847d508..8fedad348e2d 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'; @@ -60,11 +60,24 @@ 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. + const [lastValidReports, setLastValidReports] = useState | null>(null); + if (liveCurrentIndex !== -1 && 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 +88,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 +108,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 +126,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 +179,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 [hasGuardEverPassed, setHasGuardEverPassed] = useState(false); + if (isLiveGuardSatisfied && !hasGuardEverPassed) { + setHasGuardEverPassed(true); + } - if (!shouldMount) { + if (!isLiveGuardSatisfied && !hasGuardEverPassed) { return null; } From 2085db093cf696a91b8e8158cba627876f7a5950 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Thu, 30 Apr 2026 11:02:07 -0700 Subject: [PATCH 2/4] Drop redundant !! coercion in isLiveGuardSatisfied The && chain's falsy short-circuit handles undefined the same way the original shouldMount expression did, so the explicit cast is noise. --- .../MoneyRequestReportView/MoneyRequestReportNavigation.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx index 8fedad348e2d..19737302f05c 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx @@ -179,7 +179,7 @@ function MoneyRequestReportNavigation({reportID, shouldDisplayNarrowVersion}: Mo const snapshotGuardSelector = buildSnapshotGuardSelector(reportID); const [snapshotGuard = EMPTY_GUARD] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`, {selector: snapshotGuardSelector}); - const isLiveGuardSatisfied = !!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 From 00c81b19f8ba5d558d8241495809bb08452035ac Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Thu, 30 Apr 2026 11:05:34 -0700 Subject: [PATCH 3/4] =?UTF-8?q?Rename=20hasGuardEverPassed=20=E2=86=92=20s?= =?UTF-8?q?houldKeepMounted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct flag name, and the early-return check collapses to a single boolean since once shouldKeepMounted is true it stays true. --- .../MoneyRequestReportNavigation.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx index 19737302f05c..d6e6c7c8b8b9 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx @@ -186,12 +186,12 @@ function MoneyRequestReportNavigation({reportID, shouldDisplayNarrowVersion}: Mo // 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 [hasGuardEverPassed, setHasGuardEverPassed] = useState(false); - if (isLiveGuardSatisfied && !hasGuardEverPassed) { - setHasGuardEverPassed(true); + const [shouldKeepMounted, setShouldKeepMounted] = useState(false); + if (isLiveGuardSatisfied && !shouldKeepMounted) { + setShouldKeepMounted(true); } - if (!isLiveGuardSatisfied && !hasGuardEverPassed) { + if (!shouldKeepMounted) { return null; } From 6e7fc8019150685fe03b871b51620870bee1ae5a Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Thu, 30 Apr 2026 11:30:28 -0700 Subject: [PATCH 4/4] Compare allReports by content to avoid render loop useSearchSections rebuilds allReports via filter/map each render, so identity comparison would refire setLastValidReports every render and trip 'Maximum update depth exceeded'. The shallow content check converges as soon as the contents stop changing, regardless of reference identity. --- .../MoneyRequestReportNavigation.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx index d6e6c7c8b8b9..29dd908d45f6 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx @@ -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 => { @@ -66,9 +81,12 @@ function MoneyRequestReportNavigationInner({reportID, shouldDisplayNarrowVersion // 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. + // 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 && allReports !== lastValidReports) { + if (liveCurrentIndex !== -1 && !isSameReportList(allReports, lastValidReports)) { setLastValidReports(allReports); } const effectiveAllReports = liveCurrentIndex === -1 && lastValidReports ? lastValidReports : allReports;