-
Notifications
You must be signed in to change notification settings - Fork 128
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.
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
fix parsing markdown for multiline email hyperlink #750
Conversation
Tagging @hungvu193 for C+ review |
I'll review this tomorrow |
@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 Screen.Recording.2024-07-14.at.22.39.22.mov |
@hungvu193 Yes. But it requires time to investigate. For now, what I can say is that it will be problem from htmlToMarkdown parser... |
Ok. Please take a look and let's me know the result. I can try to test it tomorrow |
@hungvu193 I ran this function and it throws following error So do you want me to fix |
I believe it will be a part of this issue too, so Yes, we need to fix it. |
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. |
Yes. This PR is working correctly and the live editor issue is because they don't handle parsing correctly... |
Sure. I'm going to approve this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please link this PR and the original issue when you create PR in |
@hungvu193 I fixed in |
Thanks for your quick work 🚀 |
But problem is I must upgrade |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging this PR to unblock the other one
🚀Published to npm in v2.0.46 |
Fixed Issues
$ Expensify/App#43386
This PR is to fix parsing markdown for multiline email hyperlinks
Tests
Passed all existing test cases and Added one more test case for this purpose.
bump expensify-common version in E/App and
react-native-live-markdown
to this PR and tested locallyQA
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.