From 482d0016179a4fb7f1a9fc47ef9205a58c3a8c66 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Mon, 9 Oct 2023 09:46:59 +0300 Subject: [PATCH 01/20] Wrap ReportActionItemFragment inside a Text component --- .../home/report/ReportActionItemMessage.js | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 88223e6480ff..41f955dc6b0e 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -49,24 +49,31 @@ function ReportActionItemMessage(props) { return ( - {!props.isHidden ? ( - _.map(messages, (fragment, index) => ( - + {!props.isHidden ? ( + _.map(messages, (fragment, index) => ( + - )) - ) : ( - {props.translate('moderation.flaggedContent')} - )} + /> + )) + ) : ( + {props.translate('moderation.flaggedContent')} + )} + ); } From b591c52ed738a86d7092a0398cc10bc9256323b1 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Mon, 23 Oct 2023 12:46:55 +0300 Subject: [PATCH 02/20] Restore grey color and regular font weight to APPROVED/SUBMITTED messages --- src/pages/home/report/ReportActionItemFragment.js | 12 +++++++++++- src/pages/home/report/ReportActionItemMessage.js | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 57b51ef50519..2d750786f722 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import React, {memo} from 'react'; import PropTypes from 'prop-types'; import Str from 'expensify-common/lib/str'; @@ -63,6 +64,9 @@ const propTypes = { /** Whether the comment is a thread parent message/the first message in a thread */ isThreadParentMessage: PropTypes.bool, + /** The report's action name/type e.g. APPROVED, SUBMITTED, etc. */ + actionName: PropTypes.string, + ...windowDimensionsPropTypes, /** localization props */ @@ -86,6 +90,7 @@ const defaultProps = { delegateAccountID: 0, actorIcon: {}, isThreadParentMessage: false, + actionName: '', displayAsGroup: false, }; @@ -161,7 +166,12 @@ function ReportActionItemFragment(props) { > {props.fragment.text} diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 41f955dc6b0e..2919e598ce04 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -67,7 +67,8 @@ function ReportActionItemMessage(props) { source={lodashGet(props.action, 'originalMessage.source')} accountID={props.action.actorAccountID} style={props.style} - displayAsGroup={props.displayAsGroup} + displayAsGroup={props.displayAsGroup} + actionName={props.action.actionName} /> )) ) : ( From 51a4f16f67e7702df308038ae7815b3598700d34 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Mon, 23 Oct 2023 21:20:46 +0300 Subject: [PATCH 03/20] Conditionally surround ReportActionItemFragment with Text component --- .../home/report/ReportActionItemFragment.js | 3 +- .../home/report/ReportActionItemMessage.js | 60 +++++++++++-------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 2d750786f722..27dfbfc7095e 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -169,8 +169,7 @@ function ReportActionItemFragment(props) { style={[ styles.chatItemMessageHeaderSender, props.isSingleLine ? styles.pre : styles.preWrap, - _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.actionName) - && {color: styles.colorMuted.color, fontWeight: 'normal'}, + _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.actionName) && {color: styles.colorMuted.color, fontWeight: 'normal'}, ]} > {props.fragment.text} diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 2919e598ce04..635426c4c790 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -9,6 +9,7 @@ import * as ReportUtils from '../../../libs/ReportUtils'; import * as ReportActionsUtils from '../../../libs/ReportActionsUtils'; import reportActionPropTypes from './reportActionPropTypes'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; +import CONST from '../../../CONST'; const propTypes = { /** The report action */ @@ -47,34 +48,41 @@ function ReportActionItemMessage(props) { } } + const isApprovedOrSubmittedReportActionType = _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.action.actionName); + + const flaggedContentText = {props.translate('moderation.flaggedContent')}; + + const getReportActionItemFragment = (fragment, index) => { + return ( + + ); + }; + + const content = !props.isHidden ? _.map(messages, (fragment, index) => getReportActionItemFragment(fragment, index)) : flaggedContentText; + return ( - {/* - Wrapping ReportActionItemFragment inside 'Text' so that text isn't broken up into separate lines when - there are multiple messages of type 'TEXT', as seen when approving a report from a policy on Old - Dot and then viewing the report on New Dot. - */} - - {!props.isHidden ? ( - _.map(messages, (fragment, index) => ( - - )) - ) : ( - {props.translate('moderation.flaggedContent')} - )} - + {isApprovedOrSubmittedReportActionType ? ( + // Wrapping 'ReportActionItemFragment' inside '' so that text isn't broken up into separate lines when + // there are multiple messages of type 'TEXT', as seen when a report is submitted/approved from a + // policy on Old Dot and then viewed on New Dot. + + {content} + ) : ( + <>{content} + )} ); } From c10864549a91dca6c0d16932f14b972ab72958e0 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Mon, 23 Oct 2023 22:43:35 +0300 Subject: [PATCH 04/20] Remove return statement surrounding ReportActionItemFragment This was causing a lint error during PR checks --- .../home/report/ReportActionItemMessage.js | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 635426c4c790..a9690bb601e1 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -52,23 +52,21 @@ function ReportActionItemMessage(props) { const flaggedContentText = {props.translate('moderation.flaggedContent')}; - const getReportActionItemFragment = (fragment, index) => { - return ( - - ); - }; + const getReportActionItemFragment = (fragment, index) => ( + + ); const content = !props.isHidden ? _.map(messages, (fragment, index) => getReportActionItemFragment(fragment, index)) : flaggedContentText; From 9c0677861ff6b7eddd86ad0fe3d4ba8a61351944 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Tue, 24 Oct 2023 09:02:49 +0300 Subject: [PATCH 05/20] Surround 'props.fragment.text' in curly braces Caught this little error currently on latest main that would've rendered 'props.framgent.text' literally instead of the actual text --- src/pages/home/report/ReportActionItemFragment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 27dfbfc7095e..d6b3a40812a7 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -191,7 +191,7 @@ function ReportActionItemFragment(props) { case 'OLD_MESSAGE': return OLD_MESSAGE; default: - return props.fragment.text; + return {props.fragment.text}; } } From 9165187411f5a9f6790dfddbe9c48754d09bef4a Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Tue, 24 Oct 2023 14:23:41 +0300 Subject: [PATCH 06/20] Describe function using JS Docs syntax & use inline ternary statement --- src/pages/home/report/ReportActionItemMessage.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index a9690bb601e1..1aa38e2fe19c 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -52,7 +52,13 @@ function ReportActionItemMessage(props) { const flaggedContentText = {props.translate('moderation.flaggedContent')}; - const getReportActionItemFragment = (fragment, index) => ( + /** + * Get a ReportActionItemFragment + * @param {Object} fragment the current message fragment + * @param {Number} index the current message fragment's index + * @returns {Object} report action item fragment + */ + const renderReportActionItemFragment = (fragment, index) => ( ); - const content = !props.isHidden ? _.map(messages, (fragment, index) => getReportActionItemFragment(fragment, index)) : flaggedContentText; - return ( {isApprovedOrSubmittedReportActionType ? ( @@ -77,9 +81,9 @@ function ReportActionItemMessage(props) { // there are multiple messages of type 'TEXT', as seen when a report is submitted/approved from a // policy on Old Dot and then viewed on New Dot. - {content} + {!props.isHidden ? _.map(messages, (fragment, index) => renderReportActionItemFragment(fragment, index)) : flaggedContentText} ) : ( - <>{content} + <>{!props.isHidden ? _.map(messages, (fragment, index) => renderReportActionItemFragment(fragment, index)) : flaggedContentText} )} ); From 89baf40e1fc7f55b8c37c5027fcdc7582a764a0e Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Tue, 24 Oct 2023 14:32:37 +0300 Subject: [PATCH 07/20] Revert "Surround 'props.fragment.text' in curly braces" This reverts commit 9c0677861ff6b7eddd86ad0fe3d4ba8a61351944. --- src/pages/home/report/ReportActionItemFragment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index d6b3a40812a7..27dfbfc7095e 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -191,7 +191,7 @@ function ReportActionItemFragment(props) { case 'OLD_MESSAGE': return OLD_MESSAGE; default: - return {props.fragment.text}; + return props.fragment.text; } } From 286ed532a9c7132be46648ad1a04f5f32c23eb7b Mon Sep 17 00:00:00 2001 From: Vicktor <79470910+Victor-Nyagudi@users.noreply.github.com> Date: Thu, 26 Oct 2023 09:41:36 +0300 Subject: [PATCH 08/20] Reword comment explaining reason for wrapping ReportActionItemFragment with This is done as per @jjcoffee's suggestion. Co-authored-by: Joel Davies --- src/pages/home/report/ReportActionItemMessage.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 1aa38e2fe19c..b6f270c5b9cd 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -77,9 +77,8 @@ function ReportActionItemMessage(props) { return ( {isApprovedOrSubmittedReportActionType ? ( - // Wrapping 'ReportActionItemFragment' inside '' so that text isn't broken up into separate lines when - // there are multiple messages of type 'TEXT', as seen when a report is submitted/approved from a - // policy on Old Dot and then viewed on New Dot. + // Approving or submitting reports in oldDot results in system messages made up of multiple fragments of `TEXT` type + // which we need to wrap in `` to prevent them rendering on separate lines. {!props.isHidden ? _.map(messages, (fragment, index) => renderReportActionItemFragment(fragment, index)) : flaggedContentText} ) : ( From 1d698f3d0a362bd67fd46ee48fcbd37c14a8004f Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Thu, 26 Oct 2023 10:59:08 +0300 Subject: [PATCH 09/20] Pass isApprovedOrSubmittedReportActionType prop instead of actionName to ReportActionItemFragment --- src/pages/home/report/ReportActionItemFragment.js | 8 ++++---- src/pages/home/report/ReportActionItemMessage.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 27dfbfc7095e..fb9b3a6aa8ac 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -64,8 +64,8 @@ const propTypes = { /** Whether the comment is a thread parent message/the first message in a thread */ isThreadParentMessage: PropTypes.bool, - /** The report's action name/type e.g. APPROVED, SUBMITTED, etc. */ - actionName: PropTypes.string, + /** Whether the report action type is 'APPROVED' or 'SUBMITTED'. Used to style system messages from Old Dot */ + isApprovedOrSubmittedReportActionType: PropTypes.bool, ...windowDimensionsPropTypes, @@ -90,7 +90,7 @@ const defaultProps = { delegateAccountID: 0, actorIcon: {}, isThreadParentMessage: false, - actionName: '', + isApprovedOrSubmittedReportActionType: false, displayAsGroup: false, }; @@ -169,7 +169,7 @@ function ReportActionItemFragment(props) { style={[ styles.chatItemMessageHeaderSender, props.isSingleLine ? styles.pre : styles.preWrap, - _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.actionName) && {color: styles.colorMuted.color, fontWeight: 'normal'}, + props.isApprovedOrSubmittedReportActionType && {color: styles.colorMuted.color, fontWeight: 'normal'}, ]} > {props.fragment.text} diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index b6f270c5b9cd..51f8dd2b762e 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -70,7 +70,7 @@ function ReportActionItemMessage(props) { accountID={props.action.actorAccountID} style={props.style} displayAsGroup={props.displayAsGroup} - actionName={props.action.actionName} + isApprovedOrSubmittedReportActionType={isApprovedOrSubmittedReportActionType} /> ); From 533fbf76c1690c83de1d400215908703bde32d16 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Thu, 26 Oct 2023 11:20:58 +0300 Subject: [PATCH 10/20] Remove unused import statement --- src/pages/home/report/ReportActionItemFragment.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index fb9b3a6aa8ac..194a2ace8097 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -1,4 +1,3 @@ -import _ from 'underscore'; import React, {memo} from 'react'; import PropTypes from 'prop-types'; import Str from 'expensify-common/lib/str'; From 20e26f0f912eb73fc207080d6a6b934429132961 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Thu, 26 Oct 2023 17:01:37 +0300 Subject: [PATCH 11/20] Wrap initial ReportActionItemFragments in Text This commit also gets rid of the renderReportActionItemFragment() method and flaggedContentText variable that are no longer necessary --- .../home/report/ReportActionItemMessage.js | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 51f8dd2b762e..3d55f6e4fa08 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -50,39 +50,31 @@ function ReportActionItemMessage(props) { const isApprovedOrSubmittedReportActionType = _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.action.actionName); - const flaggedContentText = {props.translate('moderation.flaggedContent')}; - - /** - * Get a ReportActionItemFragment - * @param {Object} fragment the current message fragment - * @param {Number} index the current message fragment's index - * @returns {Object} report action item fragment - */ - const renderReportActionItemFragment = (fragment, index) => ( - - ); - return ( - {isApprovedOrSubmittedReportActionType ? ( + {!props.isHidden ? ( // Approving or submitting reports in oldDot results in system messages made up of multiple fragments of `TEXT` type // which we need to wrap in `` to prevent them rendering on separate lines. - - {!props.isHidden ? _.map(messages, (fragment, index) => renderReportActionItemFragment(fragment, index)) : flaggedContentText} + + + {_.map(messages, (fragment, index) => ( + + ))} + ) : ( - <>{!props.isHidden ? _.map(messages, (fragment, index) => renderReportActionItemFragment(fragment, index)) : flaggedContentText} + {props.translate('moderation.flaggedContent')} )} ); From 11d669e5f7538ac6a00e17f6b2e19b257a965197 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Thu, 26 Oct 2023 17:12:49 +0300 Subject: [PATCH 12/20] Ran prettier again so linter is happy --- src/pages/home/report/ReportActionItemMessage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 3d55f6e4fa08..41dfb7e39ec4 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -55,7 +55,7 @@ function ReportActionItemMessage(props) { {!props.isHidden ? ( // Approving or submitting reports in oldDot results in system messages made up of multiple fragments of `TEXT` type // which we need to wrap in `` to prevent them rendering on separate lines. - + {_.map(messages, (fragment, index) => ( Date: Thu, 26 Oct 2023 17:38:38 +0300 Subject: [PATCH 13/20] Create helper method to replace inline stlyes --- src/pages/home/report/ReportActionItemFragment.js | 2 +- src/styles/styles.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 194a2ace8097..cf720cb4afd3 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -168,7 +168,7 @@ function ReportActionItemFragment(props) { style={[ styles.chatItemMessageHeaderSender, props.isSingleLine ? styles.pre : styles.preWrap, - props.isApprovedOrSubmittedReportActionType && {color: styles.colorMuted.color, fontWeight: 'normal'}, + styles.approvedOrSubmittedMessage(props.isApprovedOrSubmittedReportActionType), ]} > {props.fragment.text} diff --git a/src/styles/styles.ts b/src/styles/styles.ts index d08b947ff680..6977a7f97100 100644 --- a/src/styles/styles.ts +++ b/src/styles/styles.ts @@ -4022,6 +4022,8 @@ const styles = (theme: ThemeDefault) => singleOptionSelectorCircle: { borderColor: theme.icon, }, + + approvedOrSubmittedMessage: (isApprovedOrSubmittedMessage: boolean) => (isApprovedOrSubmittedMessage ? {color: theme.textSupporting, fontWeight: 'normal'} : {}), } satisfies Styles); // For now we need to export the styles function that takes the theme as an argument From 244dabfd50f3449a050dfbcec9443a2967e030b3 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Thu, 26 Oct 2023 17:44:12 +0300 Subject: [PATCH 14/20] Rename isApprovedOrSubmittedReportActionType The 'type' at the end felt redundant. This can inferred from reading the comments above the prop types wherever it's used --- src/pages/home/report/ReportActionItemFragment.js | 6 +++--- src/pages/home/report/ReportActionItemMessage.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index cf720cb4afd3..0261785cdab8 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -64,7 +64,7 @@ const propTypes = { isThreadParentMessage: PropTypes.bool, /** Whether the report action type is 'APPROVED' or 'SUBMITTED'. Used to style system messages from Old Dot */ - isApprovedOrSubmittedReportActionType: PropTypes.bool, + isApprovedOrSubmittedReportAction: PropTypes.bool, ...windowDimensionsPropTypes, @@ -89,7 +89,7 @@ const defaultProps = { delegateAccountID: 0, actorIcon: {}, isThreadParentMessage: false, - isApprovedOrSubmittedReportActionType: false, + isApprovedOrSubmittedReportAction: false, displayAsGroup: false, }; @@ -168,7 +168,7 @@ function ReportActionItemFragment(props) { style={[ styles.chatItemMessageHeaderSender, props.isSingleLine ? styles.pre : styles.preWrap, - styles.approvedOrSubmittedMessage(props.isApprovedOrSubmittedReportActionType), + styles.approvedOrSubmittedMessage(props.isApprovedOrSubmittedReportAction), ]} > {props.fragment.text} diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 41dfb7e39ec4..51cc2f44cd82 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -48,7 +48,7 @@ function ReportActionItemMessage(props) { } } - const isApprovedOrSubmittedReportActionType = _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.action.actionName); + const isApprovedOrSubmittedReportAction = _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.action.actionName); return ( @@ -69,7 +69,7 @@ function ReportActionItemMessage(props) { accountID={props.action.actorAccountID} style={props.style} displayAsGroup={props.displayAsGroup} - isApprovedOrSubmittedReportActionType={isApprovedOrSubmittedReportActionType} + isApprovedOrSubmittedReportAction={isApprovedOrSubmittedReportAction} /> ))} From dbe5c08e063634e75f0b2aa0658da9dbec7dcd58 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Sat, 28 Oct 2023 18:37:04 +0300 Subject: [PATCH 15/20] Move styling helper method to StyleUtils This felt like the most logical place to put the helper method seeing how all the other helper methods are located here. --- src/pages/home/report/ReportActionItemFragment.js | 3 ++- src/styles/StyleUtils.ts | 9 +++++++++ src/styles/styles.ts | 2 -- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 0261785cdab8..3a5dcc196dde 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import Str from 'expensify-common/lib/str'; import reportActionFragmentPropTypes from './reportActionFragmentPropTypes'; import styles from '../../../styles/styles'; +import * as StyleUtils from '../../../styles/StyleUtils'; import variables from '../../../styles/variables'; import themeColors from '../../../styles/themes/default'; import RenderHTML from '../../../components/RenderHTML'; @@ -168,7 +169,7 @@ function ReportActionItemFragment(props) { style={[ styles.chatItemMessageHeaderSender, props.isSingleLine ? styles.pre : styles.preWrap, - styles.approvedOrSubmittedMessage(props.isApprovedOrSubmittedReportAction), + StyleUtils.getApprovedOrSubmittedReportTextStyles(props.isApprovedOrSubmittedReportAction), ]} > {props.fragment.text} diff --git a/src/styles/StyleUtils.ts b/src/styles/StyleUtils.ts index 48100b2efb60..5f65ffc55c50 100644 --- a/src/styles/StyleUtils.ts +++ b/src/styles/StyleUtils.ts @@ -1316,11 +1316,20 @@ function getTransparentColor(color: string) { return `${color}00`; } +/** + * Get the styles of reports submitted or approved in Old Dot in order to style them like system messages + */ +function getApprovedOrSubmittedReportTextStyles(isApprovedOrSubmittedReport = false): TextStyle { + // Font family is restored back to a regular font since text with "fontWeight: 'normal'" on Android still appears boldened + return isApprovedOrSubmittedReport ? {color: themeColors.textSupporting, fontFamily: fontFamily.EXP_NEUE, fontWeight: 'normal'} : {}; +} + export { combineStyles, displayIfTrue, getAmountFontSizeAndLineHeight, getAnimatedFABStyle, + getApprovedOrSubmittedReportTextStyles, getAutoCompleteSuggestionContainerStyle, getAutoCompleteSuggestionItemStyle, getAutoGrowHeightInputStyle, diff --git a/src/styles/styles.ts b/src/styles/styles.ts index d8b22ff6275d..c5946801abd7 100644 --- a/src/styles/styles.ts +++ b/src/styles/styles.ts @@ -4027,8 +4027,6 @@ const styles = (theme: ThemeDefault) => singleOptionSelectorCircle: { borderColor: theme.icon, }, - - approvedOrSubmittedMessage: (isApprovedOrSubmittedMessage: boolean) => (isApprovedOrSubmittedMessage ? {color: theme.textSupporting, fontWeight: 'normal'} : {}), } satisfies Styles); // For now we need to export the styles function that takes the theme as an argument From 5dda804beb9bda9d75ba24aa99c1996b1222eca6 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Sat, 28 Oct 2023 18:58:09 +0300 Subject: [PATCH 16/20] Conditionally wrap fragment text in a tooltip --- .../home/report/ReportActionItemFragment.js | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 3a5dcc196dde..1356ddab1947 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -157,25 +157,32 @@ function ReportActionItemFragment(props) { ); } - case 'TEXT': - return ( + case 'TEXT': { + const textFragment = ( + + {props.fragment.text} + + ); + + return props.isApprovedOrSubmittedReportAction ? ( + textFragment + ) : ( - - {props.fragment.text} - + {textFragment} ); + } case 'LINK': return LINK; case 'INTEGRATION_COMMENT': From cb548ebfff79b7da83ffbbb689f9a721cb8290b1 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Sun, 29 Oct 2023 09:40:12 +0300 Subject: [PATCH 17/20] Run pretteir to fix diffs --- src/pages/home/report/ReportActionItemFragment.js | 2 +- src/pages/home/report/ReportActionItemMessage.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index b84fdbaf3e49..af7d14f341eb 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -13,9 +13,9 @@ import compose from '@libs/compose'; import convertToLTR from '@libs/convertToLTR'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import * as EmojiUtils from '@libs/EmojiUtils'; -import * as StyleUtils from '@styles/StyleUtils'; import editedLabelStyles from '@styles/editedLabelStyles'; import styles from '@styles/styles'; +import * as StyleUtils from '@styles/StyleUtils'; import themeColors from '@styles/themes/default'; import variables from '@styles/variables'; import CONST from '@src/CONST'; diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 5a142429e3ed..73ce64365be4 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -7,9 +7,9 @@ import withLocalize, {withLocalizePropTypes} from '@components/withLocalize'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import styles from '@styles/styles'; +import CONST from '@src/CONST'; import ReportActionItemFragment from './ReportActionItemFragment'; import reportActionPropTypes from './reportActionPropTypes'; -import CONST from '@src/CONST'; const propTypes = { /** The report action */ From 53796b639cac3071c9a8bc1b5343dc89eea42b04 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Tue, 31 Oct 2023 10:02:30 +0300 Subject: [PATCH 18/20] Remove check in getApprovedOrSubmittedReportTextStyles Since this helper method is only used in one place so far, it's better to conditionally call it in the place it's being used rather than have the check in the method and return an empty object --- src/pages/home/report/ReportActionItemFragment.js | 2 +- src/styles/StyleUtils.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index af7d14f341eb..24301af52a76 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -164,7 +164,7 @@ function ReportActionItemFragment(props) { style={[ styles.chatItemMessageHeaderSender, props.isSingleLine ? styles.pre : styles.preWrap, - StyleUtils.getApprovedOrSubmittedReportTextStyles(props.isApprovedOrSubmittedReportAction), + props.isApprovedOrSubmittedReportAction && StyleUtils.getApprovedOrSubmittedReportTextStyles(), ]} > {props.fragment.text} diff --git a/src/styles/StyleUtils.ts b/src/styles/StyleUtils.ts index 6d29180b13c6..25b87e8f04af 100644 --- a/src/styles/StyleUtils.ts +++ b/src/styles/StyleUtils.ts @@ -1319,9 +1319,9 @@ function getTransparentColor(color: string) { /** * Get the styles of reports submitted or approved in Old Dot in order to style them like system messages */ -function getApprovedOrSubmittedReportTextStyles(isApprovedOrSubmittedReport = false): TextStyle { - // Font family is restored back to a regular font since text with "fontWeight: 'normal'" on Android still appears boldened - return isApprovedOrSubmittedReport ? {color: themeColors.textSupporting, fontFamily: fontFamily.EXP_NEUE, fontWeight: 'normal'} : {}; +function getApprovedOrSubmittedReportTextStyles(): TextStyle { + // Font family is restored back to a regular font since text with "fontWeight: 'normal'" on Android still appears in bold + return {color: themeColors.textSupporting, fontFamily: fontFamily.EXP_NEUE, fontWeight: 'normal'}; } /** From f669405e5edda8668d0c4dbfd1d2e56c37622f91 Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Fri, 3 Nov 2023 00:04:19 +0300 Subject: [PATCH 19/20] Conditionally wrap report action fragments in Text and account for RTL scenarios Arabic display names caused the text to start from the right side of the screen even though the system message is in English --- .../home/report/ReportActionItemFragment.js | 26 ++++----- .../home/report/ReportActionItemMessage.js | 54 ++++++++++++------- src/styles/StyleUtils.ts | 9 ---- 3 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 24301af52a76..322abad0ad1d 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -15,7 +15,6 @@ import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import * as EmojiUtils from '@libs/EmojiUtils'; import editedLabelStyles from '@styles/editedLabelStyles'; import styles from '@styles/styles'; -import * as StyleUtils from '@styles/StyleUtils'; import themeColors from '@styles/themes/default'; import variables from '@styles/variables'; import CONST from '@src/CONST'; @@ -67,6 +66,9 @@ const propTypes = { /** Whether the report action type is 'APPROVED' or 'SUBMITTED'. Used to style system messages from Old Dot */ isApprovedOrSubmittedReportAction: PropTypes.bool, + /** Used to format RTL display names in Old Dot system messages e.g. Arabic */ + shouldConvertToLTR: PropTypes.bool, + ...windowDimensionsPropTypes, /** localization props */ @@ -91,6 +93,7 @@ const defaultProps = { actorIcon: {}, isThreadParentMessage: false, isApprovedOrSubmittedReportAction: false, + shouldConvertToLTR: false, displayAsGroup: false, }; @@ -158,28 +161,25 @@ function ReportActionItemFragment(props) { ); } case 'TEXT': { - const textFragment = ( + return props.isApprovedOrSubmittedReportAction ? ( - {props.fragment.text} + {props.shouldConvertToLTR ? convertToLTR(props.fragment.text) : props.fragment.text} - ); - - return props.isApprovedOrSubmittedReportAction ? ( - textFragment ) : ( - {textFragment} + + {props.fragment.text} + ); } diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index 73ce64365be4..5ae0e6501e0f 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -50,29 +50,43 @@ function ReportActionItemMessage(props) { const isApprovedOrSubmittedReportAction = _.contains([CONST.REPORT.ACTIONS.TYPE.APPROVED, CONST.REPORT.ACTIONS.TYPE.SUBMITTED], props.action.actionName); + /** + * Get the ReportActionItemFragments + * @param {Boolean} shouldWrapInText determines whether the fragments are wrapped in a Text component + * @returns {Object} report action item fragments + */ + const renderReportActionItemFragments = (shouldWrapInText) => { + const reportActionItemFragments = _.map(messages, (fragment, index) => ( + + )); + + // Approving or submitting reports in oldDot results in system messages made up of multiple fragments of `TEXT` type + // which we need to wrap in `` to prevent them rendering on separate lines. + + return shouldWrapInText ? {reportActionItemFragments} : reportActionItemFragments; + }; + return ( {!props.isHidden ? ( - // Approving or submitting reports in oldDot results in system messages made up of multiple fragments of `TEXT` type - // which we need to wrap in `` to prevent them rendering on separate lines. - - - {_.map(messages, (fragment, index) => ( - - ))} - + renderReportActionItemFragments(isApprovedOrSubmittedReportAction) ) : ( {props.translate('moderation.flaggedContent')} )} diff --git a/src/styles/StyleUtils.ts b/src/styles/StyleUtils.ts index 25b87e8f04af..faece4f44335 100644 --- a/src/styles/StyleUtils.ts +++ b/src/styles/StyleUtils.ts @@ -1316,14 +1316,6 @@ function getTransparentColor(color: string) { return `${color}00`; } -/** - * Get the styles of reports submitted or approved in Old Dot in order to style them like system messages - */ -function getApprovedOrSubmittedReportTextStyles(): TextStyle { - // Font family is restored back to a regular font since text with "fontWeight: 'normal'" on Android still appears in bold - return {color: themeColors.textSupporting, fontFamily: fontFamily.EXP_NEUE, fontWeight: 'normal'}; -} - /** * Get the styles of the text next to dot indicators */ @@ -1336,7 +1328,6 @@ export { displayIfTrue, getAmountFontSizeAndLineHeight, getAnimatedFABStyle, - getApprovedOrSubmittedReportTextStyles, getAutoCompleteSuggestionContainerStyle, getAutoCompleteSuggestionItemStyle, getAutoGrowHeightInputStyle, From ffb1e9acc21efbed3b200568cdc2d57a714e4dbf Mon Sep 17 00:00:00 2001 From: Victor Nyagudi Date: Fri, 3 Nov 2023 16:40:55 +0300 Subject: [PATCH 20/20] Rename shouldConvertToLTR prop to isFragmentContainingDisplayName --- src/pages/home/report/ReportActionItemFragment.js | 6 +++--- src/pages/home/report/ReportActionItemMessage.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 322abad0ad1d..bbaa13484614 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -67,7 +67,7 @@ const propTypes = { isApprovedOrSubmittedReportAction: PropTypes.bool, /** Used to format RTL display names in Old Dot system messages e.g. Arabic */ - shouldConvertToLTR: PropTypes.bool, + isFragmentContainingDisplayName: PropTypes.bool, ...windowDimensionsPropTypes, @@ -93,7 +93,7 @@ const defaultProps = { actorIcon: {}, isThreadParentMessage: false, isApprovedOrSubmittedReportAction: false, - shouldConvertToLTR: false, + isFragmentContainingDisplayName: false, displayAsGroup: false, }; @@ -166,7 +166,7 @@ function ReportActionItemFragment(props) { numberOfLines={props.isSingleLine ? 1 : undefined} style={[styles.chatItemMessage, styles.colorMuted]} > - {props.shouldConvertToLTR ? convertToLTR(props.fragment.text) : props.fragment.text} + {props.isFragmentContainingDisplayName ? convertToLTR(props.fragment.text) : props.fragment.text} ) : ( ));