-
Notifications
You must be signed in to change notification settings - Fork 5
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
changed markdown parser #809
Conversation
title: breadcrumb.title | ||
? markdownit.renderInline(breadcrumb.title) | ||
: UNTITLED_TEXT | ||
title: breadcrumb.title ? breadcrumb.title : UNTITLED_TEXT |
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.
Just sanity checking - you don't want rendered markdown in breadcrumbs?
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'm passing the title as-is and markdownifying it in the breadcrumbs module
<ReactMarkdown | ||
source={ | ||
teachingGuide.hasPart.find(item => item.name === "Questions") | ||
.text |
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.
Did you want to allow for emphasis here?
"/primary-source-sets/" | ||
) | ||
) | ||
}} | ||
/> |
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.
Did you want to allow emphasis in the above ReactMarkdown
?
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.
yes and it is because all types are allowed by default. emphasis is specified in the cases where we only want emphasis to show up
source={ | ||
teachingGuide.hasPart.find(item => item.name === "Activity") | ||
.text | ||
} |
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.
Allow for emphasis?
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.
see above
set.hasPart.find(item => item.name === "Resources").text | ||
) | ||
}} | ||
source={set.hasPart.find(item => item.name === "Resources").text} |
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.
allow for emphasis?
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.
see above
this and most future fixes will collide with #803 so it is preferrable to merge that branch sooner rather than later
now using react-markdown for all markdown needs... also allows us to do custom link rendering for issues like #783
fixes #523