refactor: the new-transaction highlight rail and the wide RHP width model

.storybook/preview.tsx

@@ -46,7 +46,9 @@ const decorators = [
                 SearchContextProvider,
             ]}
         >
-            <ScreenWrapperStatusContext.Provider value={{didScreenTransitionEnd: true, isSafeAreaTopPaddingApplied: false, isSafeAreaBottomPaddingApplied: false}}>
+            <ScreenWrapperStatusContext.Provider
+                value={{didScreenTransitionEnd: true, shouldUseNarrowLayoutOnWideRHP: false, isSafeAreaTopPaddingApplied: false, isSafeAreaBottomPaddingApplied: false}}
+            >
                 <Story />
             </ScreenWrapperStatusContext.Provider>
         </ComposeProviders>

src/CONST/index.ts

@@ -319,6 +319,7 @@ const CONST = {
     BACKGROUND_IMAGE_TRANSITION_DURATION: 1000,
     SCREEN_TRANSITION_END_TIMEOUT: 1000,
     PENDING_TRANSACTION_DELETION_DELAY: 4000,
+    PENDING_TRANSACTION_FRESHNESS_WINDOW: 60000,
     PENDING_TRANSACTION_SCROLL_DELAY: 1000,
 
     // Delay before pre-inserting the Search fullscreen route under the RHP on the confirmation screen.

src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx

@@ -4,6 +4,7 @@ import useAppFocusEvent from '@hooks/useAppFocusEvent';
 import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
 import {useIsReportLoadPending} from '@hooks/useInFlightRequests';
 import useIsReportActionsLoaded from '@hooks/useIsReportActionsLoaded';
+import useIsReportVisible from '@hooks/useIsReportVisible';
 import useLoadReportActions from '@hooks/useLoadReportActions';
 import useLocalize from '@hooks/useLocalize';
 import useNetworkWithOfflineStatus from '@hooks/useNetworkWithOfflineStatus';
@@ -124,8 +125,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
     const [isVisible, setIsVisible] = useState(Visibility.isVisible);
     const isFocused = useIsFocused();
     const {shouldUseNarrowLayout} = useResponsiveLayoutOnWideRHP();
-    // The table is visible whenever it's wide, or — on narrow — only when focused (the RHP has closed).
-    const isReportVisible = shouldUseNarrowLayout ? isFocused : true;
+    const isReportVisible = useIsReportVisible(shouldUseNarrowLayout);
     const route = useRoute<PlatformStackRouteProp<ReportsSplitNavigatorParamList, typeof SCREENS.REPORT>>();
     const reportIDFromRoute = route?.params?.reportID;
     const isReportLoadPending = useIsReportLoadPending(reportIDFromRoute);
@@ -158,7 +158,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
     const [pendingNewTransactionIDs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportIDFromRoute}`, {
         selector: pendingNewTransactionIDsSelector,
     });
-    const newTransactions = useNewTransactions(reportLoadingState?.hasOnceLoadedReportActions, reportTransactions, pendingNewTransactionIDs, reportIDFromRoute, isFocused);
+    const newTransactions = useNewTransactions(reportLoadingState?.hasOnceLoadedReportActions, reportTransactions, pendingNewTransactionIDs, reportIDFromRoute, isReportVisible);
     const showReportActionsLoadingState = reportLoadingState?.isLoadingInitialReportActions && !reportLoadingState?.hasOnceLoadedReportActions;
     const isInitialReportLoadPending = !isOffline && isReportLoadPending && !reportLoadingState?.hasOnceLoadedReportActions;
     const reportTransactionIDs = useMemo(() => transactions.map((transaction) => transaction.transactionID), [transactions]);

src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx

@@ -3,12 +3,13 @@ import TransactionPreview from '@components/ReportActionItem/TransactionPreview'
 import {useWideRHPActions} from '@components/WideRHPContextProvider';
 
 import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
+import useIsReportVisible from '@hooks/useIsReportVisible';
 import useNetwork from '@hooks/useNetwork';
 import useNewTransactions from '@hooks/useNewTransactions';
 import useOnyx from '@hooks/useOnyx';
 import usePolicy from '@hooks/usePolicy';
 import useReportTransactionsCollection from '@hooks/useReportTransactionsCollection';
-import useResponsiveLayout from '@hooks/useResponsiveLayout';
+import useResponsiveLayoutOnWideRHP from '@hooks/useResponsiveLayoutOnWideRHP';
 import useStyleUtils from '@hooks/useStyleUtils';
 import useThemeStyles from '@hooks/useThemeStyles';
 import useTransactionViolations from '@hooks/useTransactionViolations';
@@ -69,9 +70,10 @@ function MoneyRequestReportPreview({
 }: MoneyRequestReportPreviewProps) {
     const styles = useThemeStyles();
     const StyleUtils = useStyleUtils();
-    // eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
-    const {shouldUseNarrowLayout, isSmallScreenWidth} = useResponsiveLayout();
+    const {shouldUseNarrowLayoutIgnoringWideRHP, isSmallScreenWidth} = useResponsiveLayoutOnWideRHP();
     const {markReportRHPWidth, unmarkReportRHPWidth} = useWideRHPActions();
+    // Deferred presses need focus, which is narrower than the visibility the highlight below is gated on.
+    const isFocused = useIsFocused();
     const personalDetailsList = usePersonalDetails();
     const {email: currentUserEmail, accountID: currentUserAccountID} = useCurrentUserPersonalDetails();
     const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
@@ -153,8 +155,8 @@ function MoneyRequestReportPreview({
     );
 
     const reportPreviewStyles = useMemo(
-        () => StyleUtils.getMoneyRequestReportPreviewStyle(shouldUseNarrowLayout, transactions.length, widths.currentWidth, widths.currentWrapperWidth),
-        [StyleUtils, widths, shouldUseNarrowLayout, transactions.length],
+        () => StyleUtils.getMoneyRequestReportPreviewStyle(shouldUseNarrowLayoutIgnoringWideRHP, transactions.length, widths.currentWidth, widths.currentWrapperWidth),
+        [StyleUtils, widths, shouldUseNarrowLayoutIgnoringWideRHP, transactions.length],
     );
     const shouldShowPayerAndReceiver = useMemo(() => {
         if (!isIOUReport(iouReport) && action.childType !== CONST.REPORT.TYPE.IOU) {
@@ -207,7 +209,6 @@ function MoneyRequestReportPreview({
     const [pendingNewTransactionIDs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${chatReportID}`, {
         selector: pendingNewTransactionIDsSelector,
     });
-    const isFocused = useIsFocused();
     // Transactions arrive in batches and `useNewTransactions` would diff each batch as newly added expenses.
     // Withhold the list until every transaction the report claims has arrived.
     const expectedTransactionCount = iouReport?.transactionCount ?? 0;
@@ -220,9 +221,10 @@ function MoneyRequestReportPreview({
         setHasCompletedDelivery(true);
     }
     const transactionsForDiff = isDeliveryComplete || hasCompletedDelivery ? transactions : undefined;
-    const newTransactions = useNewTransactions(hasOnceLoadedReportActions, transactionsForDiff, pendingNewTransactionIDs, chatReportID, isFocused);
     // Don't surface the highlight while the preview is covered — it'd animate the one-shot off-screen and be missed.
-    const isReportVisible = shouldUseNarrowLayout ? isFocused : true;
+    // A modal pane can be covered at any width, so this reads the flag unadjusted for wide RHP.
+    const isReportVisible = useIsReportVisible(shouldUseNarrowLayoutIgnoringWideRHP);
+    const newTransactions = useNewTransactions(hasOnceLoadedReportActions, transactionsForDiff, pendingNewTransactionIDs, chatReportID, isReportVisible);
     const newTransactionIDs = new Set(isReportVisible ? newTransactions.map((transaction) => transaction.transactionID) : []);
 
     const transactionPreviewContainerStyles = [styles.h100, reportPreviewStyles.transactionPreviewCarouselStyle];

src/components/ScreenWrapper/ScreenWrapperStatusContext.ts

@@ -2,6 +2,7 @@ import {createContext} from 'react';
 
 type ScreenWrapperStatusContextType = {
     didScreenTransitionEnd: boolean;
+    shouldUseNarrowLayoutOnWideRHP: boolean;
     isSafeAreaTopPaddingApplied: boolean;
     isSafeAreaBottomPaddingApplied: boolean;
 };

src/components/ScreenWrapper/index.tsx

@@ -9,7 +9,7 @@ import useAccessibilityFocus from '@hooks/useAccessibilityFocus';
 import useEnvironment from '@hooks/useEnvironment';
 import useNetwork from '@hooks/useNetwork';
 import useOnyx from '@hooks/useOnyx';
-import useResponsiveLayout from '@hooks/useResponsiveLayout';
+import useResponsiveLayoutOnWideRHP from '@hooks/useResponsiveLayoutOnWideRHP';
 import useSafeAreaPaddings from '@hooks/useSafeAreaPaddings';
 import useThemeStyles from '@hooks/useThemeStyles';
 
@@ -127,8 +127,7 @@ function ScreenWrapper({
     const mergedScreenWrapperRef = mergeRefs(screenWrapperRef, ref);
 
     // We need to use isSmallScreenWidth instead of shouldUseNarrowLayout for a case where we want to show the offline indicator only on small screens
-    // eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
-    const {isSmallScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout();
+    const {isSmallScreenWidth, shouldUseNarrowLayoutIgnoringWideRHP, shouldUseNarrowLayout: shouldUseNarrowLayoutOnWideRHP} = useResponsiveLayoutOnWideRHP();
 
     const styles = useThemeStyles();
     const {isDevelopment} = useEnvironment();
@@ -147,14 +146,14 @@ function ScreenWrapper({
     const includeSafeAreaPaddingBottom = isUsingEdgeToEdgeMode ? false : includeSafeAreaPaddingBottomProp;
     const isSafeAreaTopPaddingApplied = includePaddingTop;
     const statusContextValue = useMemo(
-        () => ({didScreenTransitionEnd, isSafeAreaTopPaddingApplied, isSafeAreaBottomPaddingApplied: includeSafeAreaPaddingBottom}),
-        [didScreenTransitionEnd, includeSafeAreaPaddingBottom, isSafeAreaTopPaddingApplied],
+        () => ({didScreenTransitionEnd, shouldUseNarrowLayoutOnWideRHP, isSafeAreaTopPaddingApplied, isSafeAreaBottomPaddingApplied: includeSafeAreaPaddingBottom}),
+        [didScreenTransitionEnd, shouldUseNarrowLayoutOnWideRHP, includeSafeAreaPaddingBottom, isSafeAreaTopPaddingApplied],
     );
 
     // This context allows us to disable the safe area padding offsetting the offline indicator in scrollable components like 'ScrollView', 'SelectionList' or 'FormProvider'.
     // This is useful e.g. for the RightModalNavigator, where we want to avoid the safe area padding offsetting the offline indicator because we only show the offline indicator on small screens.
     const {isInNarrowPane} = useContext(NarrowPaneContext);
-    const isMobileWebNarrowLayout = getPlatform() === CONST.PLATFORM.WEB && isMobile() && shouldUseNarrowLayout;
+    const isMobileWebNarrowLayout = getPlatform() === CONST.PLATFORM.WEB && isMobile() && shouldUseNarrowLayoutIgnoringWideRHP;
     const shouldMoveAccessibilityFocus = isMobileWebNarrowLayout && isInNarrowPane;
     const shouldHideFromAccessibility = isMobileWebNarrowLayout && !isFocused;
     const {addSafeAreaPadding, showOnSmallScreens, showOnWideScreens} = useContext(ScreenWrapperOfflineIndicatorContext);

src/components/Search/SearchList/ListItem/TransactionGroupListExpanded.tsx

@@ -239,11 +239,12 @@ function TransactionGroupListExpandedImpl({
                 });
                 return;
             }
-            markReportRHPWidth(reportID, 'wide');
             const route = ROUTES.SEARCH_REPORT.getRoute({reportID, backTo});
+            // Marked after the new-tab guard: opening in a new tab never mounts the screen that would consume the hint, so it would pin this report wide on a later visit.
             if (openInternalRouteInNewTab(route, event)) {
                 return;
             }
+            markReportRHPWidth(reportID, 'wide');
             Navigation.navigate(route);
         };
 

src/components/Search/index.tsx

@@ -673,12 +673,6 @@ function Search({
                     }
                 }
 
-                if (item.transactions.length > 1) {
-                    markReportRHPWidth(reportID, 'super-wide');
-                } else {
-                    unmarkReportRHPWidth(reportID, 'super-wide');
-                }
-
                 // Persist the current search context so prev/next navigation arrows
                 // in the report RHP can reference the correct result set.
                 saveLastSearchParams({
@@ -693,9 +687,17 @@ function Search({
                     reportID,
                     backTo,
                 });
+                if (item.transactions.length <= 1) {
+                    // A report that no longer qualifies must not keep the hint an earlier visit left behind, whether or not this click navigates.
+                    unmarkReportRHPWidth(reportID, 'super-wide');
+                }
+                // Marked after the new-tab guard: opening in a new tab never mounts the screen that would consume the hint, so it would pin this report wide on a later visit.
                 if (openInternalRouteInNewTab(route, event)) {
                     return;
                 }
+                if (item.transactions.length > 1) {
+                    markReportRHPWidth(reportID, 'super-wide');
+                }
                 requestAnimationFrame(() => Navigation.navigate(route));
                 return;
             }
@@ -728,8 +730,6 @@ function Search({
                 return;
             }
 
-            markReportRHPWidth(reportID, 'wide');
-
             if (isTransactionItem && transactionPreviewData) {
                 setOptimisticDataForTransactionThreadPreview(transactionItem, transactionPreviewData, getCurrencyDecimals, transactionItem?.reportAction?.childReportID);
             }
@@ -738,6 +738,7 @@ function Search({
             if (openInternalRouteInNewTab(route, event)) {
                 return;
             }
+            markReportRHPWidth(reportID, 'wide');
             requestAnimationFrame(() => Navigation.navigate(route));
         },
         [

src/components/WideRHPContextProvider/default.ts

@@ -17,10 +17,6 @@ const defaultWideRHPActionsContextValue: WideRHPActionsContextType = {
     markReportRHPWidth: () => {},
     unmarkReportRHPWidth: () => {},
     getReportRHPWidthHint: () => undefined,
-    syncRHPKeys: () => {},
-    clearWideRHPKeys: () => {},
-    setIsWideRHPClosing: () => {},
-    setIsSuperWideRHPClosing: () => {},
 };
 
 export {defaultWideRHPStateContextValue, defaultWideRHPActionsContextValue};

src/components/WideRHPContextProvider/getIsRHPDisplayedBelow.ts

@@ -1,9 +1,5 @@
-import getVisibleRHPKeys from './getVisibleRHPRouteKeys';
-
 // Helper function to determine if wide or super wide RHP is displayed below the currently focused route
-export default function getIsRHPDisplayedBelow(focusedRouteKey: string | undefined, allSuperWideRHPRouteKeys: string[], allWideRHPRouteKeys: string[]) {
-    const {visibleSuperWideRHPRouteKeys, visibleWideRHPRouteKeys} = getVisibleRHPKeys(allSuperWideRHPRouteKeys, allWideRHPRouteKeys);
-
+export default function getIsRHPDisplayedBelow(focusedRouteKey: string | undefined, visibleSuperWideRHPRouteKeys: string[], visibleWideRHPRouteKeys: string[]) {
     if (!focusedRouteKey) {
         return {
             isWideRHPBelow: false,

src/components/WideRHPContextProvider/getVisibleRHPRouteKeys.ts

@@ -1,54 +1,55 @@
 import extractNavigationKeys from '@libs/Navigation/helpers/extractNavigationKeys';
 import getLastVisibleRHPRouteKey from '@libs/Navigation/helpers/getLastVisibleRHPRouteKey';
-import {navigationRef} from '@libs/Navigation/Navigation';
+
+import type {NavigationState} from '@react-navigation/native';
 
 type VisibleRHPKeys = {
     visibleWideRHPRouteKeys: string[];
     visibleSuperWideRHPRouteKeys: string[];
+    presentRouteKeys: string[];
 };
 
 const emptyRHPKeysState: VisibleRHPKeys = {
     visibleWideRHPRouteKeys: [],
     visibleSuperWideRHPRouteKeys: [],
+    presentRouteKeys: [],
 };
 
 /**
- * Extracts the keys of the screens that are currently displayed from the array of all Wide/Super Wide RHP keys
- *
- * @param allWideRHPKeys - an array of all Wide/Super Wide RHP keys
+ * A key missing from the navigation state may be a screen dismissing or one that was never shown, and only the first
+ * holds a width. Callers must record `presentRouteKeys` so `seenRouteKeys` can tell those apart.
  */
-function getVisibleRHPKeys(allSuperWideRHPKeys: string[], allWideRHPKeys: string[]): VisibleRHPKeys {
-    if (!navigationRef.isReady()) {
-        return emptyRHPKeysState;
-    }
-
-    const rootState = navigationRef.getRootState();
-    if (!rootState) {
+function getVisibleRHPKeys(state: NavigationState | undefined, allWideRHPKeys: string[], allSuperWideRHPKeys: string[], seenRouteKeys: ReadonlySet<string>): VisibleRHPKeys {
+    // Nothing registered is the common case, and there is then no key to match the tree against.
+    if (!state || (!allWideRHPKeys.length && !allSuperWideRHPKeys.length)) {
         return emptyRHPKeysState;
     }
 
-    const lastVisibleRHPRouteKey = getLastVisibleRHPRouteKey(rootState);
-    const lastRHPRoute = rootState.routes.find((route) => route.key === lastVisibleRHPRouteKey);
-
-    if (!lastRHPRoute) {
-        return emptyRHPKeysState;
+    // A covered RHP needs no hand-clearing: this returns undefined for it, and its keys are still in the state below, so the displayed check rejects them too.
+    const lastVisibleRHPRouteKey = getLastVisibleRHPRouteKey(state);
+    const lastRHPRoute = state.routes.find((route) => route.key === lastVisibleRHPRouteKey);
+
+    let visibleRHPKeys = new Set<string>();
+    if (lastRHPRoute?.state?.routes) {
+        const superWideRHPIndex = lastRHPRoute.state.routes.findLastIndex((route) => route?.key && allSuperWideRHPKeys.includes(route.key));
+        const wideRHPIndex = lastRHPRoute.state.routes.findLastIndex((route) => route?.key && allWideRHPKeys.includes(route.key));
+
+        if (superWideRHPIndex > -1) {
+            visibleRHPKeys = extractNavigationKeys(lastRHPRoute.state.routes.slice(superWideRHPIndex));
+        } else if (wideRHPIndex > -1) {
+            visibleRHPKeys = extractNavigationKeys(lastRHPRoute.state.routes.slice(wideRHPIndex));
+        } else {
+            visibleRHPKeys = extractNavigationKeys(lastRHPRoute.state.routes);
+        }
     }
 
-    const superWideRHPIndex = lastRHPRoute.state?.routes.findLastIndex((route) => route?.key && allSuperWideRHPKeys.includes(route.key)) ?? -1;
-    const wideRHPIndex = lastRHPRoute.state?.routes.findLastIndex((route) => route?.key && allWideRHPKeys.includes(route.key)) ?? -1;
-
-    let visibleRHPKeys;
-    if (superWideRHPIndex > -1) {
-        visibleRHPKeys = extractNavigationKeys(lastRHPRoute.state?.routes.slice(superWideRHPIndex));
-    } else if (wideRHPIndex > -1) {
-        visibleRHPKeys = extractNavigationKeys(lastRHPRoute.state?.routes.slice(wideRHPIndex));
-    } else {
-        visibleRHPKeys = extractNavigationKeys(lastRHPRoute.state?.routes);
-    }
+    const keysInState = extractNavigationKeys(state.routes);
+    const isDisplayed = (key: string) => visibleRHPKeys.has(key) || (seenRouteKeys.has(key) && !keysInState.has(key));
 
     return {
-        visibleWideRHPRouteKeys: allWideRHPKeys.filter((key) => visibleRHPKeys.has(key)),
-        visibleSuperWideRHPRouteKeys: allSuperWideRHPKeys.filter((key) => visibleRHPKeys.has(key)),
+        visibleWideRHPRouteKeys: allWideRHPKeys.filter(isDisplayed),
+        visibleSuperWideRHPRouteKeys: allSuperWideRHPKeys.filter(isDisplayed),
+        presentRouteKeys: [...allWideRHPKeys, ...allSuperWideRHPKeys].filter((key) => keysInState.has(key)),
     };
 }