From 3be5e6909a86a33a9c7c749c2378b14ad1ab243c Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Fri, 25 Aug 2023 16:36:54 +0200 Subject: [PATCH 01/29] Separate ReportScreenContext to prevent cyclic re-renders --- .../ReportActionItemEmojiReactions.js | 4 +- src/hooks/useReportScrollManager/index.js | 4 +- .../useReportScrollManager/index.native.js | 4 +- src/pages/home/ReportScreen.js | 188 +++++++++--------- src/pages/home/ReportScreenContext.js | 9 +- src/pages/home/report/ReportActionItem.js | 4 +- src/pages/home/report/ReportActionsView.js | 7 +- 7 files changed, 110 insertions(+), 110 deletions(-) diff --git a/src/components/Reactions/ReportActionItemEmojiReactions.js b/src/components/Reactions/ReportActionItemEmojiReactions.js index ec2755f1a5dd..4f590b43e180 100644 --- a/src/components/Reactions/ReportActionItemEmojiReactions.js +++ b/src/components/Reactions/ReportActionItemEmojiReactions.js @@ -13,7 +13,7 @@ import EmojiReactionsPropTypes from './EmojiReactionsPropTypes'; import Tooltip from '../Tooltip'; import ReactionTooltipContent from './ReactionTooltipContent'; import * as EmojiUtils from '../../libs/EmojiUtils'; -import ReportScreenContext from '../../pages/home/ReportScreenContext'; +import {ReactionListContext} from '../../pages/home/ReportScreenContext'; const propTypes = { emojiReactions: EmojiReactionsPropTypes, @@ -41,7 +41,7 @@ const defaultProps = { }; function ReportActionItemEmojiReactions(props) { - const {reactionListRef} = useContext(ReportScreenContext); + const reactionListRef = useContext(ReactionListContext); const popoverReactionListAnchor = useRef(null); let totalReactionCount = 0; diff --git a/src/hooks/useReportScrollManager/index.js b/src/hooks/useReportScrollManager/index.js index 0cf09146553c..9a3303504b92 100644 --- a/src/hooks/useReportScrollManager/index.js +++ b/src/hooks/useReportScrollManager/index.js @@ -1,8 +1,8 @@ import {useContext, useCallback} from 'react'; -import ReportScreenContext from '../../pages/home/ReportScreenContext'; +import {ActionListContext} from '../../pages/home/ReportScreenContext'; function useReportScrollManager() { - const {flatListRef} = useContext(ReportScreenContext); + const flatListRef = useContext(ActionListContext); /** * Scroll to the provided index. On non-native implementations we do not want to scroll when we are scrolling because diff --git a/src/hooks/useReportScrollManager/index.native.js b/src/hooks/useReportScrollManager/index.native.js index 35af064cb062..d44a40222ca5 100644 --- a/src/hooks/useReportScrollManager/index.native.js +++ b/src/hooks/useReportScrollManager/index.native.js @@ -1,8 +1,8 @@ import {useContext, useCallback} from 'react'; -import ReportScreenContext from '../../pages/home/ReportScreenContext'; +import {ActionListContext} from '../../pages/home/ReportScreenContext'; function useReportScrollManager() { - const {flatListRef} = useContext(ReportScreenContext); + const flatListRef = useContext(ActionListContext); /** * Scroll to the provided index. diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 83f0e4a6d506..16284c2bd9c5 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -33,7 +33,7 @@ import getIsReportFullyVisible from '../../libs/getIsReportFullyVisible'; import MoneyRequestHeader from '../../components/MoneyRequestHeader'; import MoneyReportHeader from '../../components/MoneyReportHeader'; import * as ComposerActions from '../../libs/actions/Composer'; -import ReportScreenContext from './ReportScreenContext'; +import {ActionListContext, ReactionListContext} from './ReportScreenContext'; import TaskHeaderActionButton from '../../components/TaskHeaderActionButton'; import DragAndDropProvider from '../../components/DragAndDrop/Provider'; @@ -276,111 +276,107 @@ class ReportScreen extends React.Component { } return ( - - - + + - - {headerView} - {ReportUtils.isTaskReport(this.props.report) && this.props.isSmallScreenWidth && ReportUtils.isOpenTaskReport(this.props.report) && ( - - - - + + {headerView} + {ReportUtils.isTaskReport(this.props.report) && this.props.isSmallScreenWidth && ReportUtils.isOpenTaskReport(this.props.report) && ( + + + + + - - )} - - {Boolean(this.props.accountManagerReportID) && ReportUtils.isConciergeChatReport(this.props.report) && this.state.isBannerVisible && ( - - )} - - { - // Rounding this value for comparison because they can look like this: 411.9999694824219 - const skeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height); - - // Only set state when the height changes to avoid unnecessary renders - if (reportActionsListViewHeight === skeletonViewContainerHeight) return; - - // The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it - // takes up so we can set the skeleton view container height. - if (skeletonViewContainerHeight === 0) { - return; - } - reportActionsListViewHeight = skeletonViewContainerHeight; - this.setState({skeletonViewContainerHeight}); - }} - > - {this.isReportReadyForDisplay() && !isLoadingInitialReportActions && !isLoading && ( - )} - - {/* Note: The report should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then - we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */} - {(!this.isReportReadyForDisplay() || isLoadingInitialReportActions || isLoading) && ( - - )} - - {this.isReportReadyForDisplay() && ( - <> - + {Boolean(this.props.accountManagerReportID) && ReportUtils.isConciergeChatReport(this.props.report) && this.state.isBannerVisible && ( + + )} + + { + // Rounding this value for comparison because they can look like this: 411.9999694824219 + const skeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height); + + // Only set state when the height changes to avoid unnecessary renders + if (reportActionsListViewHeight === skeletonViewContainerHeight) return; + + // The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it + // takes up so we can set the skeleton view container height. + if (skeletonViewContainerHeight === 0) { + return; + } + reportActionsListViewHeight = skeletonViewContainerHeight; + this.setState({skeletonViewContainerHeight}); + }} + > + {this.isReportReadyForDisplay() && !isLoadingInitialReportActions && !isLoading && ( + - - )} + )} - {!this.isReportReadyForDisplay() && ( - - )} - - - - - + {/* Note: The report should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then + we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */} + {(!this.isReportReadyForDisplay() || isLoadingInitialReportActions || isLoading) && ( + + )} + + {this.isReportReadyForDisplay() && ( + <> + + + )} + + {!this.isReportReadyForDisplay() && ( + + )} + + + + + + ); } } diff --git a/src/pages/home/ReportScreenContext.js b/src/pages/home/ReportScreenContext.js index 2f79d6ae9432..0be1882699f4 100644 --- a/src/pages/home/ReportScreenContext.js +++ b/src/pages/home/ReportScreenContext.js @@ -1,4 +1,9 @@ import {createContext} from 'react'; -const ReportScreenContext = createContext(); -export default ReportScreenContext; +const ActionListContext = createContext(); +const ReactionListContext = createContext(); + +export { + ActionListContext, + ReactionListContext +} \ No newline at end of file diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index e5b199d1c994..4a0c0d7bd3ba 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -64,7 +64,7 @@ import * as PersonalDetailsUtils from '../../../libs/PersonalDetailsUtils'; import ReportActionItemBasicMessage from './ReportActionItemBasicMessage'; import * as store from '../../../libs/actions/ReimbursementAccount/store'; import * as BankAccounts from '../../../libs/actions/BankAccounts'; -import ReportScreenContext from '../ReportScreenContext'; +import { ReactionListContext } from '../ReportScreenContext'; import Permissions from '../../../libs/Permissions'; const propTypes = { @@ -127,7 +127,7 @@ function ReportActionItem(props) { const [isContextMenuActive, setIsContextMenuActive] = useState(ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)); const [isHidden, setIsHidden] = useState(false); const [moderationDecision, setModerationDecision] = useState(CONST.MODERATION.MODERATOR_DECISION_APPROVED); - const {reactionListRef} = useContext(ReportScreenContext); + const reactionListRef = useContext(ReactionListContext); const textInputRef = useRef(); const popoverAnchorRef = useRef(); const downloadedPreviews = useRef([]); diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index da475e61f749..70eb65ade3ea 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -19,7 +19,7 @@ import * as ReportActionsUtils from '../../../libs/ReportActionsUtils'; import reportPropTypes from '../../reportPropTypes'; import PopoverReactionList from './ReactionList/PopoverReactionList'; import getIsReportFullyVisible from '../../../libs/getIsReportFullyVisible'; -import ReportScreenContext from '../ReportScreenContext'; +import { ReactionListContext } from '../ReportScreenContext'; const propTypes = { /** The report currently being looked at */ @@ -54,10 +54,9 @@ const defaultProps = { }; function ReportActionsView(props) { - const context = useContext(ReportScreenContext); useCopySelectionHelper(); - + const reactionListRef = useContext(ReactionListContext) const didLayout = useRef(false); const didSubscribeToReportTypingEvents = useRef(false); const hasCachedActions = useRef(_.size(props.reportActions) > 0); @@ -189,7 +188,7 @@ function ReportActionsView(props) { loadMoreChats={loadMoreChats} policy={props.policy} /> - + ); } From 6125c381cde29277f150f1a4156a6b96af05f150 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 28 Aug 2023 13:44:09 +0200 Subject: [PATCH 02/29] Get rid of full list re-render when marking messages as unread --- src/pages/home/report/ReportActionsList.js | 23 +++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 7f897ee825fb..1c0c88e20ec4 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -114,9 +114,9 @@ function ReportActionsList({ const readActionSkipped = useRef(false); const reportActionSize = useRef(sortedReportActions.length); - // 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}); + // This state is used to force a re-render when the user manually marks a message as unread + // by using a timestamp you can force re-renders without having to worry about if another message was marked as unread before + const [messageManuallyMarkedUnread, setMessageManuallyMarkedUnread] = useState(0); const [isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible] = useState(false); const animatedStyles = useAnimatedStyle(() => ({ opacity: opacity.value, @@ -163,15 +163,14 @@ function ReportActionsList({ useEffect(() => { const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report); - if (!didManuallyMarkReportAsUnread) { - setMessageManuallyMarked({read: false}); - return; + if (didManuallyMarkReportAsUnread) { + // Clearing the current unread marker so that it can be recalculated + currentUnreadMarker.current = null; + setMessageManuallyMarkedUnread(new Date().getTime()); + } else { + setMessageManuallyMarkedUnread(0); } - // Clearing the current unread marker so that it can be recalculated - currentUnreadMarker.current = null; - setMessageManuallyMarked({read: true}); - // We only care when a new lastReadTime is set in the report // eslint-disable-next-line react-hooks/exhaustive-deps }, [report.lastReadTime]); @@ -230,7 +229,7 @@ function ReportActionsList({ const isCurrentMessageUnread = isMessageUnread(reportAction, report.lastReadTime); shouldDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, report.lastReadTime); - if (!messageManuallyMarked.read) { + if (!messageManuallyMarkedUnread) { shouldDisplayNewMarker = shouldDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID(); } const canDisplayMarker = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true; @@ -272,7 +271,7 @@ function ReportActionsList({ /> ); }, - [report, hasOutstandingIOU, sortedReportActions, mostRecentIOUReportActionID, messageManuallyMarked], + [report, hasOutstandingIOU, sortedReportActions, mostRecentIOUReportActionID, messageManuallyMarkedUnread], ); // Native mobile does not render updates flatlist the changes even though component did update called. From be7ea564639ef27e0ee8052c948d94f3119211b5 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 28 Aug 2023 16:31:16 +0200 Subject: [PATCH 03/29] Linting --- src/pages/home/ReportScreenContext.js | 5 +---- src/pages/home/report/ReportActionItem.js | 2 +- src/pages/home/report/ReportActionsView.js | 5 ++--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/pages/home/ReportScreenContext.js b/src/pages/home/ReportScreenContext.js index 0be1882699f4..1e8d30cf7585 100644 --- a/src/pages/home/ReportScreenContext.js +++ b/src/pages/home/ReportScreenContext.js @@ -3,7 +3,4 @@ import {createContext} from 'react'; const ActionListContext = createContext(); const ReactionListContext = createContext(); -export { - ActionListContext, - ReactionListContext -} \ No newline at end of file +export {ActionListContext, ReactionListContext}; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 4a0c0d7bd3ba..fe2ea3acf691 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -64,7 +64,7 @@ import * as PersonalDetailsUtils from '../../../libs/PersonalDetailsUtils'; import ReportActionItemBasicMessage from './ReportActionItemBasicMessage'; import * as store from '../../../libs/actions/ReimbursementAccount/store'; import * as BankAccounts from '../../../libs/actions/BankAccounts'; -import { ReactionListContext } from '../ReportScreenContext'; +import {ReactionListContext} from '../ReportScreenContext'; import Permissions from '../../../libs/Permissions'; const propTypes = { diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 70eb65ade3ea..25d01a6953d1 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -19,7 +19,7 @@ import * as ReportActionsUtils from '../../../libs/ReportActionsUtils'; import reportPropTypes from '../../reportPropTypes'; import PopoverReactionList from './ReactionList/PopoverReactionList'; import getIsReportFullyVisible from '../../../libs/getIsReportFullyVisible'; -import { ReactionListContext } from '../ReportScreenContext'; +import {ReactionListContext} from '../ReportScreenContext'; const propTypes = { /** The report currently being looked at */ @@ -54,9 +54,8 @@ const defaultProps = { }; function ReportActionsView(props) { - useCopySelectionHelper(); - const reactionListRef = useContext(ReactionListContext) + const reactionListRef = useContext(ReactionListContext); const didLayout = useRef(false); const didSubscribeToReportTypingEvents = useRef(false); const hasCachedActions = useRef(_.size(props.reportActions) > 0); From 90c9119da7a326cbcffb4ca0498fed28ef25779b Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 29 Aug 2023 13:07:54 +0200 Subject: [PATCH 04/29] create new onyx metadata collection and connect components --- src/ONYXKEYS.ts | 2 + src/libs/actions/Policy.js | 2 +- src/libs/actions/Report.js | 104 +++++++++++------- src/pages/home/ReportScreen.js | 17 ++- src/pages/home/report/ReportActionsList.js | 10 +- src/pages/home/report/ReportActionsView.js | 17 ++- .../withReportAndReportActionOrNotFound.js | 13 ++- src/pages/reportMetadataPropTypes.js | 9 ++ src/pages/reportPropTypes.js | 6 - src/types/onyx/Report.ts | 6 - src/types/onyx/ReportMetadata.ts | 9 ++ src/types/onyx/index.ts | 2 + 12 files changed, 137 insertions(+), 60 deletions(-) create mode 100644 src/pages/reportMetadataPropTypes.js create mode 100644 src/types/onyx/ReportMetadata.ts diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index 2df7dd0b5a76..de2a38cff5ae 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -235,6 +235,7 @@ const ONYXKEYS = { POLICY_CATEGORIES: 'policyCategories_', WORKSPACE_INVITE_MEMBERS_DRAFT: 'workspaceInviteMembersDraft_', REPORT: 'report_', + REPORT_METADATA: 'reportMetadata_', REPORT_ACTIONS: 'reportActions_', REPORT_ACTIONS_DRAFTS: 'reportActionsDrafts_', REPORT_ACTIONS_REACTIONS: 'reportActionsReactions_', @@ -365,6 +366,7 @@ type OnyxValues = { [ONYXKEYS.COLLECTION.DEPRECATED_POLICY_MEMBER_LIST]: OnyxTypes.PolicyMember; [ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MEMBERS_DRAFT]: Record; [ONYXKEYS.COLLECTION.REPORT]: OnyxTypes.Report; + [ONYXKEYS.COLLECTION.REPORT_METADATA]: OnyxTypes.ReportMetadata; [ONYXKEYS.COLLECTION.REPORT_ACTIONS]: OnyxTypes.ReportAction; [ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS]: string; [ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS]: OnyxTypes.ReportActionReactions; diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 87c77e722a5c..0bb0e40382cb 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -310,7 +310,7 @@ function createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs) { workspaceMembersChats.onyxFailureData.push({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReport.reportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReport.reportID}`, value: { isLoadingReportActions: false, }, diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 8b898a6aaaea..208d0e39262d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -401,43 +401,65 @@ function reportActionsExist(reportID) { * @param {Array} participantAccountIDList The list of accountIDs that are included in a new chat, not including the user creating it */ function openReport(reportID, participantLoginList = [], newReportObject = {}, parentReportActionID = '0', isFromDeepLink = false, participantAccountIDList = []) { - const optimisticReportData = { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - value: reportActionsExist(reportID) - ? {} - : { - isLoadingReportActions: true, - isLoadingMoreReportActions: false, - reportName: lodashGet(allReports, [reportID, 'reportName'], CONST.REPORT.DEFAULT_REPORT_NAME), - }, - }; - const reportSuccessData = { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - value: { - isLoadingReportActions: false, - pendingFields: { - createChat: null, + const optimisticReportData = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: reportActionsExist(reportID) + ? {} + : { + reportName: lodashGet(allReports, [reportID, 'reportName'], CONST.REPORT.DEFAULT_REPORT_NAME), + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, + value: reportActionsExist(reportID) + ? {} + : { + isLoadingReportActions: true, + isLoadingMoreReportActions: false, + }, + }, + ]; + + const reportSuccessData = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: { + pendingFields: { + createChat: null, + }, + errorFields: { + createChat: null, + }, + isOptimisticReport: false, }, - errorFields: { - createChat: null, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, + value: { + isLoadingReportActions: false, }, - isOptimisticReport: false, }, - }; - const reportFailureData = { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - value: { - isLoadingReportActions: false, + ]; + + const reportFailureData = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, + value: { + isLoadingReportActions: false, + }, }, - }; + ]; const onyxData = { - optimisticData: [optimisticReportData], - successData: [reportSuccessData], - failureData: [reportFailureData], + optimisticData: optimisticReportData, + successData: reportSuccessData, + failureData: reportFailureData, }; const params = { @@ -454,17 +476,17 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p // If we open an exist report, but it is not present in Onyx yet, we should change the method to set for this report // and we need data to be available when we navigate to the chat page if (_.isEmpty(ReportUtils.getReport(reportID))) { - optimisticReportData.onyxMethod = Onyx.METHOD.SET; + onyxData.optimisticReportData.onyxMethod = Onyx.METHOD.SET; } // If we are creating a new report, we need to add the optimistic report data and a report action if (!_.isEmpty(newReportObject)) { // Change the method to set for new reports because it doesn't exist yet, is faster, // and we need the data to be available when we navigate to the chat page - optimisticReportData.onyxMethod = Onyx.METHOD.SET; - optimisticReportData.value = { + onyxData.optimisticReportData[0].onyxMethod = Onyx.METHOD.SET; + onyxData.optimisticReportData[0].value = { reportName: CONST.REPORT.DEFAULT_REPORT_NAME, - ...optimisticReportData.value, + ...onyxData.optimisticReportData.value, ...newReportObject, pendingFields: { createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, @@ -646,17 +668,23 @@ function reconnect(reportID) { { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: { + reportName: lodashGet(allReports, [reportID, 'reportName'], CONST.REPORT.DEFAULT_REPORT_NAME), + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, value: { isLoadingReportActions: true, isLoadingMoreReportActions: false, - reportName: lodashGet(allReports, [reportID, 'reportName'], CONST.REPORT.DEFAULT_REPORT_NAME), }, }, ], successData: [ { onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, value: { isLoadingReportActions: false, }, @@ -665,7 +693,7 @@ function reconnect(reportID) { failureData: [ { onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, value: { isLoadingReportActions: false, }, diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 4e6280265949..8bf9d1a4b447 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -25,6 +25,7 @@ import ReportFooter from './report/ReportFooter'; import Banner from '../../components/Banner'; import withLocalize from '../../components/withLocalize'; import reportPropTypes from '../reportPropTypes'; +import reportMetadataPropTypes from '../reportMetadataPropTypes'; import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView'; import withViewportOffsetTop, {viewportOffsetTopPropTypes} from '../../components/withViewportOffsetTop'; import * as ReportActionsUtils from '../../libs/ReportActionsUtils'; @@ -57,6 +58,9 @@ const propTypes = { /** The report currently being looked at */ report: reportPropTypes, + /** The report metada loading states */ + reportMetadata: reportMetadataPropTypes, + /** Array of report actions for this report */ reportActions: PropTypes.arrayOf(PropTypes.shape(reportActionPropTypes)), @@ -95,7 +99,10 @@ const defaultProps = { reportActions: [], report: { hasOutstandingIOU: false, + }, + reportMetadata: { isLoadingReportActions: false, + isLoadingMoreReportActions: false, }, isComposerFullSize: false, betas: [], @@ -120,6 +127,7 @@ function ReportScreen({ betas, route, report, + reportMetadata, reportActions, accountManagerReportID, personalDetails, @@ -145,7 +153,7 @@ function ReportScreen({ const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}]; // There are no reportActions at all to display and we are still in the process of loading the next set of actions. - const isLoadingInitialReportActions = _.isEmpty(reportActions) && report.isLoadingReportActions; + const isLoadingInitialReportActions = _.isEmpty(reportActions) && reportMetadata.isLoadingReportActions; const shouldHideReport = !ReportUtils.canAccessReport(report, policies, betas); @@ -292,7 +300,7 @@ function ReportScreen({ shouldEnableKeyboardAvoidingView={isTopMostReportId} > `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, }, + reportMetadata: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`, + }, isComposerFullSize: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${getReportID(route)}`, }, diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 1c0c88e20ec4..dedf7ca89306 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -34,6 +34,9 @@ const propTypes = { /** The ID of the most recent IOU report action connected with the shown report */ mostRecentIOUReportActionID: PropTypes.string, + /** The report metada loading states */ + isLoadingReportActions: PropTypes.bool, + /** Are we loading more report actions? */ isLoadingMoreReportActions: PropTypes.bool, @@ -63,6 +66,7 @@ const defaultProps = { personalDetails: {}, onScroll: () => {}, mostRecentIOUReportActionID: '', + isLoadingReportActions: false, isLoadingMoreReportActions: false, ...withCurrentUserPersonalDetailsDefaultProps, }; @@ -92,6 +96,8 @@ function isMessageUnread(message, lastReadTime) { function ReportActionsList({ report, + isLoadingReportActions, + isLoadingMoreReportActions, sortedReportActions, windowHeight, onScroll, @@ -299,7 +305,7 @@ function ReportActionsList({ onEndReached={loadMoreChats} onEndReachedThreshold={0.75} ListFooterComponent={() => { - if (report.isLoadingMoreReportActions) { + if (isLoadingMoreReportActions) { return ; } @@ -307,7 +313,7 @@ function ReportActionsList({ // skeleton view above the created action in a newly generated optimistic chat or one with not // that many comments. const lastReportAction = _.last(sortedReportActions) || {}; - if (report.isLoadingReportActions && lastReportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) { + if (isLoadingReportActions && lastReportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) { return ( { // Only fetch more if we are not already fetching so that we don't initiate duplicate requests. - if (props.report.isLoadingMoreReportActions) { + if (props.isLoadingMoreReportActions) { return; } @@ -183,7 +191,8 @@ function ReportActionsView(props) { onLayout={recordTimeToMeasureItemLayout} sortedReportActions={props.reportActions} mostRecentIOUReportActionID={mostRecentIOUReportActionID.current} - isLoadingMoreReportActions={props.report.isLoadingMoreReportActions} + isLoadingReportActions={props.isLoadingReportActions} + isLoadingMoreReportActions={props.isLoadingMoreReportActions} loadMoreChats={loadMoreChats} policy={props.policy} /> @@ -213,11 +222,11 @@ function arePropsEqual(oldProps, newProps) { return false; } - if (oldProps.report.isLoadingMoreReportActions !== newProps.report.isLoadingMoreReportActions) { + if (oldProps.isLoadingMoreReportActions !== newProps.isLoadingMoreReportActions) { return false; } - if (oldProps.report.isLoadingReportActions !== newProps.report.isLoadingReportActions) { + if (oldProps.isLoadingReportActions !== newProps.isLoadingReportActions) { return false; } diff --git a/src/pages/home/report/withReportAndReportActionOrNotFound.js b/src/pages/home/report/withReportAndReportActionOrNotFound.js index 9bf3e73e761c..b4346504b327 100644 --- a/src/pages/home/report/withReportAndReportActionOrNotFound.js +++ b/src/pages/home/report/withReportAndReportActionOrNotFound.js @@ -6,6 +6,7 @@ import getComponentDisplayName from '../../../libs/getComponentDisplayName'; import NotFoundPage from '../../ErrorPage/NotFoundPage'; import ONYXKEYS from '../../../ONYXKEYS'; import reportPropTypes from '../../reportPropTypes'; +import reportMetadataPropTypes from '../../reportMetadataPropTypes'; import reportActionPropTypes from './reportActionPropTypes'; import FullscreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import * as ReportUtils from '../../../libs/ReportUtils'; @@ -23,6 +24,9 @@ export default function (WrappedComponent) { /** The report currently being looked at */ report: reportPropTypes, + /** The report metadata */ + reportMetadata: reportMetadataPropTypes, + /** Array of report actions for this report */ reportActions: PropTypes.shape(reportActionPropTypes), @@ -62,6 +66,10 @@ export default function (WrappedComponent) { forwardedRef: () => {}, reportActions: {}, report: {}, + reportMetadata: { + isLoadingReportActions: false, + isLoadingMoreReportActions: false, + }, policies: {}, betas: [], isLoadingReportData: true, @@ -94,7 +102,7 @@ export default function (WrappedComponent) { // Perform all the loading checks const isLoadingReport = props.isLoadingReportData && (_.isEmpty(props.report) || !props.report.reportID); - const isLoadingReportAction = _.isEmpty(props.reportActions) || (props.report.isLoadingReportActions && _.isEmpty(getReportAction())); + const isLoadingReportAction = _.isEmpty(props.reportActions) || (props.reportMetadata.isLoadingReportActions && _.isEmpty(getReportAction())); const shouldHideReport = !isLoadingReport && (_.isEmpty(props.report) || !props.report.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas)); if ((isLoadingReport || isLoadingReportAction) && !shouldHideReport) { @@ -135,6 +143,9 @@ export default function (WrappedComponent) { report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, }, + reportMetadata: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${route.params.reportID}`, + }, isLoadingReportData: { key: ONYXKEYS.IS_LOADING_REPORT_DATA, }, diff --git a/src/pages/reportMetadataPropTypes.js b/src/pages/reportMetadataPropTypes.js new file mode 100644 index 000000000000..a75d71aef7b3 --- /dev/null +++ b/src/pages/reportMetadataPropTypes.js @@ -0,0 +1,9 @@ +import PropTypes from 'prop-types'; + +export default PropTypes.shape({ + /** Are we loading more report actions? */ + isLoadingMoreReportActions: PropTypes.bool, + + /** Flag to check if the report actions data are loading */ + isLoadingReportActions: PropTypes.bool, +}); diff --git a/src/pages/reportPropTypes.js b/src/pages/reportPropTypes.js index da90e0a4ac5c..a2c41b5a8147 100644 --- a/src/pages/reportPropTypes.js +++ b/src/pages/reportPropTypes.js @@ -13,12 +13,6 @@ export default PropTypes.shape({ /** List of icons for report participants */ icons: PropTypes.arrayOf(avatarPropTypes), - /** Are we loading more report actions? */ - isLoadingMoreReportActions: PropTypes.bool, - - /** Flag to check if the report actions data are loading */ - isLoadingReportActions: PropTypes.bool, - /** Whether the user is not an admin of policyExpenseChat chat */ isOwnPolicyExpenseChat: PropTypes.bool, diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index 8660837ba874..7ba0f6c69621 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -12,12 +12,6 @@ type Report = { /** List of icons for report participants */ icons?: OnyxCommon.Icon[]; - /** Are we loading more report actions? */ - isLoadingMoreReportActions?: boolean; - - /** Flag to check if the report actions data are loading */ - isLoadingReportActions?: boolean; - /** Whether the user is not an admin of policyExpenseChat chat */ isOwnPolicyExpenseChat?: boolean; diff --git a/src/types/onyx/ReportMetadata.ts b/src/types/onyx/ReportMetadata.ts new file mode 100644 index 000000000000..3e389c8cff4f --- /dev/null +++ b/src/types/onyx/ReportMetadata.ts @@ -0,0 +1,9 @@ +type ReportMetadata = { + /** Are we loading more report actions? */ + isLoadingMoreReportActions?: boolean; + + /** Flag to check if the report actions data are loading */ + isLoadingReportActions?: boolean; +}; + +export default ReportMetadata; diff --git a/src/types/onyx/index.ts b/src/types/onyx/index.ts index 8d48eccf1464..71aa97bc3b9c 100644 --- a/src/types/onyx/index.ts +++ b/src/types/onyx/index.ts @@ -36,6 +36,7 @@ import Download from './Download'; import PolicyMember from './PolicyMember'; import Policy from './Policy'; import Report from './Report'; +import ReportMetadata from './ReportMetadata'; import ReportAction from './ReportAction'; import ReportActionReactions from './ReportActionReactions'; import SecurityGroup from './SecurityGroup'; @@ -82,6 +83,7 @@ export type { PolicyMember, Policy, Report, + ReportMetadata, ReportAction, ReportActionReactions, SecurityGroup, From 9829fd82ed78743f1f383eecc60ea44b45dcaafd Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 30 Aug 2023 09:49:29 +0200 Subject: [PATCH 05/29] Clean code on ReportScreen main body --- src/pages/home/ReportScreen.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 8bf9d1a4b447..3aac348fc33a 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -249,6 +249,18 @@ function ReportScreen({ [route], ); + const calculateSkeletonViewHeight = (event) => { + // Rounding this value for comparison because they can look like this: 411.9999694824219 + const newSkeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height); + + // The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it + // takes up so we can set the skeleton view container height. + if (newSkeletonViewContainerHeight === 0) { + return; + } + setSkeletonViewContainerHeight(newSkeletonViewContainerHeight); + }; + useEffect(() => { const unsubscribeVisibilityListener = Visibility.onVisibilityChange(() => { // If the report is not fully visible (AKA on small screen devices and LHR is open) or the report is optimistic (AKA not yet created) @@ -337,17 +349,7 @@ function ReportScreen({ { - // Rounding this value for comparison because they can look like this: 411.9999694824219 - const newSkeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height); - - // The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it - // takes up so we can set the skeleton view container height. - if (newSkeletonViewContainerHeight === 0) { - return; - } - setSkeletonViewContainerHeight(newSkeletonViewContainerHeight); - }} + onLayout={calculateSkeletonViewHeight} > {isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && ( )} - {/* Note: The report should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then + {/* Note: The ReportActionsSkeletonView should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */} {(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && } - {isReportReadyForDisplay && ( + {isReportReadyForDisplay ? ( <> - )} - - {!isReportReadyForDisplay && ( + ) : ( Date: Wed, 30 Aug 2023 11:23:33 +0200 Subject: [PATCH 06/29] Do not use context on SidebarLinks --- src/pages/home/sidebar/SidebarLinks.js | 2 -- src/pages/home/sidebar/SidebarLinksData.js | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 0057e16aed7c..e09f5accafc2 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -30,7 +30,6 @@ import * as Session from '../../../libs/actions/Session'; import KeyboardShortcut from '../../../libs/KeyboardShortcut'; import onyxSubscribe from '../../../libs/onyxSubscribe'; import * as ReportActionContextMenu from '../report/ContextMenu/ReportActionContextMenu'; -import withCurrentReportID from '../../../components/withCurrentReportID'; import SignInOrAvatarWithOptionalStatus from './SignInOrAvatarWithOptionalStatus'; const basePropTypes = { @@ -204,7 +203,6 @@ SidebarLinks.defaultProps = defaultProps; export default compose( withLocalize, withWindowDimensions, - withCurrentReportID, withOnyx({ report: { key: ({currentReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${currentReportID}`, diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 3eca506f7591..8681e01f1a67 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -88,6 +88,7 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr style={[styles.flex1, styles.h100]} > Date: Wed, 30 Aug 2023 13:41:50 +0200 Subject: [PATCH 07/29] Eagerly initialize date-fns locale --- src/components/withLocalize.js | 11 +++++++++++ src/libs/DateUtils.js | 1 + src/pages/home/sidebar/SidebarLinks.js | 11 ++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/components/withLocalize.js b/src/components/withLocalize.js index 913d085f9428..bea0e1790edb 100755 --- a/src/components/withLocalize.js +++ b/src/components/withLocalize.js @@ -28,6 +28,9 @@ const withLocalizePropTypes = { /** Formats a datetime to local date and time string */ datetimeToCalendarTime: PropTypes.func.isRequired, + /** Updates date-fns internal locale */ + updateLocale: PropTypes.func.isRequired, + /** Returns a locally converted phone number for numbers from the same region * and an internationally converted phone number with the country code for numbers from other regions */ formatPhoneNumber: PropTypes.func.isRequired, @@ -72,6 +75,7 @@ class LocaleContextProvider extends React.Component { numberFormat: this.numberFormat.bind(this), datetimeToRelative: this.datetimeToRelative.bind(this), datetimeToCalendarTime: this.datetimeToCalendarTime.bind(this), + updateLocale: this.updateLocale.bind(this), formatPhoneNumber: this.formatPhoneNumber.bind(this), fromLocaleDigit: this.fromLocaleDigit.bind(this), toLocaleDigit: this.toLocaleDigit.bind(this), @@ -115,6 +119,13 @@ class LocaleContextProvider extends React.Component { return DateUtils.datetimeToCalendarTime(this.props.preferredLocale, datetime, includeTimezone, lodashGet(this.props, 'currentUserPersonalDetails.timezone.selected'), isLowercase); } + /** + * Updates date-fns internal locale to the user preferredLocale + */ + updateLocale() { + DateUtils.setLocale(this.props.preferredLocale); + } + /** * @param {String} phoneNumber * @returns {String} diff --git a/src/libs/DateUtils.js b/src/libs/DateUtils.js index b33a1b1b2a73..5c8cbd175082 100644 --- a/src/libs/DateUtils.js +++ b/src/libs/DateUtils.js @@ -344,6 +344,7 @@ const DateUtils = { subtractMillisecondsFromDateTime, getDateStringFromISOTimestamp, getStatusUntilDate, + setLocale, }; export default DateUtils; diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index e09f5accafc2..933511f0a86b 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -1,6 +1,6 @@ /* eslint-disable rulesdir/onyx-props-must-have-default */ import React from 'react'; -import {View} from 'react-native'; +import {View, InteractionManager} from 'react-native'; import _ from 'underscore'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; @@ -87,6 +87,15 @@ class SidebarLinks extends React.PureComponent { SidebarUtils.setIsSidebarLoadedReady(); this.isSidebarLoaded = true; + // Eagerly set the locale on date-fns, it helps navigating to the report screen faster + InteractionManager.runAfterInteractions(() => { + requestAnimationFrame(() => { + requestIdleCallback(() => { + this.props.updateLocale(); + }); + }); + }); + let modal = {}; this.unsubscribeOnyxModal = onyxSubscribe({ key: ONYXKEYS.MODAL, From b3b33eb1f15241eed74281b3db692705604bf5c9 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 30 Aug 2023 15:08:05 +0200 Subject: [PATCH 08/29] Update src/pages/home/ReportScreen.js Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- src/pages/home/ReportScreen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 3aac348fc33a..c6370ea92e3f 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -58,7 +58,7 @@ const propTypes = { /** The report currently being looked at */ report: reportPropTypes, - /** The report metada loading states */ + /** The report metadata loading states */ reportMetadata: reportMetadataPropTypes, /** Array of report actions for this report */ From 40d48bb9df4c5afcc1bb8d6d32a788906c3de736 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 30 Aug 2023 15:08:13 +0200 Subject: [PATCH 09/29] Update src/pages/home/report/ReportActionsList.js Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- src/pages/home/report/ReportActionsList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index dedf7ca89306..febf9d809d7f 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -34,7 +34,7 @@ const propTypes = { /** The ID of the most recent IOU report action connected with the shown report */ mostRecentIOUReportActionID: PropTypes.string, - /** The report metada loading states */ + /** The report metadata loading states */ isLoadingReportActions: PropTypes.bool, /** Are we loading more report actions? */ From 14808d94edadef4cc1e3d6043d4acf05a357459c Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 30 Aug 2023 15:34:18 +0200 Subject: [PATCH 10/29] Remove requestIdleCallback as it might not be available everywhere --- src/pages/home/sidebar/SidebarLinks.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 933511f0a86b..f0dc6639c8a5 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -90,9 +90,7 @@ class SidebarLinks extends React.PureComponent { // Eagerly set the locale on date-fns, it helps navigating to the report screen faster InteractionManager.runAfterInteractions(() => { requestAnimationFrame(() => { - requestIdleCallback(() => { - this.props.updateLocale(); - }); + this.props.updateLocale(); }); }); From 6effb37df8cd05395136575d7856ccd37b8c875d Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Fri, 8 Sep 2023 10:28:09 +0200 Subject: [PATCH 11/29] Fix wrong variable name --- src/libs/actions/Report.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 208d0e39262d..de7b0797acd9 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -414,12 +414,10 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, - value: reportActionsExist(reportID) - ? {} - : { - isLoadingReportActions: true, - isLoadingMoreReportActions: false, - }, + value: { + isLoadingReportActions: true, + isLoadingMoreReportActions: false, + }, }, ]; @@ -476,17 +474,17 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p // If we open an exist report, but it is not present in Onyx yet, we should change the method to set for this report // and we need data to be available when we navigate to the chat page if (_.isEmpty(ReportUtils.getReport(reportID))) { - onyxData.optimisticReportData.onyxMethod = Onyx.METHOD.SET; + onyxData.optimisticData[0].onyxMethod = Onyx.METHOD.SET; } // If we are creating a new report, we need to add the optimistic report data and a report action if (!_.isEmpty(newReportObject)) { // Change the method to set for new reports because it doesn't exist yet, is faster, // and we need the data to be available when we navigate to the chat page - onyxData.optimisticReportData[0].onyxMethod = Onyx.METHOD.SET; - onyxData.optimisticReportData[0].value = { + onyxData.optimisticData[0].onyxMethod = Onyx.METHOD.SET; + onyxData.optimisticData[0].value = { reportName: CONST.REPORT.DEFAULT_REPORT_NAME, - ...onyxData.optimisticReportData.value, + ...onyxData.optimisticData[0].value, ...newReportObject, pendingFields: { createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, From 3619a252ee31267f6c25d7f0fe0448032163466b Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 5 Sep 2023 14:16:40 +0200 Subject: [PATCH 12/29] Report screen performance improvements - Add initial values to different components to allow immediate mounting/rendering - Delay the initialization of certain values to allow React to flush the queue immediately - Remove unnecessary Onyx values --- src/components/ExceededCommentLength.js | 1 + .../ReportActionItem/TaskPreview.js | 2 + .../AppNavigator/ReportScreenIDSetter.js | 129 ++++++++++++++++++ .../AppNavigator/ReportScreenWrapper.js | 112 ++------------- src/libs/Navigation/NavigationRoot.js | 5 +- src/pages/home/HeaderView.js | 36 ++--- src/pages/home/ReportScreen.js | 89 +++++++----- .../SilentCommentUpdater.js | 1 + src/pages/home/report/ReportActionItem.js | 2 + src/pages/home/report/ReportActionsList.js | 61 +++++---- src/pages/home/report/ReportActionsView.js | 5 + src/pages/home/report/ReportFooter.js | 1 + .../home/report/ReportTypingIndicator.js | 1 + 13 files changed, 263 insertions(+), 182 deletions(-) create mode 100644 src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js diff --git a/src/components/ExceededCommentLength.js b/src/components/ExceededCommentLength.js index 2aa50779e10f..97ce63cc4144 100644 --- a/src/components/ExceededCommentLength.js +++ b/src/components/ExceededCommentLength.js @@ -54,5 +54,6 @@ ExceededCommentLength.displayName = 'ExceededCommentLength'; export default withOnyx({ comment: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, + initialValue: null, }, })(ExceededCommentLength); diff --git a/src/components/ReportActionItem/TaskPreview.js b/src/components/ReportActionItem/TaskPreview.js index ca4103624440..47716c0ec65a 100644 --- a/src/components/ReportActionItem/TaskPreview.js +++ b/src/components/ReportActionItem/TaskPreview.js @@ -116,9 +116,11 @@ export default compose( withOnyx({ taskReport: { key: ({taskReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${taskReportID}`, + initialValue: {}, }, personalDetailsList: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, + initialValue: {}, }, }), )(TaskPreview); diff --git a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js new file mode 100644 index 000000000000..85e34ae36aa3 --- /dev/null +++ b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js @@ -0,0 +1,129 @@ +import {useEffect} from 'react'; +import PropTypes from 'prop-types'; +import lodashGet from 'lodash/get'; +import {withOnyx} from 'react-native-onyx'; +import ONYXKEYS from '../../../ONYXKEYS'; +import * as ReportUtils from '../../ReportUtils'; +import reportPropTypes from '../../../pages/reportPropTypes'; +import {withNavigationPropTypes} from '../../../components/withNavigation'; +import * as App from '../../actions/App'; +import usePermissions from '../../../hooks/usePermissions'; +import CONST from '../../../CONST'; +import Navigation from '../Navigation'; + +const propTypes = { + /** Available reports that would be displayed in this navigator */ + reports: PropTypes.objectOf(reportPropTypes), + + /** The policies which the user has access to */ + policies: PropTypes.objectOf( + PropTypes.shape({ + /** The policy name */ + name: PropTypes.string, + + /** The type of the policy */ + type: PropTypes.string, + }), + ), + + isFirstTimeNewExpensifyUser: PropTypes.bool, + + /** Navigation route context info provided by react navigation */ + route: PropTypes.shape({ + /** Route specific parameters used on this screen */ + params: PropTypes.shape({ + /** If the admin room should be opened */ + openOnAdminRoom: PropTypes.bool, + + /** The ID of the report this screen should display */ + reportID: PropTypes.string, + }), + }).isRequired, + + ...withNavigationPropTypes, +}; + +const defaultProps = { + reports: {}, + policies: {}, + isFirstTimeNewExpensifyUser: false, +}; + +/** + * Get the most recently accessed report for the user + * + * @param {Object} reports + * @param {Boolean} [ignoreDefaultRooms] + * @param {Object} policies + * @param {Boolean} isFirstTimeNewExpensifyUser + * @param {Boolean} openOnAdminRoom + * @returns {Number} + */ +const getLastAccessedReportID = (reports, ignoreDefaultRooms, policies, isFirstTimeNewExpensifyUser, openOnAdminRoom) => { + // If deeplink url is of an attachment, we should show the report that the attachment comes from. + const currentRoute = Navigation.getActiveRoute(); + const matches = CONST.REGEX.ATTACHMENT_ROUTE.exec(currentRoute); + const reportID = lodashGet(matches, 1, null); + if (reportID) { + return reportID; + } + + const lastReport = ReportUtils.findLastAccessedReport(reports, ignoreDefaultRooms, policies, isFirstTimeNewExpensifyUser, openOnAdminRoom); + + return lodashGet(lastReport, 'reportID'); +}; + +// This wrapper is reponsible for opening the last accessed report if there is no reportID specified in the route params +function ReportScreenIDSetter(props) { + const {canUseDefaultRooms} = usePermissions(); + + useEffect(() => { + // Don't update if there is a reportID in the params already + if (lodashGet(props.route, 'params.reportID', null)) { + App.confirmReadyToOpenApp(); + return; + } + + // If there is no reportID in route, try to find last accessed and use it for setParams + const reportID = getLastAccessedReportID( + props.reports, + !canUseDefaultRooms, + props.policies, + props.isFirstTimeNewExpensifyUser, + lodashGet(props.route, 'params.openOnAdminRoom', false), + ); + + // It's possible that props.reports aren't fully loaded yet + // in that case the reportID is undefined + if (reportID) { + performance.mark('ReportScreenConfirmer_hook'); + performance.measure('ReportScreenConfirmer_hook_meassure', {start: 'ReportScreenConfirmer_hook', duration: 100}); + props.navigation.setParams({reportID: String(reportID)}); + } else { + App.confirmReadyToOpenApp(); + } + }, [props.route, props.navigation, props.reports, canUseDefaultRooms, props.policies, props.isFirstTimeNewExpensifyUser]); + + // The ReportScreen without the reportID set will display a skeleton + // until the reportID is loaded and set in the route param + return null; +} + +ReportScreenIDSetter.propTypes = propTypes; +ReportScreenIDSetter.defaultProps = defaultProps; +ReportScreenIDSetter.displayName = 'ReportScreenIDSetter'; + +export default withOnyx({ + reports: { + key: ONYXKEYS.COLLECTION.REPORT, + allowStaleData: true, + }, + policies: { + key: ONYXKEYS.COLLECTION.POLICY, + allowStaleData: true, + }, + isFirstTimeNewExpensifyUser: { + key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER, + initialValue: false, + }, +})(ReportScreenIDSetter); diff --git a/src/libs/Navigation/AppNavigator/ReportScreenWrapper.js b/src/libs/Navigation/AppNavigator/ReportScreenWrapper.js index f1743e1a2269..767bd9793ac2 100644 --- a/src/libs/Navigation/AppNavigator/ReportScreenWrapper.js +++ b/src/libs/Navigation/AppNavigator/ReportScreenWrapper.js @@ -1,36 +1,10 @@ -import React, {useEffect} from 'react'; import PropTypes from 'prop-types'; -import lodashGet from 'lodash/get'; -import {withOnyx} from 'react-native-onyx'; - -import ONYXKEYS from '../../../ONYXKEYS'; - -import ReportScreen from '../../../pages/home/ReportScreen'; -import * as ReportUtils from '../../ReportUtils'; -import reportPropTypes from '../../../pages/reportPropTypes'; +import React from 'react'; import {withNavigationPropTypes} from '../../../components/withNavigation'; -import * as App from '../../actions/App'; -import usePermissions from '../../../hooks/usePermissions'; -import CONST from '../../../CONST'; -import Navigation from '../Navigation'; +import ReportScreen from '../../../pages/home/ReportScreen'; +import ReportScreenIDSetter from './ReportScreenIDSetter'; const propTypes = { - /** Available reports that would be displayed in this navigator */ - reports: PropTypes.objectOf(reportPropTypes), - - /** The policies which the user has access to */ - policies: PropTypes.objectOf( - PropTypes.shape({ - /** The policy name */ - name: PropTypes.string, - - /** The type of the policy */ - type: PropTypes.string, - }), - ), - - isFirstTimeNewExpensifyUser: PropTypes.bool, - /** Navigation route context info provided by react navigation */ route: PropTypes.shape({ /** Route specific parameters used on this screen */ @@ -46,82 +20,24 @@ const propTypes = { ...withNavigationPropTypes, }; -const defaultProps = { - reports: {}, - policies: {}, - isFirstTimeNewExpensifyUser: false, -}; - -/** - * Get the most recently accessed report for the user - * - * @param {Object} reports - * @param {Boolean} [ignoreDefaultRooms] - * @param {Object} policies - * @param {Boolean} isFirstTimeNewExpensifyUser - * @param {Boolean} openOnAdminRoom - * @returns {Number} - */ -const getLastAccessedReportID = (reports, ignoreDefaultRooms, policies, isFirstTimeNewExpensifyUser, openOnAdminRoom) => { - // If deeplink url is of an attachment, we should show the report that the attachment comes from. - const currentRoute = Navigation.getActiveRoute(); - const matches = CONST.REGEX.ATTACHMENT_ROUTE.exec(currentRoute); - const reportID = lodashGet(matches, 1, null); - if (reportID) { - return reportID; - } - - const lastReport = ReportUtils.findLastAccessedReport(reports, ignoreDefaultRooms, policies, isFirstTimeNewExpensifyUser, openOnAdminRoom); - - return lodashGet(lastReport, 'reportID'); -}; +const defaultProps = {}; -// This wrapper is reponsible for opening the last accessed report if there is no reportID specified in the route params function ReportScreenWrapper(props) { - const {canUseDefaultRooms} = usePermissions(); - - useEffect(() => { - // Don't update if there is a reportID in the params already - if (lodashGet(props.route, 'params.reportID', null)) { - App.confirmReadyToOpenApp(); - return; - } - - // If there is no reportID in route, try to find last accessed and use it for setParams - const reportID = getLastAccessedReportID( - props.reports, - !canUseDefaultRooms, - props.policies, - props.isFirstTimeNewExpensifyUser, - lodashGet(props.route, 'params.openOnAdminRoom', false), - ); - - // It's possible that props.reports aren't fully loaded yet - // in that case the reportID is undefined - if (reportID) { - props.navigation.setParams({reportID: String(reportID)}); - } else { - App.confirmReadyToOpenApp(); - } - }, [props.route, props.navigation, props.reports, canUseDefaultRooms, props.policies, props.isFirstTimeNewExpensifyUser]); - // The ReportScreen without the reportID set will display a skeleton // until the reportID is loaded and set in the route param - return ; + return ( + <> + + + + ); } ReportScreenWrapper.propTypes = propTypes; ReportScreenWrapper.defaultProps = defaultProps; ReportScreenWrapper.displayName = 'ReportScreenWrapper'; -export default withOnyx({ - reports: { - key: ONYXKEYS.COLLECTION.REPORT, - }, - policies: { - key: ONYXKEYS.COLLECTION.POLICY, - }, - isFirstTimeNewExpensifyUser: { - key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER, - }, -})(ReportScreenWrapper); +export default ReportScreenWrapper; diff --git a/src/libs/Navigation/NavigationRoot.js b/src/libs/Navigation/NavigationRoot.js index 42d6627d6699..b992961f60ed 100644 --- a/src/libs/Navigation/NavigationRoot.js +++ b/src/libs/Navigation/NavigationRoot.js @@ -116,7 +116,10 @@ function NavigationRoot(props) { if (!state) { return; } - updateCurrentReportID(state); + // Performance optimization to avoid context consumers to delay first render + setTimeout(() => { + updateCurrentReportID(state); + }, 0); parseAndLogRoute(state); animateStatusBarBackgroundColor(); }; diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index 74994e4dc9d0..d9251a89af3b 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -25,7 +25,6 @@ import reportPropTypes from '../reportPropTypes'; import ONYXKEYS from '../../ONYXKEYS'; import ThreeDotsMenu from '../../components/ThreeDotsMenu'; import * as Task from '../../libs/actions/Task'; -import reportActionPropTypes from './report/reportActionPropTypes'; import PressableWithoutFeedback from '../../components/Pressable/PressableWithoutFeedback'; import PinButton from '../../components/PinButton'; import TaskHeaderActionButton from '../../components/TaskHeaderActionButton'; @@ -44,33 +43,22 @@ const propTypes = { /** Onyx Props */ parentReport: reportPropTypes, - /** The details about the account that the user is signing in with */ - account: PropTypes.shape({ - /** URL to the assigned guide's appointment booking calendar */ - guideCalendarLink: PropTypes.string, - }), + /** URL to the assigned guide's appointment booking calendar */ + guideCalendarLink: PropTypes.string, /** Current user session */ session: PropTypes.shape({ accountID: PropTypes.number, }), - /** The report actions from the parent report */ - // TO DO: Replace with HOC https://github.com/Expensify/App/issues/18769. - // eslint-disable-next-line react/no-unused-prop-types - parentReportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)), - ...windowDimensionsPropTypes, ...withLocalizePropTypes, }; const defaultProps = { personalDetails: {}, - parentReportActions: {}, report: null, - account: { - guideCalendarLink: null, - }, + guideCalendarLink: null, parentReport: {}, session: { accountID: 0, @@ -92,11 +80,10 @@ function HeaderView(props) { const parentNavigationSubtitleData = ReportUtils.getParentNavigationSubtitle(reportHeaderData); const isConcierge = ReportUtils.hasSingleParticipant(props.report) && _.contains(participants, CONST.ACCOUNT_ID.CONCIERGE); const isAutomatedExpensifyAccount = ReportUtils.hasSingleParticipant(props.report) && ReportUtils.hasAutomatedExpensifyAccountIDs(participants); - const guideCalendarLink = lodashGet(props.account, 'guideCalendarLink'); // We hide the button when we are chatting with an automated Expensify account since it's not possible to contact // these users via alternative means. It is possible to request a call with Concierge so we leave the option for them. - const shouldShowCallButton = (isConcierge && guideCalendarLink) || (!isAutomatedExpensifyAccount && !isTaskReport); + const shouldShowCallButton = (isConcierge && props.guideCalendarLink) || (!isAutomatedExpensifyAccount && !isTaskReport); const threeDotMenuItems = []; if (isTaskReport) { const canModifyTask = Task.canModifyTask(props.report, props.session.accountID); @@ -219,7 +206,7 @@ function HeaderView(props) { {shouldShowCallButton && ( )} @@ -244,17 +231,10 @@ export default compose( withWindowDimensions, withLocalize, withOnyx({ - account: { + guideCalendarLink: { key: ONYXKEYS.ACCOUNT, - selector: (account) => - account && { - guideCalendarLink: account.guideCalendarLink, - primaryLogin: account.primaryLogin, - }, - }, - parentReportActions: { - key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`, - canEvict: false, + selector: (account) => (account && account.guideCalendarLink ? account.guideCalendarLink : null), + initialValue: null, }, parentReport: { key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID || report.reportID}`, diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index c6370ea92e3f..80f4cf8b42ef 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -90,6 +90,9 @@ const propTypes = { /** All of the personal details for everyone */ personalDetails: PropTypes.objectOf(personalDetailsPropType), + /** Onyx function that marks the component ready for hydration */ + allowOnyxUpdates: PropTypes.func, + ...windowDimensionsPropTypes, ...viewportOffsetTopPropTypes, }; @@ -109,6 +112,7 @@ const defaultProps = { policies: {}, accountManagerReportID: null, personalDetails: {}, + allowOnyxUpdates: null, }; /** @@ -131,6 +135,7 @@ function ReportScreen({ reportActions, accountManagerReportID, personalDetails, + allowOnyxUpdates, policies, translate, network, @@ -157,7 +162,7 @@ function ReportScreen({ const shouldHideReport = !ReportUtils.canAccessReport(report, policies, betas); - const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails) || firstRenderRef.current; + const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails); const parentReportAction = ReportActionsUtils.getParentReportAction(report); const isDeletedParentAction = ReportActionsUtils.isDeletedParentAction(parentReportAction); @@ -304,6 +309,14 @@ function ReportScreen({ ComposerActions.setShouldShowComposeInput(true); }, [route, report, errors, fetchReportIfNeeded, prevReport.reportID]); + const onListLayout = useCallback(() => { + if (!allowOnyxUpdates) { + return; + } + allowOnyxUpdates(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + return ( @@ -336,7 +349,7 @@ function ReportScreen({ )} - {Boolean(accountManagerReportID) && ReportUtils.isConciergeChatReport(report) && isBannerVisible && ( + {!!accountManagerReportID && ReportUtils.isConciergeChatReport(report) && isBannerVisible && ( `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`, - canEvict: false, - selector: ReportActionsUtils.getSortedReportActionsForDisplay, + withOnyx( + { + isSidebarLoaded: { + key: ONYXKEYS.IS_SIDEBAR_LOADED, + }, + reportActions: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`, + canEvict: false, + selector: ReportActionsUtils.getSortedReportActionsForDisplay, + }, + report: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, + allowStaleData: true, + }, + reportMetadata: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`, + initialValue: { + isLoadingReportActions: false, + isLoadingMoreReportActions: false, + }, + }, + isComposerFullSize: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${getReportID(route)}`, + initialValue: false, + }, + betas: { + key: ONYXKEYS.BETAS, + }, + policies: { + key: ONYXKEYS.COLLECTION.POLICY, + allowStaleData: true, + }, + accountManagerReportID: { + key: ONYXKEYS.ACCOUNT_MANAGER_REPORT_ID, + initialValue: null, + }, + personalDetails: { + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + }, }, - report: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, + { + delayUpdates: true, }, - reportMetadata: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`, - }, - isComposerFullSize: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${getReportID(route)}`, - }, - betas: { - key: ONYXKEYS.BETAS, - }, - policies: { - key: ONYXKEYS.COLLECTION.POLICY, - }, - accountManagerReportID: { - key: ONYXKEYS.ACCOUNT_MANAGER_REPORT_ID, - }, - personalDetails: { - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - }, - }), + ), )(ReportScreen); diff --git a/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js b/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js index da5dc326d421..90b9fcaa6668 100644 --- a/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js +++ b/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js @@ -68,5 +68,6 @@ SilentCommentUpdater.displayName = 'SilentCommentUpdater'; export default withOnyx({ comment: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, + initialValue: null, }, })(SilentCommentUpdater); diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 324d47080f88..9962409dd6da 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -632,12 +632,14 @@ export default compose( withOnyx({ preferredSkinTone: { key: ONYXKEYS.PREFERRED_EMOJI_SKIN_TONE, + initialValue: CONST.EMOJI_DEFAULT_SKIN_TONE, }, iouReport: { key: ({action}) => `${ONYXKEYS.COLLECTION.REPORT}${ReportActionsUtils.getIOUReportIDFromReportActionPreview(action)}`, }, emojiReactions: { key: ({action}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${action.reportActionID}`, + initialValue: {}, }, }), )( diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index febf9d809d7f..52916fca7655 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -119,6 +119,7 @@ function ReportActionsList({ const scrollingVerticalOffset = useRef(0); const readActionSkipped = useRef(false); const reportActionSize = useRef(sortedReportActions.length); + const firstRenderRef = useRef(true); // This state is used to force a re-render when the user manually marks a message as unread // by using a timestamp you can force re-renders without having to worry about if another message was marked as unread before @@ -286,6 +287,40 @@ function ReportActionsList({ const hideComposer = ReportUtils.shouldDisableWriteActions(report); const shouldShowReportRecipientLocalTime = ReportUtils.canShowReportRecipientLocalTime(personalDetailsList, report, currentUserPersonalDetails.accountID) && !isComposerFullSize; + const renderFooter = useCallback(() => { + if (firstRenderRef.current) { + firstRenderRef.current = false; + return null; + } + + if (isLoadingMoreReportActions) { + return ; + } + + // Make sure the oldest report action loaded is not the first. This is so we do not show the + // skeleton view above the created action in a newly generated optimistic chat or one with not + // that many comments. + const lastReportAction = _.last(sortedReportActions) || {}; + if (isLoadingReportActions && lastReportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) { + return ( + + ); + } + + return null; + }, [isLoadingMoreReportActions, isLoadingReportActions, sortedReportActions, isOffline, skeletonViewHeight]); + + const onLayoutInner = useCallback( + (event) => { + setSkeletonViewHeight(event.nativeEvent.layout.height); + onLayout(event); + }, + [onLayout], + ); + return ( <> { - if (isLoadingMoreReportActions) { - return ; - } - - // Make sure the oldest report action loaded is not the first. This is so we do not show the - // skeleton view above the created action in a newly generated optimistic chat or one with not - // that many comments. - const lastReportAction = _.last(sortedReportActions) || {}; - if (isLoadingReportActions && lastReportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) { - return ( - - ); - } - - return null; - }} + ListFooterComponent={renderFooter} keyboardShouldPersistTaps="handled" - onLayout={(event) => { - setSkeletonViewHeight(event.nativeEvent.layout.height); - onLayout(event); - }} + onLayout={onLayoutInner} onScroll={trackVerticalScrolling} extraData={extraData} /> diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 73f4e37e152f..f21884fe4770 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -50,6 +50,9 @@ const propTypes = { avatar: PropTypes.string, }), + /** Used by parent component to tell Onyx when to hydrate data */ + onLayout: PropTypes.func.isRequired, + ...windowDimensionsPropTypes, ...withLocalizePropTypes, }; @@ -163,6 +166,8 @@ function ReportActionsView(props) { * Runs when the FlatList finishes laying out */ const recordTimeToMeasureItemLayout = () => { + props.onLayout(); + if (didLayout.current) { return; } diff --git a/src/pages/home/report/ReportFooter.js b/src/pages/home/report/ReportFooter.js index 8d92c09b7a6e..80486b514106 100644 --- a/src/pages/home/report/ReportFooter.js +++ b/src/pages/home/report/ReportFooter.js @@ -102,5 +102,6 @@ export default compose( withWindowDimensions, withOnyx({ shouldShowComposeInput: {key: ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT}, + initialValue: false, }), )(ReportFooter); diff --git a/src/pages/home/report/ReportTypingIndicator.js b/src/pages/home/report/ReportTypingIndicator.js index 4de649c7eb49..db97f712d65f 100755 --- a/src/pages/home/report/ReportTypingIndicator.js +++ b/src/pages/home/report/ReportTypingIndicator.js @@ -74,6 +74,7 @@ export default compose( withOnyx({ userTypingStatuses: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, + initialValue: {}, }, }), )(ReportTypingIndicator); From eb44749ed2314086412b1358c3b51cc2ebb6f97f Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 11 Sep 2023 11:56:02 +0200 Subject: [PATCH 13/29] Prevent SidebarLinks from re-rendering on opening report --- src/pages/home/sidebar/SidebarLinks.js | 19 +------------------ src/pages/home/sidebar/SidebarLinksData.js | 1 - 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index f0dc6639c8a5..d276cb935941 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -3,7 +3,6 @@ import React from 'react'; import {View, InteractionManager} from 'react-native'; import _ from 'underscore'; import PropTypes from 'prop-types'; -import {withOnyx} from 'react-native-onyx'; import styles from '../../../styles/styles'; import * as StyleUtils from '../../../styles/StyleUtils'; import ONYXKEYS from '../../../ONYXKEYS'; @@ -52,22 +51,11 @@ const propTypes = { priorityMode: PropTypes.oneOf(_.values(CONST.PRIORITY_MODE)), - /** The top most report id */ - currentReportID: PropTypes.string, - - /* Onyx Props */ - report: PropTypes.shape({ - /** reportID (only present when there is a matching report) */ - reportID: PropTypes.string, - }), - ...withLocalizePropTypes, }; const defaultProps = { priorityMode: CONST.PRIORITY_MODE.DEFAULT, - currentReportID: '', - report: {}, }; class SidebarLinks extends React.PureComponent { @@ -151,7 +139,7 @@ class SidebarLinks extends React.PureComponent { // or when clicking the active LHN row // or when continuously clicking different LHNs, only apply to small screen // since getTopmostReportId always returns on other devices - if (this.props.isCreateMenuOpen || this.props.currentReportID === option.reportID || (this.props.isSmallScreenWidth && Navigation.getTopmostReportId())) { + if (this.props.isCreateMenuOpen || option.reportID === Navigation.getTopmostReportId() || (this.props.isSmallScreenWidth && Navigation.getTopmostReportId())) { return; } Navigation.navigate(ROUTES.getReportRoute(option.reportID)); @@ -210,10 +198,5 @@ SidebarLinks.defaultProps = defaultProps; export default compose( withLocalize, withWindowDimensions, - withOnyx({ - report: { - key: ({currentReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${currentReportID}`, - }, - }), )(SidebarLinks); export {basePropTypes}; diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 8681e01f1a67..3eca506f7591 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -88,7 +88,6 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr style={[styles.flex1, styles.h100]} > Date: Tue, 12 Sep 2023 11:35:59 +0200 Subject: [PATCH 14/29] Change delayUpdates prop on ReportScreen to new shouldDelayUpdates --- src/pages/home/ReportScreen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 80f4cf8b42ef..6364eed581d4 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -458,7 +458,7 @@ export default compose( }, }, { - delayUpdates: true, + shouldDelayUpdates: true, }, ), )(ReportScreen); From ceda6442aa6fd2ed0ce73675c04184a6c269acc5 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 14 Sep 2023 12:38:30 +0200 Subject: [PATCH 15/29] Remove options object from ReportScreen withOnyx, replace with shouldDelayUpdates flag --- src/pages/home/ReportScreen.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index bfc963450e26..9f28df274d3c 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -462,8 +462,6 @@ export default compose( key: ONYXKEYS.PERSONAL_DETAILS_LIST, }, }, - { - shouldDelayUpdates: true, - }, + true ), )(ReportScreen); From 12c8fa91a32d824da8a8e87d8db490a49163ef6b Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 14 Sep 2023 15:48:37 +0200 Subject: [PATCH 16/29] Get rid of unused currentReportId prop --- src/pages/home/sidebar/SidebarLinksData.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 4baf9fe13952..5d0a21038d68 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -92,7 +92,6 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr style={[styles.flex1, styles.h100]} > Date: Thu, 14 Sep 2023 16:22:05 +0200 Subject: [PATCH 17/29] Remove containerHeight from ReportActionsSkeletonView because onLayoutEvent is too slow --- .../ReportActionsSkeletonView/index.js | 6 ++-- src/pages/home/ReportScreen.js | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/components/ReportActionsSkeletonView/index.js b/src/components/ReportActionsSkeletonView/index.js index 2fe7e590afef..c2fe040b1e15 100644 --- a/src/components/ReportActionsSkeletonView/index.js +++ b/src/components/ReportActionsSkeletonView/index.js @@ -1,12 +1,10 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {View} from 'react-native'; +import {View, Dimensions} from 'react-native'; import SkeletonViewLines from './SkeletonViewLines'; import CONST from '../../CONST'; const propTypes = { - /** Height of the container component */ - containerHeight: PropTypes.number.isRequired, /** Whether to animate the skeleton view */ shouldAnimate: PropTypes.bool, @@ -18,7 +16,7 @@ const defaultProps = { function ReportActionsSkeletonView(props) { // Determines the number of content items based on container height - const possibleVisibleContentItems = Math.ceil(props.containerHeight / CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT); + const possibleVisibleContentItems = Math.ceil(Dimensions.get('window').height / CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT); const skeletonViewLines = []; for (let index = 0; index < possibleVisibleContentItems; index++) { const iconIndex = (index + 1) % 4; diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 347999fd3dec..29060d5d3fa2 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -368,22 +368,23 @@ function ReportScreen({ /> )} - {/* Note: The ReportActionsSkeletonView should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then - we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */} - {(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && } + {/* Note: The ReportActionsSkeletonView should be allowed to mount even if the initial report actions are not loaded. + If we prevent rendering the report while they are loading then + we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */} + {(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && + + } {isReportReadyForDisplay ? ( - <> - - + ) : ( Date: Thu, 14 Sep 2023 19:06:57 +0200 Subject: [PATCH 18/29] Lint --- src/components/Reactions/ReportActionItemEmojiReactions.js | 1 - src/components/ReportActionsSkeletonView/index.js | 1 - src/pages/home/ReportScreen.js | 4 +--- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/Reactions/ReportActionItemEmojiReactions.js b/src/components/Reactions/ReportActionItemEmojiReactions.js index a8fa69c2b8fd..e72c9d9381fa 100644 --- a/src/components/Reactions/ReportActionItemEmojiReactions.js +++ b/src/components/Reactions/ReportActionItemEmojiReactions.js @@ -41,7 +41,6 @@ const defaultProps = { }; function ReportActionItemEmojiReactions(props) { - const reactionListRef = useContext(ReactionListContext); const popoverReactionListAnchors = useRef({}); diff --git a/src/components/ReportActionsSkeletonView/index.js b/src/components/ReportActionsSkeletonView/index.js index c2fe040b1e15..6bdc993c2055 100644 --- a/src/components/ReportActionsSkeletonView/index.js +++ b/src/components/ReportActionsSkeletonView/index.js @@ -5,7 +5,6 @@ import SkeletonViewLines from './SkeletonViewLines'; import CONST from '../../CONST'; const propTypes = { - /** Whether to animate the skeleton view */ shouldAnimate: PropTypes.bool, }; diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 29060d5d3fa2..5ffa4ed9202c 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -371,9 +371,7 @@ function ReportScreen({ {/* Note: The ReportActionsSkeletonView should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */} - {(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && - - } + {(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && } {isReportReadyForDisplay ? ( Date: Fri, 15 Sep 2023 11:25:48 +0200 Subject: [PATCH 19/29] Removed skeleton height calculation --- src/pages/home/ReportScreen.js | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index f765d2003c1b..653ec7bb27fd 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -148,8 +148,6 @@ function ReportScreen({ const flatListRef = useRef(); const reactionListRef = useRef(); const prevReport = usePrevious(report); - - const [skeletonViewContainerHeight, setSkeletonViewContainerHeight] = useState(0); const [isBannerVisible, setIsBannerVisible] = useState(true); const reportID = getReportID(route); @@ -253,18 +251,6 @@ function ReportScreen({ [route], ); - const calculateSkeletonViewHeight = (event) => { - // Rounding this value for comparison because they can look like this: 411.9999694824219 - const newSkeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height); - - // The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it - // takes up so we can set the skeleton view container height. - if (newSkeletonViewContainerHeight === 0) { - return; - } - setSkeletonViewContainerHeight(newSkeletonViewContainerHeight); - }; - useEffect(() => { const unsubscribeVisibilityListener = Visibility.onVisibilityChange(() => { const isTopMostReportID = Navigation.getTopmostReportId() === getReportID(route); @@ -353,10 +339,7 @@ function ReportScreen({ /> )} - + {isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && ( )} From 035874c7f0a7c7c01795618f33d238ac1f5e61c8 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 21 Sep 2023 09:25:52 +0200 Subject: [PATCH 20/29] Fix botched merge --- src/pages/home/report/ReportActionsList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 2e1ef3d80a35..1290203c1f7b 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -178,7 +178,7 @@ function ReportActionsList({ const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report); if (didManuallyMarkReportAsUnread) { // Clearing the current unread marker so that it can be recalculated - currentUnreadMarker.current = null; + setCurrentUnreadMarker(null); setMessageManuallyMarkedUnread(new Date().getTime()); } else { setMessageManuallyMarkedUnread(0); From 492f847995c7d74b8b2d6ffc32d25943679ada24 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 21 Sep 2023 10:41:54 +0200 Subject: [PATCH 21/29] Change onListLayout from actions to container --- src/pages/home/ReportScreen.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index fd21a6e06643..892c60e4c5fc 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -321,6 +321,7 @@ function ReportScreen({ if (!markReadyForHydration) { return; } + markReadyForHydration(); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -375,11 +376,13 @@ function ReportScreen({ /> )} - + {isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && ( Date: Thu, 21 Sep 2023 11:48:09 +0200 Subject: [PATCH 22/29] Add check before calling onLayout on ReportActionsView --- src/pages/home/report/ReportActionsView.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 377c66e2eee7..d8ec76457503 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -166,7 +166,9 @@ function ReportActionsView(props) { * Runs when the FlatList finishes laying out */ const recordTimeToMeasureItemLayout = () => { - props.onLayout(); + if (props.onLayout) { + props.onLayout(); + } if (didLayout.current) { return; From 9eddbea83a437983a5f31b6027baab4f1af0a9bf Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 21 Sep 2023 11:59:51 +0200 Subject: [PATCH 23/29] Completely remove onLayout prop from ReportActionsView --- src/pages/home/report/ReportActionsView.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index d8ec76457503..f58c6644cd47 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -50,9 +50,6 @@ const propTypes = { avatar: PropTypes.string, }), - /** Used by parent component to tell Onyx when to hydrate data */ - onLayout: PropTypes.func.isRequired, - ...windowDimensionsPropTypes, ...withLocalizePropTypes, }; @@ -166,10 +163,6 @@ function ReportActionsView(props) { * Runs when the FlatList finishes laying out */ const recordTimeToMeasureItemLayout = () => { - if (props.onLayout) { - props.onLayout(); - } - if (didLayout.current) { return; } From 66670d541d32b594938bc69e6274017bf421693f Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 25 Sep 2023 08:11:21 +0200 Subject: [PATCH 24/29] PR comments --- src/components/ExceededCommentLength.js | 2 +- src/pages/home/HeaderView.js | 2 +- src/pages/home/ReportScreen.js | 14 +------------- src/pages/home/report/ReportActionsList.js | 15 +++++---------- src/pages/home/report/ReportFooter.js | 8 +++++++- 5 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/components/ExceededCommentLength.js b/src/components/ExceededCommentLength.js index f68f0c4532f9..9c785cec0395 100644 --- a/src/components/ExceededCommentLength.js +++ b/src/components/ExceededCommentLength.js @@ -63,6 +63,6 @@ ExceededCommentLength.displayName = 'ExceededCommentLength'; export default withOnyx({ comment: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, - initialValue: null, + initialValue: '', }, })(ExceededCommentLength); diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index 775fbfc1eb30..b7d9a923d0bf 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -236,7 +236,7 @@ export default compose( withOnyx({ guideCalendarLink: { key: ONYXKEYS.ACCOUNT, - selector: (account) => (account && account.guideCalendarLink ? account.guideCalendarLink : null), + selector: (account) => (account && account.guideCalendarLink) || null, initialValue: null, }, parentReport: { diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 892c60e4c5fc..c41294ec54d4 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -16,10 +16,8 @@ import * as ReportUtils from '../../libs/ReportUtils'; import ReportActionsView from './report/ReportActionsView'; import ReportActionsSkeletonView from '../../components/ReportActionsSkeletonView'; import reportActionPropTypes from './report/reportActionPropTypes'; -import {withNetwork} from '../../components/OnyxProvider'; import compose from '../../libs/compose'; import Visibility from '../../libs/Visibility'; -import networkPropTypes from '../../components/networkPropTypes'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/withWindowDimensions'; import OfflineWithFeedback from '../../components/OfflineWithFeedback'; import ReportFooter from './report/ReportFooter'; @@ -84,9 +82,6 @@ const propTypes = { }), ), - /** Information about the network */ - network: networkPropTypes.isRequired, - /** The account manager report ID */ accountManagerReportID: PropTypes.string, @@ -152,7 +147,6 @@ function ReportScreen({ markReadyForHydration, policies, translate, - network, isSmallScreenWidth, isSidebarLoaded, viewportOffsetTop, @@ -338,7 +332,6 @@ function ReportScreen({ ) : ( - + )} @@ -429,7 +418,6 @@ export default compose( withViewportOffsetTop, withLocalize, withWindowDimensions, - withNetwork(), withCurrentReportID, withOnyx( { diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 1290203c1f7b..74c87ec7e7c7 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -138,7 +138,6 @@ function ReportActionsList({ useEffect(() => { opacity.value = withTiming(1, {duration: 100}); }, [opacity]); - const [skeletonViewHeight, setSkeletonViewHeight] = useState(0); useEffect(() => { // If the reportID changes, we reset the userActiveSince to null, we need to do it because @@ -340,13 +339,15 @@ function ReportActionsList({ const shouldShowReportRecipientLocalTime = ReportUtils.canShowReportRecipientLocalTime(personalDetailsList, report, currentUserPersonalDetails.accountID) && !isComposerFullSize; const renderFooter = useCallback(() => { + // Skip this hook on the first render, as we are not sure if more actions are going to be loaded + // Therefore showing the skeleton on footer might be misleading if (firstRenderRef.current) { firstRenderRef.current = false; return null; } if (isLoadingMoreReportActions) { - return ; + return ; } // Make sure the oldest report action loaded is not the first. This is so we do not show the @@ -354,20 +355,14 @@ function ReportActionsList({ // that many comments. const lastReportAction = _.last(sortedReportActions) || {}; if (isLoadingReportActions && lastReportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) { - return ( - - ); + return ; } return null; - }, [isLoadingMoreReportActions, isLoadingReportActions, sortedReportActions, isOffline, skeletonViewHeight]); + }, [isLoadingMoreReportActions, isLoadingReportActions, sortedReportActions, isOffline]); const onLayoutInner = useCallback( (event) => { - setSkeletonViewHeight(event.nativeEvent.layout.height); onLayout(event); }, [onLayout], diff --git a/src/pages/home/report/ReportFooter.js b/src/pages/home/report/ReportFooter.js index 80486b514106..e9c6d9d66252 100644 --- a/src/pages/home/report/ReportFooter.js +++ b/src/pages/home/report/ReportFooter.js @@ -17,6 +17,8 @@ import reportActionPropTypes from './reportActionPropTypes'; import reportPropTypes from '../../reportPropTypes'; import * as ReportUtils from '../../../libs/ReportUtils'; import * as Session from '../../../libs/actions/Session'; +import {withNetwork} from '../../../components/OnyxProvider'; +import networkPropTypes from '../../../components/networkPropTypes'; const propTypes = { /** Report object for the current report */ @@ -40,6 +42,9 @@ const propTypes = { /** Whether user interactions should be disabled */ shouldDisableCompose: PropTypes.bool, + /** Information about the network */ + network: networkPropTypes.isRequired, + ...windowDimensionsPropTypes, }; @@ -53,7 +58,7 @@ const defaultProps = { }; function ReportFooter(props) { - const chatFooterStyles = {...styles.chatFooter, minHeight: !props.isOffline ? CONST.CHAT_FOOTER_MIN_HEIGHT : 0}; + const chatFooterStyles = {...styles.chatFooter, minHeight: !props.network.isOffline ? CONST.CHAT_FOOTER_MIN_HEIGHT : 0}; const isArchivedRoom = ReportUtils.isArchivedRoom(props.report); const isAnonymousUser = Session.isAnonymousUser(); @@ -100,6 +105,7 @@ ReportFooter.propTypes = propTypes; ReportFooter.defaultProps = defaultProps; export default compose( withWindowDimensions, + withNetwork(), withOnyx({ shouldShowComposeInput: {key: ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT}, initialValue: false, From 6ecee830735a1cbaa9383edcbbbdc20c7d04cb3c Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 25 Sep 2023 11:35:59 +0200 Subject: [PATCH 25/29] Fix memo on ReportActionItem to re-render when a task is canceled --- src/pages/home/report/ReportActionItem.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 65343f6a61bf..b0447ebf5bec 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -705,6 +705,7 @@ export default compose( _.isEqual(prevProps.emojiReactions, nextProps.emojiReactions) && _.isEqual(prevProps.action, nextProps.action) && _.isEqual(prevProps.report.pendingFields, nextProps.report.pendingFields) && + _.isEqual(prevProps.report.isDeletedParentAction, nextProps.report.isDeletedParentAction) && _.isEqual(prevProps.report.errorFields, nextProps.report.errorFields) && lodashGet(prevProps.report, 'statusNum') === lodashGet(nextProps.report, 'statusNum') && lodashGet(prevProps.report, 'stateNum') === lodashGet(nextProps.report, 'stateNum') && From e3fd39ed7d0179c3d00da040ed3ac1898377ffe4 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 25 Sep 2023 15:57:43 +0200 Subject: [PATCH 26/29] Replace withNetwork HOC with useNetwork hook --- src/pages/home/report/ReportFooter.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/pages/home/report/ReportFooter.js b/src/pages/home/report/ReportFooter.js index e9c6d9d66252..51a8490162e5 100644 --- a/src/pages/home/report/ReportFooter.js +++ b/src/pages/home/report/ReportFooter.js @@ -11,14 +11,13 @@ import ArchivedReportFooter from '../../../components/ArchivedReportFooter'; import compose from '../../../libs/compose'; import ONYXKEYS from '../../../ONYXKEYS'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; +import useNetwork from '../../../hooks/useNetwork'; import styles from '../../../styles/styles'; import variables from '../../../styles/variables'; import reportActionPropTypes from './reportActionPropTypes'; import reportPropTypes from '../../reportPropTypes'; import * as ReportUtils from '../../../libs/ReportUtils'; import * as Session from '../../../libs/actions/Session'; -import {withNetwork} from '../../../components/OnyxProvider'; -import networkPropTypes from '../../../components/networkPropTypes'; const propTypes = { /** Report object for the current report */ @@ -27,9 +26,6 @@ const propTypes = { /** Report actions for the current report */ reportActions: PropTypes.arrayOf(PropTypes.shape(reportActionPropTypes)), - /** Offline status */ - isOffline: PropTypes.bool.isRequired, - /** Callback fired when the comment is submitted */ onSubmitComment: PropTypes.func, @@ -42,9 +38,6 @@ const propTypes = { /** Whether user interactions should be disabled */ shouldDisableCompose: PropTypes.bool, - /** Information about the network */ - network: networkPropTypes.isRequired, - ...windowDimensionsPropTypes, }; @@ -58,7 +51,8 @@ const defaultProps = { }; function ReportFooter(props) { - const chatFooterStyles = {...styles.chatFooter, minHeight: !props.network.isOffline ? CONST.CHAT_FOOTER_MIN_HEIGHT : 0}; + const {isOffline} = useNetwork(); + const chatFooterStyles = {...styles.chatFooter, minHeight: !isOffline ? CONST.CHAT_FOOTER_MIN_HEIGHT : 0}; const isArchivedRoom = ReportUtils.isArchivedRoom(props.report); const isAnonymousUser = Session.isAnonymousUser(); @@ -105,7 +99,6 @@ ReportFooter.propTypes = propTypes; ReportFooter.defaultProps = defaultProps; export default compose( withWindowDimensions, - withNetwork(), withOnyx({ shouldShowComposeInput: {key: ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT}, initialValue: false, From 38253711515494761274127fbc42f49294e4cb54 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 26 Sep 2023 08:31:47 +0200 Subject: [PATCH 27/29] Revert firstRenderRef calculation of isLoading flag --- src/pages/home/ReportScreen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 5840bf7748ad..5f3c907d6d17 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -177,7 +177,7 @@ function ReportScreen({ const shouldHideReport = !ReportUtils.canAccessReport(report, policies, betas); - const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails); + const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails) || firstRenderRef.current; const parentReportAction = ReportActionsUtils.getParentReportAction(report); const isDeletedParentAction = ReportActionsUtils.isDeletedParentAction(parentReportAction); From 24710b76b32a74a19b6bf2b1e94ac1e1ccf86bff Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 26 Sep 2023 13:58:51 +0200 Subject: [PATCH 28/29] Move firstRenderRef from isLoading to shouldShowNotFoundPage --- src/pages/home/ReportScreen.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 5f3c907d6d17..a6101ae6f1fe 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -177,7 +177,7 @@ function ReportScreen({ const shouldHideReport = !ReportUtils.canAccessReport(report, policies, betas); - const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails) || firstRenderRef.current; + const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails); const parentReportAction = ReportActionsUtils.getParentReportAction(report); const isDeletedParentAction = ReportActionsUtils.isDeletedParentAction(parentReportAction); @@ -364,7 +364,16 @@ function ReportScreen({ // eslint-disable-next-line rulesdir/no-negated-variables const shouldShowNotFoundPage = useMemo( - () => (!_.isEmpty(report) && !isDefaultReport && !report.reportID && !isOptimisticDelete && !report.isLoadingReportActions && !isLoading && !userLeavingStatus) || shouldHideReport, + () => + (!firstRenderRef.current && + !_.isEmpty(report) && + !isDefaultReport && + !report.reportID && + !isOptimisticDelete && + !report.isLoadingReportActions && + !isLoading && + !userLeavingStatus) || + shouldHideReport, [report, isLoading, shouldHideReport, isDefaultReport, isOptimisticDelete, userLeavingStatus], ); From f006f1dbb47974a3d2062b4339244e4d35fc5f7b Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 27 Sep 2023 08:19:17 +0200 Subject: [PATCH 29/29] PR Comments --- .../AppNavigator/ReportScreenIDSetter.js | 24 +++++++------------ .../SilentCommentUpdater.js | 2 +- src/pages/home/report/ReportActionsList.js | 5 ++-- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js index 85e34ae36aa3..24f855645870 100644 --- a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js +++ b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.js @@ -53,7 +53,7 @@ const defaultProps = { * Get the most recently accessed report for the user * * @param {Object} reports - * @param {Boolean} [ignoreDefaultRooms] + * @param {Boolean} ignoreDefaultRooms * @param {Object} policies * @param {Boolean} isFirstTimeNewExpensifyUser * @param {Boolean} openOnAdminRoom @@ -74,35 +74,27 @@ const getLastAccessedReportID = (reports, ignoreDefaultRooms, policies, isFirstT }; // This wrapper is reponsible for opening the last accessed report if there is no reportID specified in the route params -function ReportScreenIDSetter(props) { +function ReportScreenIDSetter({route, reports, policies, isFirstTimeNewExpensifyUser, navigation}) { const {canUseDefaultRooms} = usePermissions(); useEffect(() => { // Don't update if there is a reportID in the params already - if (lodashGet(props.route, 'params.reportID', null)) { + if (lodashGet(route, 'params.reportID', null)) { App.confirmReadyToOpenApp(); return; } // If there is no reportID in route, try to find last accessed and use it for setParams - const reportID = getLastAccessedReportID( - props.reports, - !canUseDefaultRooms, - props.policies, - props.isFirstTimeNewExpensifyUser, - lodashGet(props.route, 'params.openOnAdminRoom', false), - ); - - // It's possible that props.reports aren't fully loaded yet + const reportID = getLastAccessedReportID(reports, !canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser, lodashGet(route, 'params.openOnAdminRoom', false)); + + // It's possible that reports aren't fully loaded yet // in that case the reportID is undefined if (reportID) { - performance.mark('ReportScreenConfirmer_hook'); - performance.measure('ReportScreenConfirmer_hook_meassure', {start: 'ReportScreenConfirmer_hook', duration: 100}); - props.navigation.setParams({reportID: String(reportID)}); + navigation.setParams({reportID: String(reportID)}); } else { App.confirmReadyToOpenApp(); } - }, [props.route, props.navigation, props.reports, canUseDefaultRooms, props.policies, props.isFirstTimeNewExpensifyUser]); + }, [route, navigation, reports, canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser]); // The ReportScreen without the reportID set will display a skeleton // until the reportID is loaded and set in the route param diff --git a/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js b/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js index 90b9fcaa6668..09f9d368bdcc 100644 --- a/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js +++ b/src/pages/home/report/ReportActionCompose/SilentCommentUpdater.js @@ -68,6 +68,6 @@ SilentCommentUpdater.displayName = 'SilentCommentUpdater'; export default withOnyx({ comment: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, - initialValue: null, + initialValue: '', }, })(SilentCommentUpdater); diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index e2363e1da685..0163a7ff2b4f 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -177,10 +177,11 @@ function ReportActionsList({ // Clearing the current unread marker so that it can be recalculated setCurrentUnreadMarker(null); setMessageManuallyMarkedUnread(new Date().getTime()); - } else { - setMessageManuallyMarkedUnread(0); + return; } + setMessageManuallyMarkedUnread(0); + // We only care when a new lastReadTime is set in the report // eslint-disable-next-line react-hooks/exhaustive-deps }, [report.lastReadTime]);