[skyweb331]

Fixed Issues

$ Expensify/App#43386
This PR is to fix parsing markdown for multiline email hyperlinks

Tests

1. What unit/integration tests cover your change? What autoQA tests cover your change?
   Passed all existing test cases and Added one more test case for this purpose.
2. What tests did you perform that validates your changed worked?
   bump expensify-common version in E/App and react-native-live-markdown to this PR and tested locally

QA

1. What does QA need to do to validate your changes?
   bump expensify-common version in E/App and try to input multiline email hyperlinks ( i.e. test\ntest ) and see if there's any errors or wrong parsing.

[marcochavezf]

Tagging @hungvu193 for C+ review

[hungvu193]

I'll review this tomorrow

[hungvu193]

@skyweb331 One question, when I inserted line break between content, the email won't be highlighted anymore. I assume that this will be fixed when we apply this change from this PR to our react-native-live-markdown right?

Screen.Recording.2024-07-14.at.22.39.22.mov

[skyweb331]

@hungvu193 Yes. But it requires time to investigate. For now, what I can say is that it will be problem from htmlToMarkdown parser...

[hungvu193]

Ok. Please take a look and let's me know the result. I can try to test it tomorrow

[skyweb331]

@hungvu193
I checked this bug and it is related to react-native-live-markdown

Specifically, this function: https://github.com/Expensify/react-native-live-markdown/blob/9c9769ef004657f1dbe71d3e54b34d50c9d87635/parser/index.ts#L90-L199

I ran this function and it throws following error

So do you want me to fix react-native-live-markdown for this?

[hungvu193]

I believe it will be a part of this issue too, so Yes, we need to fix it.

[skyweb331]

OK. GOT IT. ( I thought react-native-live-markdown is managed by software-mansion so this should be something they should do??? )

[hungvu193]

OK. GOT IT. ( I thought react-native-live-markdown is managed by software-mansion so this should be something they should do??? )

They will help us review our PR once we create it 😄 . So I think here's the plan, we will try to prioritize this PR then create another PR in live markdown to fix the console error.

[skyweb331]

Yes. This PR is working correctly and the live editor issue is because they don't handle parsing correctly...
So how about merging this first and meanwhile let them know there is issue when they break parsed string into tokens...

[hungvu193]

Sure. I'm going to approve this PR.

[hungvu193]

LGTM

[hungvu193]

Yes. This PR is working correctly and the live editor issue is because they don't handle parsing correctly... So how about merging this first and meanwhile let them know there is issue when they break parsed string into tokens...

Please link this PR and the original issue when you create PR in react-native-live-markdown.

[skyweb331]

@hungvu193 I fixed in react-native-live-markdown and will create PR there.

[hungvu193]

@hungvu193 I fixed in react-native-live-markdown and will create PR there.

Thanks for your quick work 🚀

[skyweb331]

But problem is I must upgrade expensify-common version in react-native-live-markdown after merge this PR. otherwise, my PR there will fail test.
So plan is let's merge this first and then I will create a PR in react-native-live-markdown with bumping expensify-common version to newer version and my updates.

[hungvu193]

Yes. Correct. We need to wait for this PR to be merged first, then we will use new expensify-common version and update it to our live markdown PR.

[marcochavezf]

Merging this PR to unblock the other one

[github-actions]

🚀Published to npm in v2.0.46