Render markdown on frontend #86

paulocheque · 2016-05-27T06:41:17Z

This PR is not done yet. I have just created it to discuss about it.

I rebased your render_markdown_on_frontend branch to the origin/master. I guess we should deploy the Fixed #84 #85 first to avoid merge conflicts. I can rebase it again later.
Before deploying this feature, I believe we should clean the Redis cache. I am using rendered_text = False, but I am not sure if this prevent some cache of html.
Now, the preview and the article's page looks identical. If we need to change something in the layout, we need to use one common css file for both pages, to keep the idea.
I removed the github.css files because they were screwing the design up. But maybe some useful css, that let the design closer to the GitHub may be lost. It is not critical I think.
I believe we can remove all the backend code related to the rendered = True part. Is that ok? Or we are using this in another feature?
I guess some js/css files can be renamed to be more understanding. For example: editor_controller.js instead of editor_utils.js. But I believe this should be the last commit to avoid merge issues.

paulocheque · 2016-05-30T21:22:17Z

I saw other features are using it, like the FAQ page. I guess the best option would be removing all rendered text. I changed it here but I did not commit it yet. If will do that, we should clean the faq page's cache too.

durden · 2016-06-13T08:58:00Z

Let's go ahead and move the FAQ and contest pages over to using the new approach.

I think we should leave rendered=True in the code now. It's not too much code, and it could be useful if we want to use the lower-level functions later to fetch markdown from the github API for other purposes.

durden · 2016-06-13T09:27:38Z

It looks like the syntax highlighting is different due to this change. My guess is github uses a different syntax coloring theme than what we're using by default with highlightjs.

Here's the version where we render the markdown on the front-end:

Here's the version when github renders the markdown:

You can use this guide as a basic test.

paulocheque · 2016-06-13T16:38:41Z

GitHub uses (Linguist)[https://github.com/github/linguist] to highlight the codes. But I dont think it would be good for us to use that, since it is a ruby lib.

I guess we can use another or improve the current highlight js theme.

durden · 2016-06-14T08:39:37Z

pskb_website/static/js/utils.js

@@ -150,16 +150,6 @@ function create_responsive_tables(div) {
    tables.addClass('table');
 }

-/* Change all external links to open in a new tab/window */
-function create_external_links(id) {


Just for reference, this code is handled in the pskb_website/static/js/marked_settings.js file now.

durden · 2016-06-14T08:43:34Z

Why does it matter if Github uses linguist instead of highlight js? I thought the problem was the syntax coloring not the actual language detection, which is what linguist does right?

Isn't that what this file is supposed to be handling? I was under the impression that CSS file was mimicing the colors, etc. of the github syntax styling.

As a sidenote, we aren't using this file anymore after you removed it from the article.html page in this commit. Maybe we can adapt it to fix this issue or remove the file altogether since it's not used?

paulocheque · 2016-06-14T18:35:03Z

Exactly. Actually there were two github.css files. I stop using them because they were changing badly some html components. But it was in the TODO list to check this design issues after removing that. So, we can create/customise the github.css to fit our needs.

paulocheque · 2016-06-14T20:43:19Z

The included github theme of highlightjs should be obsolete. The github-gist theme is more similar, so maybe we can use this. There is a good live demo here about the themes: https://highlightjs.org/static/demo/

I created an article for testing these:

I will work on the css now.

durden · 2016-06-15T08:37:51Z

Oh ok that all makes sense. Thanks for clarifying. I forgot you mentioned we'd need to update the CSS afterwards. Thanks for the highlightjs theme link, good way to test it!

paulocheque · 2016-06-16T15:19:11Z

Do we need to turn our rendered markdown as close as possible to the GitHub pages?

Because now I am using https://sindresorhus.com/github-markdown-css/ and the css is very close to the GitHub's css.

Also, I am using the highlight-js github-gist.css instead of the github.css theme.

Now the Editor's preview, the Article page and the FAQ page are using the same JS code, the same CSS and all of them are very similar to the GitHub's. Including the font-family. Is that the objective?

http://guides-dev.herokuapp.com//python/markdown-test-paulo?saved=1&status=draft
http://guides-dev.herokuapp.com/write/python/markdown-test-paulo?branch=master
http://guides-dev.herokuapp.com//faq/

durden · 2016-06-16T15:52:30Z

Sounds good. The objective is to have our preview and guide pages look as
close as possible to github's rendering. We can tolerate minor differences
in fact that's almost inevitable since GitHub can change at any minute.

On Thursday, June 16, 2016, Paulo Cheque notifications@github.com wrote:

Do we need to turn our rendered markdown as close as possible to the
GitHub pages?

Because now I am using https://sindresorhus.com/github-markdown-css/ and
the css is very close to the GitHub's css.

Also, I am using the highlight-js github-gist.css instead of the
github.css theme.

Now the Editor's preview, the Article page and the FAQ page are using the
same JS code, the same CSS and all of them are very similar to the
GitHub's. Including the font-family. Is that the objective?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#86 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AADiz0j34-G4E2fhSlsWnRC9xmx6WAzrks5qMWlwgaJpZM4IoN0_
.

paulocheque · 2016-06-22T16:18:02Z

Rebased to origin/master

durden · 2016-06-23T07:51:17Z

Looks like there is a problem with some CSS or the rendering leading to a really narrow column of text for the guide. Maybe it's an error with the guide's markdown? You can grab the raw markdown to test with here.

Here's how the guide looks with our current implementation:

I think this guide's text might also have newlines in it which is leading to the weird wrapping. I guess the github renderer or our previous JS on the front-end cleaned this up?

Another issue is the link text is blue in the new version. Does anyone have a preference on this? Our existing CSS uses orange, which matches the rest of the design. However, maybe blue is better?

I also noticed there's a lot more whitespace between the title and the guide itself when using the new renderer. I'm guessing this is just some padding in the gist CSS that we didn't have before. I think it looks better to keep the text a bit closer to the title. What do you think @paulocheque?

New:

Old:

paulocheque · 2016-06-23T19:57:57Z

Fixed!

durden · 2016-06-24T09:01:48Z

Nice! It looks like some of the changes are causing us to not use the Gotham font on the guides. Looks like these rules are the problem, which makes the text on the guide look different than the rest of the site.

I noticed another subtle difference in the markdown parsing. For example, test this guide on the staging site with this branch. Notice the extra   between paragraphs in the new version. Maybe this has something to do with hard line-breaks in the text?

This issue leads to a lot of extra whitespace between columns on this guide.

Finally, there still seems to be a bunch of extra left margin/padding on the guide text and code blocks, which leads to the code block and text not being left-aligned and too much whitespace on the left.

Old:

New:

Sorry for all the nitpicking! This is a big change, which changes the most useful feature of the entire project, so I want to test it fully. This is not an easy change so thanks so much for taking it on!

paulocheque · 2016-06-24T17:28:12Z

About the fonts, you are right, I just removed that css and it is fixed. Before, the font was the same of the GitHub's.

About the line breaks, this is a cache issue. If you clean your redis cache of articles, it should be fixed automatically.

Now we have to think about the deploy. We can temporarily disable the cache and deploy. After a couple of hours we can deploy again with the cache enabled again.

Or we can clean the cache manually.

durden · 2016-06-27T07:51:12Z

The cache was part of the problem. However, there is still a difference in how the new parsing handles line breaks. I've cleared the cache on the staging server, and I still see a major difference between the rendering of this guide. Notice the article itself has some line-breaks in it, but github's rendering doesn't render these as   tags, whereas the new approach does.

New:

Old (rendering from github.com):

durden · 2016-06-27T08:07:43Z

Also, here are some other CSS tweaks that we need to integrate to this PR (I don't have push access).

The font-size is a little too large in the new approach. Also that diff cleans up the left padding on code blocks and the blue links. We want to keep the links orange to match the rest of the site.

paulocheque · 2016-06-27T17:02:14Z

Are you sure this is not a browser's cache issue? Because I load the same article and it the line endings appear to be ok here.

paulocheque · 2016-06-27T17:13:50Z

Well, I am not sure about this line endings, but if the artilcle has a line break, the renderer replace that by a  .

This happens because we are using the gfm line breaks (https://github.com/chjj/marked#breaks).

So I believe that if some article has these extra line breaks, the article content should be adjusted. But I think the only way to fix that is manually.

A simple test here:

Paragraph 1 with a line break right now (no line breaks here)

Paragraph 2 with a line break right
now (in the next line)

Paragraph 3 with a line break right

now (every \n is replaced by a
)

durden · 2016-06-27T17:21:55Z

Interesting. So the gfm mode in marked inserts   but the real github flavored markdown doesn't do this? We were allowing github.com itself to do the rendering before and I guess these extra   didn't show up?

durden · 2016-06-27T17:24:51Z

Is this a bug in the gfm mode of marked then? Why would their github mode render   in places differently than the real github?

I think this is the starting point for seeing how github's API actually does this.

paulocheque · 2016-06-27T17:26:08Z

Compare the gfm line breaks enabled and disabled.

…ticle view - Experimenting with ideas mentioned in pluralsight#39. This commit removes the following features: - No automatic header tags like on github.com - This is the little 'octicon link' icons that show up to the left of headers when you hover over them. - This means our headers do not have in-page anchors associated with them anymore so users cannot link to sections of a guide. - We definitely want this added back but not sure the best way to do it yet on the front-end.

…e. Stop using the rendered article html in the backend.

…render object.

…/article/faq pages. Using highlight-js github-gist.css theme instead of github.css theme.

…not support this markdown syntax, but hack.guides does.

paulocheque · 2016-06-27T17:34:31Z

There is no bug. It just a line ending convention. But the marked lib doc is wrong (https://github.com/chjj/marked#breaks), so the GitHub line ending convention is the opposite.

So I guess we should disable this line ending style.

marked.setOptions({
    renderer: renderer,
    gfm: true,
    tables: true,
    breaks: true, # <----------------------- We should disable this
    pedantic: false,
    sanitize: false,
    smartLists: true,
    smartypants: false,
});

paulocheque · 2016-06-27T17:38:02Z

The GitHub's doc is very short. But the real example before doesn't lie.

…'s line ending convention. All the discussion and screenshots can be found in the PR pluralsight#86

durden · 2016-06-28T08:53:03Z

Got this all fixed and deployed. I made a few small changes. You can see the commits for details, but in short I removed all the custom font-weight and font-size from the headers and fixed a bug injected here 836b220 that was leading to an extra bit of the white background showing on the homepage.

The article pages now use the header stylings that are used throughout the site and suggested by PS's designer. This is different than github.com but it makes everything match across our site.

paulocheque · 2016-06-28T10:16:33Z

Awesome!

paulocheque force-pushed the render_markdown_on_frontend branch 2 times, most recently from c757c31 to 3c218a2 Compare June 9, 2016 20:34

durden reviewed Jun 14, 2016
View reviewed changes

paulocheque force-pushed the render_markdown_on_frontend branch from 3c218a2 to 3633bde Compare June 16, 2016 14:17

paulocheque force-pushed the render_markdown_on_frontend branch from fd63660 to e586c84 Compare June 22, 2016 16:17

paulocheque mentioned this pull request Jun 23, 2016

Highlight JS issue on Python doc strings #100

Closed

paulocheque force-pushed the render_markdown_on_frontend branch from c6d7533 to 421719e Compare June 23, 2016 19:57

paulocheque force-pushed the render_markdown_on_frontend branch from 01bcb6e to 402869a Compare June 24, 2016 17:24

durden mentioned this pull request Jun 27, 2016

CSS Fixes #101

Closed

durden and others added 10 commits June 27, 2016 14:29

Using the frontend js code to render the article in the article's pag…

aef16ae

…e. Stop using the rendered article html in the backend.

Removed duplicate code for rendering markdown links.

fd8a819

Rendered markdown tables should be responsive. Moved the code to the …

4582735

…render object.

Using our markdown render system to create the FAQ page

e22a008

Using github-markdown-css.css to the rendered markdown in the preview…

7617534

…/article/faq pages. Using highlight-js github-gist.css theme instead of github.css theme.

Improve layout for headers (h#) inside of lists (ol/ul). GitHub does …

262e1e9

…not support this markdown syntax, but hack.guides does.

CSS improved for the rendered markdown of Preview/Article/FAQ pages

836b220

Using hack.guides fonts in the rendered markdown

371e5bb

CSS adjustments (by Luke)

007ab8c

paulocheque force-pushed the render_markdown_on_frontend branch from 3d32fc7 to 007ab8c Compare June 27, 2016 17:29

Disabled the Marked's breaks option. It does not represent the GitHub…

594c6d8

…'s line ending convention. All the discussion and screenshots can be found in the PR pluralsight#86

durden merged commit 594c6d8 into pluralsight:master Jun 28, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render markdown on frontend #86

Render markdown on frontend #86

paulocheque commented May 27, 2016 •

edited

paulocheque commented May 30, 2016 •

edited

durden commented Jun 13, 2016

durden commented Jun 13, 2016

paulocheque commented Jun 13, 2016

durden Jun 14, 2016

durden commented Jun 14, 2016

paulocheque commented Jun 14, 2016

paulocheque commented Jun 14, 2016 •

edited

durden commented Jun 15, 2016

paulocheque commented Jun 16, 2016 •

edited

durden commented Jun 16, 2016

paulocheque commented Jun 22, 2016

durden commented Jun 23, 2016 •

edited

paulocheque commented Jun 23, 2016

durden commented Jun 24, 2016

paulocheque commented Jun 24, 2016

durden commented Jun 27, 2016

durden commented Jun 27, 2016

paulocheque commented Jun 27, 2016

paulocheque commented Jun 27, 2016 •

edited

durden commented Jun 27, 2016

durden commented Jun 27, 2016

paulocheque commented Jun 27, 2016

paulocheque commented Jun 27, 2016 •

edited

paulocheque commented Jun 27, 2016 •

edited

durden commented Jun 28, 2016

paulocheque commented Jun 28, 2016

Render markdown on frontend #86

Render markdown on frontend #86

Conversation

paulocheque commented May 27, 2016 • edited

paulocheque commented May 30, 2016 • edited

durden commented Jun 13, 2016

durden commented Jun 13, 2016

paulocheque commented Jun 13, 2016

durden Jun 14, 2016

Choose a reason for hiding this comment

durden commented Jun 14, 2016

paulocheque commented Jun 14, 2016

paulocheque commented Jun 14, 2016 • edited

durden commented Jun 15, 2016

paulocheque commented Jun 16, 2016 • edited

durden commented Jun 16, 2016

paulocheque commented Jun 22, 2016

durden commented Jun 23, 2016 • edited

paulocheque commented Jun 23, 2016

durden commented Jun 24, 2016

paulocheque commented Jun 24, 2016

durden commented Jun 27, 2016

durden commented Jun 27, 2016

paulocheque commented Jun 27, 2016

paulocheque commented Jun 27, 2016 • edited

durden commented Jun 27, 2016

durden commented Jun 27, 2016

paulocheque commented Jun 27, 2016

paulocheque commented Jun 27, 2016 • edited

paulocheque commented Jun 27, 2016 • edited

durden commented Jun 28, 2016

paulocheque commented Jun 28, 2016

paulocheque commented May 27, 2016 •

edited

paulocheque commented May 30, 2016 •

edited

paulocheque commented Jun 14, 2016 •

edited

paulocheque commented Jun 16, 2016 •

edited

durden commented Jun 23, 2016 •

edited

paulocheque commented Jun 27, 2016 •

edited

paulocheque commented Jun 27, 2016 •

edited

paulocheque commented Jun 27, 2016 •

edited