Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated logic to handle unread messages case #25935

Merged
merged 43 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
fab7506
Updated logic to handle the shouldDisplayNewMarker
gedu Aug 25, 2023
47dd5cd
Sending an event when the user mark as unread a message
gedu Sep 14, 2023
922fbf3
lint fixes
gedu Sep 14, 2023
85dddae
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Sep 21, 2023
b9f1f17
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Sep 22, 2023
ec2e0fb
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Sep 27, 2023
3646848
Listening to the specific reports and clearing unread marker cache
gedu Sep 27, 2023
c71ecae
disable dependency hook, no need to listen to the messageManuallyMarked
gedu Sep 28, 2023
ebdeddb
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Sep 29, 2023
de5b8fe
removed console logs
gedu Sep 29, 2023
c483c10
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 3, 2023
8925515
fixed lint issues
gedu Oct 3, 2023
7149558
Moved the DeviceEventEmitter into the markCommentAsUnread function
gedu Oct 9, 2023
89d15d2
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 9, 2023
748c8ad
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 11, 2023
a434c3a
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 11, 2023
f40773b
Updated scrolling direction and amount
gedu Oct 13, 2023
c784057
Fix to add report into map
gedu Oct 18, 2023
51f8117
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 18, 2023
eee1e83
Fixed green line placement
gedu Oct 18, 2023
f1de924
fixed lint errors
gedu Oct 18, 2023
c8d4853
fixed lint issue
gedu Oct 18, 2023
c9722d9
Fixed test
gedu Oct 20, 2023
8782bf7
fixed lint error
gedu Oct 20, 2023
d3ddefd
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 23, 2023
cea0759
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 25, 2023
3ff86e2
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 30, 2023
3630dac
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Oct 30, 2023
57978b6
updated dependency array
gedu Oct 30, 2023
6546ba5
Prettier fixes
gedu Oct 30, 2023
6b2beed
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Nov 2, 2023
dc93b9e
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Nov 8, 2023
95cc451
Merge branch 'main' into edu/23171_manually_unread_marking
gedu Nov 14, 2023
cdb1b80
reverting changes
gedu Nov 14, 2023
5dde6f2
Fixed comments
gedu Nov 14, 2023
2b83ba6
fixed more comments
gedu Nov 15, 2023
c4541fb
improved the comments
gedu Nov 15, 2023
58e2f9c
added new line after pods
gedu Nov 15, 2023
12a92a7
explaining why we cache
gedu Nov 15, 2023
823f3b7
Update src/pages/home/report/ReportActionsList.js
gedu Nov 15, 2023
d1a347c
Merged main into edu/23171_manually_unread_marking
gedu Nov 16, 2023
0a578dd
Moved out the the prevReportID
gedu Nov 16, 2023
8bb02a4
commenting the new variable
gedu Nov 16, 2023
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
3 changes: 3 additions & 0 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,8 @@ function readNewestAction(reportID) {
*
* @param {String} reportID
* @param {String} reportActionCreated
*
* @returns {String} lastReadTime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird. I'm not sure if we're returning unrelated parameters from an action elsewhere. A better way would be to just use onyx to get lastReadTime from the report.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was for emitting the even, but I will move the emitting into the function, so it won't need to return it

*/
function markCommentAsUnread(reportID, reportActionCreated) {
// If no action created date is provided, use the last action's
Expand Down Expand Up @@ -843,6 +845,7 @@ function markCommentAsUnread(reportID, reportActionCreated) {
],
},
);
return lastReadTime;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/pages/home/report/ContextMenu/ContextMenuActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import _ from 'underscore';
import ExpensiMark from 'expensify-common/lib/ExpensiMark';
import lodashGet from 'lodash/get';
import {DeviceEventEmitter} from 'react-native';
import * as Expensicons from '../../../../components/Icon/Expensicons';
import * as Report from '../../../../libs/actions/Report';
import * as Download from '../../../../libs/actions/Download';
Expand Down Expand Up @@ -261,7 +262,8 @@ export default [
shouldShow: (type, reportAction, isArchivedRoom, betas, anchor, isChronosReport, reportID, isPinnedChat, isUnreadChat) =>
type === CONTEXT_MENU_TYPES.REPORT_ACTION || (type === CONTEXT_MENU_TYPES.REPORT && !isUnreadChat),
onPress: (closePopover, {reportAction, reportID}) => {
Report.markCommentAsUnread(reportID, reportAction.created);
const lastReadTime = Report.markCommentAsUnread(reportID, reportAction.created);
DeviceEventEmitter.emit('unreadAction', lastReadTime);
if (closePopover) {
hideContextMenu(true, ReportActionComposeFocusManager.focus);
}
Expand Down
73 changes: 40 additions & 33 deletions src/pages/home/report/ReportActionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,26 @@ import PropTypes from 'prop-types';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import _ from 'underscore';
import CONST from '../../../CONST';
import InvertedFlatList from '../../../components/InvertedFlatList';
import {DeviceEventEmitter} from 'react-native';
import compose from '../../../libs/compose';
import styles from '../../../styles/styles';
import * as ReportUtils from '../../../libs/ReportUtils';
import * as Report from '../../../libs/actions/Report';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsPropTypes, withCurrentUserPersonalDetailsDefaultProps} from '../../../components/withCurrentUserPersonalDetails';
import {withPersonalDetails} from '../../../components/OnyxProvider';
import ReportActionsSkeletonView from '../../../components/ReportActionsSkeletonView';
import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsDefaultProps, withCurrentUserPersonalDetailsPropTypes} from '../../../components/withCurrentUserPersonalDetails';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import variables from '../../../styles/variables';
import reportActionPropTypes from './reportActionPropTypes';
import CONST from '../../../CONST';
import InvertedFlatList from '../../../components/InvertedFlatList';
import useLocalize from '../../../hooks/useLocalize';
import useNetwork from '../../../hooks/useNetwork';
import useReportScrollManager from '../../../hooks/useReportScrollManager';
import DateUtils from '../../../libs/DateUtils';
import * as ReportUtils from '../../../libs/ReportUtils';
import * as Report from '../../../libs/actions/Report';
import compose from '../../../libs/compose';
import styles from '../../../styles/styles';
import variables from '../../../styles/variables';
import reportPropTypes from '../../reportPropTypes';
import FloatingMessageCounter from './FloatingMessageCounter';
import ReportActionsListItemRenderer from './ReportActionsListItemRenderer';
import reportActionPropTypes from './reportActionPropTypes';

const propTypes = {
/** The report currently being looked at */
Expand Down Expand Up @@ -68,10 +69,6 @@ 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
Expand Down Expand Up @@ -113,14 +110,16 @@ function ReportActionsList({
const {isOffline} = useNetwork();
const opacity = useSharedValue(0);
const userActiveSince = useRef(null);
const prevReportID = useRef(null);
const [currentUnreadMarker, setCurrentUnreadMarker] = useState(null);
const scrollingVerticalOffset = useRef(0);
const readActionSkipped = useRef(false);
const reportActionSize = useRef(sortedReportActions.length);
const lastReadRef = useRef(report.lastReadTime);
MonilBhavsar marked this conversation as resolved.
Show resolved Hide resolved

// Considering that renderItem is enclosed within a useCallback, marking it as "read" twice will retain the value as "true," preventing the useCallback from re-executing.
// However, if we create and listen to an object, it will lead to a new useCallback execution.
const [messageManuallyMarked, setMessageManuallyMarked] = useState({read: false});
const [messageManuallyMarked, setMessageManuallyMarked] = useState(0);
const [isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible] = useState(false);
const animatedStyles = useAnimatedStyle(() => ({
opacity: opacity.value,
Expand All @@ -135,16 +134,16 @@ function ReportActionsList({
// If the reportID changes, we reset the userActiveSince to null, we need to do it because
// the parent component is sending the previous reportID even when the user isn't active
// on the report
if (userActiveSince.current && prevReportID && prevReportID !== report.reportID) {
if (userActiveSince.current && prevReportID.current && prevReportID.current !== report.reportID) {
userActiveSince.current = null;
} else {
userActiveSince.current = DateUtils.getDBTime();
}
prevReportID = report.reportID;
prevReportID.current = report.reportID;
}, [report.reportID]);

useEffect(() => {
if (!userActiveSince.current || report.reportID !== prevReportID) {
if (!userActiveSince.current || report.reportID !== prevReportID.current) {
return;
}

Expand All @@ -166,19 +165,23 @@ function ReportActionsList({
}, [sortedReportActions.length, report.reportID]);

useEffect(() => {
const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report);
if (!didManuallyMarkReportAsUnread) {
setMessageManuallyMarked({read: false});
if (!userActiveSince.current || report.reportID !== prevReportID.current) {
return;
}

// Clearing the current unread marker so that it can be recalculated
setCurrentUnreadMarker(null);
setMessageManuallyMarked({read: true});
lastReadRef.current = report.lastReadTime;
setMessageManuallyMarked(0);
}, [report.lastReadTime, report.reportID]);

// We only care when a new lastReadTime is set in the report
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [report.lastReadTime]);
useEffect(() => {
const unreadActionSubscription = DeviceEventEmitter.addListener('unreadAction', (newLastReadTime) => {
setCurrentUnreadMarker(null);
lastReadRef.current = newLastReadTime;
setMessageManuallyMarked(new Date().getTime());
});

return () => unreadActionSubscription.remove();
}, []);

useEffect(() => {
// Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function?
Expand Down Expand Up @@ -278,16 +281,20 @@ function ReportActionsList({

MonilBhavsar marked this conversation as resolved.
Show resolved Hide resolved
if (!currentUnreadMarker) {
const nextMessage = sortedReportActions[index + 1];
const isCurrentMessageUnread = isMessageUnread(reportAction, report.lastReadTime);
shouldDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, report.lastReadTime);
const isCurrentMessageUnread = isMessageUnread(reportAction, lastReadRef.current);
let canDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, lastReadRef.current);

MonilBhavsar marked this conversation as resolved.
Show resolved Hide resolved
if (!messageManuallyMarked.read) {
shouldDisplayNewMarker = shouldDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID();
if (!messageManuallyMarked) {
canDisplayNewMarker = canDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID();
}
const canDisplayMarker = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;

if (!currentUnreadMarker && shouldDisplayNewMarker && canDisplayMarker) {
let isMessageInScope = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;
if (messageManuallyMarked) {
isMessageInScope = true;
}
if (!currentUnreadMarker && canDisplayNewMarker && isMessageInScope) {
setCurrentUnreadMarker(reportAction.reportActionID);
shouldDisplayNewMarker = true;
}
} else {
shouldDisplayNewMarker = reportAction.reportActionID === currentUnreadMarker;
Expand Down
Loading