-
Notifications
You must be signed in to change notification settings - Fork 72
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
(feat) O3-3946: Add support for markdown questions #355
base: main
Are you sure you want to change the base?
Changes from 17 commits
5a50c2b
7185afd
a329aec
9dc9747
d555079
8325371
0928980
7e0923f
526cb1f
c932a2e
5953379
6a4552b
4afe95e
6e35125
01a5a9a
a34c7fb
be8822a
9596868
deb634c
f751243
af8d7b4
8591f6b
265dda8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
**/dist/* | ||
**/node_modules/* | ||
**/*.d.tsx | ||
__mocks__/* | ||
__mocks__/* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import React from 'react'; | ||
|
||
export default function ({ children }) { | ||
return <>{children}</>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import React, { useCallback } from 'react'; | ||
import ReactMarkdown from 'react-markdown'; | ||
import classNames from 'classnames'; | ||
import { useDraggable } from '@dnd-kit/core'; | ||
import { CSS } from '@dnd-kit/utilities'; | ||
|
@@ -69,6 +70,45 @@ const DraggableQuestion: React.FC<DraggableQuestionProps> = ({ | |
} | ||
}, [handleDuplicateQuestion, isDragging, question, pageIndex, sectionIndex]); | ||
|
||
const MarkdownWrapper: React.FC<{ markdown: string | Array<string> }> = ({ markdown }) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Split this out as it's own component in it's own file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated this! |
||
const delimiters = ['***', '**', '*', '__', '_']; | ||
|
||
function shortenMarkdownText(markdown: string | Array<string>, limit: number, delimiters: Array<string>) { | ||
const inputString = Array.isArray(markdown) ? markdown.join('\n') : markdown; | ||
let truncatedContent = inputString.length <= limit ? inputString : inputString.slice(0, limit).trimEnd(); | ||
const delimiterPattern = /[*_#]+$/; | ||
if (delimiterPattern.test(truncatedContent)) { | ||
truncatedContent = truncatedContent.replace(delimiterPattern, '').trimEnd(); | ||
} | ||
let mutableString = truncatedContent | ||
const unmatchedDelimiters = []; | ||
|
||
for (const delimiter of delimiters) { | ||
const firstIndex = mutableString.indexOf(delimiter); | ||
const secondIndex = mutableString.indexOf(delimiter, firstIndex + delimiter.length); | ||
if (firstIndex !== -1) { | ||
if (secondIndex === -1) { | ||
unmatchedDelimiters.push(delimiter); | ||
mutableString = mutableString.replace(delimiter, ''); | ||
} else { | ||
mutableString = mutableString.replace(delimiter, '').replace(delimiter, ''); | ||
} | ||
} | ||
} | ||
return truncatedContent + unmatchedDelimiters.reverse().join('') + (inputString.length > limit ? ' ...' : ''); | ||
} | ||
|
||
const shortMarkdownText = shortenMarkdownText(markdown, 15, delimiters); | ||
|
||
return ( | ||
<ReactMarkdown | ||
children={shortMarkdownText} | ||
unwrapDisallowed={true} | ||
allowedElements={['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'strong', 'em']} | ||
/> | ||
); | ||
}; | ||
|
||
return ( | ||
<div | ||
className={classNames({ | ||
|
@@ -89,7 +129,9 @@ const DraggableQuestion: React.FC<DraggableQuestionProps> = ({ | |
<Draggable /> | ||
</IconButton> | ||
</div> | ||
<p className={styles.questionLabel}>{question.label}</p> | ||
<p className={styles.questionLabel}> | ||
{question.label ? question.label : <MarkdownWrapper markdown={question.value} />} | ||
</p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an even weirder set of assumptions here, i.e., that any question without a label must be rendered via markdown. It would be better to check that the value is intended to be rendered in markdown. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ibacher , does this imply that question type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe in the future, probably. It would be better to check the rendering type === markdown as a condition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated this! |
||
</div> | ||
<div className={styles.buttonsContainer}> | ||
<CopyButton | ||
|
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 love this type of mocking. It means that the tests cannot actually test this component. Is there a reason we need to stub it out like this?
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.
This is a very bad response from me to say this as the reason, but I did this following what we had done in form-engine, because jest threw an error.