Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand All @@ -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 ?? '';
}
Expand Down Expand Up @@ -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]);

Expand Down Expand Up @@ -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({
Expand Down
9 changes: 6 additions & 3 deletions src/hooks/useMarkAsRead.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand All @@ -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]);

Expand All @@ -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 ?? '';
}
Expand All @@ -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]);

Expand All @@ -162,7 +165,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible
};

const completeSkippedMarkAsRead = () => {
if (!readActionSkippedRef.current) {
if (!readActionSkippedRef.current || !Visibility.hasFocus()) {
return;
}
markNewestActionAsRead();
Expand Down
2 changes: 2 additions & 0 deletions src/hooks/useUnreadMarker.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 10 additions & 1 deletion src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type ShouldDisplayNewMarkerOnReportActionParams = {

/** Whether the network is offline */
isOffline: boolean;

/** Whether the app window is focused */
hasWindowFocus?: boolean;
};

/**
Expand All @@ -41,6 +44,7 @@ const shouldDisplayNewMarkerOnReportAction = ({
prevSortedVisibleReportActionsObjects,
isScrolledOverThreshold,
isOffline,
hasWindowFocus = true,
}: ShouldDisplayNewMarkerOnReportActionParams): boolean => {
const isNextMessageUnread = !!nextMessage && isReportActionUnread(nextMessage, unreadMarkerTime);

Expand Down Expand Up @@ -82,7 +86,7 @@ const shouldDisplayNewMarkerOnReportAction = ({
return !shouldIgnoreUnreadForCurrentUserMessage;
}

return !isNewMessage || isScrolledOverThreshold;
return !isNewMessage || isScrolledOverThreshold || !hasWindowFocus;
};

export default shouldDisplayNewMarkerOnReportAction;
Expand Down Expand Up @@ -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;
};

/**
Expand All @@ -130,6 +137,7 @@ const getUnreadMarkerReportAction = ({
isOffline,
isReversed,
isAnonymousUser = false,
hasWindowFocus = true,
}: GetUnreadMarkerReportActionParams): [string | null, number] => {
if (isAnonymousUser) {
return [null, -1];
Expand Down Expand Up @@ -169,6 +177,7 @@ const getUnreadMarkerReportAction = ({
unreadMarkerTime,
isScrolledOverThreshold,
isOffline,
hasWindowFocus,
});

if (shouldShowMarker) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
12 changes: 12 additions & 0 deletions tests/unit/useMarkAsReadTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ const REPORT_ID = '1';

let mockIsUnread = true;
let mockIsVisible = true;
let mockHasFocus = true;
let mockIsFocused = true;
let mockReferrer: string | undefined;

jest.mock('@libs/Visibility', () => ({
__esModule: true,
default: {
isVisible: () => mockIsVisible,
hasFocus: () => mockHasFocus,
onVisibilityChange: () => () => {},
},
}));
Expand Down Expand Up @@ -77,6 +79,7 @@ describe('useMarkAsRead', () => {
jest.clearAllMocks();
mockIsUnread = true;
mockIsVisible = true;
mockHasFocus = true;
mockIsFocused = true;
mockReferrer = undefined;
});
Expand Down Expand Up @@ -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);
});
});
Loading