Skip to content

[SVCS-418] MFR Markdown update + Mathjax #272

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

Conversation

AddisonSchiller
Copy link
Contributor

@AddisonSchiller AddisonSchiller commented Aug 11, 2017

refs: https://openscience.atlassian.net/browse/SVCS-418

Purpose

Currently MFR and the OSF wiki pages do not use the same flavor of markdown to render .md files. This inconsitency could be a problem, and MFR should be upgraded to use the same renderer as the OSF wiki.

Summary of Changes

Added support for mathjax (via cdn), markdown-it and its noted extensions.
While the old renderer was solely python, this one is mostly in javascript.

Updating the markdown renderer to use markdown-it instead of original markdown. Includes support for various extensions such as:

@centerforopenscience/markdown-it-toc
markdown-it-highlightjs
markdown-it-ins-del
markdown-it-sanitizer
markdown-it-mathjax

Testing Notes/QA Notes

Nearly all markdown files should be able to be rendered. Code blocks should be highlighted, and inline math equations etc should display properly. Inline html should work. Here is an example equation that should render correctly:

$$\sum_{i=0}^n i^2 = \frac{(n^2+n)(2n+1)}{6}$$

HTML tags in .md files should also be tested thoroughly. This could be a problem due to the fact that html sanitization has to happen before being rendered. This means that html tags will not appear as inserted html, but as text. (this is contrary to what is happening on the wiki)

Try breaking it with unclosed html tags as well.

Other Notes

Once MFR has its javascript library packaging re-worked, this ticket will also need to be looked at. Currently we are grabbing mathjax from a CDN. During original testing I had it local, but the package is so large that it is not in good practice to include it in this commit ( as would be needed due to MFRs current js package set up)

This md renderer is not exactly like the osf wiki, there are a few issues:
Style isn't exact, I had trouble matching style.
Scroll bar issues. There are some issues with scrollbars appearing when they shouldn't (iframe problems)
Escape characters dont work exactly like they do on the wiki,
ie, < shows up as just < on the wiki. On MFR it will show up as &lt, which is a standard way of displaying a "Less than" sign

@AddisonSchiller AddisonSchiller force-pushed the feature/markdown-upgrade branch 2 times, most recently from 727621f to 26e5b64 Compare August 15, 2017 18:20
@AddisonSchiller AddisonSchiller changed the title MFR Markdown update + Mathjax [SVCS-418] MFR Markdown update + Mathjax Oct 5, 2017
@cslzchen cslzchen requested review from felliott and removed request for felliott November 21, 2017 16:52
Updating the markdown renderer to use markdown-it instead of original markdown.

Upgrade bleach
@AddisonSchiller AddisonSchiller force-pushed the feature/markdown-upgrade branch from 92625c4 to 7cd21a8 Compare November 28, 2017 14:50
Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

First pass done. As discussed, here is a few things in addition to the comments:

  • Fix firefox scrollbar issue.
  • Fix html tag sanitizing issue.
  • Try and figure out the difference between mfr and wiki when javascript is turned off

FYI, here is link for sanitization in OSF for wiki:
https://github.com/CenterForOpenScience/osf.io/blob/develop/addons/wiki/models.py#L72, which calls bleach with whitelist.


to get all the libraries needed:

```
Copy link
Contributor

Choose a reason for hiding this comment

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

change to ```bash

Add note that you do not need to use npm, can just get off of github as well.


To add a new library that is not already set up to export to `root` can be a bit tricky, but the gist of it is, wrap the plugin in this code:

```
Copy link
Contributor

Choose a reason for hiding this comment

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

add ```javascript


And then modify it to work in this context. See other plugins for examples.

Then, in md.js, you can add a plugin to the markdown renderer by adding a `.use(<window.<PLUGIN_NAME>)`
Copy link
Contributor

Choose a reason for hiding this comment

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

add comment on how to load into template.mako

})(this, function () {


return function(md){
Copy link
Contributor

Choose a reason for hiding this comment

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

indent nicer


def render(self):
"""Render a markdown file to html."""
with open(self.file_path, 'r') as fp:
body = markdown.markdown(fp.read(), extensions=[EscapeHtml()])
body = bleach.clean(fp.read())
Copy link
Contributor

Choose a reason for hiding this comment

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

comment on why we bleach before sending to template

npm install [email protected]
npm install [email protected]
npm install [email protected]
npm install [email protected]
Copy link
Contributor

Choose a reason for hiding this comment

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

Add links to github versions, so we can see differences in the files you modified.

</div>

<script src="/static/js/mfr.js"></script>
<script src="/static/js/mfr.child.js"></script>
<script src="${base}/js/markdown-it.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Order matters, add comment for order.

<link rel="stylesheet" href="static/css/default.css">
<div style="word-wrap: break-word;" class="mfrViewer">
${body}
<link href='https://fonts.googleapis.com/css?family=Open+Sans:400,600,300' rel='stylesheet' type='text/css'>
Copy link
Contributor

Choose a reason for hiding this comment

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

mess with versions and re add the other osf file, see if it makes a difference.

<link rel="stylesheet" href="static/css/default.css">
<div style="word-wrap: break-word;" class="mfrViewer">
${body}
<link href='https://fonts.googleapis.com/css?family=Open+Sans:400,600,300' rel='stylesheet' type='text/css'>
Copy link
Contributor

Choose a reason for hiding this comment

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

style the file a bit better

};

var markdown = new MarkdownIt('commonmark', {
html: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

check default on this, see if true makes it more like the osf side

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It indeed does when proper sanitization is used 👍

Instead of bundling up assets, serve the smaller ones.
Larger ones can be loaded from cdn.
@AddisonSchiller AddisonSchiller force-pushed the feature/markdown-upgrade branch 2 times, most recently from a90d297 to d7ef557 Compare November 29, 2017 20:06
@cslzchen
Copy link
Contributor

cslzchen commented Dec 7, 2017

Replaced by #305.

@cslzchen cslzchen closed this Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants