-
Notifications
You must be signed in to change notification settings - Fork 135
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 problem with parsing links containing mailto: prefix followed by an email #604
Fix problem with parsing links containing mailto: prefix followed by an email #604
Conversation
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.
Looks good to me. Thanks for adding proper unit tests.
lib/ExpensiMark.js
Outdated
let label = g1.trim(); | ||
const href = `mailto:${g2}`; | ||
if (label === href) { | ||
label = g2; | ||
} | ||
return `<a href="${href}">${label}</a>`; |
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.
I don't like the fact that we mutate label
here but honestly I cannot think of anything better that wouldn't require duplicating <a href="...">
string.
@jasperhuangg bump |
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.
Sorry for the delay on this was OOO
This PR fixes potential problems with parsing links containing mailto: prefix followed by an email. The changes were added to enhance parsing inside Live Markdown Preview.
The problem was mainly with parsing
[mailto:[email protected]](mailto:[email protected])
. ExpensiMark instead of one link returns two separate tags inside brackets:To fix the problem we decided to collapse such links, similarly like it is done after writing
[[email protected]]([email protected])
.So now following strings:
[[email protected]](mailto:[email protected])
[mailto:[email protected]]([email protected])
[mailto:[email protected]](mailto:[email protected])
will be collapsed into:
<a href=\"mailto:[email protected]\">[email protected]</a>
mailto.parsing.mov
Fixed Issues
$ Expensify/App#31181
Tests
To test these changes I've added proper tests connected to all needed behaviors for Live Markdown Preview. To test it by yourself you should:
QA
Same as tests