Skip to content

Fix quoted text disappearing after edit #13536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcochavezf merged 4 commits into from
Dec 15, 2022
Merged

Fix quoted text disappearing after edit #13536

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eaad93b
Also replace the existing message so that we are comparing the same t…
stitesExpensify Dec 12, 2022
e42a721
DRY up filter code
stitesExpensify Dec 13, 2022
e1630c1
Merge branch 'main' of github.com:Expensify/App into stites-fixQuoteEdit
stitesExpensify Dec 13, 2022
d2f08c9
Merge branch 'main' of github.com:Expensify/App into stites-fixQuoteEdit
stitesExpensify Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,9 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {

// Do not autolink if someone explicitly tries to remove a link from message.
// https://github.com/Expensify/App/issues/9090
const htmlForNewComment = parser.replace(textForNewComment, {filterRules: _.filter(_.pluck(parser.rules, 'name'), name => name !== 'autolink')});
const autolinkFilter = {filterRules: _.filter(_.pluck(parser.rules, 'name'), name => name !== 'autolink')};
const htmlForNewComment = parser.replace(textForNewComment, autolinkFilter);
const originalMessageHTML = parser.replace(originalReportAction.message[0].html, autolinkFilter);

@yuwenmemon yuwenmemon Dec 19, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stitesExpensify coming from #13661, I don't believe this PR caused this original issue (because there's some issue with escaping spaces as well) but I am seeing that by passing HTML into parser.replace we're effectively treating the HTML like markdown, is that not correct?
Screenshot 2022-12-19 at 12 01 44 PM

@yuwenmemon yuwenmemon Dec 19, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay yes I can confirm, PR incoming

@yuwenmemon yuwenmemon Dec 19, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should resolve it: #13703

@stitesExpensify stitesExpensify Dec 19, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're effectively treating the HTML like markdown, is that not correct?

I don't really understand the question TBH.

In this issue, the problem (at least I thought) was that we were comparing parsed HTML to unparsed HTML, so when the strings were the same it was failing because it was comparing " to ", which is why I parsed the original HTML so that we were comparing the same things.

Looking at your screenshot, there are 3 different interpretations of the same HTML which seems super messy 😅 I think ideally we would want to just compare the originalCommentHTML to the htmlForNewComment but even those are in different formats.


// Delete the comment if it's empty
if (_.isEmpty(htmlForNewComment)) {
Expand All @@ -913,7 +915,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
}

// Skip the Edit if message is not changed
if (originalReportAction.message[0].html === htmlForNewComment.trim()) {
if (originalMessageHTML === htmlForNewComment.trim()) {
return;
}

Expand Down