From 55845ca42f7965fe2524ab27358b3f6f8d6bfb3e Mon Sep 17 00:00:00 2001 From: Edu Date: Wed, 23 Aug 2023 07:12:33 -0300 Subject: [PATCH 1/6] checking the last message changes --- src/pages/home/report/ReportActionsList.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 7f897ee825fb..84be0cf8a8e8 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -112,8 +112,7 @@ function ReportActionsList({ const currentUnreadMarker = useRef(null); const scrollingVerticalOffset = useRef(0); const readActionSkipped = useRef(false); - const reportActionSize = useRef(sortedReportActions.length); - + const lastMessage = sortedReportActions.length > 0 ? sortedReportActions[0] : null; // Considering that renderItem is enclosed within a useCallback, marking it as "read" twice will retain the value as "true," preventing the useCallback from re-executing. // However, if we create and listen to an object, it will lead to a new useCallback execution. const [messageManuallyMarked, setMessageManuallyMarked] = useState({read: false}); @@ -143,8 +142,9 @@ function ReportActionsList({ if (!userActiveSince.current || report.reportID !== prevReportID) { return; } + const isUnreadMessage = ReportUtils.isUnread(report); - if (ReportUtils.isUnread(report)) { + if (isUnreadMessage) { if (scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) { Report.readNewestAction(report.reportID); } else { @@ -152,14 +152,18 @@ function ReportActionsList({ } } - if (currentUnreadMarker.current || reportActionSize.current === sortedReportActions.length) { + // Deleted message is marked as 'deleted', if the last message is deleted, we have to remove the unread marker + if (!(isUnreadMessage && lastMessage && lastMessage.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !isOffline)) { + return; + } + + if (currentUnreadMarker.current) { return; } - reportActionSize.current = sortedReportActions.length; currentUnreadMarker.current = null; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [sortedReportActions.length, report.reportID]); + }, [lastMessage, report.reportID, isOffline]); useEffect(() => { const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report); From 5263c234be7fd78e9022566296182b3832fbe111 Mon Sep 17 00:00:00 2001 From: Edu Date: Wed, 23 Aug 2023 16:22:43 -0300 Subject: [PATCH 2/6] moving currentMarker out --- src/pages/home/report/ReportActionsList.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 84be0cf8a8e8..04f317952304 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -74,6 +74,8 @@ const MSG_VISIBLE_THRESHOLD = 250; // the useRef value gets reset when the reportID changes, so we use a global variable to keep track let prevReportID = null; +const currentUnreadMarker = {current: null}; + /** * Create a unique key for each action in the FlatList. * We use the reportActionID that is a string representation of a random 64-bit int, which should be @@ -109,7 +111,6 @@ function ReportActionsList({ const {isOffline} = useNetwork(); const opacity = useSharedValue(0); const userActiveSince = useRef(null); - const currentUnreadMarker = useRef(null); const scrollingVerticalOffset = useRef(0); const readActionSkipped = useRef(false); const lastMessage = sortedReportActions.length > 0 ? sortedReportActions[0] : null; @@ -135,6 +136,10 @@ function ReportActionsList({ } else { userActiveSince.current = DateUtils.getDBTime(); } + + if (prevReportID !== report.reportID) { + currentUnreadMarker.current = null; + } prevReportID = report.reportID; }, [report.reportID]); @@ -143,7 +148,6 @@ function ReportActionsList({ return; } const isUnreadMessage = ReportUtils.isUnread(report); - if (isUnreadMessage) { if (scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) { Report.readNewestAction(report.reportID); @@ -157,28 +161,27 @@ function ReportActionsList({ return; } - if (currentUnreadMarker.current) { - return; - } - currentUnreadMarker.current = null; // eslint-disable-next-line react-hooks/exhaustive-deps }, [lastMessage, report.reportID, isOffline]); useEffect(() => { + if (!userActiveSince.current || report.reportID !== prevReportID) { + return; + } const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report); + if (!didManuallyMarkReportAsUnread) { setMessageManuallyMarked({read: false}); return; } - // Clearing the current unread marker so that it can be recalculated currentUnreadMarker.current = null; setMessageManuallyMarked({read: true}); // We only care when a new lastReadTime is set in the report // eslint-disable-next-line react-hooks/exhaustive-deps - }, [report.lastReadTime]); + }, [report.lastReadTime, report.reportID]); /** * Show/hide the new floating message counter when user is scrolling back/forth in the history of messages. From a64a152892698c676084f86e5c70958b9575c1e5 Mon Sep 17 00:00:00 2001 From: Edu Date: Fri, 25 Aug 2023 09:55:04 -0300 Subject: [PATCH 3/6] updated logic to handle the shouldDisplayMarker --- src/pages/home/report/ReportActionsList.js | 37 +++++++++++----------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 04f317952304..b59403cf21fb 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -70,12 +70,6 @@ const defaultProps = { const VERTICAL_OFFSET_THRESHOLD = 200; const MSG_VISIBLE_THRESHOLD = 250; -// Seems that there is an architecture issue that prevents us from using the reportID with useRef -// the useRef value gets reset when the reportID changes, so we use a global variable to keep track -let prevReportID = null; - -const currentUnreadMarker = {current: null}; - /** * Create a unique key for each action in the FlatList. * We use the reportActionID that is a string representation of a random 64-bit int, which should be @@ -113,6 +107,8 @@ function ReportActionsList({ const userActiveSince = useRef(null); const scrollingVerticalOffset = useRef(0); const readActionSkipped = useRef(false); + const prevReportID = useRef(null); + const currentUnreadMarker = useRef(null); const lastMessage = sortedReportActions.length > 0 ? sortedReportActions[0] : null; // Considering that renderItem is enclosed within a useCallback, marking it as "read" twice will retain the value as "true," preventing the useCallback from re-executing. // However, if we create and listen to an object, it will lead to a new useCallback execution. @@ -131,20 +127,20 @@ function ReportActionsList({ // If the reportID changes, we reset the userActiveSince to null, we need to do it because // the parent component is sending the previous reportID even when the user isn't active // on the report - if (userActiveSince.current && prevReportID && prevReportID !== report.reportID) { + if (userActiveSince.current && prevReportID.current && prevReportID.current !== report.reportID) { userActiveSince.current = null; } else { userActiveSince.current = DateUtils.getDBTime(); } - if (prevReportID !== report.reportID) { - currentUnreadMarker.current = null; - } - prevReportID = report.reportID; + // if (prevReportID.current !== report.reportID) { + // currentUnreadMarker.current = null; + // } + prevReportID.current = report.reportID; }, [report.reportID]); useEffect(() => { - if (!userActiveSince.current || report.reportID !== prevReportID) { + if (!userActiveSince.current || report.reportID !== prevReportID.current) { return; } const isUnreadMessage = ReportUtils.isUnread(report); @@ -166,15 +162,15 @@ function ReportActionsList({ }, [lastMessage, report.reportID, isOffline]); useEffect(() => { - if (!userActiveSince.current || report.reportID !== prevReportID) { + if (!userActiveSince.current || report.reportID !== prevReportID.current) { return; } const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report); - if (!didManuallyMarkReportAsUnread) { setMessageManuallyMarked({read: false}); return; } + // Clearing the current unread marker so that it can be recalculated currentUnreadMarker.current = null; setMessageManuallyMarked({read: true}); @@ -235,15 +231,18 @@ function ReportActionsList({ if (!currentUnreadMarker.current) { const nextMessage = sortedReportActions[index + 1]; const isCurrentMessageUnread = isMessageUnread(reportAction, report.lastReadTime); - shouldDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, report.lastReadTime); + let canDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, report.lastReadTime); if (!messageManuallyMarked.read) { - shouldDisplayNewMarker = shouldDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID(); + canDisplayNewMarker = shouldDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID(); } - const canDisplayMarker = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true; - - if (!currentUnreadMarker.current && shouldDisplayNewMarker && canDisplayMarker) { + let isMessageInScope = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true; + if (messageManuallyMarked.read) { + isMessageInScope = true; + } + if (!currentUnreadMarker.current && canDisplayNewMarker && isMessageInScope) { currentUnreadMarker.current = reportAction.reportActionID; + shouldDisplayNewMarker = true; } } else { shouldDisplayNewMarker = reportAction.reportActionID === currentUnreadMarker.current; From 8b49c6f15873ea47b473ceb3fd64d1624edd018f Mon Sep 17 00:00:00 2001 From: Edu Date: Fri, 25 Aug 2023 10:24:37 -0300 Subject: [PATCH 4/6] updated logic --- src/pages/home/report/ReportActionsList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index b59403cf21fb..be91ad809d73 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -234,7 +234,7 @@ function ReportActionsList({ let canDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, report.lastReadTime); if (!messageManuallyMarked.read) { - canDisplayNewMarker = shouldDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID(); + canDisplayNewMarker = canDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID(); } let isMessageInScope = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true; if (messageManuallyMarked.read) { From 2fb7f33e11b278954bb32d6d44dafa84bfa9552c Mon Sep 17 00:00:00 2001 From: Eduardo Date: Wed, 13 Sep 2023 11:03:16 +0200 Subject: [PATCH 5/6] clean up --- src/pages/home/report/ReportActionsList.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index be91ad809d73..99464b9041e3 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -133,9 +133,6 @@ function ReportActionsList({ userActiveSince.current = DateUtils.getDBTime(); } - // if (prevReportID.current !== report.reportID) { - // currentUnreadMarker.current = null; - // } prevReportID.current = report.reportID; }, [report.reportID]); From 29d4c3d116b5da8cbff43e9005953e8c0cfbe056 Mon Sep 17 00:00:00 2001 From: Eduardo Date: Wed, 6 Mar 2024 10:12:23 -0300 Subject: [PATCH 6/6] keeping the unread marker when a message is deleted --- .../ContextMenu/PopoverReportActionContextMenu.tsx | 4 +++- src/pages/home/report/ReportActionsList.js | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.tsx b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.tsx index 47cbc559a67b..c052bc0c4435 100644 --- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.tsx +++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.tsx @@ -3,7 +3,7 @@ import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useRef, /* eslint-disable no-restricted-imports */ import type {EmitterSubscription, GestureResponderEvent, NativeTouchEvent, View} from 'react-native'; -import {Dimensions} from 'react-native'; +import {DeviceEventEmitter, Dimensions} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import ConfirmModal from '@components/ConfirmModal'; import PopoverWithMeasuredContent from '@components/PopoverWithMeasuredContent'; @@ -259,6 +259,8 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef { + if (cacheUnreadMarkers.get(report.reportID) !== reportActionID) { + return; + } + + setMessageManuallyMarkedUnread(new Date().getTime()); + }); + return () => { unreadActionSubscription.remove(); readNewestActionSubscription.remove(); + deletedReportActionSubscription.remove(); }; }, [report.reportID]);