Chat - The Paragraphs are not displayed and some spaces missing between text after copied test from the opened HTML file

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Issue found when executing PR https://github.com/Expensify/App/pull/11926

Action Performed:

1. Go to staging.new.expensify.com
2. Log in with any account
3. Select any chat
4. Open HTML file in another tab
5. Copy and paste as is
6. Send it

Expected Result:

The Paragraphs and spaces between text are displayed after copied test from the opened HTML file

Actual Result:

The Paragraphs are not displayed and some spaces missing between text after copied test from the opened HTML file

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.19.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/198123365-fb5fdc99-134c-4e5a-ad93-7b051121093e.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot[bot]]

Triggered auto assignment to @davidcardoza (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

[melvin-bot[bot]]

Triggered auto assignment to @tgolen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Gonals]

Marking as external!

[tgolen]

Waiting for proposals. Bumping up to daily

[melvin-bot[bot]]

@tgolen, @davidcardoza, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Santhosh-Sellavel]

We are still waiting for proposals here! ~Maybe we should double this~ @davidcardoza please post the Upwork job! Maybe we could get some proposals! I think we could start this job for $500 or $1000!

cc: @tgolen!

[Santhosh-Sellavel]

Not overdue!

[tgolen]

Still waiting for proposals.

[puneetlath]

@tgolen To help us clear out the large backlog of /App bugs, we're putting the spotlight every bug in the repo already than 4 weeks old. To help unblock the roadmap and get our bug pipeline back in equilibrium, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is
• For any that can't, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #bug-zero

[fabiocberg]

Hi, my name is Fabio, I've been working as a web, backend and mobile developer for over 16 years. I can help you with your paste issue. The problem is with your input component. This is accepting only single lines, so after pasting the text, it automatically removes break lines. It can be fixed by creating another input component (or adapting the current one) to accept multiline text.

Best regards,

Fabio Bergmann

[tgolen]

@fabiocberg Thank you for your proposal. Can you please give more details about the root of the problem (add links to the code where it shows it's only accepting single lines) and then also show the specific code changes you would make to solve the problem?

[Santhosh-Sellavel]

@fabiocberg bump!

Still looking for proposals!

[tjferriss]

Reviewing the issue based on the weekly bug update chore:

@tgolen this is one of the oldest issues in the /App repo. To help us clear out the large backlog of bugs, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is
• For any that can't, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #bug-zero

[cead22]

I thought adding multiline={true} could do the trick, but no luck

• Adding multiline={true} to the RNTextInput here didn't work
• Adding multiline={true} to TextInput here didn't work either

[Santhosh-Sellavel]

Looking for proposals

[eh2077]

Proposal

We can support pasting HTML p tag and unordered list in ExpensiMark.js of expensify-common. Two changes will be added into ExpensiMark.js

1. Add space for unordered list item https://github.com/Expensify/expensify-common/blob/db2c245c1e6b39232092b1415b9235fadda2a529/lib/ExpensiMark.js#L226-L231

                name: 'heading1',
                regex: /\s*<(h1)(?:"[^"]*"|'[^']*'|[^'">])*>(.*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))\s*/gi,
                replacement: '\n# $2\n',
            },
+            {
+                name: 'listItem',
+                regex: /\s*<(li)(?:"[^"]*"|'[^']*'|[^'">])*>(.*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))\s*/gi,
+                replacement: '<li>  $2</li>',
+            },
             {
                 name: 'italic',

2. Add line break for HTML p and li tag https://github.com/Expensify/expensify-common/blob/db2c245c1e6b39232092b1415b9235fadda2a529/lib/ExpensiMark.js#L465-L474

        this.htmlToMarkdownRules.forEach((rule) => {
            // Pre-processes input HTML before applying regex
            if (rule.pre) {
                generatedMarkdown = rule.pre(generatedMarkdown);
            }
            generatedMarkdown = generatedMarkdown.replace(rule.regex, rule.replacement);
        });
-        generatedMarkdown = generatedMarkdown.replace(/<div.*?>|<\/div>|<comment.*?>|\n<\/comment>|<\/comment>|<h2>|<\/h2>|<h3>|<\/h3>|<h4>|<\/h4>|<h5>|<\/h5>|<h6>|<\/h6>/gm, '[block]');
+        generatedMarkdown = generatedMarkdown.replace(/<div.*?>|<\/div>|<comment.*?>|\n<\/comment>|<\/comment>|<h2>|<\/h2>|<h3>|<\/h3>|<h4>|<\/h4>|<h5>|<\/h5>|<h6>|<\/h6>|<p>|<\/p>|<li>|<\/li>/gm, '[block]');
         return Str.htmlDecode(this.replaceBlockWithNewLine(Str.stripHTML(generatedMarkdown)));

Video demo about the change

https://user-images.githubusercontent.com/117511920/204067360-c1d011b6-669c-4f64-8ae6-ac2d1d755f7d.mov

Test html

<!DOCTYPE html>
<html>
<body>
    <h1>HTML paragraphs and unordered list copy test</h1>
    <p>This is a HTML paragraph</p>
    <br>
    <p>An Unordered HTML List</p>
    <ul>
        <li><a href="https://example.com">Coffee</a> -- Coffee</li>
        <li><a href="https://example.com">Tea</a> -- Tea</li>
        <li><a href="https://example.com">Milk</a> -- Milk</li>
    </ul>
</body>
</html>

To have least changes in ExpensiMark.js and keep it simple, this proposal converts HTML list items to new lines with 2 prefix spaces instead of markdown list.

I'll be happy to hear feedback from you! cc @Santhosh-Sellavel

[melvin-bot[bot]]

@tgolen, @davidcardoza, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Santhosh-Sellavel]

We have a proposal and will review it shortly!

[Santhosh-Sellavel]

@tgolen this is more like a feature than a bug, shall we proceed with this?

@eh2077 Not working for me, can you check and update please thanks!

[tgolen]

I agree that this is a new feature. @davidcardoza I'm going to place this on HOLD until we get through What'sAppQuality 👍

[eh2077]

@eh2077 Not working for me, can you check and update please thanks!

Hi @Santhosh-Sellavel, thanks for checking the proposal!

I apply the diff of upstream lib expensify-common to App project through following steps. First, clone expensify-common to local disk and apply following diff

diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js
index 8eb6b92..f9ad8cf 100644
--- a/lib/ExpensiMark.js
+++ b/lib/ExpensiMark.js
@@ -227,6 +227,11 @@ export default class ExpensiMark {
                 regex: /\s*<(h1)(?:"[^"]*"|'[^']*'|[^'">])*>(.*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))\s*/gi,
                 replacement: '\n# $2\n',
             },
+            {
+                name: 'listItem',
+                regex: /\s*<(li)(?:"[^"]*"|'[^']*'|[^'">])*>(.*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))\s*/gi,
+                replacement: '<li>  $2</li>',
+            },
             {
                 name: 'italic',
                 regex: /<(em|i)(?:"[^"]*"|'[^']*'|[^'">])*>(.*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))/gi,
@@ -469,7 +474,7 @@ export default class ExpensiMark {
             }
             generatedMarkdown = generatedMarkdown.replace(rule.regex, rule.replacement);
         });
-        generatedMarkdown = generatedMarkdown.replace(/<div.*?>|<\/div>|<comment.*?>|\n<\/comment>|<\/comment>|<h2>|<\/h2>|<h3>|<\/h3>|<h4>|<\/h4>|<h5>|<\/h5>|<h6>|<\/h6>/gm, '[block]');
+        generatedMarkdown = generatedMarkdown.replace(/<div.*?>|<\/div>|<comment.*?>|\n<\/comment>|<\/comment>|<h2>|<\/h2>|<h3>|<\/h3>|<h4>|<\/h4>|<h5>|<\/h5>|<h6>|<\/h6>|<p>|<\/p>|<li>|<\/li>/gm, '[block]');
         return Str.htmlDecode(this.replaceBlockWithNewLine(Str.stripHTML(generatedMarkdown)));
     }

Second, switch to local dependency in the package.json file of App project

diff --git a/package.json b/package.json
index fb2e1d98cf..fe2e41901e 100644
--- a/package.json
+++ b/package.json
@@ -67,7 +67,7 @@
     "dom-serializer": "^0.2.2",
     "domhandler": "^4.3.0",
     "dotenv": "^8.2.0",
-    "expensify-common": "git+https://github.com/Expensify/expensify-common.git#87d6599f2fadd3601067ac9cfa5d10681fa34811",
+    "expensify-common": "file:/Users/erich/path/to/Expensify/expensify-common",
     "fbjs": "^3.0.2",
     "file-loader": "^6.0.0",
     "html-entities": "^1.3.1",

Last, run npm install --save and start the App.

Hope this helps though this new feature is currently marked as hold.

[Santhosh-Sellavel]

@eh2077 This is on hold, please focus on other open issues for now. When this is off-hold I will review this, thanks!

[tgolen]

I'm going to move this to a weekly issue while it's on HOLD

[tgolen]

Still on HOLD

[davidcardoza]

On HOLD until we get through WAQ

[tgolen]

Still on HOLD

[tgolen]

I'm taking this off HOLD now that WAQ is mostly behind us. I think that this proposal looks fine to me @eh2077 and @Santhosh-Sellavel

[Santhosh-Sellavel]

@tgolen Can you assign another C+ continue , I am unassigning myself here due to limited availability.