From b5627212762d9621852476036d5f472254d815e3 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Wed, 26 Jun 2024 09:43:32 +0700 Subject: [PATCH 1/8] Fix blocking view not found page displayed too early Signed-off-by: Tsaqif --- src/CONST.ts | 1 + src/pages/home/ReportScreen.tsx | 56 ++++++++++++++++----- src/pages/home/report/ReportActionsView.tsx | 12 ----- 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index f8ce1a574d49..1e848e9d0d91 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -652,6 +652,7 @@ const CONST = { MEMBER: 'member', }, MAX_COUNT_BEFORE_FOCUS_UPDATE: 30, + INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY: 15, SPLIT_REPORTID: '-2', ACTIONS: { LIMIT: 50, diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index f8f652d21acc..1b8191aa34ce 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -287,11 +287,21 @@ 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 isLinkedMessageAvailable = useMemo( - (): boolean => sortedAllReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)) > -1, + const indexOfLinkedMessage = useMemo( + (): number => sortedAllReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)), [sortedAllReportActions, 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 minimum amount (15) messages (count as 1 page) above it, to fill the screen. + // If it is too much (same as web pagination size/50) and there is no cached messages on the report, + // the OpenReport will be called each time user scroll up report a bit, click on reportpreview then go back + const isLinkedMessagePageReady = + isLinkedMessageAvailable && (sortedAllReportActions.length - indexOfLinkedMessage > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || 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'); const shouldHideReport = !hasHelpfulErrors && !ReportUtils.canAccessReport(report, policies, betas); @@ -380,15 +390,18 @@ function ReportScreen({ return reportIDFromRoute !== '' && !!report.reportID && !isTransitioning; }, [report, reportIDFromRoute]); + const isInitialPageReady = isOffline + ? reportActions.length > 0 + : reportActions.length > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || isPendingActionExist || doesCreatedActionExists(); const isLoading = isLoadingApp ?? (!reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty()); const shouldShowSkeleton = - !isLinkedMessageAvailable && - (isLinkingToMessage || - !isCurrentReportLoadedFromOnyx || - (reportActions.length === 0 && !!reportMetadata?.isLoadingInitialReportActions) || - isLoading || - (!!reportActionIDFromRoute && reportMetadata?.isLoadingInitialReportActions)); - const shouldShowReportActionList = isCurrentReportLoadedFromOnyx && !isLoading; + (isLinkingToMessage && !isLinkedMessagePageReady) || + !isCurrentReportLoadedFromOnyx || + (!isLinkingToMessage && !isInitialPageReady) || + isLoadingReportOnyx || + isLoading || + !reportIDFromRoute; + const currentReportIDFormRoute = route.params?.reportID; // eslint-disable-next-line rulesdir/no-negated-variables @@ -411,7 +424,6 @@ function ReportScreen({ if (shouldHideReport) { return true; } - return !!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute); }, [isLoadingApp, finishedLoadingApp, report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute]); @@ -513,6 +525,18 @@ function ReportScreen({ // eslint-disable-next-line 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-hooks/exhaustive-deps + }, [route, isLinkedMessagePageReady]); + // 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) { @@ -683,7 +707,15 @@ function ReportScreen({ }); }, [isInaccessibleWhisper]); - if ((!isInaccessibleWhisper && isLinkedReportActionDeleted) ?? (!shouldShowSkeleton && reportActionIDFromRoute && reportActions?.length === 0 && !isLinkingToMessage)) { + if ( + (!isInaccessibleWhisper && isLinkedReportActionDeleted) ?? + (shouldShowSkeleton && + !reportMetadata.isLoadingInitialReportActions && + reportActionIDFromRoute && + sortedAllReportActions.length > 0 && + reportActions.length === 0 && + !isLinkingToMessage) + ) { return ( - {shouldShowReportActionList && ( + {!shouldShowSkeleton && ( 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-hooks/exhaustive-deps - }, [route, indexOfLinkedAction]); - useEffect(() => { const wasLoginChangedDetected = prevAuthTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS && !session?.authTokenType; if (wasLoginChangedDetected && didUserLogInDuringSession() && isUserCreatedPolicyRoom(report)) { From d61422a0c5e010638db85f07ed2878a6ca8b7285 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Wed, 26 Jun 2024 20:56:19 +0700 Subject: [PATCH 2/8] Using a Single Source to Determine the Report ID from the Route in ReportScreen, ReportActionsView, and ReportActionsList Signed-off-by: Tsaqif --- src/pages/home/ReportScreen.tsx | 9 ++++++--- src/pages/home/report/ReportActionsList.tsx | 6 +++++- src/pages/home/report/ReportActionsView.tsx | 19 ++++++++++--------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 1b8191aa34ce..6758b05821aa 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -392,14 +392,16 @@ function ReportScreen({ const isInitialPageReady = isOffline ? reportActions.length > 0 - : reportActions.length > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || isPendingActionExist || doesCreatedActionExists(); + : reportActions.length > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || isPendingActionExist || (doesCreatedActionExists() && reportActions.length > 0); const isLoading = isLoadingApp ?? (!reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty()); + const shouldShowSkeleton = (isLinkingToMessage && !isLinkedMessagePageReady) || - !isCurrentReportLoadedFromOnyx || - (!isLinkingToMessage && !isInitialPageReady) || + !isInitialPageReady || isLoadingReportOnyx || + !isCurrentReportLoadedFromOnyx || isLoading || + (!!reportActionIDFromRoute && !!reportMetadata?.isLoadingInitialReportActions) || !reportIDFromRoute; const currentReportIDFormRoute = route.params?.reportID; @@ -781,6 +783,7 @@ function ReportScreen({ ; @@ -142,6 +145,7 @@ function ReportActionsList({ report, transactionThreadReport, reportActions = [], + reportActionIDFromRoute: linkedReportActionID = undefined, parentReportAction, isLoadingInitialReportActions = false, isLoadingOlderReportActions = false, @@ -167,6 +171,7 @@ function ReportActionsList({ const {isSmallScreenWidth, windowHeight} = useWindowDimensions(); const {isOffline} = useNetwork(); const route = useRoute>(); + const opacity = useSharedValue(0); const reportScrollManager = useReportScrollManager(); const userActiveSince = useRef(null); @@ -218,7 +223,6 @@ function ReportActionsList({ const previousLastIndex = useRef(lastActionIndex); const isLastPendingActionIsDelete = sortedReportActions?.[0]?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; - 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 diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index da6485ef9233..c3339c5fd18f 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -1,5 +1,4 @@ -import type {RouteProp} from '@react-navigation/native'; -import {useIsFocused, useRoute} from '@react-navigation/native'; +import {useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import {InteractionManager} from 'react-native'; @@ -12,7 +11,6 @@ import usePrevious from '@hooks/usePrevious'; import useWindowDimensions from '@hooks/useWindowDimensions'; import DateUtils from '@libs/DateUtils'; import getIsReportFullyVisible from '@libs/getIsReportFullyVisible'; -import type {AuthScreensParamList} from '@libs/Navigation/types'; import * as NumberUtils from '@libs/NumberUtils'; import {generateNewRandomInt} from '@libs/NumberUtils'; import Performance from '@libs/Performance'; @@ -26,7 +24,6 @@ import * as Report from '@userActions/Report'; import Timing from '@userActions/Timing'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import getInitialPaginationSize from './getInitialPaginationSize'; @@ -52,6 +49,9 @@ type ReportActionsViewProps = ReportActionsViewOnyxProps & { /** Array of report actions for this report */ reportActions?: OnyxTypes.ReportAction[]; + /** Linked report action ID */ + reportActionIDFromRoute?: string | undefined; + /** The report's parentReportAction */ parentReportAction: OnyxEntry; @@ -89,6 +89,7 @@ function ReportActionsView({ session, parentReportAction, reportActions: allReportActions = [], + reportActionIDFromRoute: reportActionID = undefined, transactionThreadReportActions = [], isLoadingInitialReportActions = false, isLoadingOlderReportActions = false, @@ -99,8 +100,6 @@ function ReportActionsView({ }: ReportActionsViewProps) { useCopySelectionHelper(); const reactionListRef = useContext(ReactionListContext); - const route = useRoute>(); - const reportActionID = route?.params?.reportActionID; const didLayout = useRef(false); const didLoadOlderChats = useRef(false); const didLoadNewerChats = useRef(false); @@ -139,7 +138,7 @@ function ReportActionsView({ useLayoutEffect(() => { setCurrentReportActionID(''); - }, [route]); + }, [reportActionID]); // Change the list ID only for comment linking to get the positioning right const listID = useMemo(() => { @@ -152,7 +151,7 @@ function ReportActionsView({ listOldID = newID; return newID; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [route, isLoadingInitialReportActions, reportActionID]); + }, [isLoadingInitialReportActions, reportActionID]); // Get a sorted array of reportActions for both the current report and the transaction thread report associated with this report (if there is one) // so that we display transaction-level and report-level report actions in order in the one-transaction view @@ -352,7 +351,7 @@ function ReportActionsView({ didLoadNewerChats.current = true; - if ((reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) { + if ((!!reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) { handleReportActionPagination({firstReportActionID: newestReportAction?.reportActionID}); } }, @@ -513,6 +512,7 @@ function ReportActionsView({ if (!reportActions.length) { return null; } + // AutoScroll is disabled when we do linking to a specific reportAction const shouldEnableAutoScroll = hasNewestReportAction && (!reportActionID || !isNavigatingToLinkedMessage); return ( @@ -521,6 +521,7 @@ function ReportActionsView({ report={report} transactionThreadReport={transactionThreadReport} reportActions={reportActions} + reportActionIDFromRoute={reportActionID} parentReportAction={parentReportAction} parentReportActionForTransactionThread={parentReportActionForTransactionThread} onLayout={recordTimeToMeasureItemLayout} From 6c8cdf8234dc4a0b1a675619bc7ab4ab670c3f55 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Tue, 9 Jul 2024 22:30:11 +0700 Subject: [PATCH 3/8] Change isLinkedMessagePageReady check condition and prevent fetchNewerMessage when there is hasNewestReportAction Signed-off-by: Tsaqif --- src/pages/home/ReportScreen.tsx | 24 ++++++++---------- src/pages/home/report/ReportActionsList.tsx | 6 +---- src/pages/home/report/ReportActionsView.tsx | 28 +++++++++++---------- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 21fc00cda133..c83083066d3f 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -319,19 +319,19 @@ function ReportScreen({ const isEmptyChat = useMemo(() => ReportUtils.isEmptyReport(report), [report]); const isOptimisticDelete = report.statusNum === CONST.REPORT.STATUS_NUM.CLOSED; const indexOfLinkedMessage = useMemo( - (): number => sortedAllReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)), - [sortedAllReportActions, reportActionIDFromRoute], + (): 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 minimum amount (15) messages (count as 1 page) above it, to fill the screen. - // If it is too much (same as web pagination size/50) and there is no cached messages on the report, - // the OpenReport will be called each time user scroll up report a bit, click on reportpreview then go back + // 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 && (sortedAllReportActions.length - indexOfLinkedMessage > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || doesCreatedActionExists()); + isLinkedMessageAvailable && (reportActions.length - indexOfLinkedMessage > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || 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'); @@ -424,7 +424,6 @@ function ReportScreen({ const isInitialPageReady = isOffline ? reportActions.length > 0 : reportActions.length > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || isPendingActionExist || (doesCreatedActionExists() && reportActions.length > 0); - const isLoading = isLoadingApp ?? (!reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty()); /** * Using logical OR operator because with nullish coalescing operator, when `isLoadingApp` is false, the right hand side of the operator @@ -435,11 +434,11 @@ function ReportScreen({ const isLoading = isLoadingApp || !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty(); const shouldShowSkeleton = (isLinkingToMessage && !isLinkedMessagePageReady) || - !isInitialPageReady || + (!!reportActionIDFromRoute && !!reportMetadata?.isLoadingInitialReportActions) || + (!isLinkingToMessage && !isInitialPageReady) || isLoadingReportOnyx || !isCurrentReportLoadedFromOnyx || - isLoading || - (!!reportActionIDFromRoute && !!reportMetadata?.isLoadingInitialReportActions); + isLoading; const currentReportIDFormRoute = route.params?.reportID; @@ -563,8 +562,8 @@ function ReportScreen({ // 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-hooks/exhaustive-deps - }, [route, isLinkedMessagePageReady]); + // 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(() => { @@ -821,7 +820,6 @@ function ReportScreen({ ; @@ -144,7 +141,6 @@ function ReportActionsList({ report, transactionThreadReport, reportActions = [], - reportActionIDFromRoute: linkedReportActionID = undefined, parentReportAction, isLoadingInitialReportActions = false, isLoadingOlderReportActions = false, @@ -170,7 +166,6 @@ function ReportActionsList({ const {isSmallScreenWidth, windowHeight} = useWindowDimensions(); const {isOffline} = useNetwork(); const route = useRoute>(); - const opacity = useSharedValue(0); const reportScrollManager = useReportScrollManager(); const userActiveSince = useRef(null); @@ -222,6 +217,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'; // 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 diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index 0cfe04898d6c..ee9e3ac7d938 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -1,4 +1,5 @@ -import {useIsFocused} from '@react-navigation/native'; +import type {RouteProp} from '@react-navigation/native'; +import {useIsFocused, useRoute} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import {InteractionManager} from 'react-native'; @@ -11,6 +12,7 @@ import usePrevious from '@hooks/usePrevious'; import useWindowDimensions from '@hooks/useWindowDimensions'; import DateUtils from '@libs/DateUtils'; import getIsReportFullyVisible from '@libs/getIsReportFullyVisible'; +import type {AuthScreensParamList} from '@libs/Navigation/types'; import * as NumberUtils from '@libs/NumberUtils'; import {generateNewRandomInt} from '@libs/NumberUtils'; import Performance from '@libs/Performance'; @@ -24,6 +26,7 @@ import * as Report from '@userActions/Report'; import Timing from '@userActions/Timing'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import getInitialPaginationSize from './getInitialPaginationSize'; @@ -49,9 +52,6 @@ type ReportActionsViewProps = ReportActionsViewOnyxProps & { /** Array of report actions for this report */ reportActions?: OnyxTypes.ReportAction[]; - /** Linked report action ID */ - reportActionIDFromRoute?: string | undefined; - /** The report's parentReportAction */ parentReportAction: OnyxEntry; @@ -89,7 +89,6 @@ function ReportActionsView({ session, parentReportAction, reportActions: allReportActions = [], - reportActionIDFromRoute: reportActionID = undefined, transactionThreadReportActions = [], isLoadingInitialReportActions = false, isLoadingOlderReportActions = false, @@ -101,6 +100,8 @@ function ReportActionsView({ }: ReportActionsViewProps) { useCopySelectionHelper(); const reactionListRef = useContext(ReactionListContext); + const route = useRoute>(); + const reportActionID = route?.params?.reportActionID; const didLayout = useRef(false); const didLoadOlderChats = useRef(false); const didLoadNewerChats = useRef(false); @@ -139,7 +140,7 @@ function ReportActionsView({ useLayoutEffect(() => { setCurrentReportActionID(''); - }, [reportActionID]); + }, [route]); // Change the list ID only for comment linking to get the positioning right const listID = useMemo(() => { @@ -153,7 +154,7 @@ function ReportActionsView({ listOldID = newID; return newID; // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [isLoadingInitialReportActions, reportActionID]); + }, [route, isLoadingInitialReportActions, reportActionID]); // When we are offline before opening an IOU/Expense report, // the total of the report and sometimes the expense aren't displayed because these actions aren't returned until `OpenReport` API is complete. @@ -293,24 +294,26 @@ function ReportActionsView({ const hasMoreCached = reportActions.length < combinedReportActions.length; const newestReportAction = useMemo(() => reportActions?.[0], [reportActions]); + const hasNewestReportAction = reportActions[0]?.created === report.lastVisibleActionCreated || reportActions[0]?.created === transactionThreadReport?.lastVisibleActionCreated; const handleReportActionPagination = useCallback( ({firstReportActionID}: {firstReportActionID: string}) => { // This function is a placeholder as the actual pagination is handled by visibleReportActions if (!hasMoreCached) { isFirstLinkedActionRender.current = false; - fetchNewerAction(newestReportAction); + if (!hasNewestReportAction) { + fetchNewerAction(newestReportAction); + } } if (isFirstLinkedActionRender.current) { isFirstLinkedActionRender.current = false; } setCurrentReportActionID(firstReportActionID); }, - [fetchNewerAction, hasMoreCached, newestReportAction], + [fetchNewerAction, hasMoreCached, newestReportAction, hasNewestReportAction], ); const mostRecentIOUReportActionID = useMemo(() => ReportActionsUtils.getMostRecentIOURequestActionID(reportActions), [reportActions]); const hasCachedActionOnFirstRender = useInitialValue(() => reportActions.length > 0); - const hasNewestReportAction = reportActions[0]?.created === report.lastVisibleActionCreated || reportActions[0]?.created === transactionThreadReport?.lastVisibleActionCreated; const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]); const hasCreatedAction = oldestReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED; @@ -415,7 +418,7 @@ function ReportActionsView({ didLoadNewerChats.current = true; - if ((!!reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) { + if ((!!reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded && !hasNewestReportAction)) { handleReportActionPagination({firstReportActionID: newestReportAction?.reportActionID}); } }, @@ -431,6 +434,7 @@ function ReportActionsView({ isFocused, hasLoadingNewerReportActionsError, hasMoreCached, + hasNewestReportAction, ], ); @@ -484,7 +488,6 @@ function ReportActionsView({ if (!reportActions.length) { return null; } - // AutoScroll is disabled when we do linking to a specific reportAction const shouldEnableAutoScroll = hasNewestReportAction && (!reportActionID || !isNavigatingToLinkedMessage); return ( @@ -493,7 +496,6 @@ function ReportActionsView({ report={report} transactionThreadReport={transactionThreadReport} reportActions={reportActions} - reportActionIDFromRoute={reportActionID} parentReportAction={parentReportAction} parentReportActionForTransactionThread={parentReportActionForTransactionThread} onLayout={recordTimeToMeasureItemLayout} From c897aa0f889b8fb55e9c0778d08a0286aa7cf8ba Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Tue, 9 Jul 2024 22:42:41 +0700 Subject: [PATCH 4/8] revert add check on fetchNewerActions Signed-off-by: Tsaqif --- src/pages/home/report/ReportActionsView.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index ee9e3ac7d938..658eba77cb4a 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -294,15 +294,12 @@ function ReportActionsView({ const hasMoreCached = reportActions.length < combinedReportActions.length; const newestReportAction = useMemo(() => reportActions?.[0], [reportActions]); - const hasNewestReportAction = reportActions[0]?.created === report.lastVisibleActionCreated || reportActions[0]?.created === transactionThreadReport?.lastVisibleActionCreated; const handleReportActionPagination = useCallback( ({firstReportActionID}: {firstReportActionID: string}) => { // This function is a placeholder as the actual pagination is handled by visibleReportActions if (!hasMoreCached) { isFirstLinkedActionRender.current = false; - if (!hasNewestReportAction) { - fetchNewerAction(newestReportAction); - } + fetchNewerAction(newestReportAction); } if (isFirstLinkedActionRender.current) { isFirstLinkedActionRender.current = false; @@ -314,6 +311,7 @@ function ReportActionsView({ const mostRecentIOUReportActionID = useMemo(() => ReportActionsUtils.getMostRecentIOURequestActionID(reportActions), [reportActions]); const hasCachedActionOnFirstRender = useInitialValue(() => reportActions.length > 0); + const hasNewestReportAction = reportActions[0]?.created === report.lastVisibleActionCreated || reportActions[0]?.created === transactionThreadReport?.lastVisibleActionCreated; const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]); const hasCreatedAction = oldestReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED; @@ -418,7 +416,7 @@ function ReportActionsView({ didLoadNewerChats.current = true; - if ((!!reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded && !hasNewestReportAction)) { + if ((!!reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) { handleReportActionPagination({firstReportActionID: newestReportAction?.reportActionID}); } }, @@ -434,7 +432,6 @@ function ReportActionsView({ isFocused, hasLoadingNewerReportActionsError, hasMoreCached, - hasNewestReportAction, ], ); From 3740265e8dcdbcb08595ddbb2bbc984f4b180665 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Wed, 10 Jul 2024 06:57:22 +0700 Subject: [PATCH 5/8] Fix lint, typecheck and test failures Signed-off-by: Tsaqif --- src/CONST.ts | 2 +- src/pages/home/ReportScreen.tsx | 5 ++--- src/pages/home/report/ReportActionsView.tsx | 4 ++-- tests/ui/PaginationTest.tsx | 8 ++++---- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 45c2a2abeb67..295ea7643f93 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -662,7 +662,7 @@ const CONST = { MEMBER: 'member', }, MAX_COUNT_BEFORE_FOCUS_UPDATE: 30, - INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY: 15, + MIN_INITIAL_REPORT_ACTION_COUNT: 15, SPLIT_REPORTID: '-2', ACTIONS: { LIMIT: 50, diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index c83083066d3f..98cc255c276a 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -330,8 +330,7 @@ function ReportScreen({ // 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.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || doesCreatedActionExists()); + 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'); @@ -423,7 +422,7 @@ function ReportScreen({ const isInitialPageReady = isOffline ? reportActions.length > 0 - : reportActions.length > CONST.REPORT.INITIAL_REPORT_ACTION_COUNT_TO_DISPLAY || isPendingActionExist || (doesCreatedActionExists() && 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 diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index 658eba77cb4a..44fc443c0903 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -306,7 +306,7 @@ function ReportActionsView({ } setCurrentReportActionID(firstReportActionID); }, - [fetchNewerAction, hasMoreCached, newestReportAction, hasNewestReportAction], + [fetchNewerAction, hasMoreCached, newestReportAction], ); const mostRecentIOUReportActionID = useMemo(() => ReportActionsUtils.getMostRecentIOURequestActionID(reportActions), [reportActions]); @@ -416,7 +416,7 @@ function ReportActionsView({ didLoadNewerChats.current = true; - if ((!!reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) { + if ((reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) { handleReportActionPagination({firstReportActionID: newestReportAction?.reportActionID}); } }, diff --git a/tests/ui/PaginationTest.tsx b/tests/ui/PaginationTest.tsx index d578ec245972..277f6b88e78f 100644 --- a/tests/ui/PaginationTest.tsx +++ b/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. From 2b522e833367f25f8993b8fc46153e0f644d4f06 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Thu, 11 Jul 2024 13:49:04 +0700 Subject: [PATCH 6/8] Fix opening report with reportActionID won't call OpenReport when the page is not ready Signed-off-by: Tsaqif --- src/pages/home/ReportScreen.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 98cc255c276a..09f8e434bb92 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -494,12 +494,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); @@ -544,7 +544,7 @@ 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; } From cbfc9ddb6b1da45a09b187f3856b6bfdbbb9b966 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Sat, 13 Jul 2024 08:55:05 +0700 Subject: [PATCH 7/8] fix prettier Signed-off-by: Tsaqif --- src/pages/home/ReportScreen.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 62b16b8fd29e..f44ea21e7fca 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -729,7 +729,7 @@ function ReportScreen({ }, [report]); if ( - (!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) || + (!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) || (shouldShowSkeleton && !reportMetadata.isLoadingInitialReportActions && reportActionIDFromRoute && From c49c935a57fb71015d61c39222ee19baa81a151e Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Sat, 13 Jul 2024 09:23:26 +0700 Subject: [PATCH 8/8] fix typecheck and lint errors Signed-off-by: Tsaqif --- src/hooks/usePaginatedReportActions.ts | 2 -- src/pages/home/ReportScreen.tsx | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 24e4cfc9ed47..918831f27545 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -30,8 +30,6 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { [reportActionID, sortedAllReportActions], ); - console.log({reportActions, sortedAllReportActions}); - return { reportActions, linkedAction, diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index f44ea21e7fca..9b4a1863cc79 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -314,7 +314,7 @@ function ReportScreen({ ); const isPendingActionExist = !!reportActions.at(0)?.pendingAction; - const doesCreatedActionExists = useCallback(() => !!sortedAllReportActions.findLast((action) => ReportActionsUtils.isCreatedAction(action)), [sortedAllReportActions]); + 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. @@ -733,7 +733,8 @@ function ReportScreen({ (shouldShowSkeleton && !reportMetadata.isLoadingInitialReportActions && reportActionIDFromRoute && - sortedAllReportActions.length > 0 && + sortedAllReportActions && + sortedAllReportActions?.length > 0 && reportActions.length === 0 && !isLinkingToMessage) ) {