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

#2356 - Fix the chat markdown bug related to a line-break #2361

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

SebinSong
Copy link
Collaborator

@SebinSong SebinSong commented Sep 22, 2024

closes #2356

Screenshot for the fix.

@SebinSong SebinSong self-assigned this Sep 22, 2024
Comment on lines +43 to +44
.replace(/<br>\n(\s*)(\d+\.|-)/g, '\n\n$1$2') // [1] custom-handling the case where <br> is directly followed by the start of ordered/unordered lists
.replace(/(\d+\.|-)(\s.+)\n<br>/g, '$1$2\n\n') // [2] this is a custom-logic added so that the end of ordered/un-ordered lists are correctly detected by markedjs.
Copy link
Collaborator Author

@SebinSong SebinSong Sep 22, 2024

Choose a reason for hiding this comment

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

As inline-commented in the same file, markedjs with gfm style has a default behaviour where it collapses multiple consecutive line-breaks into one which is not what we want for the app. [1] and [2] here are the custom logics added to work around this. But [2] here didn't capture an edge-case where the last-child of a list ends with <code> element.

(The reason for this edge case is that, we split the whole text by markdown code elements first before applying these [1] and [2] because no content wrapped within <code>...</code> should be converted. eg. we already made a fix before for a bug where something like a<=b within <code> element looks incorrect.)

So added a fix in L51 below to take this into account.

Copy link

cypress bot commented Sep 22, 2024

group-income    Run #3166

Run Properties:  status check passed Passed #3166  •  git commit 125cda8c1b ℹ️: Merge d25c910c3f7b95dd1ff322e99b9c496431ab43c3 into 9fd5e1643cf0396ad0dfb58b6d45...
Project group-income
Branch Review sebin/task/#2356-markdown-linebreak-bug
Run status status check passed Passed #3166
Run duration 09m 27s
Commit git commit 125cda8c1b ℹ️: Merge d25c910c3f7b95dd1ff322e99b9c496431ab43c3 into 9fd5e1643cf0396ad0dfb58b6d45...
Committer Sebin Song
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 111
View all changes introduced in this branch ↗︎

@SebinSong SebinSong requested a review from taoeffect September 22, 2024 22:12
Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Great work @SebinSong! 👏

@taoeffect taoeffect merged commit 1a0de11 into master Sep 30, 2024
4 checks passed
@taoeffect taoeffect deleted the sebin/task/#2356-markdown-linebreak-bug branch September 30, 2024 15:47
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.

Minor markdown formatting bug in chat
2 participants