diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 4860af2cfe60..b497bdd45bf3 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -387,14 +387,19 @@ function setLocalIOUReportData(iouReportObject, chatReportID) { * Update the lastRead actionID and timestamp in local memory and Onyx * * @param {Number} reportID - * @param {Number} sequenceNumber + * @param {Number} lastReadSequenceNumber */ -function setLocalLastRead(reportID, sequenceNumber) { - lastReadSequenceNumbers[reportID] = sequenceNumber; +function setLocalLastRead(reportID, lastReadSequenceNumber) { + lastReadSequenceNumbers[reportID] = lastReadSequenceNumber; + const reportMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; + + // Determine the number of unread actions by deducting the last read sequence from the total. If, for some reason, + // the last read sequence is higher than the actual last sequence, let's just assume all actions are read + const unreadActionCount = Math.max(reportMaxSequenceNumber - lastReadSequenceNumber, 0); - // Update the report optimistically + // Update the report optimistically. Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - unreadActionCount: 0, + unreadActionCount, lastVisitedTimestamp: Date.now(), }); } @@ -929,21 +934,32 @@ function addAction(reportID, text, file) { * network layer handle the delayed write. * * @param {Number} reportID - * @param {Number} sequenceNumber + * @param {Number} [sequenceNumber] This can be used to set the last read actionID to a specific + * spot (eg. mark-as-unread). Otherwise, when this param is omitted, the highest sequence number becomes the one that + * is last read (meaning that the entire report history has been read) */ function updateLastReadActionID(reportID, sequenceNumber) { - const currentMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; - if (sequenceNumber < currentMaxSequenceNumber) { - return; - } + // Need to subtract 1 from sequenceNumber so that the "New" marker appears in the right spot (the last read + // action). If 1 isn't subtracted then the "New" marker appears one row below the action (the first unread action) + const lastReadSequenceNumber = (sequenceNumber - 1) || reportMaxSequenceNumbers[reportID]; - setLocalLastRead(reportID, sequenceNumber); + setLocalLastRead(reportID, lastReadSequenceNumber); // Mark the report as not having any unread items API.Report_UpdateLastRead({ accountID: currentUserAccountID, reportID, - sequenceNumber, + sequenceNumber: lastReadSequenceNumber, + }); +} + +/** + * @param {Number} reportID + * @param {Number} sequenceNumber + */ +function setNewMarkerPosition(reportID, sequenceNumber) { + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + newMarkerSequenceNumber: sequenceNumber, }); } @@ -1031,6 +1047,7 @@ export { fetchOrCreateChatReport, addAction, updateLastReadActionID, + setNewMarkerPosition, subscribeToReportTypingEvents, subscribeToUserEvents, unsubscribeFromReportChannel, diff --git a/src/pages/home/report/ReportActionContextMenu.js b/src/pages/home/report/ReportActionContextMenu.js index 78f9fd8c1066..5edbd19eff68 100644 --- a/src/pages/home/report/ReportActionContextMenu.js +++ b/src/pages/home/report/ReportActionContextMenu.js @@ -7,81 +7,19 @@ import { Clipboard as ClipboardIcon, LinkCopy, Mail, Pencil, Trashcan, Checkmark, } from '../../../components/Icon/Expensicons'; import getReportActionContextMenuStyles from '../../../styles/getReportActionContextMenuStyles'; +import {setNewMarkerPosition, updateLastReadActionID} from '../../../libs/actions/Report'; import ReportActionContextMenuItem from './ReportActionContextMenuItem'; import ReportActionPropTypes from './ReportActionPropTypes'; import Clipboard from '../../../libs/Clipboard'; import {isReportMessageAttachment} from '../../../libs/reportUtils'; -/** - * A list of all the context actions in this menu. - */ -const CONTEXT_ACTIONS = [ - // Copy to clipboard - { - text: 'Copy to Clipboard', - icon: ClipboardIcon, - successText: 'Copied!', - successIcon: Checkmark, - shouldShow: true, - - // If return value is true, we switch the `text` and `icon` on - // `ReportActionContextMenuItem` with `successText` and `successIcon` which will fallback to - // the `text` and `icon` - onPress: (action) => { - const message = _.last(lodashGet(action, 'message', null)); - const html = lodashGet(message, 'html', ''); - const text = lodashGet(message, 'text', ''); - const isAttachment = _.has(action, 'isAttachment') - ? action.isAttachment - : isReportMessageAttachment(text); - if (!isAttachment) { - Clipboard.setString(text); - } else { - Clipboard.setString(html); - } - }, - }, - - // Copy chat link - { - text: 'Copy Link', - icon: LinkCopy, - shouldShow: false, - onPress: () => {}, - }, - - // Mark as Unread - { - text: 'Mark as Unread', - icon: Mail, - shouldShow: false, - onPress: () => {}, - }, - - // Edit Comment - { - text: 'Edit Comment', - icon: Pencil, - shouldShow: false, - onPress: () => {}, - }, - - // Delete Comment - { - text: 'Delete Comment', - icon: Trashcan, - shouldShow: false, - onPress: () => {}, - }, -]; - const propTypes = { // The ID of the report this report action is attached to. // eslint-disable-next-line react/no-unused-prop-types reportID: PropTypes.number.isRequired, // The report action this context menu is attached to. - reportAction: PropTypes.shape(ReportActionPropTypes), + reportAction: PropTypes.shape(ReportActionPropTypes).isRequired, // If true, this component will be a small, row-oriented menu that displays icons but not text. // If false, this component will be a larger, column-oriented menu that displays icons alongside text in each row. @@ -92,29 +30,96 @@ const propTypes = { }; const defaultProps = { - reportAction: {}, isMini: false, isVisible: false, }; -const ReportActionContextMenu = (props) => { - const wrapperStyle = getReportActionContextMenuStyles(props.isMini); - return props.isVisible && ( - - {CONTEXT_ACTIONS.map(contextAction => contextAction.shouldShow && ( - contextAction.onPress(props.reportAction)} - key={contextAction.text} - /> - ))} - - ); -}; +class ReportActionContextMenu extends React.Component { + constructor(props) { + super(props); + + // A list of all the context actions in this menu. + this.CONTEXT_ACTIONS = [ + // Copy to clipboard + { + text: 'Copy to Clipboard', + icon: ClipboardIcon, + successText: 'Copied!', + successIcon: Checkmark, + shouldShow: true, + + // If return value is true, we switch the `text` and `icon` on + // `ReportActionContextMenuItem` with `successText` and `successIcon` which will fallback to + // the `text` and `icon` + onPress: () => { + const message = _.last(lodashGet(this.props.reportAction, 'message', null)); + const html = lodashGet(message, 'html', ''); + const text = lodashGet(message, 'text', ''); + const isAttachment = _.has(this.props.reportAction, 'isAttachment') + ? this.props.reportAction.isAttachment + : isReportMessageAttachment(text); + if (!isAttachment) { + Clipboard.setString(text); + } else { + Clipboard.setString(html); + } + }, + }, + + { + text: 'Copy Link', + icon: LinkCopy, + shouldShow: false, + onPress: () => {}, + }, + + { + text: 'Mark as Unread', + icon: Mail, + successIcon: Checkmark, + shouldShow: true, + onPress: () => { + updateLastReadActionID(this.props.reportID, this.props.reportAction.sequenceNumber); + setNewMarkerPosition(this.props.reportID, this.props.reportAction.sequenceNumber); + }, + }, + + { + text: 'Edit Comment', + icon: Pencil, + shouldShow: false, + onPress: () => {}, + }, + + { + text: 'Delete Comment', + icon: Trashcan, + shouldShow: false, + onPress: () => {}, + }, + ]; + + this.wrapperStyle = getReportActionContextMenuStyles(this.props.isMini); + } + + render() { + return this.props.isVisible && ( + + {this.CONTEXT_ACTIONS.map(contextAction => contextAction.shouldShow && ( + + ))} + + ); + } +} ReportActionContextMenu.propTypes = propTypes; ReportActionContextMenu.defaultProps = defaultProps; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 82878a2c384b..7929fd272407 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -155,7 +155,7 @@ class ReportActionItem extends Component { measureContent={() => ( )} diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 2661a20c400d..f267ef26880c 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -13,6 +13,7 @@ import Text from '../../../components/Text'; import { fetchActions, updateLastReadActionID, + setNewMarkerPosition, subscribeToReportTypingEvents, unsubscribeFromReportChannel, } from '../../../libs/actions/Report'; @@ -44,6 +45,9 @@ const propTypes = { // The largest sequenceNumber on this report maxSequenceNumber: PropTypes.number, + // The current position of the new marker + newMarkerSequenceNumber: PropTypes.number, + // Whether there is an outstanding amount in IOU hasOutstandingIOU: PropTypes.bool, @@ -81,17 +85,12 @@ class ReportActionsView extends React.Component { this.renderItem = this.renderItem.bind(this); this.renderCell = this.renderCell.bind(this); this.scrollToListBottom = this.scrollToListBottom.bind(this); - this.recordMaxAction = this.recordMaxAction.bind(this); this.onVisibilityChange = this.onVisibilityChange.bind(this); this.loadMoreChats = this.loadMoreChats.bind(this); this.startRecordMaxActionTimer = this.startRecordMaxActionTimer.bind(this); this.sortedReportActions = []; this.timers = []; - this.initialNewMarkerPosition = props.report.unreadActionCount === 0 - ? 0 - : (props.report.maxSequenceNumber + 1) - props.report.unreadActionCount; - this.state = { isLoadingMoreChats: false, }; @@ -104,7 +103,17 @@ class ReportActionsView extends React.Component { AppState.addEventListener('change', this.onVisibilityChange); subscribeToReportTypingEvents(this.props.reportID); this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); - this.recordMaxAction(); + updateLastReadActionID(this.props.reportID); + + // Since we want the New marker to remain in place even if newer messages come in, we set it once on mount. + // We determine the last read action by deducting the number of unread actions from the total number. + // Then, we add 1 because we want the New marker displayed over the oldest unread sequence. + const oldestUnreadSequenceNumber = this.props.report.unreadActionCount === 0 + ? 0 + : (this.props.report.maxSequenceNumber - this.props.report.unreadActionCount) + 1; + + setNewMarkerPosition(this.props.reportID, oldestUnreadSequenceNumber); + fetchActions(this.props.reportID); Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } @@ -116,6 +125,13 @@ class ReportActionsView extends React.Component { return true; } + // If the new marker has changed places (because the user manually marked a comment as Unread), we have to + // update the component. + if (nextProps.report.newMarkerSequenceNumber > 0 + && nextProps.report.newMarkerSequenceNumber !== this.props.report.newMarkerSequenceNumber) { + return true; + } + if (nextState.isLoadingMoreChats !== this.state.isLoadingMoreChats) { return true; } @@ -194,7 +210,7 @@ class ReportActionsView extends React.Component { * @memberof ReportActionsView */ startRecordMaxActionTimer() { - this.timers.push(setTimeout(this.recordMaxAction, 3000)); + this.timers.push(setTimeout(() => updateLastReadActionID(this.props.reportID), 3000)); } /** @@ -263,25 +279,6 @@ class ReportActionsView extends React.Component { return currentAction.action.actorEmail === previousAction.action.actorEmail; } - /** - * Recorded when the report first opens and when the list is scrolled to the bottom - */ - recordMaxAction() { - const reportActions = lodashGet(this.props, 'reportActions', {}); - const maxVisibleSequenceNumber = _.chain(reportActions) - - // We want to avoid marking any pending actions as read since - // 1. Any action ID that hasn't been delivered by the server is a temporary action ID. - // 2. We already set a comment someone has authored as the lastReadActionID_ rNVP on the server - // and should sync it locally when we handle it via Pusher or Airship - .reject(action => action.loading) - .pluck('sequenceNumber') - .max() - .value(); - - updateLastReadActionID(this.props.reportID, maxVisibleSequenceNumber); - } - /** * Finds and updates most recent IOU report action number * @@ -304,7 +301,7 @@ class ReportActionsView extends React.Component { if (this.actionListElement) { this.actionListElement.scrollToIndex({animated: false, index: 0}); } - this.recordMaxAction(); + updateLastReadActionID(this.props.reportID); } /** @@ -342,13 +339,14 @@ class ReportActionsView extends React.Component { item, index, }) { + const shouldDisplayNewIndicator = this.props.report.newMarkerSequenceNumber > 0 + && item.action.sequenceNumber === this.props.report.newMarkerSequenceNumber; return ( 0 - && item.action.sequenceNumber === this.initialNewMarkerPosition} + shouldDisplayNewIndicator={shouldDisplayNewIndicator} isMostRecentIOUReportAction={item.action.sequenceNumber === this.mostRecentIOUReportSequenceNumber} iouReportID={this.props.report.iouReportID} hasOutstandingIOU={this.props.report.hasOutstandingIOU}