Merge pull request #44488 from tsa321/fixLoadingSkeletonOnReport2

Fix the loading skeleton behavior when opening a report

src/CONST.ts

@@ -664,6 +664,7 @@ const CONST = {
             MEMBER: 'member',
         },
         MAX_COUNT_BEFORE_FOCUS_UPDATE: 30,
+        MIN_INITIAL_REPORT_ACTION_COUNT: 15,
         SPLIT_REPORTID: '-2',
         ACTIONS: {
             LIMIT: 50,

src/hooks/usePaginatedReportActions.ts

@@ -33,6 +33,7 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) {
     return {
         reportActions,
         linkedAction,
+        sortedAllReportActions,
     };
 }
 

src/pages/home/ReportScreen.tsx

@@ -285,7 +285,7 @@ function ReportScreen({
     const [isLinkingToMessage, setIsLinkingToMessage] = useState(!!reportActionIDFromRoute);
 
     const [currentUserAccountID = -1] = useOnyx(ONYXKEYS.SESSION, {selector: (value) => value?.accountID});
-    const {reportActions, linkedAction} = usePaginatedReportActions(report.reportID, reportActionIDFromRoute);
+    const {reportActions, linkedAction, sortedAllReportActions} = usePaginatedReportActions(report.reportID, reportActionIDFromRoute);
 
     // Define here because reportActions are recalculated before mount, allowing data to display faster than useEffect can trigger.
     // If we have cached reportActions, they will be shown immediately.
@@ -310,6 +310,19 @@ function ReportScreen({
     const screenWrapperStyle: ViewStyle[] = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}];
     const isEmptyChat = useMemo(() => ReportUtils.isEmptyReport(report), [report]);
     const isOptimisticDelete = report.statusNum === CONST.REPORT.STATUS_NUM.CLOSED;
+    const indexOfLinkedMessage = useMemo(
+        (): number => reportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)),
+        [reportActions, reportActionIDFromRoute],
+    );
+
+    const isPendingActionExist = !!reportActions.at(0)?.pendingAction;
+    const doesCreatedActionExists = useCallback(() => !!sortedAllReportActions?.findLast((action) => ReportActionsUtils.isCreatedAction(action)), [sortedAllReportActions]);
+    const isLinkedMessageAvailable = useMemo(() => indexOfLinkedMessage > -1, [indexOfLinkedMessage]);
+
+    // The linked report actions should have at least 15 messages (counting as 1 page) above them to fill the screen.
+    // If the count is too high (equal to or exceeds the web pagination size / 50) and there are no cached messages in the report,
+    // OpenReport will be called each time the user scrolls up the report a bit, clicks on report preview, and then goes back."
+    const isLinkedMessagePageReady = isLinkedMessageAvailable && (reportActions.length - indexOfLinkedMessage >= CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT || doesCreatedActionExists());
 
     // If there's a non-404 error for the report we should show it instead of blocking the screen
     const hasHelpfulErrors = Object.keys(report?.errorFields ?? {}).some((key) => key !== 'notFound');
@@ -399,6 +412,10 @@ function ReportScreen({
         return reportIDFromRoute !== '' && !!report.reportID && !isTransitioning;
     }, [report, reportIDFromRoute]);
 
+    const isInitialPageReady = isOffline
+        ? reportActions.length > 0
+        : reportActions.length >= CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT || isPendingActionExist || (doesCreatedActionExists() && reportActions.length > 0);
+
     /**
      * Using logical OR operator because with nullish coalescing operator, when `isLoadingApp` is false, the right hand side of the operator
      * is not evaluated. This causes issues where we have `isLoading` set to false and later set to true and then set to false again.
@@ -407,13 +424,13 @@ function ReportScreen({
     // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
     const isLoading = isLoadingApp || !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty();
     const shouldShowSkeleton =
-        !linkedAction &&
-        (isLinkingToMessage ||
-            !isCurrentReportLoadedFromOnyx ||
-            (reportActions.length === 0 && !!reportMetadata?.isLoadingInitialReportActions) ||
-            isLoading ||
-            (!!reportActionIDFromRoute && reportMetadata?.isLoadingInitialReportActions));
-    const shouldShowReportActionList = isCurrentReportLoadedFromOnyx && !isLoading;
+        (isLinkingToMessage && !isLinkedMessagePageReady) ||
+        (!!reportActionIDFromRoute && !!reportMetadata?.isLoadingInitialReportActions) ||
+        (!isLinkingToMessage && !isInitialPageReady) ||
+        isLoadingReportOnyx ||
+        !isCurrentReportLoadedFromOnyx ||
+        isLoading;
+
     const currentReportIDFormRoute = route.params?.reportID;
 
     // eslint-disable-next-line rulesdir/no-negated-variables
@@ -436,7 +453,6 @@ function ReportScreen({
         if (shouldHideReport) {
             return true;
         }
-
         return !!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute);
     }, [isLoadingApp, finishedLoadingApp, report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute]);
 
@@ -470,12 +486,12 @@ function ReportScreen({
             return;
         }
 
-        if (!shouldFetchReport(report)) {
+        if (!shouldFetchReport(report) && (isInitialPageReady || isLinkedMessagePageReady)) {
             return;
         }
 
         fetchReport();
-    }, [report, fetchReport, reportIDFromRoute, isLoadingApp]);
+    }, [report, fetchReport, reportIDFromRoute, isLoadingApp, isInitialPageReady, isLinkedMessagePageReady]);
 
     const dismissBanner = useCallback(() => {
         setIsBannerVisible(false);
@@ -520,14 +536,26 @@ function ReportScreen({
 
     useEffect(() => {
         // Call OpenReport only if we are not linking to a message or the report is not available yet
-        if (isLoadingReportOnyx || (reportActionIDFromRoute && report.reportID)) {
+        if (isLoadingReportOnyx || (reportActionIDFromRoute && report.reportID && isLinkedMessagePageReady)) {
             return;
         }
 
         fetchReportIfNeeded();
         // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
     }, [isLoadingReportOnyx]);
 
+    useEffect(() => {
+        if (isLoadingReportOnyx || !reportActionIDFromRoute || isLinkedMessagePageReady) {
+            return;
+        }
+
+        // This function is triggered when a user clicks on a link to navigate to a report.
+        // For each link click, we retrieve the report data again, even though it may already be cached.
+        // There should be only one openReport execution per page start or navigating
+        fetchReportIfNeeded();
+        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
+    }, [route, isLinkedMessagePageReady, isLoadingReportOnyx, reportActionIDFromRoute]);
+
     // If a user has chosen to leave a thread, and then returns to it (e.g. with the back button), we need to call `openReport` again in order to allow the user to rejoin and to receive real-time updates
     useEffect(() => {
         if (!shouldUseNarrowLayout || !isFocused || prevIsFocused || !ReportUtils.isChatThread(report) || report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
@@ -705,7 +733,16 @@ function ReportScreen({
         Report.readNewestAction(report.reportID);
     }, [report]);
 
-    if ((!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) ?? (!shouldShowSkeleton && reportActionIDFromRoute && reportActions?.length === 0 && !isLinkingToMessage)) {
+    if (
+        (!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) ||
+        (shouldShowSkeleton &&
+            !reportMetadata.isLoadingInitialReportActions &&
+            reportActionIDFromRoute &&
+            sortedAllReportActions &&
+            sortedAllReportActions?.length > 0 &&
+            reportActions.length === 0 &&
+            !isLinkingToMessage)
+    ) {
         return (
             <BlockingView
                 icon={Illustrations.ToddBehindCloud}
@@ -768,7 +805,7 @@ function ReportScreen({
                                 onLayout={onListLayout}
                                 testID="report-actions-view-wrapper"
                             >
-                                {shouldShowReportActionList && (
+                                {!shouldShowSkeleton && (
                                     <ReportActionsView
                                         reportActions={reportActions}
                                         report={report}

src/pages/home/report/ReportActionsList.tsx

@@ -220,7 +220,7 @@ function ReportActionsList({
     const previousLastIndex = useRef(lastActionIndex);
 
     const isLastPendingActionIsDelete = sortedReportActions?.[0]?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
-    const linkedReportActionID = route.params?.reportActionID ?? '-1';
+    const linkedReportActionID = route?.params?.reportActionID ?? '-1';
 
     // This state is used to force a re-render when the user manually marks a message as unread
     // by using a timestamp you can force re-renders without having to worry about if another message was marked as unread before

src/pages/home/report/ReportActionsView.tsx

@@ -314,18 +314,6 @@ function ReportActionsView({
     const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]);
     const hasCreatedAction = oldestReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED;
 
-    useEffect(() => {
-        if (!reportActionID || indexOfLinkedAction > -1) {
-            return;
-        }
-
-        // This function is triggered when a user clicks on a link to navigate to a report.
-        // For each link click, we retrieve the report data again, even though it may already be cached.
-        // There should be only one openReport execution per page start or navigating
-        Report.openReport(reportID, reportActionID);
-        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
-    }, [route, indexOfLinkedAction]);
-
     useEffect(() => {
         const wasLoginChangedDetected = prevAuthTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS && !session?.authTokenType;
         if (wasLoginChangedDetected && didUserLogInDuringSession() && isUserCreatedPolicyRoom(report)) {

tests/ui/PaginationTest.tsx

@@ -281,13 +281,13 @@ describe('Pagination', () => {
     });
 
     it('opens a chat and load older messages', async () => {
-        mockOpenReport(5, '8');
+        mockOpenReport(CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT, '18');
         mockGetOlderActions(5);
 
         await signInAndGetApp();
         await navigateToSidebarOption(REPORT_ID);
 
-        expect(getReportActions()).toHaveLength(5);
+        expect(getReportActions()).toHaveLength(CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT);
         TestHelper.expectAPICommandToHaveBeenCalled('OpenReport', 1);
         TestHelper.expectAPICommandToHaveBeenCalledWith('OpenReport', 0, {reportID: REPORT_ID, reportActionID: ''});
         TestHelper.expectAPICommandToHaveBeenCalled('GetOlderActions', 0);
@@ -304,9 +304,9 @@ describe('Pagination', () => {
 
         await waitForBatchedUpdatesWithAct();
 
-        // We now have 8 messages. 5 from the initial OpenReport and 3 from GetOlderActions.
+        // We now have 18 messages. 15 (MIN_INITIAL_REPORT_ACTION_COUNT) from the initial OpenReport and 3 from GetOlderActions.
         // GetOlderActions only returns 3 actions since it reaches id '1', which is the created action.
-        expect(getReportActions()).toHaveLength(8);
+        expect(getReportActions()).toHaveLength(18);
     });
 
     // Currently broken on main by https://github.com/Expensify/App/pull/42582.