From fa7023ee52a25b09b07da0131c96a06bfad5abb7 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 12 Oct 2024 21:57:19 +0800 Subject: [PATCH 1/9] fix unread marker shows when sends a new message --- src/pages/home/report/ReportActionsList.tsx | 29 ++++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index ce925d4375af..71cd01026890 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -183,6 +183,7 @@ function ReportActionsList({ const readActionSkipped = useRef(false); const hasHeaderRendered = useRef(false); const hasFooterRendered = useRef(false); + const lastActionAddedByCurrentUser = useRef(); const linkedReportActionID = route?.params?.reportActionID ?? '-1'; const sortedVisibleReportActions = useMemo( @@ -205,6 +206,7 @@ function ReportActionsList({ * - switches reports * - marks a message as read/unread * - reads a new message as it is received + * - sends a new message, update with both optimistic and BE created value */ const [unreadMarkerTime, setUnreadMarkerTime] = useState(report.lastReadTime ?? ''); useEffect(() => { @@ -213,6 +215,20 @@ function ReportActionsList({ // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [report.reportID]); + const lastAction = sortedVisibleReportActions.at(0); + useEffect(() => { + if (!lastActionAddedByCurrentUser.current || lastAction?.reportActionID !== lastActionAddedByCurrentUser.current) { + return; + } + setUnreadMarkerTime(lastAction.created); + + if (lastAction.isOptimisticAction) { + return; + } + + lastActionAddedByCurrentUser.current = undefined; + }, [lastAction]); + /** * The reportActionID the unread marker should display above */ @@ -223,7 +239,7 @@ function ReportActionsList({ const isNextMessageRead = !nextMessage || !isMessageUnread(nextMessage, unreadMarkerTime); const shouldDisplay = isCurrentMessageUnread && isNextMessageRead && !ReportActionsUtils.shouldHideNewMarker(reportAction); const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < (userActiveSince.current ?? '') : true; - return shouldDisplay && isWithinVisibleThreshold; + return shouldDisplay && isWithinVisibleThreshold && reportAction.reportActionID !== lastActionAddedByCurrentUser.current; }; // Scan through each visible report action until we find the appropriate action to show the unread marker @@ -245,9 +261,11 @@ function ReportActionsList({ const unreadActionSubscription = DeviceEventEmitter.addListener(`unreadAction_${report.reportID}`, (newLastReadTime: string) => { setUnreadMarkerTime(newLastReadTime); userActiveSince.current = DateUtils.getDBTime(); + lastActionAddedByCurrentUser.current = undefined; }); const readNewestActionSubscription = DeviceEventEmitter.addListener(`readNewestAction_${report.reportID}`, (newLastReadTime: string) => { setUnreadMarkerTime(newLastReadTime); + lastActionAddedByCurrentUser.current = undefined; }); return () => { @@ -339,19 +357,22 @@ function ReportActionsList({ }, []); const scrollToBottomForCurrentUserAction = useCallback( - (isFromCurrentUser: boolean) => { + (isFromCurrentUser: boolean, reportActionID: string | undefined) => { // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where // they are now in the list. if (!isFromCurrentUser) { return; } + if (!unreadMarkerReportActionID) { + lastActionAddedByCurrentUser.current = reportActionID; + } if (!hasNewestReportActionRef.current) { Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)); return; } InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom()); }, - [reportScrollManager, report.reportID], + [reportScrollManager, report.reportID, unreadMarkerReportActionID], ); useEffect(() => { // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function? @@ -382,7 +403,7 @@ function ReportActionsList({ return cleanup; // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [report.reportID]); + }, [report.reportID, scrollToBottomForCurrentUserAction]); /** * Show/hide the new floating message counter when user is scrolling back/forth in the history of messages. From e2c9845a78b217b2c7406ae643f3d68526194293 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 19 Oct 2024 16:15:21 +0800 Subject: [PATCH 2/9] fix unread marker shown when user send message --- src/libs/actions/Report.ts | 2 -- src/libs/actions/User.ts | 11 ++++++ src/pages/home/report/ReportActionsList.tsx | 39 +++++++++++---------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 7071c96f8612..a51704b47045 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2494,8 +2494,6 @@ function showReportActionNotification(reportID: string, reportAction: ReportActi } else { LocalNotification.showCommentNotification(report, reportAction, onClick); } - - notifyNewAction(reportID, reportAction.actorAccountID, reportAction.reportActionID); } /** Clear the errors associated with the IOUs of a given report. */ diff --git a/src/libs/actions/User.ts b/src/libs/actions/User.ts index 9ea29506accc..938d2c79cc55 100644 --- a/src/libs/actions/User.ts +++ b/src/libs/actions/User.ts @@ -902,6 +902,17 @@ function subscribeToUserEvents() { return; } + // Notify the subscriber that a new action has been received. This code was previously placed inside triggerNotifications, + // but it's actually unrelated to app notifications. We want to notify ReportActionsList before updating the onyx that a new action has been added, + // allowing us to avoid displaying an unread marker for messages from the current user. + pushJSON.forEach((update) => { + if (!update.key.startsWith(ONYXKEYS.COLLECTION.REPORT_ACTIONS)) { + return; + } + const reportID = update.key.replace(ONYXKEYS.COLLECTION.REPORT_ACTIONS, ''); + const reportActions = Object.values((update.value as OnyxCollection) ?? {}); + reportActions.forEach((action) => action && Report.notifyNewAction(reportID, action.actorAccountID, action.reportActionID)); + }); const onyxUpdatePromise = Onyx.update(pushJSON).then(() => { triggerNotifications(pushJSON); }); diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index d5df9c002473..11e7fd995a9e 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -198,6 +198,7 @@ function ReportActionsList({ ), [sortedReportActions, isOffline], ); + const lastAction = sortedVisibleReportActions.at(0); /** * The timestamp for the unread marker. @@ -215,20 +216,6 @@ function ReportActionsList({ // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [report.reportID]); - const lastAction = sortedVisibleReportActions.at(0); - useEffect(() => { - if (!lastActionAddedByCurrentUser.current || lastAction?.reportActionID !== lastActionAddedByCurrentUser.current) { - return; - } - setUnreadMarkerTime(lastAction.created); - - if (lastAction.isOptimisticAction) { - return; - } - - lastActionAddedByCurrentUser.current = undefined; - }, [lastAction]); - /** * The reportActionID the unread marker should display above */ @@ -279,13 +266,25 @@ function ReportActionsList({ * the latest report action. When new report actions are received and the user is not viewing them (they're above * the MSG_VISIBLE_THRESHOLD), the unread marker will display over those new messages rather than the initial * lastReadTime. + * + * However, if the new message is from the current user, we want to always push the unreadMarkerTime down to the timestamp of + * the latest report action, only if there is no existing unread marker. This is achieved by setting the lastActionAddedByCurrentUser + * whenever a new action from the current user is received (and there is no existing unread marker) which will avoid + * the new action to be marked as unread. */ useEffect(() => { if (unreadMarkerReportActionID) { return; } - const mostRecentReportActionCreated = sortedVisibleReportActions.at(0)?.created ?? ''; + // We only want to update the unread marker time to be the same as the new current user message once, + // but if the action is still an optimistic action, we want to keep lastActionAddedByCurrentUser because + // the action created value from the BE will be different. + if (!lastAction?.isOptimisticAction) { + lastActionAddedByCurrentUser.current = undefined; + } + + const mostRecentReportActionCreated = lastAction?.created ?? ''; if (mostRecentReportActionCreated <= unreadMarkerTime) { return; } @@ -293,15 +292,15 @@ function ReportActionsList({ setUnreadMarkerTime(mostRecentReportActionCreated); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [sortedVisibleReportActions]); + }, [lastAction?.created]); - const lastActionIndex = sortedVisibleReportActions.at(0)?.reportActionID; + const lastActionIndex = lastAction?.reportActionID; const reportActionSize = useRef(sortedVisibleReportActions.length); const lastVisibleActionCreated = (transactionThreadReport?.lastVisibleActionCreated ?? '') > (report.lastVisibleActionCreated ?? '') ? transactionThreadReport?.lastVisibleActionCreated : report.lastVisibleActionCreated; - const hasNewestReportAction = sortedVisibleReportActions.at(0)?.created === lastVisibleActionCreated; + const hasNewestReportAction = lastAction?.created === lastVisibleActionCreated; const hasNewestReportActionRef = useRef(hasNewestReportAction); hasNewestReportActionRef.current = hasNewestReportAction; const previousLastIndex = useRef(lastActionIndex); @@ -368,6 +367,8 @@ function ReportActionsList({ return; } if (!unreadMarkerReportActionID) { + // This tells the component that the unread marker time needs to be updated with the last action created time, + // but only if there is no existing unread marker so we don't override it. lastActionAddedByCurrentUser.current = reportActionID; } if (!hasNewestReportActionRef.current) { @@ -496,7 +497,7 @@ function ReportActionsList({ if (!isVisible || !isFocused) { if (!lastMessageTime.current) { - lastMessageTime.current = sortedVisibleReportActions.at(0)?.created ?? ''; + lastMessageTime.current = lastAction?.created ?? ''; } return; } From ebbf3faf52609983b252d18bd8251c9c2b6063da Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 19 Oct 2024 22:18:38 +0800 Subject: [PATCH 3/9] prettier --- src/pages/home/report/ReportActionsList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 11e7fd995a9e..91eae2017718 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -266,7 +266,7 @@ function ReportActionsList({ * the latest report action. When new report actions are received and the user is not viewing them (they're above * the MSG_VISIBLE_THRESHOLD), the unread marker will display over those new messages rather than the initial * lastReadTime. - * + * * However, if the new message is from the current user, we want to always push the unreadMarkerTime down to the timestamp of * the latest report action, only if there is no existing unread marker. This is achieved by setting the lastActionAddedByCurrentUser * whenever a new action from the current user is received (and there is no existing unread marker) which will avoid From d6851380d4c3f0fb1a8c79003149abd555cf0541 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Mon, 21 Oct 2024 10:51:19 +0800 Subject: [PATCH 4/9] don't show unread marker if there is a pending unread marker update from current user message --- src/pages/home/report/ReportActionsList.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 91eae2017718..5e8d6190f1d4 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -226,7 +226,7 @@ function ReportActionsList({ const isNextMessageRead = !nextMessage || !isMessageUnread(nextMessage, unreadMarkerTime); const shouldDisplay = isCurrentMessageUnread && isNextMessageRead && !ReportActionsUtils.shouldHideNewMarker(reportAction); const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < (userActiveSince.current ?? '') : true; - return shouldDisplay && isWithinVisibleThreshold && reportAction.reportActionID !== lastActionAddedByCurrentUser.current; + return shouldDisplay && isWithinVisibleThreshold && !lastActionAddedByCurrentUser.current; }; // Scan through each visible report action until we find the appropriate action to show the unread marker @@ -280,7 +280,7 @@ function ReportActionsList({ // We only want to update the unread marker time to be the same as the new current user message once, // but if the action is still an optimistic action, we want to keep lastActionAddedByCurrentUser because // the action created value from the BE will be different. - if (!lastAction?.isOptimisticAction) { + if (!lastAction?.isOptimisticAction && lastActionAddedByCurrentUser.current === lastAction?.reportActionID) { lastActionAddedByCurrentUser.current = undefined; } From e60bf62e32d4c916ef8681b94b01f88094f62b4e Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 24 Oct 2024 16:03:14 +0800 Subject: [PATCH 5/9] revert --- src/libs/actions/Report.ts | 2 ++ src/libs/actions/User.ts | 11 -------- src/pages/home/report/ReportActionsList.tsx | 29 +++------------------ 3 files changed, 6 insertions(+), 36 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index af354f5306ec..4af2357fc572 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2505,6 +2505,8 @@ function showReportActionNotification(reportID: string, reportAction: ReportActi } else { LocalNotification.showCommentNotification(report, reportAction, onClick); } + + notifyNewAction(reportID, reportAction.actorAccountID, reportAction.reportActionID); } /** Clear the errors associated with the IOUs of a given report. */ diff --git a/src/libs/actions/User.ts b/src/libs/actions/User.ts index 1e1c3628ae21..754563b57429 100644 --- a/src/libs/actions/User.ts +++ b/src/libs/actions/User.ts @@ -898,17 +898,6 @@ function subscribeToUserEvents() { return; } - // Notify the subscriber that a new action has been received. This code was previously placed inside triggerNotifications, - // but it's actually unrelated to app notifications. We want to notify ReportActionsList before updating the onyx that a new action has been added, - // allowing us to avoid displaying an unread marker for messages from the current user. - pushJSON.forEach((update) => { - if (!update.key.startsWith(ONYXKEYS.COLLECTION.REPORT_ACTIONS)) { - return; - } - const reportID = update.key.replace(ONYXKEYS.COLLECTION.REPORT_ACTIONS, ''); - const reportActions = Object.values((update.value as OnyxCollection) ?? {}); - reportActions.forEach((action) => action && Report.notifyNewAction(reportID, action.actorAccountID, action.reportActionID)); - }); const onyxUpdatePromise = Onyx.update(pushJSON).then(() => { triggerNotifications(pushJSON); }); diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index aba9eb34bdcb..2a6c8cf36113 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -183,7 +183,6 @@ function ReportActionsList({ const readActionSkipped = useRef(false); const hasHeaderRendered = useRef(false); const hasFooterRendered = useRef(false); - const lastActionAddedByCurrentUser = useRef(); const linkedReportActionID = route?.params?.reportActionID ?? '-1'; const sortedVisibleReportActions = useMemo( @@ -207,7 +206,6 @@ function ReportActionsList({ * - switches reports * - marks a message as read/unread * - reads a new message as it is received - * - sends a new message, update with both optimistic and BE created value */ const [unreadMarkerTime, setUnreadMarkerTime] = useState(report.lastReadTime ?? ''); useEffect(() => { @@ -226,7 +224,7 @@ function ReportActionsList({ const isNextMessageRead = !nextMessage || !isMessageUnread(nextMessage, unreadMarkerTime); const shouldDisplay = isCurrentMessageUnread && isNextMessageRead && !ReportActionsUtils.shouldHideNewMarker(reportAction); const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < (userActiveSince.current ?? '') : true; - return shouldDisplay && isWithinVisibleThreshold && !lastActionAddedByCurrentUser.current; + return shouldDisplay && isWithinVisibleThreshold; }; // Scan through each visible report action until we find the appropriate action to show the unread marker @@ -248,11 +246,9 @@ function ReportActionsList({ const unreadActionSubscription = DeviceEventEmitter.addListener(`unreadAction_${report.reportID}`, (newLastReadTime: string) => { setUnreadMarkerTime(newLastReadTime); userActiveSince.current = DateUtils.getDBTime(); - lastActionAddedByCurrentUser.current = undefined; }); const readNewestActionSubscription = DeviceEventEmitter.addListener(`readNewestAction_${report.reportID}`, (newLastReadTime: string) => { setUnreadMarkerTime(newLastReadTime); - lastActionAddedByCurrentUser.current = undefined; }); return () => { @@ -266,24 +262,12 @@ function ReportActionsList({ * the latest report action. When new report actions are received and the user is not viewing them (they're above * the MSG_VISIBLE_THRESHOLD), the unread marker will display over those new messages rather than the initial * lastReadTime. - * - * However, if the new message is from the current user, we want to always push the unreadMarkerTime down to the timestamp of - * the latest report action, only if there is no existing unread marker. This is achieved by setting the lastActionAddedByCurrentUser - * whenever a new action from the current user is received (and there is no existing unread marker) which will avoid - * the new action to be marked as unread. */ useEffect(() => { if (unreadMarkerReportActionID) { return; } - // We only want to update the unread marker time to be the same as the new current user message once, - // but if the action is still an optimistic action, we want to keep lastActionAddedByCurrentUser because - // the action created value from the BE will be different. - if (!lastAction?.isOptimisticAction && lastActionAddedByCurrentUser.current === lastAction?.reportActionID) { - lastActionAddedByCurrentUser.current = undefined; - } - const mostRecentReportActionCreated = lastAction?.created ?? ''; if (mostRecentReportActionCreated <= unreadMarkerTime) { return; @@ -360,17 +344,12 @@ function ReportActionsList({ }, []); const scrollToBottomForCurrentUserAction = useCallback( - (isFromCurrentUser: boolean, reportActionID: string | undefined) => { + (isFromCurrentUser: boolean) => { // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where // they are now in the list. if (!isFromCurrentUser) { return; } - if (!unreadMarkerReportActionID) { - // This tells the component that the unread marker time needs to be updated with the last action created time, - // but only if there is no existing unread marker so we don't override it. - lastActionAddedByCurrentUser.current = reportActionID; - } if (!hasNewestReportActionRef.current) { if (isInNarrowPaneModal) { return; @@ -380,7 +359,7 @@ function ReportActionsList({ } InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom()); }, - [isInNarrowPaneModal, reportScrollManager, report.reportID, unreadMarkerReportActionID], + [isInNarrowPaneModal, reportScrollManager, report.reportID], ); useEffect(() => { // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function? @@ -411,7 +390,7 @@ function ReportActionsList({ return cleanup; // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [report.reportID, scrollToBottomForCurrentUserAction]); + }, [report.reportID]); /** * Show/hide the new floating message counter when user is scrolling back/forth in the history of messages. From 15d4a5757d1e474b7764c2974466208d8bfe68eb Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 24 Oct 2024 16:08:04 +0800 Subject: [PATCH 6/9] don't show unread marker for new current user message --- src/pages/home/report/ReportActionsList.tsx | 24 +++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 2a6c8cf36113..8f30016c63c2 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -14,6 +14,7 @@ import {usePersonalDetails} from '@components/OnyxProvider'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; +import usePrevious from '@hooks/usePrevious'; import useReportScrollManager from '@hooks/useReportScrollManager'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -170,6 +171,7 @@ function ReportActionsList({ const isFocused = useIsFocused(); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`); + const [accountID] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.accountID}); useEffect(() => { const unsubscriber = Visibility.onVisibilityChange(() => { @@ -198,6 +200,15 @@ function ReportActionsList({ [sortedReportActions, isOffline], ); const lastAction = sortedVisibleReportActions.at(0); + const sortedVisibleReportActionsObjects: OnyxTypes.ReportActions = useMemo( + () => + sortedVisibleReportActions.reduce((acc, action) => { + Object.assign(acc, {[action.reportActionID]: action}); + return acc; + }, {}), + [sortedVisibleReportActions], + ); + const prevSortedVisibleReportActionsObjects = usePrevious(sortedVisibleReportActionsObjects); /** * The timestamp for the unread marker. @@ -214,6 +225,7 @@ function ReportActionsList({ // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [report.reportID]); + const prevUnreadMarkerReportActionID = useRef(null); /** * The reportActionID the unread marker should display above */ @@ -224,7 +236,14 @@ function ReportActionsList({ const isNextMessageRead = !nextMessage || !isMessageUnread(nextMessage, unreadMarkerTime); const shouldDisplay = isCurrentMessageUnread && isNextMessageRead && !ReportActionsUtils.shouldHideNewMarker(reportAction); const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < (userActiveSince.current ?? '') : true; - return shouldDisplay && isWithinVisibleThreshold; + + // Don't set unread marker for newly added message from the current user, only if there is no existing unread marker + const isFromCurrentUser = accountID === (ReportActionsUtils.isReportPreviewAction(reportAction) ? !reportAction.childLastActorAccountID : reportAction.actorAccountID); + const isNewMessage = !prevSortedVisibleReportActionsObjects[reportAction.reportActionID]; + const isPreviouslyOptimistic = !!prevSortedVisibleReportActionsObjects[reportAction.reportActionID]?.isOptimisticAction && !reportAction.isOptimisticAction; + const shouldIgnoreUnreadForCurrentUserMessage = !prevUnreadMarkerReportActionID.current && isFromCurrentUser && (isNewMessage || isPreviouslyOptimistic); + + return shouldDisplay && isWithinVisibleThreshold && !shouldIgnoreUnreadForCurrentUserMessage; }; // Scan through each visible report action until we find the appropriate action to show the unread marker @@ -237,7 +256,8 @@ function ReportActionsList({ } return null; - }, [sortedVisibleReportActions, unreadMarkerTime]); + }, [sortedVisibleReportActions, unreadMarkerTime, accountID]); + prevUnreadMarkerReportActionID.current = unreadMarkerReportActionID; /** * Subscribe to read/unread events and update our unreadMarkerTime From d6a8b8be6d0b65f8857bc02bb1bd225eac3b704e Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 24 Oct 2024 16:08:51 +0800 Subject: [PATCH 7/9] lint --- src/pages/home/report/ReportActionsList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 8f30016c63c2..e9df0598a3ec 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -256,7 +256,7 @@ function ReportActionsList({ } return null; - }, [sortedVisibleReportActions, unreadMarkerTime, accountID]); + }, [sortedVisibleReportActions, unreadMarkerTime, accountID, prevSortedVisibleReportActionsObjects]); prevUnreadMarkerReportActionID.current = unreadMarkerReportActionID; /** From 3252a3e688580269ec1e594c9ba52fac049cdfc3 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 25 Oct 2024 22:50:40 +0800 Subject: [PATCH 8/9] rename --- src/pages/home/report/ReportActionsList.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index e9df0598a3ec..95bc9d7c3878 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -202,9 +202,9 @@ function ReportActionsList({ const lastAction = sortedVisibleReportActions.at(0); const sortedVisibleReportActionsObjects: OnyxTypes.ReportActions = useMemo( () => - sortedVisibleReportActions.reduce((acc, action) => { - Object.assign(acc, {[action.reportActionID]: action}); - return acc; + sortedVisibleReportActions.reduce((actions, action) => { + Object.assign(actions, {[action.reportActionID]: action}); + return actions; }, {}), [sortedVisibleReportActions], ); From c275f3938a42d3b0766536de9364dfa990967a5d Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 25 Oct 2024 23:25:15 +0800 Subject: [PATCH 9/9] update comment --- src/pages/home/report/ReportActionsList.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 95bc9d7c3878..9329fbdc55db 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -237,9 +237,13 @@ function ReportActionsList({ const shouldDisplay = isCurrentMessageUnread && isNextMessageRead && !ReportActionsUtils.shouldHideNewMarker(reportAction); const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < (userActiveSince.current ?? '') : true; - // Don't set unread marker for newly added message from the current user, only if there is no existing unread marker + // If no unread marker exists, don't set an unread marker for newly added messages from the current user. const isFromCurrentUser = accountID === (ReportActionsUtils.isReportPreviewAction(reportAction) ? !reportAction.childLastActorAccountID : reportAction.actorAccountID); const isNewMessage = !prevSortedVisibleReportActionsObjects[reportAction.reportActionID]; + // The unread marker will show if the action's `created` time is later than `unreadMarkerTime`. + // The `unreadMarkerTime` has already been updated to match the optimistic action created time, + // but once the new action is saved on the backend, the actual created time will be later than the optimistic one. + // Therefore, we also need to prevent the unread marker from appearing for previously optimistic actions. const isPreviouslyOptimistic = !!prevSortedVisibleReportActionsObjects[reportAction.reportActionID]?.isOptimisticAction && !reportAction.isOptimisticAction; const shouldIgnoreUnreadForCurrentUserMessage = !prevUnreadMarkerReportActionID.current && isFromCurrentUser && (isNewMessage || isPreviouslyOptimistic);