diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 9f2744a058d1..9fc1224f96c0 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -135,6 +135,9 @@ const EmojiPicker = forwardRef((props, ref) => { }); }); return () => { + if (!emojiPopoverDimensionListener) { + return; + } emojiPopoverDimensionListener.remove(); }; }, [isEmojiPickerVisible, isSmallScreenWidth, emojiPopoverAnchorOrigin]); diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 22a1bc5441e6..58d7a9399533 100644 --- a/src/libs/actions/Report.js +++ b/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); } /** diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index eb555500a557..dd537959c91f 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/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}) => ( diff --git a/tests/ui/UnreadIndicatorsTest.js b/tests/ui/UnreadIndicatorsTest.js index b8f920954a34..ed44d3088ae0 100644 --- a/tests/ui/UnreadIndicatorsTest.js +++ b/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');