Skip to content
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 newline parsing in MD to HTML parser #393

Merged
merged 2 commits into from
Jul 14, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Jul 14, 2021

@Jag96 will you please review this?

[Explanation of the change or anything fishy that is going on]
If a br tag is followed by a newline then we should parse both as single newline. e.g.
<b>Joe likes to eat <b><br>\n Apple & Doughnut.

Fixed Issues

$ Expensify/App#4009 (comment)

Tests

  1. What unit/integration tests cover your change? What autoQA tests cover your change?
    UNit
  2. What tests did you perform that validates your changed worked?
    Unit

QA

  1. What does QA need to do to validate your changes?
    NA
  2. What areas to they need to test for regressions?
    NA

@parasharrajat parasharrajat requested a review from a team as a code owner July 14, 2021 03:25
@MelvinBot MelvinBot requested review from robertjchen and removed request for a team July 14, 2021 03:25
@@ -174,7 +174,7 @@ export default class ExpensiMark {

// Replaces open and closing <br><br/> tags with a single <br/>
pre: inputString => inputString.replace('<br></br>', '<br/>').replace('<br><br/>', '<br/>'),
regex: /<br\s*[/]?>/gi,
regex: /<br\s*[/]?>\n?/gi,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update one of the tests to ensure this case is covered?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@parasharrajat
Copy link
Member Author

Done

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Jag96 Jag96 merged commit 77b43a2 into Expensify:master Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants