From eac55be68c7cec230e4bfb6837e40bbbe80c20c0 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Thu, 26 May 2022 18:23:04 -0400 Subject: [PATCH 1/7] Remove whitespace: pre because extra empty lines bug --- src/styles/styles.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/styles/styles.js b/src/styles/styles.js index 54729c21abdd..923870fd3abd 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -135,7 +135,6 @@ const webViewStyles = { fontSize: variables.fontSizeNormal, fontFamily: fontFamily.GTA, flex: 1, - whiteSpace: 'pre', }, }; From c7e2d84196cb890343fe923ddec528db8d225c9e Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Thu, 9 Jun 2022 13:31:30 -0700 Subject: [PATCH 2/7] WIP Use flag for comments coming from email reply A comment flag with source = 'email' should be displayed with whitespace: 'normal'; --- .../BaseHTMLEngineProvider.js | 5 ++ .../HTMLEngineProvider/htmlEngineUtils.js | 2 +- .../home/report/ReportActionItemFragment.js | 59 ++++++++++++------- .../home/report/ReportActionItemMessage.js | 1 + src/styles/styles.js | 1 + 5 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js index 874a7364d084..1476a81633b3 100755 --- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js +++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js @@ -33,6 +33,11 @@ const customHTMLElementModels = { }), comment: defaultHTMLElementModels.div.extend({ tagName: 'comment', + mixedUAStyles: {whiteSpace: 'pre', backgroundColor: 'blue'}, + }), + 'email-comment': defaultHTMLElementModels.div.extend({ + tagName: 'email-comment', + mixedUAStyles: {whiteSpace: 'normal', backgroundColor: 'green'}, }), }; diff --git a/src/components/HTMLEngineProvider/htmlEngineUtils.js b/src/components/HTMLEngineProvider/htmlEngineUtils.js index dc678b99d601..19468a82d342 100644 --- a/src/components/HTMLEngineProvider/htmlEngineUtils.js +++ b/src/components/HTMLEngineProvider/htmlEngineUtils.js @@ -26,7 +26,7 @@ function computeEmbeddedMaxWidth(tagName, contentWidth) { function isInsideComment(tnode) { let currentNode = tnode; while (currentNode.parent) { - if (currentNode.domNode.name === 'comment') { + if (currentNode.domNode.name === 'comment' || currentNode.domNode.name === 'email-comment') { return true; } currentNode = currentNode.parent; diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index e70becf96f45..12a9f4b651be 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -44,6 +44,9 @@ const propTypes = { /** Does this fragment belong to a reportAction that has not yet loaded? */ loading: PropTypes.bool, + /** The reportAction's source */ + source: PropTypes.string, + /** Should this fragment be contained in a single line? */ isSingleLine: PropTypes.bool, @@ -64,6 +67,7 @@ const defaultProps = { loading: false, isSingleLine: false, tooltipText: '', + source: '', }; const ReportActionItemFragment = (props) => { @@ -86,39 +90,52 @@ const ReportActionItemFragment = (props) => { ) ); } + let {html, text} = props.fragment; // If the only difference between fragment.text and fragment.html is
tags // we replace them with line breaks and render it as text, not as html. // This is done to render emojis with line breaks between them as text. - const differByLineBreaksOnly = props.fragment.html.replaceAll('
', ' ') === props.fragment.text; + const differByLineBreaksOnly = html.replaceAll('
', ' ') === text; if (differByLineBreaksOnly) { - const textWithLineBreaks = props.fragment.html.replaceAll('
', '\n'); + const textWithLineBreaks = html.replaceAll('
', '\n'); // eslint-disable-next-line no-param-reassign - props.fragment = {...props.fragment, text: textWithLineBreaks, html: textWithLineBreaks}; + html = textWithLineBreaks; + text = textWithLineBreaks; } // Only render HTML if we have html in the fragment - return props.fragment.html !== props.fragment.text - ? ( + if (html !== text) { + if (props.source === 'email') { + // Messages from email replies usually have complex HTML structure and they don't rely in white-space: pre to preserve spacing, + // instead, the normally use   + return ( + ${html + (props.fragment.isEdited ? '' : '')}`} + /> + ); + } + return ( ${props.fragment.html + (props.fragment.isEdited ? '' : '')}`} + html={`${html + (props.fragment.isEdited ? '' : '')}`} /> - ) : ( - - {Str.htmlDecode(props.fragment.text)} - {props.fragment.isEdited && ( - - {` ${props.translate('reportActionCompose.edited')}`} - - )} - ); + } + return ( + + {Str.htmlDecode(text)} + {props.fragment.isEdited && ( + + {` ${props.translate('reportActionCompose.edited')}`} + + )} + + ); } case 'TEXT': return ( diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index c1eb02a8326d..c034170795a7 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -33,6 +33,7 @@ const ReportActionItemMessage = (props) => { isAttachment={props.action.isAttachment} attachmentInfo={props.action.attachmentInfo} loading={props.action.loading} + source={props.action.originalMessage.source} /> ))} diff --git a/src/styles/styles.js b/src/styles/styles.js index d23d89b17ded..c5eb979e7ee4 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -135,6 +135,7 @@ const webViewStyles = { fontSize: variables.fontSizeNormal, fontFamily: fontFamily.GTA, flex: 1, + // whiteSpace: 'pre', }, }; From db4ae6bd6c7985f57360dbfbe6910dcaa53064fa Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 10 Jun 2022 13:28:31 -0700 Subject: [PATCH 3/7] Remove comment and bg color for debugging --- src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js | 4 ++-- src/styles/styles.js | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js index 1476a81633b3..4d066a08f22b 100755 --- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js +++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js @@ -33,11 +33,11 @@ const customHTMLElementModels = { }), comment: defaultHTMLElementModels.div.extend({ tagName: 'comment', - mixedUAStyles: {whiteSpace: 'pre', backgroundColor: 'blue'}, + mixedUAStyles: {whiteSpace: 'pre'}, }), 'email-comment': defaultHTMLElementModels.div.extend({ tagName: 'email-comment', - mixedUAStyles: {whiteSpace: 'normal', backgroundColor: 'green'}, + mixedUAStyles: {whiteSpace: 'normal'}, }), }; diff --git a/src/styles/styles.js b/src/styles/styles.js index c5eb979e7ee4..d23d89b17ded 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -135,7 +135,6 @@ const webViewStyles = { fontSize: variables.fontSizeNormal, fontFamily: fontFamily.GTA, flex: 1, - // whiteSpace: 'pre', }, }; From 0b9a43f085ee55ed51278a5376c1c986bbecb9d5 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 10 Jun 2022 13:38:24 -0700 Subject: [PATCH 4/7] Avoid crash if action.originalMessage is missing This happens when you type a new message and the message getting rendered is the optimistic data --- src/pages/home/report/ReportActionItemMessage.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index c034170795a7..d2369fb1dd29 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -2,6 +2,7 @@ import React from 'react'; import {View} from 'react-native'; import PropTypes from 'prop-types'; import _ from 'underscore'; +import lodashGet from 'lodash/get'; import styles from '../../../styles/styles'; import ReportActionItemFragment from './ReportActionItemFragment'; import reportActionPropTypes from './reportActionPropTypes'; @@ -33,7 +34,7 @@ const ReportActionItemMessage = (props) => { isAttachment={props.action.isAttachment} attachmentInfo={props.action.attachmentInfo} loading={props.action.loading} - source={props.action.originalMessage.source} + source={lodashGet(props.action, 'originalMessage.source')} /> ))} From 66d1e79d8bd561e4bb1fee4e3b56be3fee0e51c9 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 10 Jun 2022 16:12:17 -0700 Subject: [PATCH 5/7] Apply same logic for email-comment and comment --- src/libs/SelectionScraper/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/SelectionScraper/index.js b/src/libs/SelectionScraper/index.js index 363367072631..a30e71b8b6f7 100644 --- a/src/libs/SelectionScraper/index.js +++ b/src/libs/SelectionScraper/index.js @@ -103,7 +103,7 @@ const replaceNodes = (dom) => { } // Adding a new line after each comment here, because adding after each range is not working for chrome. - if (dom.attribs[tagAttribute] === 'comment') { + if (dom.attribs[tagAttribute] === 'comment' || dom.attribs[tagAttribute] === 'email-comment') { dom.children.push(new Element('br', {})); } } From a732f4b530ca99bdd1cc2e4120fe509de8b00fbd Mon Sep 17 00:00:00 2001 From: Aldo Canepa Garay <87341702+aldo-expensify@users.noreply.github.com> Date: Tue, 28 Jun 2022 14:21:36 -0700 Subject: [PATCH 6/7] Update comment Co-authored-by: Rory Abraham <47436092+roryabraham@users.noreply.github.com> --- 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 c75671043483..90800d9f696e 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -106,7 +106,7 @@ const ReportActionItemFragment = (props) => { if (html !== text) { if (props.source === 'email') { // Messages from email replies usually have complex HTML structure and they don't rely in white-space: pre to preserve spacing, - // instead, the normally use   + // instead, they normally use   return ( ${html + (props.fragment.isEdited ? '' : '')}`} From de3301c92428e7963bb1bf11ca31d8c49a3511de Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Tue, 28 Jun 2022 14:46:05 -0700 Subject: [PATCH 7/7] DRY code --- .../HTMLEngineProvider/htmlEngineUtils.js | 13 ++++++++++++- src/libs/SelectionScraper/index.js | 3 ++- .../home/report/ReportActionItemFragment.js | 17 ++++++----------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/components/HTMLEngineProvider/htmlEngineUtils.js b/src/components/HTMLEngineProvider/htmlEngineUtils.js index 19468a82d342..71a5e9267835 100644 --- a/src/components/HTMLEngineProvider/htmlEngineUtils.js +++ b/src/components/HTMLEngineProvider/htmlEngineUtils.js @@ -17,6 +17,16 @@ function computeEmbeddedMaxWidth(tagName, contentWidth) { return contentWidth; } +/** + * Check if tagName is equal to any of our custom tags wrapping chat comments. + * + * @param {string} tagName + * @returns {Boolean} + */ +function isCommentTag(tagName) { + return tagName === 'email-comment' || tagName === 'comment'; +} + /** * Check if there is an ancestor node with name 'comment'. * Finding node with name 'comment' flags that we are rendering a comment. @@ -26,7 +36,7 @@ function computeEmbeddedMaxWidth(tagName, contentWidth) { function isInsideComment(tnode) { let currentNode = tnode; while (currentNode.parent) { - if (currentNode.domNode.name === 'comment' || currentNode.domNode.name === 'email-comment') { + if (isCommentTag(currentNode.domNode.name)) { return true; } currentNode = currentNode.parent; @@ -37,4 +47,5 @@ function isInsideComment(tnode) { export { computeEmbeddedMaxWidth, isInsideComment, + isCommentTag, }; diff --git a/src/libs/SelectionScraper/index.js b/src/libs/SelectionScraper/index.js index a30e71b8b6f7..7f0a9d69959d 100644 --- a/src/libs/SelectionScraper/index.js +++ b/src/libs/SelectionScraper/index.js @@ -4,6 +4,7 @@ import {parseDocument} from 'htmlparser2'; import {Element} from 'domhandler'; import _ from 'underscore'; import Str from 'expensify-common/lib/str'; +import {isCommentTag} from '../../components/HTMLEngineProvider/htmlEngineUtils'; const elementsWillBeSkipped = ['html', 'body']; const tagAttribute = 'data-testid'; @@ -103,7 +104,7 @@ const replaceNodes = (dom) => { } // Adding a new line after each comment here, because adding after each range is not working for chrome. - if (dom.attribs[tagAttribute] === 'comment' || dom.attribs[tagAttribute] === 'email-comment') { + if (isCommentTag(dom.attribs[tagAttribute])) { dom.children.push(new Element('br', {})); } } diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 90800d9f696e..964142ca5b95 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -45,7 +45,7 @@ const propTypes = { loading: PropTypes.bool, /** The reportAction's source */ - source: PropTypes.string, + source: PropTypes.oneOf(['Chronos', 'email', 'ios', 'android', 'web', 'email', '']), /** Should this fragment be contained in a single line? */ isSingleLine: PropTypes.bool, @@ -104,18 +104,13 @@ const ReportActionItemFragment = (props) => { // Only render HTML if we have html in the fragment if (html !== text) { - if (props.source === 'email') { - // Messages from email replies usually have complex HTML structure and they don't rely in white-space: pre to preserve spacing, - // instead, they normally use   - return ( - ${html + (props.fragment.isEdited ? '' : '')}`} - /> - ); - } + const editedTag = props.fragment.isEdited ? '' : ''; + const htmlContent = html + editedTag; return ( ${html + (props.fragment.isEdited ? '' : '')}`} + html={props.source === 'email' + ? `${htmlContent}` + : `${htmlContent}`} /> ); }