-
Notifications
You must be signed in to change notification settings - Fork 60
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) Make the label in FormField optional to support Markdown content #411
(feat) Make the label in FormField optional to support Markdown content #411
Conversation
package.json
Outdated
"rehype-raw": "^7.0.0", | ||
"remark-gfm": "^4.0.0" |
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.
Do we need these three libraries?
@Twiineenock I think you need to commit the yarn.lock |
@@ -57,7 +57,7 @@ export interface FormSection { | |||
} | |||
|
|||
export interface FormField { | |||
label: string; | |||
label?: string; |
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.
@samuelmale this was changed to be optional because markdown type inputs don't have a label. Does this seem okay?
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.
That makes sense to me.
@Twiineenock can you resolve the conflicts and update your branch? |
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.
Thanks for working on this. I've left a comment seeking some clarification about the changes here.
allowedElements={[ | ||
'h1', | ||
'h2', | ||
'h3', | ||
'h4', | ||
'h5', | ||
'h6', | ||
'p', | ||
'strong', | ||
'em', | ||
]} |
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.
@Twiineenock AFAICT, it doesn't seem as though allowedElements
has changed per this this diff. Does that invalidate the PR title? Is the main change here that we're just bumping react-markdown
?
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.
@denniskigen , I had added more allowedElements
in the first commit.
I was later asked to maintain the current allowedElements
as in this thread.
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.
And yes the PR title has to change! Thanks!
LGTM! Thanks @Twiineenock! |
Requirements
Summary
Problem Description
This pull request is part of ongoing work in PR #355.
In the interactive form builder, users should have the ability to create questions with markdown-rendered content. The markdown feature will allow question content to be formatted for better readability and accessibility.
Key Changes
Upgrade
react-markdown
Version:react-markdown
from version^7.1.2
to^9.0.1
to support an expanded range of markdown formatting options.Extended Markdown Capabilities:
ReactMarkdown
component to support more plugins andallowedElements
for enhanced markdown rendering.FormField
interface to handle cases where thelabel
is absent, but avalue
is present. This change provides more flexibility for displaying markdown content without requiring a label.Technical Details
Screenshots
Related Issue
https://openmrs.atlassian.net/browse/O3-3946
Other