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

#2512 - Fix a couple of chat regressions #2521

Merged
merged 6 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ export default ({
generateTextObjectsFromText (text) {
const containsMentionChar = str => new RegExp(`[${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${CHATROOM_CHANNEL_MENTION_SPECIAL_CHAR}]`, 'g').test(text)
const wrapEmojis = str => {
const emojiRegex = /(\p{Emoji_Presentation}|\p{Emoji}\uFE0F|[\u2615-\u27BF]|\u200D)/gu
// We should be able to style the emojis in message-text (reference issue: https://github.com/okTurtles/group-income/issues/2464)
return str.replace(/(\p{Emoji})/gu, '<span class="chat-emoji">$1</span>')
return str.replace(emojiRegex, '<span class="chat-emoji">$1</span>')
}

if (!text) { return [] }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const RenderMessageWithMarkdown: any = {
render: function (createElement: any): any {
const { text, edited = false, isReplyingMessage = false } = this.$props
const domTree = htmlStringToDomObjectTree(renderMarkdown(text))

// Turns a dom tree object structure into the equivalent recursive createElement(...) call structure.
const recursiveCall = (entry: any): any => {
if (entry.tagName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ export function htmlStringToDomObjectTree (htmlString: string): Array<DomObject>
return createRecursiveDomObjects(rootNode)?.children || []
}

function isOnlyNewlines (str: any): boolean {
return /^[\n]*$/.test(str)
}

function createRecursiveDomObjects (element: any): DomObject {
/*
This function takes the virtual DOM tree generated as a reult of calling the DOMParser method,
Expand Down Expand Up @@ -61,7 +65,6 @@ function createRecursiveDomObjects (element: any): DomObject {

const isNodeTypeText = element?.nodeType === Node.TEXT_NODE
const isNodeCodeElement = element?.nodeName === 'CODE' // <code> ... </code> element needs a special treatment in the chat.
const isBodyElement = element?.nodeName === 'BODY'

const nodeObj: DomObject = isNodeTypeText
? { tagName: null, attributes: {}, text: element.textContent }
Expand Down Expand Up @@ -90,11 +93,7 @@ function createRecursiveDomObjects (element: any): DomObject {

nodeObj.children = nodeObj.children.filter(child => {
if (child.tagName) return true
else {
return isBodyElement
? child.text !== '\n' // DOMParser.parseFromString() adds a '\n' at the end of the body content which needs to be removed.
: (child.text || '').length
}
else return Boolean(child.text?.length) && !isOnlyNewlines(child.text)
})
}

Expand Down
14 changes: 6 additions & 8 deletions frontend/views/utils/markdown-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,23 @@ export function renderMarkdown (str: string): any {
entryText = entryText.replace(/</g, '&lt;')
.replace(/(?<!(^|\n))>/g, '&gt;') // Replace all '>' with '&gt;' except for the ones that are not preceded by a line-break or start of the string (e.g. '> asdf' is a blockquote).

entryText = entryText.replace(/\n(?=\n)/g, '\n<br>')
.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.
.replace(/(>)(\s.+)\n<br>/gs, '$1$2\n\n') // [3] this is a custom-logic added so that the end of blockquotes are correctly detected by markedjs. ('s' flag is needed to account for multi-line strings)

// GI needs to keep the line-breaks in the markdown but the markedjs with 'gfm' option doesn't fully support it.
// So we need to manually add <br/> tags here before passing it to markedjs.
// (Reference: https://github.com/markedjs/marked/issues/190#issuecomment-865303317)
entryText = entryText.replace(/\n(?=\n)/g, '\n\n<br/>\n')
entry.text = entryText
}
})

str = combineMarkdownSegmentListIntoString(strSplitByCodeMarkdown)
str = str.replace(/(\d+\.|-)(\s.+)\n<br>/g, '$1$2\n\n')
.replace(/(>)(\s.+)\n<br>/gs, '$1$2\n\n') // Check for [2], [3] above once more to resolve edge-cases (reference: https://github.com/okTurtles/group-income/issues/2356)
Comment on lines -43 to -54
Copy link
Collaborator Author

@SebinSong SebinSong Jan 18, 2025

Choose a reason for hiding this comment

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

To give a context here, these multiple lines of regExp logics to manually add <br> here had been added over time to resolve issues related to multiple line-breaks in the chat. (markedjs with 'gfm' option does not support the behavior we want. eg. we would want \n\n\n to become <br><br><br> but it collapses them all and convert to a single <br> or sometimes even removes them all)

I re-assessed these part and enhanced/simplified as much as I could because it had been growing in complexity over time, which made it hard to maintain. I made sure this change doesn't break the previous issues we fixed before:

[Complex markdown structure 1 - nested lists]

image

[Complex markdown structure 2 - blockquotes]

image


// STEP 2. convert the markdown into html DOM string.
let converted = marked.parse(str, { gfm: true })

// STEP 3. Remove the unecessary starting/end line-breaks added in/outside of the converted html tags.
converted = converted.replace(/<([a-z]+)>\n/g, '<$1>')
.replace(/\n<\/([a-z]+)>/g, '</$1>')

return converted
}

Expand Down Expand Up @@ -159,7 +157,7 @@ export function splitStringByMarkdownCode (
// (e.g. `asdf`, ```const var = 123```)

const regExCodeMultiple = /(```\n[\s\S]*?```$)/gm // Detecting multi-line code-block by reg-exp - reference: https://regexr.com/4h9sh
const regExCodeInline = /(`.+`)/g
const regExCodeInline = /(`[^`]+`)/g
const splitByMulitpleCode = str.split(regExCodeMultiple)
const finalArr = []

Expand Down