Merge pull request Expensify#25935 from gedu/edu/23171_manually_unrea…

…d_marking

Updated logic to handle unread messages case

src/components/EmojiPicker/EmojiPicker.js

@@ -135,6 +135,9 @@ const EmojiPicker = forwardRef((props, ref) => {
             });
         });
         return () => {
+            if (!emojiPopoverDimensionListener) {
+                return;
+            }
             emojiPopoverDimensionListener.remove();
         };
     }, [isEmojiPickerVisible, isSmallScreenWidth, emojiPopoverAnchorOrigin]);

src/libs/actions/Report.js

@@ -3,7 +3,7 @@ import ExpensiMark from 'expensify-common/lib/ExpensiMark';
 import Str from 'expensify-common/lib/str';
 import lodashDebounce from 'lodash/debounce';
 import lodashGet from 'lodash/get';
-import {InteractionManager} from 'react-native';
+import {DeviceEventEmitter, InteractionManager} from 'react-native';
 import Onyx from 'react-native-onyx';
 import _ from 'underscore';
 import * as ActiveClientManager from '@libs/ActiveClientManager';
@@ -937,6 +937,7 @@ function markCommentAsUnread(reportID, reportActionCreated) {
             ],
         },
     );
+    DeviceEventEmitter.emit(`unreadAction_${reportID}`, lastReadTime);
 }
 
 /**

src/pages/home/report/ReportActionsList.js

@@ -2,6 +2,7 @@ import {useRoute} from '@react-navigation/native';
 import lodashGet from 'lodash/get';
 import PropTypes from 'prop-types';
 import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
+import {DeviceEventEmitter} from 'react-native';
 import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
 import _ from 'underscore';
 import InvertedFlatList from '@components/InvertedFlatList';
@@ -82,16 +83,23 @@ 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;
-
 // In the component we are subscribing to the arrival of new actions.
 // As there is the possibility that there are multiple instances of a ReportScreen
 // for the same report, we only ever want one subscription to be active, as
 // the subscriptions could otherwise be conflicting.
 const newActionUnsubscribeMap = {};
 
+// Caching the reportID and reportActionID for unread markers ensures persistent tracking
+// across multiple reports, preserving the green line placement and allowing retrieval
+// of the relevant reportActionID for displaying the green line.
+// We need to persist it across reports because there are at least 3 ReportScreen components created so the
+// internal states are resetted or recreated.
+const cacheUnreadMarkers = new Map();
+
+// 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;
+
 /**
  * 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
@@ -137,12 +145,21 @@ function ReportActionsList({
     const route = useRoute();
     const opacity = useSharedValue(0);
     const userActiveSince = useRef(null);
-    const [currentUnreadMarker, setCurrentUnreadMarker] = useState(null);
+    const unreadActionSubscription = useRef(null);
+    const markerInit = () => {
+        if (!cacheUnreadMarkers.has(report.reportID)) {
+            return null;
+        }
+        return cacheUnreadMarkers.get(report.reportID);
+    };
+    const [currentUnreadMarker, setCurrentUnreadMarker] = useState(markerInit);
     const scrollingVerticalOffset = useRef(0);
     const readActionSkipped = useRef(false);
     const hasHeaderRendered = useRef(false);
     const hasFooterRendered = useRef(false);
     const reportActionSize = useRef(sortedReportActions.length);
+    const lastReadTimeRef = useRef(report.lastReadTime);
+
     const linkedReportActionID = lodashGet(route, 'params.reportActionID', '');
 
     // This state is used to force a re-render when the user manually marks a message as unread
@@ -186,25 +203,41 @@ function ReportActionsList({
             return;
         }
 
+        cacheUnreadMarkers.delete(report.reportID);
         reportActionSize.current = sortedReportActions.length;
         setCurrentUnreadMarker(null);
         // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [sortedReportActions.length, report.reportID]);
 
     useEffect(() => {
-        const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report);
-        if (didManuallyMarkReportAsUnread) {
-            // Clearing the current unread marker so that it can be recalculated
-            setCurrentUnreadMarker(null);
-            setMessageManuallyMarkedUnread(new Date().getTime());
+        if (!userActiveSince.current || report.reportID !== prevReportID) {
             return;
         }
-
+        if (!messageManuallyMarkedUnread && lastReadTimeRef.current && lastReadTimeRef.current < report.lastReadTime) {
+            cacheUnreadMarkers.delete(report.reportID);
+        }
+        lastReadTimeRef.current = report.lastReadTime;
         setMessageManuallyMarkedUnread(0);
 
-        // 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]);
+
+    useEffect(() => {
+        // If the reportID changes, we reset the userActiveSince to null, we need to do it because
+        // this component doesn't unmount when the reportID changes
+        if (unreadActionSubscription.current) {
+            unreadActionSubscription.current.remove();
+            unreadActionSubscription.current = null;
+        }
+
+        // Listen to specific reportID for unread event and set the marker to new message
+        unreadActionSubscription.current = DeviceEventEmitter.addListener(`unreadAction_${report.reportID}`, (newLastReadTime) => {
+            cacheUnreadMarkers.delete(report.reportID);
+            lastReadTimeRef.current = newLastReadTime;
+            setCurrentUnreadMarker(null);
+            setMessageManuallyMarkedUnread(new Date().getTime());
+        });
+    }, [report.reportID]);
 
     useEffect(() => {
         // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function?
@@ -303,17 +336,21 @@ function ReportActionsList({
             let shouldDisplay = false;
             if (!currentUnreadMarker) {
                 const nextMessage = sortedReportActions[index + 1];
-                const isCurrentMessageUnread = isMessageUnread(reportAction, report.lastReadTime);
-                shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, report.lastReadTime));
+                const isCurrentMessageUnread = isMessageUnread(reportAction, lastReadTimeRef.current);
+                shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current));
                 if (!messageManuallyMarkedUnread) {
                     shouldDisplay = shouldDisplay && reportAction.actorAccountID !== Report.getCurrentUserAccountID();
                 }
+                if (shouldDisplay) {
+                    cacheUnreadMarkers.set(report.reportID, reportAction.reportActionID);
+                }
             } else {
                 shouldDisplay = reportAction.reportActionID === currentUnreadMarker;
             }
+
             return shouldDisplay;
         },
-        [currentUnreadMarker, sortedReportActions, report.lastReadTime, messageManuallyMarkedUnread],
+        [currentUnreadMarker, sortedReportActions, report.reportID, messageManuallyMarkedUnread],
     );
 
     useEffect(() => {
@@ -327,13 +364,14 @@ function ReportActionsList({
             }
             markerFound = true;
             if (!currentUnreadMarker && currentUnreadMarker !== reportAction.reportActionID) {
+                cacheUnreadMarkers.set(report.reportID, reportAction.reportActionID);
                 setCurrentUnreadMarker(reportAction.reportActionID);
             }
         });
         if (!markerFound) {
             setCurrentUnreadMarker(null);
         }
-    }, [sortedReportActions, report.lastReadTime, messageManuallyMarkedUnread, shouldDisplayNewMarker, currentUnreadMarker]);
+    }, [sortedReportActions, report.lastReadTime, report.reportID, messageManuallyMarkedUnread, shouldDisplayNewMarker, currentUnreadMarker]);
 
     const renderItem = useCallback(
         ({item: reportAction, index}) => (

tests/ui/UnreadIndicatorsTest.js

@@ -3,7 +3,7 @@ import {addSeconds, format, subMinutes, subSeconds} from 'date-fns';
 import {utcToZonedTime} from 'date-fns-tz';
 import lodashGet from 'lodash/get';
 import React from 'react';
-import {AppState, Linking} from 'react-native';
+import {AppState, DeviceEventEmitter, Linking} from 'react-native';
 import Onyx from 'react-native-onyx';
 import App from '../../src/App';
 import CONFIG from '../../src/CONFIG';
@@ -78,7 +78,7 @@ function scrollUpToRevealNewMessagesBadge() {
 function isNewMessagesBadgeVisible() {
     const hintText = Localize.translateLocal('accessibilityHints.scrollToNewestMessages');
     const badge = screen.queryByAccessibilityHint(hintText);
-    return Math.round(badge.props.style.transform[0].translateY) === 10;
+    return Math.round(badge.props.style.transform[0].translateY) === -40;
 }
 
 /**
@@ -249,8 +249,12 @@ describe('Unread Indicators', () => {
         signInAndGetAppWithUnreadChat()
             // Navigate to the unread chat from the sidebar
             .then(() => navigateToSidebarOption(0))
-            // Navigate to the unread chat from the sidebar
-            .then(() => navigateToSidebarOption(0))
+            .then(() => {
+                // Verify the unread indicator is present
+                const newMessageLineIndicatorHintText = Localize.translateLocal('accessibilityHints.newMessageLineIndicator');
+                const unreadIndicator = screen.queryAllByLabelText(newMessageLineIndicatorHintText);
+                expect(unreadIndicator).toHaveLength(1);
+            })
             .then(() => {
                 expect(areYouOnChatListScreen()).toBe(false);
                 // Then navigate back to the sidebar
@@ -259,9 +263,15 @@ describe('Unread Indicators', () => {
             .then(() => {
                 // Verify the LHN is now open
                 expect(areYouOnChatListScreen()).toBe(true);
+
                 // Tap on the chat again
                 return navigateToSidebarOption(0);
             })
+            .then(() => {
+                // Sending event to clear the unread indicator cache, given that the test doesn't behave as the app
+                DeviceEventEmitter.emit(`unreadAction_${REPORT_ID}`, format(new Date(), CONST.DATE.FNS_DB_FORMAT_STRING));
+                return waitForBatchedUpdatesWithAct();
+            })
             .then(() => {
                 // Verify the unread indicator is not present
                 const newMessageLineIndicatorHintText = Localize.translateLocal('accessibilityHints.newMessageLineIndicator');