diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index a24c6510f5ce..e048d32a237b 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -354,7 +354,10 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) // Currently, there's no programmatic way to dismiss the notification center panel. // To handle this, we use the 'referrer' parameter to check if the current navigation is triggered from a notification. const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION; - if ((isVisible || isFromNotification) && scrollingVerticalBottomOffset.current < CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD) { + const isScrolledToEnd = scrollingVerticalBottomOffset.current < CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD; + const shouldReadOnReportChange = ((isVisible && Visibility.hasFocus()) || isFromNotification) && isScrolledToEnd; + + if (shouldReadOnReportChange) { readNewestAction(report?.reportID, !!reportLoadingState?.hasOnceLoadedReportActions); if (isFromNotification) { Navigation.setParams({referrer: undefined}); @@ -363,11 +366,12 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) readActionSkipped.current = true; } } + // This effect should only run when the newest visible action changes, otherwise every action/report object update can prematurely consume unread state. // eslint-disable-next-line react-hooks/exhaustive-deps }, [report?.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, report?.reportID, isVisible, reportLoadingState?.hasOnceLoadedReportActions]); useEffect(() => { - if (!isVisible || !isFocused) { + if (!isVisible || !Visibility.hasFocus() || !isFocused) { if (!lastMessageTime.current) { lastMessageTime.current = lastAction?.created ?? ''; } @@ -395,6 +399,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) // is changed to visible(meaning user switched to app/web, while user was previously using different tab or application). // We will mark the report as read in the above case which marks the LHN report item as read while showing the new message // marker for the chat messages received while the user wasn't focused on the report or on another browser tab for web. + // This effect should only run when app visibility/focus changes; the helper reads the latest report/action values without making every action update mark the report as read. // eslint-disable-next-line react-hooks/exhaustive-deps }, [isFocused, isVisible, reportLoadingState?.hasOnceLoadedReportActions]); @@ -422,6 +427,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) isScrolledOverThreshold: scrollingVerticalBottomOffset.current >= CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD, isOffline, isReversed: true, + hasWindowFocus: Visibility.hasFocus(), }); const {isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible, trackVerticalScrolling, onViewableItemsChanged} = useReportUnreadMessageScrollTracking({ diff --git a/src/hooks/useMarkAsRead.ts b/src/hooks/useMarkAsRead.ts index e428a206e37c..9e64cf0d9ae2 100644 --- a/src/hooks/useMarkAsRead.ts +++ b/src/hooks/useMarkAsRead.ts @@ -105,8 +105,9 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible return; } const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION; + const shouldReadOnReportChange = ((isVisible && Visibility.hasFocus()) || isFromNotification) && !hasNewerActions && isScrolledToEnd; - if ((isVisible || isFromNotification) && !hasNewerActions && isScrolledToEnd) { + if (shouldReadOnReportChange) { readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); if (isFromNotification) { Navigation.setParams({referrer: undefined}); @@ -116,6 +117,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible } readActionSkippedRef.current = true; + // This effect should only run when the newest visible action changes, otherwise every action/report object update can prematurely consume unread state. // eslint-disable-next-line react-hooks/exhaustive-deps }, [report?.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, reportID, isVisible, reportLoadingState?.hasOnceLoadedReportActions]); @@ -128,7 +130,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible return; } - if (!isVisible || !isFocused) { + if (!isVisible || !Visibility.hasFocus() || !isFocused) { if (!lastMessageTime.current) { lastMessageTime.current = lastAction?.created ?? ''; } @@ -153,6 +155,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); userActiveSince.current = DateUtils.getDBTime(); + // This effect should only run when app visibility/focus changes; the helper reads the latest report/action values without making every action update mark the report as read. // eslint-disable-next-line react-hooks/exhaustive-deps }, [isVisible, isFocused, reportLoadingState?.hasOnceLoadedReportActions]); @@ -162,7 +165,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible }; const completeSkippedMarkAsRead = () => { - if (!readActionSkippedRef.current) { + if (!readActionSkippedRef.current || !Visibility.hasFocus()) { return; } markNewestActionAsRead(); diff --git a/src/hooks/useUnreadMarker.ts b/src/hooks/useUnreadMarker.ts index a6172bd43e6c..6dcdd4a46582 100644 --- a/src/hooks/useUnreadMarker.ts +++ b/src/hooks/useUnreadMarker.ts @@ -1,6 +1,7 @@ import {useEffect, useState} from 'react'; import {DeviceEventEmitter} from 'react-native'; import {wasMessageReceivedWhileOffline} from '@libs/ReportActionsUtils'; +import Visibility from '@libs/Visibility'; import {getUnreadMarkerReportAction} from '@pages/inbox/report/shouldDisplayNewMarkerOnReportAction'; import ONYXKEYS from '@src/ONYXKEYS'; import type * as OnyxTypes from '@src/types/onyx'; @@ -110,6 +111,7 @@ function useUnreadMarker({ isOffline, isReversed: false, isAnonymousUser, + hasWindowFocus: Visibility.hasFocus(), }); // Pagination is anchored to the oldest unread on first open; that anchor does not change when the user // marks read or unread, or when messages are deleted. Prefer the scan when it does not match that stale id. diff --git a/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts b/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts index 7f9106ced655..839a0e31cd12 100644 --- a/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts +++ b/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts @@ -26,6 +26,9 @@ type ShouldDisplayNewMarkerOnReportActionParams = { /** Whether the network is offline */ isOffline: boolean; + + /** Whether the app window is focused */ + hasWindowFocus?: boolean; }; /** @@ -41,6 +44,7 @@ const shouldDisplayNewMarkerOnReportAction = ({ prevSortedVisibleReportActionsObjects, isScrolledOverThreshold, isOffline, + hasWindowFocus = true, }: ShouldDisplayNewMarkerOnReportActionParams): boolean => { const isNextMessageUnread = !!nextMessage && isReportActionUnread(nextMessage, unreadMarkerTime); @@ -82,7 +86,7 @@ const shouldDisplayNewMarkerOnReportAction = ({ return !shouldIgnoreUnreadForCurrentUserMessage; } - return !isNewMessage || isScrolledOverThreshold; + return !isNewMessage || isScrolledOverThreshold || !hasWindowFocus; }; export default shouldDisplayNewMarkerOnReportAction; @@ -114,6 +118,9 @@ type GetUnreadMarkerReportActionParams = { /** Whether the current user is anonymous — skips the scan entirely */ isAnonymousUser?: boolean; + + /** Whether the app window is focused */ + hasWindowFocus?: boolean; }; /** @@ -130,6 +137,7 @@ const getUnreadMarkerReportAction = ({ isOffline, isReversed, isAnonymousUser = false, + hasWindowFocus = true, }: GetUnreadMarkerReportActionParams): [string | null, number] => { if (isAnonymousUser) { return [null, -1]; @@ -169,6 +177,7 @@ const getUnreadMarkerReportAction = ({ unreadMarkerTime, isScrolledOverThreshold, isOffline, + hasWindowFocus, }); if (shouldShowMarker) { diff --git a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts index 65e094bcefc2..3608dea2b7fe 100644 --- a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts +++ b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts @@ -132,7 +132,7 @@ export default function useReportUnreadMessageScrollTracking({ } // when the unread action scrolls into view, the consumer decides whether a skipped mark-as-read needs completing - if (unreadActionVisible) { + if (hasUnreadMarkerReportAction && unreadActionVisible) { ref.current.onUnreadActionVisible(); } diff --git a/tests/unit/useMarkAsReadTest.ts b/tests/unit/useMarkAsReadTest.ts index 0d737ce4f722..b44cdc6e00b3 100644 --- a/tests/unit/useMarkAsReadTest.ts +++ b/tests/unit/useMarkAsReadTest.ts @@ -10,6 +10,7 @@ const REPORT_ID = '1'; let mockIsUnread = true; let mockIsVisible = true; +let mockHasFocus = true; let mockIsFocused = true; let mockReferrer: string | undefined; @@ -17,6 +18,7 @@ jest.mock('@libs/Visibility', () => ({ __esModule: true, default: { isVisible: () => mockIsVisible, + hasFocus: () => mockHasFocus, onVisibilityChange: () => () => {}, }, })); @@ -77,6 +79,7 @@ describe('useMarkAsRead', () => { jest.clearAllMocks(); mockIsUnread = true; mockIsVisible = true; + mockHasFocus = true; mockIsFocused = true; mockReferrer = undefined; }); @@ -121,4 +124,13 @@ describe('useMarkAsRead', () => { expect(readNewestAction).toHaveBeenCalledWith(REPORT_ID, false); expect(NavigationMock.setParams).toHaveBeenCalledWith({referrer: undefined}); }); + + it('does not mark the report as read on report change when the app is visible but unfocused', () => { + mockHasFocus = false; + + renderMarkAsRead({isScrolledToEnd: true}); + + expect(readNewestAction).toHaveBeenCalledTimes(1); + expect(readNewestAction).toHaveBeenCalledWith(REPORT_ID, false); + }); });