From b0c4771d04134ced5211167cc212cb84463d0280 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Fri, 15 Dec 2023 06:58:09 -0600 Subject: [PATCH 1/7] revert PR 32474 --- src/libs/ReportActionsUtils.ts | 2 +- src/pages/home/ReportScreen.js | 12 ++++-------- tests/unit/ReportActionsUtilsTest.js | 3 +-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 9a3099ba6c02..21c382346e57 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -375,7 +375,7 @@ function shouldReportActionBeVisible(reportAction: OnyxEntry, key: // All other actions are displayed except thread parents, deleted, or non-pending actions const isDeleted = isDeletedAction(reportAction); - const isPending = !!reportAction.pendingAction && !(!isNetworkOffline && reportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + const isPending = !!reportAction.pendingAction; return !isDeleted || isPending || isDeletedParentAction(reportAction) || isReversedTransaction(reportAction); } diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index fd5caeea24f4..dc087c96a3cc 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -16,7 +16,6 @@ import TaskHeaderActionButton from '@components/TaskHeaderActionButton'; import withCurrentReportID, {withCurrentReportIDDefaultProps, withCurrentReportIDPropTypes} from '@components/withCurrentReportID'; import withViewportOffsetTop from '@components/withViewportOffsetTop'; import useLocalize from '@hooks/useLocalize'; -import useNetwork from '@hooks/useNetwork'; import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; @@ -155,7 +154,6 @@ function ReportScreen({ const styles = useThemeStyles(); const {translate} = useLocalize(); const {isSmallScreenWidth} = useWindowDimensions(); - const {isOffline} = useNetwork(); const firstRenderRef = useRef(true); const flatListRef = useRef(); @@ -175,11 +173,8 @@ function ReportScreen({ const {addWorkspaceRoomOrChatPendingAction, addWorkspaceRoomOrChatErrors} = ReportUtils.getReportOfflinePendingActionAndErrors(report); const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}]; - // eslint-disable-next-line react-hooks/exhaustive-deps -- need to re-filter the report actions when network status changes - const filteredReportActions = useMemo(() => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true), [isOffline, reportActions]); - // There are no reportActions at all to display and we are still in the process of loading the next set of actions. - const isLoadingInitialReportActions = _.isEmpty(filteredReportActions) && reportMetadata.isLoadingInitialReportActions; + const isLoadingInitialReportActions = _.isEmpty(reportActions) && reportMetadata.isLoadingInitialReportActions; const isOptimisticDelete = lodashGet(report, 'statusNum') === CONST.REPORT.STATUS.CLOSED; @@ -437,7 +432,7 @@ function ReportScreen({ > {isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && ( `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`, canEvict: false, + selector: (reportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true), }, report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, diff --git a/tests/unit/ReportActionsUtilsTest.js b/tests/unit/ReportActionsUtilsTest.js index 545d442e4799..b8b6eb5e7673 100644 --- a/tests/unit/ReportActionsUtilsTest.js +++ b/tests/unit/ReportActionsUtilsTest.js @@ -288,7 +288,7 @@ describe('ReportActionsUtils', () => { expect(result).toStrictEqual(input); }); - it('should filter out deleted and delete-pending comments', () => { + it('should filter out deleted, non-pending comments', () => { const input = [ { created: '2022-11-13 22:27:01.825', @@ -312,7 +312,6 @@ describe('ReportActionsUtils', () => { ]; const result = ReportActionsUtils.getSortedReportActionsForDisplay(input); input.pop(); - input.pop(); expect(result).toStrictEqual(input); }); }); From 7aa66e9b669b0983ae03e85055f07cbf999ba66b Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Fri, 15 Dec 2023 06:59:15 -0600 Subject: [PATCH 2/7] fix: hide new marker for pending deleted action in online mode --- src/libs/ReportActionsUtils.ts | 11 +++++++++++ src/pages/home/report/ReportActionItem.js | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 21c382346e57..0f49cad2c9b9 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -379,6 +379,16 @@ function shouldReportActionBeVisible(reportAction: OnyxEntry, key: return !isDeleted || isPending || isDeletedParentAction(reportAction) || isReversedTransaction(reportAction); } +/** + * Checks if the new marker should be shown for the report action. + */ +function shouldShowNewMarker(reportAction: OnyxEntry): boolean { + if (!reportAction) { + return false; + } + return !(!isNetworkOffline && reportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); +} + /** * Checks if a reportAction is fit for display as report last action, meaning that * it satisfies shouldReportActionBeVisible, it's not whisper action and not deleted. @@ -816,6 +826,7 @@ export { isWhisperAction, isReimbursementQueuedAction, shouldReportActionBeVisible, + shouldShowNewMarker, shouldReportActionBeVisibleAsLastAction, hasRequestFromCurrentAccount, getFirstVisibleReportActionID, diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index a08f025e0530..f24701a2afbe 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -679,7 +679,7 @@ function ReportActionItem(props) { > {(hovered) => ( - {props.shouldDisplayNewMarker && } + {props.shouldDisplayNewMarker && ReportActionsUtils.shouldShowNewMarker(props.action) && } Date: Sat, 16 Dec 2023 05:09:51 -0600 Subject: [PATCH 3/7] fix move new marker logic to ReportActionsList --- src/libs/ReportActionsUtils.ts | 4 ++-- src/pages/home/report/ReportActionItem.js | 2 +- src/pages/home/report/ReportActionsList.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 0f49cad2c9b9..cb04f1e42ed4 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -382,7 +382,7 @@ function shouldReportActionBeVisible(reportAction: OnyxEntry, key: /** * Checks if the new marker should be shown for the report action. */ -function shouldShowNewMarker(reportAction: OnyxEntry): boolean { +function shouldHideNewMarker(reportAction: OnyxEntry): boolean { if (!reportAction) { return false; } @@ -826,7 +826,7 @@ export { isWhisperAction, isReimbursementQueuedAction, shouldReportActionBeVisible, - shouldShowNewMarker, + shouldHideNewMarker, shouldReportActionBeVisibleAsLastAction, hasRequestFromCurrentAccount, getFirstVisibleReportActionID, diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index f24701a2afbe..a08f025e0530 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -679,7 +679,7 @@ function ReportActionItem(props) { > {(hovered) => ( - {props.shouldDisplayNewMarker && ReportActionsUtils.shouldShowNewMarker(props.action) && } + {props.shouldDisplayNewMarker && } ), [report, linkedReportActionID, sortedReportActions, mostRecentIOUReportActionID, shouldHideThreadDividerLine, shouldDisplayNewMarker], From 4fb31f2255272236cc2e97dd0af5b6f92d8fe500 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Sat, 16 Dec 2023 05:15:40 -0600 Subject: [PATCH 4/7] fix: wrong logic to hide new marker --- src/libs/ReportActionsUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index cb04f1e42ed4..61a34f44200c 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -384,9 +384,9 @@ function shouldReportActionBeVisible(reportAction: OnyxEntry, key: */ function shouldHideNewMarker(reportAction: OnyxEntry): boolean { if (!reportAction) { - return false; + return true; } - return !(!isNetworkOffline && reportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + return !isNetworkOffline && reportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; } /** From 8e4bb461137c74ef00a0d7392866d8a34a488447 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Wed, 3 Jan 2024 09:15:00 -0600 Subject: [PATCH 5/7] fix: move should-hide logic to the shouldDisplayNewMarker --- src/pages/home/report/ReportActionsList.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index b0a50bdc4614..b51d61a0daf1 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -350,7 +350,7 @@ function ReportActionsList({ if (!currentUnreadMarker) { const nextMessage = sortedReportActions[index + 1]; const isCurrentMessageUnread = isMessageUnread(reportAction, lastReadTimeRef.current); - shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current)); + shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current)) && !ReportActionsUtils.shouldHideNewMarker(reportAction); if (shouldDisplay && !messageManuallyMarkedUnread) { const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true; // Prevent displaying a new marker line when report action is of type "REPORTPREVIEW" and last actor is the current user @@ -400,7 +400,7 @@ function ReportActionsList({ displayAsGroup={ReportActionsUtils.isConsecutiveActionMadeByPreviousActor(sortedReportActions, index)} mostRecentIOUReportActionID={mostRecentIOUReportActionID} shouldHideThreadDividerLine={shouldHideThreadDividerLine} - shouldDisplayNewMarker={shouldDisplayNewMarker(reportAction, index) && !ReportActionsUtils.shouldHideNewMarker(reportAction)} + shouldDisplayNewMarker={shouldDisplayNewMarker(reportAction, index)} /> ), [report, linkedReportActionID, sortedReportActions, mostRecentIOUReportActionID, shouldHideThreadDividerLine, shouldDisplayNewMarker], From 2dea8cd52d7e95ac875e5754255474f69b198419 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Thu, 4 Jan 2024 05:53:02 -0600 Subject: [PATCH 6/7] fix: comment for shouldHideNewMarker function --- src/libs/ReportActionsUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index e55532d6bad9..213e882993a3 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -380,7 +380,7 @@ function shouldReportActionBeVisible(reportAction: OnyxEntry, key: } /** - * Checks if the new marker should be shown for the report action. + * Checks if the new marker should be hidden for the report action. */ function shouldHideNewMarker(reportAction: OnyxEntry): boolean { if (!reportAction) { From a070efa6a5b4466663c4242761cf1fa2dd949813 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Thu, 4 Jan 2024 12:22:41 -0600 Subject: [PATCH 7/7] fix: get unread marker from visible actions --- src/pages/home/report/ReportActionsList.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index b51d61a0daf1..e06d8e30b11e 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -159,7 +159,10 @@ function ReportActionsList({ const lastVisibleActionCreatedRef = useRef(report.lastVisibleActionCreated); const lastReadTimeRef = useRef(report.lastReadTime); - const sortedVisibleReportActions = _.filter(sortedReportActions, (s) => isOffline || s.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || s.errors); + const sortedVisibleReportActions = useMemo( + () => _.filter(sortedReportActions, (s) => isOffline || s.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || s.errors), + [sortedReportActions, isOffline], + ); const lastActionIndex = lodashGet(sortedVisibleReportActions, [0, 'reportActionID']); const reportActionSize = useRef(sortedVisibleReportActions.length); @@ -348,7 +351,7 @@ function ReportActionsList({ (reportAction, index) => { let shouldDisplay = false; if (!currentUnreadMarker) { - const nextMessage = sortedReportActions[index + 1]; + const nextMessage = sortedVisibleReportActions[index + 1]; const isCurrentMessageUnread = isMessageUnread(reportAction, lastReadTimeRef.current); shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current)) && !ReportActionsUtils.shouldHideNewMarker(reportAction); if (shouldDisplay && !messageManuallyMarkedUnread) { @@ -367,7 +370,7 @@ function ReportActionsList({ return shouldDisplay; }, - [currentUnreadMarker, sortedReportActions, report.reportID, messageManuallyMarkedUnread], + [currentUnreadMarker, sortedVisibleReportActions, report.reportID, messageManuallyMarkedUnread], ); useEffect(() => { @@ -375,7 +378,7 @@ function ReportActionsList({ // This is to avoid a warning of: // Cannot update a component (ReportActionsList) while rendering a different component (CellRenderer). let markerFound = false; - _.each(sortedReportActions, (reportAction, index) => { + _.each(sortedVisibleReportActions, (reportAction, index) => { if (!shouldDisplayNewMarker(reportAction, index)) { return; } @@ -388,7 +391,7 @@ function ReportActionsList({ if (!markerFound) { setCurrentUnreadMarker(null); } - }, [sortedReportActions, report.lastReadTime, report.reportID, messageManuallyMarkedUnread, shouldDisplayNewMarker, currentUnreadMarker]); + }, [sortedVisibleReportActions, report.lastReadTime, report.reportID, messageManuallyMarkedUnread, shouldDisplayNewMarker, currentUnreadMarker]); const renderItem = useCallback( ({item: reportAction, index}) => (