From a95ddfc5f0ac12ad9072a9fec6e682f79e31ea23 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 20 Dec 2022 14:06:09 -0700 Subject: [PATCH 1/7] Replace double newlines with a single newline --- src/components/Composer/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Composer/index.js b/src/components/Composer/index.js index 34046295f1bf..d5f9246502de 100755 --- a/src/components/Composer/index.js +++ b/src/components/Composer/index.js @@ -265,7 +265,9 @@ class Composer extends React.Component { return; } - const plainText = event.clipboardData.getData('text/plain'); + // The regex replace is there because when doing a "paste without formatting", there are extra line breaks added to the content (I do not know why). To fix the problem, anytime + // there is a double set of newline characters, they are replaced with a single newline. This preserves the same number of newlines between pasting with and without formatting. + const plainText = event.clipboardData.getData('text/plain').replace(/\n\n/g, '\n'); this.paste(Str.htmlDecode(plainText)); } From de90a623424cb6e6f88784b3dbd602722b80afb3 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 20 Dec 2022 14:57:44 -0700 Subject: [PATCH 2/7] Change "newlines" to "new lines" --- src/components/Composer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Composer/index.js b/src/components/Composer/index.js index d5f9246502de..d188bd0810c5 100755 --- a/src/components/Composer/index.js +++ b/src/components/Composer/index.js @@ -266,7 +266,7 @@ class Composer extends React.Component { } // The regex replace is there because when doing a "paste without formatting", there are extra line breaks added to the content (I do not know why). To fix the problem, anytime - // there is a double set of newline characters, they are replaced with a single newline. This preserves the same number of newlines between pasting with and without formatting. + // there is a double set of new lines, they are replaced with a single new line. This preserves the same number of new lines between pasting with and without formatting. const plainText = event.clipboardData.getData('text/plain').replace(/\n\n/g, '\n'); this.paste(Str.htmlDecode(plainText)); } From 6b695965dc6186ea75793aa21b9335ffaf29f527 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 28 Dec 2022 09:31:23 -0700 Subject: [PATCH 3/7] Add extra newlines when copying from context-menu --- src/libs/Clipboard/index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libs/Clipboard/index.js b/src/libs/Clipboard/index.js index efffd9626e03..1c83777ca000 100644 --- a/src/libs/Clipboard/index.js +++ b/src/libs/Clipboard/index.js @@ -70,7 +70,12 @@ const setHtml = (html, text) => { // eslint-disable-next-line no-undef new ClipboardItem({ 'text/html': new Blob([html], {type: 'text/html'}), - 'text/plain': new Blob([text], {type: 'text/plain'}), + + // Thanks to how browsers work, when text is highlighted and CTRL+c is pressed, browsers end up doubling the amount newlines. Since the code in this file is triggered from + // a context menu and not CTRL+c, the newlines need to be doubled so that the content that goes into the clipboard is consistent with CTRL+c behavior. The extra newlines + // are stripped when the contents are pasted into the compose input, but if the contents are pasted outside of the comment composer, it will contain extra newlines and that's + // OK because it is consistent with CTRL+c behavior. + 'text/plain': new Blob([text.replace(/\n/g, '\n\n')], {type: 'text/plain'}), }), ]); } From 19b65461376140307cbf74188f22856e545afa5b Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 3 Jan 2023 09:27:39 -0800 Subject: [PATCH 4/7] Move the place where newlines are doubled --- src/libs/Clipboard/index.js | 7 +------ src/pages/home/report/ContextMenu/ContextMenuActions.js | 7 ++++++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libs/Clipboard/index.js b/src/libs/Clipboard/index.js index 1c83777ca000..efffd9626e03 100644 --- a/src/libs/Clipboard/index.js +++ b/src/libs/Clipboard/index.js @@ -70,12 +70,7 @@ const setHtml = (html, text) => { // eslint-disable-next-line no-undef new ClipboardItem({ 'text/html': new Blob([html], {type: 'text/html'}), - - // Thanks to how browsers work, when text is highlighted and CTRL+c is pressed, browsers end up doubling the amount newlines. Since the code in this file is triggered from - // a context menu and not CTRL+c, the newlines need to be doubled so that the content that goes into the clipboard is consistent with CTRL+c behavior. The extra newlines - // are stripped when the contents are pasted into the compose input, but if the contents are pasted outside of the comment composer, it will contain extra newlines and that's - // OK because it is consistent with CTRL+c behavior. - 'text/plain': new Blob([text.replace(/\n/g, '\n\n')], {type: 'text/plain'}), + 'text/plain': new Blob([text], {type: 'text/plain'}), }), ]); } diff --git a/src/pages/home/report/ContextMenu/ContextMenuActions.js b/src/pages/home/report/ContextMenu/ContextMenuActions.js index c4075aa5fbb2..a08e7022d9df 100644 --- a/src/pages/home/report/ContextMenu/ContextMenuActions.js +++ b/src/pages/home/report/ContextMenu/ContextMenuActions.js @@ -110,7 +110,12 @@ export default [ if (!Clipboard.canSetHtml()) { Clipboard.setString(parser.htmlToMarkdown(content)); } else { - Clipboard.setHtml(content, Str.htmlDecode(parser.htmlToText(content))); + // Thanks to how browsers work, when text is highlighted and CTRL+c is pressed, browsers end up doubling the amount of newlines. Since the code in this file is + // triggered from a context menu and not CTRL+c, the newlines need to be doubled so that the content that goes into the clipboard is consistent with CTRL+c behavior. + // The extra newlines are stripped when the contents are pasted into the compose input, but if the contents are pasted outside of the comment composer, it will + // contain extra newlines and that's OK because it is consistent with CTRL+c behavior. + const plainText = Str.htmlDecode(parser.htmlToText(content)).replace(/\n/g, '\n\n'); + Clipboard.setHtml(content, plainText); } } } else { From 5ff34b5b5b4b3c8ce990e00da3a3e1deef932157 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 4 Jan 2023 16:00:10 -0800 Subject: [PATCH 5/7] Add another place where newlines are removed --- src/libs/SelectionScraper/index.js | 5 ----- src/pages/home/report/ReportActionItem.js | 6 +++++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/libs/SelectionScraper/index.js b/src/libs/SelectionScraper/index.js index 051e0f9ec7b6..32fea24e96b3 100644 --- a/src/libs/SelectionScraper/index.js +++ b/src/libs/SelectionScraper/index.js @@ -100,11 +100,6 @@ const replaceNodes = (dom) => { if (!elementsWillBeSkipped.includes(dom.attribs[tagAttribute])) { domName = dom.attribs[tagAttribute]; } - - // Adding a new line after each comment here, because adding after each range is not working for chrome. - if (htmlEngineUtils.isCommentTag(dom.attribs[tagAttribute])) { - dom.children.push(new Element('br', {})); - } } else if (dom.name === 'div' && dom.children.length === 1 && dom.children[0].type !== 'text') { // We are excluding divs that have only one child and no text nodes and don't have a tagAttribute to prevent // additional newlines from being added in the HTML to Markdown conversion process. diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 7158f15f37bb..c80e3a435b43 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -117,7 +117,11 @@ class ReportActionItem extends Component { } this.setState({isContextMenuActive: true}); - const selection = SelectionScraper.getCurrentSelection(); + + // Newline characters need to be removed here because getCurrentSelection() returns html mixed with newlines, and when + //
tags are converted later to markdown, it creates duplicate newline characters. This means that when the content + // is pasted, there are extra newlines in the content that we want to avoid. + const selection = SelectionScraper.getCurrentSelection().replace(/\n/g, ''); ReportActionContextMenu.showContextMenu( ContextMenuActions.CONTEXT_MENU_TYPES.REPORT_ACTION, event, From c778843e72badfc05edc44a8eebdb77b8640d636 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 5 Jan 2023 09:35:16 -0800 Subject: [PATCH 6/7] Remove unused imports --- src/libs/SelectionScraper/index.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libs/SelectionScraper/index.js b/src/libs/SelectionScraper/index.js index 32fea24e96b3..1b50cda4196a 100644 --- a/src/libs/SelectionScraper/index.js +++ b/src/libs/SelectionScraper/index.js @@ -1,9 +1,7 @@ import render from 'dom-serializer'; import {parseDocument} from 'htmlparser2'; -import {Element} from 'domhandler'; import _ from 'underscore'; import Str from 'expensify-common/lib/str'; -import * as htmlEngineUtils from '../../components/HTMLEngineProvider/htmlEngineUtils'; const elementsWillBeSkipped = ['html', 'body']; const tagAttribute = 'data-testid'; From cb94d2bd225ae8090a1aef0edda544bd3c48fa09 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 5 Jan 2023 16:32:51 -0800 Subject: [PATCH 7/7] Fix removal of newlines when copying inside the composer --- src/components/Composer/index.js | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/components/Composer/index.js b/src/components/Composer/index.js index d188bd0810c5..eacf074b5582 100755 --- a/src/components/Composer/index.js +++ b/src/components/Composer/index.js @@ -11,6 +11,7 @@ import themeColors from '../../styles/themes/default'; import updateIsFullComposerAvailable from '../../libs/ComposerUtils/updateIsFullComposerAvailable'; import getNumberOfLines from '../../libs/ComposerUtils/index'; import * as Browser from '../../libs/Browser'; +import Clipboard from '../../libs/Clipboard'; const propTypes = { /** Maximum number of lines in the text input */ @@ -116,11 +117,12 @@ class Composer extends React.Component { end: initialValue.length, }, }; - this.paste = this.paste.bind(this); + this.paste = this.paste.bind(this); this.handlePaste = this.handlePaste.bind(this); this.handlePastedHTML = this.handlePastedHTML.bind(this); this.handleWheel = this.handleWheel.bind(this); + this.putSelectionInClipboard = this.putSelectionInClipboard.bind(this); this.shouldCallUpdateNumberOfLines = this.shouldCallUpdateNumberOfLines.bind(this); } @@ -140,6 +142,7 @@ class Composer extends React.Component { if (this.textInput) { this.textInput.addEventListener('paste', this.handlePaste); this.textInput.addEventListener('wheel', this.handleWheel); + this.textInput.addEventListener('keydown', this.putSelectionInClipboard); } } @@ -265,9 +268,8 @@ class Composer extends React.Component { return; } - // The regex replace is there because when doing a "paste without formatting", there are extra line breaks added to the content (I do not know why). To fix the problem, anytime - // there is a double set of new lines, they are replaced with a single new line. This preserves the same number of new lines between pasting with and without formatting. const plainText = event.clipboardData.getData('text/plain').replace(/\n\n/g, '\n'); + this.paste(Str.htmlDecode(plainText)); } @@ -285,6 +287,21 @@ class Composer extends React.Component { event.stopPropagation(); } + putSelectionInClipboard(event) { + // If anything happens that isn't cmd+c or cmd+x, ignore the event because it's not a copy command + if (!event.metaKey || (event.key !== 'c' && event.key !== 'x')) { + return; + } + + // The user might have only highlighted a portion of the message to copy, so using the selection will ensure that + // the only stuff put into the clipboard is what the user selected. + const selectedText = event.target.value.substring(this.state.selection.start, this.state.selection.end); + + // The plaintext portion that is put into the clipboard needs to have the newlines duplicated. This is because + // the paste functionality is stripping all duplicate newlines to try and provide consistent behavior. + Clipboard.setHtml(selectedText, selectedText.replace(/\n/g, '\n\n')); + } + /** * We want to call updateNumberOfLines only when the parent doesn't provide value in props * as updateNumberOfLines is already being called when value changes in componentDidUpdate