From dea9ed463c2b400377eff6f2da3c17d84e617dc8 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 30 Nov 2023 14:02:47 -0700 Subject: [PATCH 1/4] Remove side-loading of parent report action --- src/components/AvatarWithDisplayName.js | 92 ++++++++++++++++--------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/src/components/AvatarWithDisplayName.js b/src/components/AvatarWithDisplayName.js index de6d6b8ef6e2..bb8bb79c09d1 100644 --- a/src/components/AvatarWithDisplayName.js +++ b/src/components/AvatarWithDisplayName.js @@ -1,18 +1,21 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React from 'react'; +import React, {useCallback, useRef, useEffect} from 'react'; import {View} from 'react-native'; +import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import compose from '@libs/compose'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; -import reportPropTypes from '@pages/reportPropTypes'; import * as StyleUtils from '@styles/StyleUtils'; +import reportPropTypes from '@pages/reportPropTypes'; +import reportActionPropTypes from '@pages/home/report/reportActionPropTypes'; import useTheme from '@styles/themes/useTheme'; import useThemeStyles from '@styles/useThemeStyles'; import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import DisplayNames from './DisplayNames'; import MultipleAvatars from './MultipleAvatars'; @@ -45,6 +48,10 @@ const propTypes = { shouldEnableDetailPageNavigation: PropTypes.bool, + /* Onyx Props */ + /** All of the actions of the report */ + parentReportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)), + ...windowDimensionsPropTypes, ...withLocalizePropTypes, }; @@ -53,41 +60,12 @@ const defaultProps = { personalDetails: {}, policy: {}, report: {}, + parentReportActions: {}, isAnonymous: false, size: CONST.AVATAR_SIZE.DEFAULT, shouldEnableDetailPageNavigation: false, }; -const showActorDetails = (report, shouldEnableDetailPageNavigation = false) => { - // We should navigate to the details page if the report is a IOU/expense report - if (shouldEnableDetailPageNavigation) { - return ReportUtils.navigateToDetailsPage(report); - } - - if (ReportUtils.isExpenseReport(report)) { - Navigation.navigate(ROUTES.PROFILE.getRoute(report.ownerAccountID)); - return; - } - - if (ReportUtils.isIOUReport(report)) { - Navigation.navigate(ROUTES.REPORT_PARTICIPANTS.getRoute(report.reportID)); - return; - } - - if (ReportUtils.isChatThread(report)) { - const parentReportAction = ReportActionsUtils.getParentReportAction(report); - const actorAccountID = lodashGet(parentReportAction, 'actorAccountID', -1); - // in an ideal situation account ID won't be 0 - if (actorAccountID > 0) { - Navigation.navigate(ROUTES.PROFILE.getRoute(actorAccountID)); - return; - } - } - - // report detail route is added as fallback but based on the current implementation this route won't be executed - Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID)); -}; - function AvatarWithDisplayName(props) { const theme = useTheme(); const styles = useThemeStyles(); @@ -102,13 +80,47 @@ function AvatarWithDisplayName(props) { const isExpenseRequest = ReportUtils.isExpenseRequest(props.report); const defaultSubscriptSize = isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : props.size; const avatarBorderColor = props.isAnonymous ? theme.highlightBG : theme.componentBG; + const actorAccountID = useRef(null); + + useEffect(() => { + const parentReportAction = lodashGet(props.parentReportActions, [props.report.parentReportActionID], {}); + actorAccountID.current = lodashGet(parentReportAction, 'actorAccountID', -1); + }, [props]); + + const showActorDetails = useCallback(() => { + // We should navigate to the details page if the report is a IOU/expense report + if (props.shouldEnableDetailPageNavigation) { + return ReportUtils.navigateToDetailsPage(props.report); + } + + if (ReportUtils.isExpenseReport(props.report)) { + Navigation.navigate(ROUTES.PROFILE.getRoute(props.report.ownerAccountID)); + return; + } + + if (ReportUtils.isIOUReport(props.report)) { + Navigation.navigate(ROUTES.REPORT_PARTICIPANTS.getRoute(props.report.reportID)); + return; + } + + if (ReportUtils.isChatThread(props.report)) { + // In an ideal situation account ID won't be 0 + if (actorAccountID.current > 0) { + Navigation.navigate(ROUTES.PROFILE.getRoute(actorAccountID.current)); + return; + } + } + + // Report detail route is added as fallback but based on the current implementation this route won't be executed + Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(props.report.reportID)); + }, [props]); const headerView = ( {Boolean(props.report && title) && ( showActorDetails(props.report, props.shouldEnableDetailPageNavigation)} + onPress={showActorDetails} accessibilityLabel={title} role={CONST.ACCESSIBILITY_ROLE.BUTTON} > @@ -175,4 +187,16 @@ AvatarWithDisplayName.propTypes = propTypes; AvatarWithDisplayName.displayName = 'AvatarWithDisplayName'; AvatarWithDisplayName.defaultProps = defaultProps; -export default compose(withWindowDimensions, withLocalize)(AvatarWithDisplayName); +export default compose( + withWindowDimensions, + withLocalize, + withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, + parentReportActions: { + key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`, + canEvict: false, + }, + }), +)(AvatarWithDisplayName); From a1562004b31ced6633296cd93c998def494b2b3f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 30 Nov 2023 14:10:11 -0700 Subject: [PATCH 2/4] Destructure props and remove unused props --- src/components/AvatarWithDisplayName.js | 83 ++++++++++++------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/src/components/AvatarWithDisplayName.js b/src/components/AvatarWithDisplayName.js index bb8bb79c09d1..baa546e5b6a2 100644 --- a/src/components/AvatarWithDisplayName.js +++ b/src/components/AvatarWithDisplayName.js @@ -4,10 +4,8 @@ import React, {useCallback, useRef, useEffect} from 'react'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; -import compose from '@libs/compose'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; -import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import * as StyleUtils from '@styles/StyleUtils'; import reportPropTypes from '@pages/reportPropTypes'; @@ -24,8 +22,6 @@ import participantPropTypes from './participantPropTypes'; import PressableWithoutFeedback from './Pressable/PressableWithoutFeedback'; import SubscriptAvatar from './SubscriptAvatar'; import Text from './Text'; -import withLocalize, {withLocalizePropTypes} from './withLocalize'; -import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions'; const propTypes = { /** The report currently being looked at */ @@ -51,9 +47,6 @@ const propTypes = { /* Onyx Props */ /** All of the actions of the report */ parentReportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)), - - ...windowDimensionsPropTypes, - ...withLocalizePropTypes, }; const defaultProps = { @@ -66,44 +59,52 @@ const defaultProps = { shouldEnableDetailPageNavigation: false, }; -function AvatarWithDisplayName(props) { +function AvatarWithDisplayName({ + report, + policy, + size, + isAnonymous, + parentReportActions, + personalDetails, + shouldEnableDetailPageNavigation, +}) { const theme = useTheme(); const styles = useThemeStyles(); - const title = ReportUtils.getReportName(props.report); - const subtitle = ReportUtils.getChatRoomSubtitle(props.report); - const parentNavigationSubtitleData = ReportUtils.getParentNavigationSubtitle(props.report); - const isMoneyRequestOrReport = ReportUtils.isMoneyRequestReport(props.report) || ReportUtils.isMoneyRequest(props.report); - const icons = ReportUtils.getIcons(props.report, props.personalDetails, props.policy); - const ownerPersonalDetails = OptionsListUtils.getPersonalDetailsForAccountIDs([props.report.ownerAccountID], props.personalDetails); + const title = ReportUtils.getReportName(report); + const subtitle = ReportUtils.getChatRoomSubtitle(report); + const parentNavigationSubtitleData = ReportUtils.getParentNavigationSubtitle(report); + const isMoneyRequestOrReport = ReportUtils.isMoneyRequestReport(report) || ReportUtils.isMoneyRequest(report); + const icons = ReportUtils.getIcons(report, personalDetails, policy); + const ownerPersonalDetails = OptionsListUtils.getPersonalDetailsForAccountIDs([report.ownerAccountID], personalDetails); const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(_.values(ownerPersonalDetails), false); - const shouldShowSubscriptAvatar = ReportUtils.shouldReportShowSubscript(props.report); - const isExpenseRequest = ReportUtils.isExpenseRequest(props.report); - const defaultSubscriptSize = isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : props.size; - const avatarBorderColor = props.isAnonymous ? theme.highlightBG : theme.componentBG; - const actorAccountID = useRef(null); + const shouldShowSubscriptAvatar = ReportUtils.shouldReportShowSubscript(report); + const isExpenseRequest = ReportUtils.isExpenseRequest(report); + const defaultSubscriptSize = isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : size; + const avatarBorderColor = isAnonymous ? theme.highlightBG : theme.componentBG; + const actorAccountID = useRef(null); useEffect(() => { - const parentReportAction = lodashGet(props.parentReportActions, [props.report.parentReportActionID], {}); + const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID], {}); actorAccountID.current = lodashGet(parentReportAction, 'actorAccountID', -1); - }, [props]); + }, [parentReportActions, report]); const showActorDetails = useCallback(() => { // We should navigate to the details page if the report is a IOU/expense report - if (props.shouldEnableDetailPageNavigation) { - return ReportUtils.navigateToDetailsPage(props.report); + if (shouldEnableDetailPageNavigation) { + return ReportUtils.navigateToDetailsPage(report); } - if (ReportUtils.isExpenseReport(props.report)) { - Navigation.navigate(ROUTES.PROFILE.getRoute(props.report.ownerAccountID)); + if (ReportUtils.isExpenseReport(report)) { + Navigation.navigate(ROUTES.PROFILE.getRoute(report.ownerAccountID)); return; } - if (ReportUtils.isIOUReport(props.report)) { - Navigation.navigate(ROUTES.REPORT_PARTICIPANTS.getRoute(props.report.reportID)); + if (ReportUtils.isIOUReport(report)) { + Navigation.navigate(ROUTES.REPORT_PARTICIPANTS.getRoute(report.reportID)); return; } - if (ReportUtils.isChatThread(props.report)) { + if (ReportUtils.isChatThread(report)) { // In an ideal situation account ID won't be 0 if (actorAccountID.current > 0) { Navigation.navigate(ROUTES.PROFILE.getRoute(actorAccountID.current)); @@ -112,12 +113,12 @@ function AvatarWithDisplayName(props) { } // Report detail route is added as fallback but based on the current implementation this route won't be executed - Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(props.report.reportID)); - }, [props]); + Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID)); + }, [report, shouldEnableDetailPageNavigation]); const headerView = ( - {Boolean(props.report && title) && ( + {Boolean(report && title) && ( )} @@ -145,13 +146,13 @@ function AvatarWithDisplayName(props) { displayNamesWithTooltips={displayNamesWithTooltips} tooltipEnabled numberOfLines={1} - textStyles={[props.isAnonymous ? styles.headerAnonymousFooter : styles.headerText, styles.pre]} - shouldUseFullTitle={isMoneyRequestOrReport || props.isAnonymous} + textStyles={[isAnonymous ? styles.headerAnonymousFooter : styles.headerText, styles.pre]} + shouldUseFullTitle={isMoneyRequestOrReport || isAnonymous} /> {!_.isEmpty(parentNavigationSubtitleData) && ( )} {!_.isEmpty(subtitle) && ( @@ -168,13 +169,13 @@ function AvatarWithDisplayName(props) { ); - if (!props.shouldEnableDetailPageNavigation) { + if (!shouldEnableDetailPageNavigation) { return headerView; } return ( ReportUtils.navigateToDetailsPage(props.report)} + onPress={() => ReportUtils.navigateToDetailsPage(report)} style={[styles.flexRow, styles.alignItemsCenter, styles.flex1]} accessibilityLabel={title} role={CONST.ACCESSIBILITY_ROLE.BUTTON} @@ -187,10 +188,7 @@ AvatarWithDisplayName.propTypes = propTypes; AvatarWithDisplayName.displayName = 'AvatarWithDisplayName'; AvatarWithDisplayName.defaultProps = defaultProps; -export default compose( - withWindowDimensions, - withLocalize, - withOnyx({ +export default withOnyx({ session: { key: ONYXKEYS.SESSION, }, @@ -198,5 +196,4 @@ export default compose( key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`, canEvict: false, }, - }), -)(AvatarWithDisplayName); + })(AvatarWithDisplayName); From 7025f47116f8d7f651a292f6052a1b34380e4491 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 30 Nov 2023 14:46:31 -0700 Subject: [PATCH 3/4] Fix style --- src/components/AvatarWithDisplayName.js | 32 ++++++++++--------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/components/AvatarWithDisplayName.js b/src/components/AvatarWithDisplayName.js index baa546e5b6a2..89d74a6ad3bb 100644 --- a/src/components/AvatarWithDisplayName.js +++ b/src/components/AvatarWithDisplayName.js @@ -1,15 +1,15 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useCallback, useRef, useEffect} from 'react'; +import React, {useCallback, useEffect, useRef} from 'react'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import * as ReportUtils from '@libs/ReportUtils'; -import * as StyleUtils from '@styles/StyleUtils'; -import reportPropTypes from '@pages/reportPropTypes'; import reportActionPropTypes from '@pages/home/report/reportActionPropTypes'; +import reportPropTypes from '@pages/reportPropTypes'; +import * as StyleUtils from '@styles/StyleUtils'; import useTheme from '@styles/themes/useTheme'; import useThemeStyles from '@styles/useThemeStyles'; import CONST from '@src/CONST'; @@ -59,15 +59,7 @@ const defaultProps = { shouldEnableDetailPageNavigation: false, }; -function AvatarWithDisplayName({ - report, - policy, - size, - isAnonymous, - parentReportActions, - personalDetails, - shouldEnableDetailPageNavigation, -}) { +function AvatarWithDisplayName({report, policy, size, isAnonymous, parentReportActions, personalDetails, shouldEnableDetailPageNavigation}) { const theme = useTheme(); const styles = useThemeStyles(); const title = ReportUtils.getReportName(report); @@ -189,11 +181,11 @@ AvatarWithDisplayName.displayName = 'AvatarWithDisplayName'; AvatarWithDisplayName.defaultProps = defaultProps; export default withOnyx({ - session: { - key: ONYXKEYS.SESSION, - }, - parentReportActions: { - key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`, - canEvict: false, - }, - })(AvatarWithDisplayName); + session: { + key: ONYXKEYS.SESSION, + }, + parentReportActions: { + key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`, + canEvict: false, + }, +})(AvatarWithDisplayName); From e056672266b7a09d782209987ec7e9be94e7ea8c Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 1 Dec 2023 10:41:17 -0700 Subject: [PATCH 4/4] Remove unused Onyx key --- src/components/AvatarWithDisplayName.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/AvatarWithDisplayName.js b/src/components/AvatarWithDisplayName.js index 89d74a6ad3bb..9d7e6467e804 100644 --- a/src/components/AvatarWithDisplayName.js +++ b/src/components/AvatarWithDisplayName.js @@ -181,9 +181,6 @@ AvatarWithDisplayName.displayName = 'AvatarWithDisplayName'; AvatarWithDisplayName.defaultProps = defaultProps; export default withOnyx({ - session: { - key: ONYXKEYS.SESSION, - }, parentReportActions: { key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`, canEvict: false,