[$1000] Copying code blocks from chat is broken #7644

mvtglobally · 2022-02-09T05:54:04Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Navigate to any chat
Copy any code block
paste in chat

Expected Result:

User should be able to copy and paste code blocks

Actual Result:

What ends up being copied to your clipboard is not the content that is highlighted

Workaround:

unknown

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: 1.1.37-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Recording.391.mp4

Expensify/Expensify Issue URL:
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1643914668204029

Job Post https://www.upwork.com/jobs/~0103d56637d4154cbd

View all open jobs on GitHub

MelvinBot · 2022-02-09T05:54:06Z

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

MelvinBot · 2022-02-09T14:53:41Z

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

MelvinBot · 2022-02-10T17:00:43Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (Exported)

MelvinBot · 2022-02-10T17:00:46Z

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

ihamzanizami · 2022-02-10T19:03:02Z

Hey,
I saw your job posting and I understand that you are looking for a react native developer to solve an existing issue in your code that is related to copying code blocks.
I have been working on React Native for over 5 years now and I have used the clipboard feature of react-native in a number of projects.
I will be using the clipboard feature provided by React native which will get a string and that string will be set to a data value and the user will be able to paste that data anywhere he wants.
I am willing to start right away and can provide you with a solution in less than 36 hours.
I would like to discuss further details with you before proceeding, I would suggest that we hop onto a call right away!
Looking forward to your response,
Kind regards,
Hamza Nizami

aldo-expensify · 2022-02-10T19:26:40Z

👋 @ihamzanizami , I suggest you read https://github.com/Expensify/App/blob/main/CONTRIBUTING.md to learn how contributors can participate and propose solutions.

iwiznia · 2022-02-10T19:32:56Z

I don't understand this issue. I see that the code is pasted correctly in that video. Can someone explain the problem in more detail please?

aldo-expensify · 2022-02-10T19:56:48Z

I understand that the problem are the line breaks. When I copy code blocks, I always have to fix line breaks manually.

iwiznia · 2022-02-10T19:58:35Z

Oh, I see. FWIW it works properly if you CMD+SHIFT+V (paste without format)

ahmdshrif · 2022-02-10T22:41:30Z

Proposal

issue happend on htmlconverter . since the code block is formated with html .
code formate use <div> as a new line and we net parse this on library.

we can fix this by just parsing the div as new line /n or <br/> and html converter will count it as a new line.

in

App/src/components/TextInputFocusable/index.js

Line 244 in 403fc28

const markdownText = parser.htmlToMarkdown(html);

+ const fixHtml = html.replaceAll('<div>', '').replaceAll('</div>', '<br/>');
- const markdownText = parser.htmlToMarkdown(html); 
+ const markdownText = parser.htmlToMarkdown(fixHtml);

we can fix this from the library also by edit this.

            {
                name: 'newline',

                // Replaces open and closing <br><br/> tags with a single <br/>
                // Slack uses special <span> tag for empty lines instead of <br> tag
                pre: inputString => inputString.replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
                    .replace(SLACK_SPAN_NEW_LINE_TAG + SLACK_SPAN_NEW_LINE_TAG, '<br/><br/><br/>').replace(SLACK_SPAN_NEW_LINE_TAG, '<br/><br/>'),

                // Include the immediately followed newline as `<br>\n` should be equal to one \n.
                regex: /<br(?:"[^"]*"|'[^']*'|[^'"><])*>\n?/gi,
                replacement: '\n'
            },

to

                pre: inputString => inputString .replaceAll('<div>', '').replaceAll('</div>', '<br/>')replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
                    .replace(SLACK_SPAN_NEW_LINE_TAG + SLACK_SPAN_NEW_LINE_TAG, '<br/><br/><br/>').replace(SLACK_SPAN_NEW_LINE_TAG, '<br/><br/>'),

rushatgabhane · 2022-02-14T14:21:34Z

            pre: inputString => inputString .replaceAll('<div>', '').replaceAll('</div>', '<br/>')replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
                .replace(SLACK_SPAN_NEW_LINE_TAG + SLACK_SPAN_NEW_LINE_TAG, '<br/><br/><br/>').replace(SLACK_SPAN_NEW_LINE_TAG, '<br/><br/>'),

@ahmdshrif that didn't work for me.
I modified the new line rule of htmlToMarkdownRules

ahmdshrif · 2022-02-14T14:24:42Z

We just modified the pre with this not all object

ahmdshrif · 2022-02-14T14:35:50Z

you can apply the changes on
/node_modules/expensify-common/lib/ExpensiMark.js
line 197

https://github.com/Expensify/expensify-common/blob/main/lib/ExpensiMark.js#L197

 {
                name: 'newline',

                // Replaces open and closing <br><br/> tags with a single <br/>
                // Slack uses special <span> tag for empty lines instead of <br> tag
-                pre: inputString => inputString.replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
+                pre: inputString => inputString .replaceAll('<div>', '').replaceAll('</div>', '<br/>').replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
                .replace(SLACK_SPAN_NEW_LINE_TAG + SLACK_SPAN_NEW_LINE_TAG, '<br/><br/><br/>').replace(SLACK_SPAN_NEW_LINE_TAG, '<br/><br/>'),
                

                // Include the immediately followed newline as `<br>\n` should be equal to one \n.
                regex: /<br(?:"[^"]*"|'[^']*'|[^'"><])*>\n?/gi,
                replacement: '\n'
            },

sure we will not edit on node module but we will add the changes on expensify-common library repo but you can test on by change on node module

rushatgabhane · 2022-02-14T21:57:32Z

@ahmdshrif yep, that's exactly what I tried. It didn't work.

Btw, I'm testing by pasting this text -

App/src/components/TextInputFocusable/index.js

Lines 263 to 267 in 403fc28

    
           if (files.length > 0) { 
        
               // Prevent the default so we do not post the file name into the text box 
        
               event.preventDefault(); 
        
               this.props.onPasteFile(event.clipboardData.files[0]); 
        
               return;

ahmdshrif · 2022-02-14T22:20:42Z

@rushatgabhane ok, GitHub uses td and tr Tags for new line, not div as VScode.
so just add them. like this will work on both.

pre: inputString => inputString.replaceAll('<div>', '').replaceAll('</div>', '<br/>').replaceAll('</td></tr>', '<br/>').replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
.replace(SLACK_SPAN_NEW_LINE_TAG + SLACK_SPAN_NEW_LINE_TAG, '<br/><br/><br/>').replace(SLACK_SPAN_NEW_LINE_TAG, '<br/><br/>'),

rushatgabhane · 2022-02-15T21:40:08Z

@ahmdshrif thanks, that fixed it for copying form github.

Copying 1 line from vscode pastes 2 extra lines.

rushatgabhane · 2022-03-21T22:09:35Z

@ahmdshrif could you please repost your proposal here? And slightly customize it for this issue.
It'll make it easier for @ iwiznia to review it. Thanks a lot! 🙇‍♂️

ahmdshrif · 2022-03-22T07:26:12Z

sure @rushatgabhane
@iwiznia we have an issue here when copying text from the text editor because it uses div to make a new line
and that is what we try to fix on both issues.

so I add the regex before here and it work fine but did not work on the second issue so I replace it with some logic explained here #7352 (comment) to work with both.

but we still have here another issue here when copy code form github . github use <td> and <tr> (tables) to map new lines and to fix that i add this regex .replaceAll(/(<tr.*?<\/tr>)/g,"$1<br/>")

I also explain this regex and the first regex (we replace it ) on this comment #7644 (comment)

as summary we will fix mapping dev on the second issue but we still need to fix copy form github buy use this regex
.replaceAll(/(<tr.*?<\/tr>)/g,"$1<br/>")

thanks .

rushatgabhane · 2022-03-23T13:33:06Z

🎀 👀 🎀 C+ reviewed

@iwiznia I know this isn't very pretty, but the only other alternative is to use a third party markdown-html parser. But as Rory and I discussed, it's a very long term project that may or may not come into fruition.

I recommend @ahmdshrif's proposal.

iwiznia · 2022-03-24T20:19:13Z

Where exactly will that code go? (sorry if it is linked somewhere, but could not find it following the linked comments)

Wouldn't those regexes also match other things? Like what if I am pasting an HTML string? Ie: what if what I am pasting is <tr><td>Hello!</td></tr>, would that get replaced or is the escaping of html characters happening before and thus would work?

rushatgabhane · 2022-03-25T05:46:06Z

#7644 (comment)

@ahmdshrif ^^

ahmdshrif · 2022-03-25T11:56:46Z

sorry @iwiznia for making it confusing
I mention it early here #7644 (comment) and forget to add it to the final proposal

when past text we check if it contains HTML format and if yes we pare it with htmlToMarkdown

App/src/components/TextInputFocusable/index.js

Line 244 in 403fc28

const markdownText = parser.htmlToMarkdown(html);

so the library has its own mechanism to just pares the HTML style tags.

but the issue in the library is it only maps br tag to newline and not handle the table case
so we only will add this line to the HTML parser role
https://github.com/Expensify/expensify-common/blob/main/lib/ExpensiMark.js#L197

 {
                name: 'newline',

                // Replaces open and closing <br><br/> tags with a single <br/>
                // Slack uses special <span> tag for empty lines instead of <br> tag
              pre: inputString => inputString.replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>')
+               .replaceAll(/(<tr.*?<\/tr>)/g,"$1<br/>")
                .replace(SLACK_SPAN_NEW_LINE_TAG + SLACK_SPAN_NEW_LINE_TAG, '<br/><br/><br/>').replace(SLACK_SPAN_NEW_LINE_TAG, '<br/><br/>'),
                

                // Include the immediately followed newline as `<br>\n` should be equal to one \n.
                regex: /<br(?:"[^"]*"|'[^']*'|[^'"><])*>\n?/gi,
                replacement: '\n'
            },

for div handling l already implement it on this PR https://github.com/Expensify/expensify-common/pull/436/files

copy you example : -

copy form github

iwiznia · 2022-03-25T18:08:32Z

Got it, thanks for clarifying! The proposal makes sense @jboniface please hire @ahmdshrif

jboniface · 2022-03-25T18:22:20Z

@ahmdshrif please apply here ~~https://www.upwork.com/ab/applicants/1491819288094076928/applicants~~ https://www.upwork.com/jobs/~0103d56637d4154cbd

ahmdshrif · 2022-03-25T18:27:59Z

@jboniface the link does not work

jboniface · 2022-03-25T18:29:58Z

@ahmdshrif oops, i sent you the link for the proposal reviews 😵‍💫
it's this: https://www.upwork.com/jobs/~0103d56637d4154cbd

ahmdshrif · 2022-03-25T18:33:53Z

no problem @jboniface 😄
applied.

ahmdshrif · 2022-03-29T13:32:12Z

pr ready here Expensify/expensify-common#436

rushatgabhane · 2022-04-12T14:11:32Z

Review almost done

rushatgabhane · 2022-04-24T21:33:21Z

All set here, PR merged

rushatgabhane · 2022-05-15T10:12:07Z

PR #8743 hit production 2 days ago.

ahmdshrif · 2022-05-20T14:42:45Z

Bump @jboniface

jboniface · 2022-05-23T15:57:09Z

@rushatgabhane please apply for the C+ payment https://www.upwork.com/jobs/~0103d56637d4154cbd

rushatgabhane · 2022-05-23T15:58:08Z

@jboniface unfortunately, that job is no longer available.

jboniface · 2022-05-23T16:00:18Z

@rushatgabhane ok, let's use this one https://www.upwork.com/jobs/~01b9b2c4c22ea68332

rushatgabhane · 2022-05-23T16:12:25Z

Thanks!

mallenexpensify · 2022-06-01T16:36:51Z

@ahmdshrif has been paid (by Jeremy), is there a reason he wasn't assigned to this issue?
@rushatgabhane I hired you, let me know when you've accepted and we'll pay.
https://www.upwork.com/jobs/~01b9b2c4c22ea68332

rushatgabhane · 2022-06-01T16:40:20Z

is there a reason he wasn't assigned to this issue

@ahmdshrif was given the approval, so it's proly a small boo boo

@mallenexpensify accepted, thank you!

mallenexpensify · 2022-06-02T16:18:28Z

Paid @rushatgabhane $1k for C+

Thanks for the help y'all!

mvtglobally added Engineering Daily KSv2 labels Feb 9, 2022

MelvinBot assigned aldo-expensify Feb 9, 2022

aldo-expensify added the External Added to denote the issue can be worked on by a contributor label Feb 9, 2022

MelvinBot assigned kevinksullivan Feb 9, 2022

aldo-expensify unassigned kevinksullivan and aldo-expensify Feb 9, 2022

kevinksullivan self-assigned this Feb 10, 2022

kevinksullivan added the Exported label Feb 10, 2022

botify removed the Daily KSv2 label Feb 10, 2022

MelvinBot added the Weekly KSv2 label Feb 10, 2022

MelvinBot assigned rushatgabhane Feb 10, 2022

MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 10, 2022

MelvinBot assigned iwiznia Feb 10, 2022

rushatgabhane mentioned this issue Mar 25, 2022

fix new line issue Expensify/expensify-common#436

Merged

90 tasks

MelvinBot added the Overdue label Apr 4, 2022

jboniface added the Reviewing Has a PR in review label Apr 6, 2022

melvin-bot bot removed the Overdue label Apr 6, 2022

ahmdshrif mentioned this issue Apr 21, 2022

update expensify-common #8743

Merged

90 tasks

jboniface assigned mallenexpensify May 25, 2022

mallenexpensify closed this as completed Jun 2, 2022

[$1000] Copying code blocks from chat is broken #7644

[$1000] Copying code blocks from chat is broken #7644

Comments

mvtglobally commented Feb 9, 2022 • edited by kevinksullivan Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platform:

MelvinBot commented Feb 9, 2022

MelvinBot commented Feb 9, 2022

MelvinBot commented Feb 10, 2022

MelvinBot commented Feb 10, 2022

ihamzanizami commented Feb 10, 2022

aldo-expensify commented Feb 10, 2022

iwiznia commented Feb 10, 2022

aldo-expensify commented Feb 10, 2022

iwiznia commented Feb 10, 2022

ahmdshrif commented Feb 10, 2022 • edited Loading

Proposal

rushatgabhane commented Feb 14, 2022

ahmdshrif commented Feb 14, 2022

ahmdshrif commented Feb 14, 2022 • edited Loading

rushatgabhane commented Feb 14, 2022 • edited Loading

ahmdshrif commented Feb 14, 2022 • edited Loading

rushatgabhane commented Feb 15, 2022

rushatgabhane commented Mar 21, 2022 • edited Loading

ahmdshrif commented Mar 22, 2022 • edited Loading

rushatgabhane commented Mar 23, 2022

iwiznia commented Mar 24, 2022

rushatgabhane commented Mar 25, 2022

ahmdshrif commented Mar 25, 2022

iwiznia commented Mar 25, 2022

jboniface commented Mar 25, 2022 • edited Loading

ahmdshrif commented Mar 25, 2022 • edited Loading

jboniface commented Mar 25, 2022

ahmdshrif commented Mar 25, 2022 • edited Loading

ahmdshrif commented Mar 29, 2022

rushatgabhane commented Apr 12, 2022

rushatgabhane commented Apr 24, 2022

rushatgabhane commented May 15, 2022

ahmdshrif commented May 20, 2022

jboniface commented May 23, 2022

rushatgabhane commented May 23, 2022

jboniface commented May 23, 2022

rushatgabhane commented May 23, 2022

mallenexpensify commented Jun 1, 2022

rushatgabhane commented Jun 1, 2022 • edited Loading

mallenexpensify commented Jun 2, 2022

mvtglobally commented Feb 9, 2022 •

edited by kevinksullivan

Loading

ahmdshrif commented Feb 10, 2022 •

edited

Loading

ahmdshrif commented Feb 14, 2022 •

edited

Loading

rushatgabhane commented Feb 14, 2022 •

edited

Loading

ahmdshrif commented Feb 14, 2022 •

edited

Loading

rushatgabhane commented Mar 21, 2022 •

edited

Loading

ahmdshrif commented Mar 22, 2022 •

edited

Loading

jboniface commented Mar 25, 2022 •

edited

Loading

ahmdshrif commented Mar 25, 2022 •

edited

Loading

ahmdshrif commented Mar 25, 2022 •

edited

Loading

rushatgabhane commented Jun 1, 2022 •

edited

Loading