Skip to content
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(markdown): Use markdown rendered on GitHub side #386

Merged
merged 16 commits into from
Sep 30, 2017
Merged

feat(markdown): Use markdown rendered on GitHub side #386

merged 16 commits into from
Sep 30, 2017

Conversation

machour
Copy link
Member

@machour machour commented Sep 29, 2017

Silenced all missing keys warnings.
Fixed an issue where a mention or a issue link wasn't recognized at the start of a line.

#381 <- should work
@housseindjirdeh <- should work too

@machour
Copy link
Member Author

machour commented Sep 29, 2017

Noticed yet a few more problems
screen shot 2017-09-29 at 2 03 04 pm

@machour
Copy link
Member Author

machour commented Sep 29, 2017

Ok, so this is turning out to be more than small fixes for markdown.
I discovered that when we pass the HTML accept header to github API, all markdown gets rendered server side, and a clean HTML transformation is provided in body_html.

So we're getting rid of MarkdownHtmlView in favor of GithubHtmlView.

Still work in progress to fix some small glitches

@andrewda andrewda changed the title More markdown fixes fix(markdown): more markdown fixes Sep 29, 2017
@machour
Copy link
Member Author

machour commented Sep 30, 2017

bummer: syntax highlighting doesn't work anymore.. fixing it now.

Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

Looks solid. Noticed one thing you may have missed :)

<MarkdownHtmlView
source={comment.body}
<GithubHtmlView
source={comment.body_html}
Copy link
Member

Choose a reason for hiding this comment

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

I think you might have meant to use comment.body. Noticing this when I open an issue:

image

From the looks of it, you may have accidentally swapped it by accident :)

image

Copy link
Member Author

Choose a reason for hiding this comment

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

@housseindjirdeh We should be using only comment.body_html from now on instead. I tried the branch again and everything works fine for me. Could you give it a new try?
Fair enough, your comment have made me realize that Adding a comment was broken, I fixed it and completely got rid of comment.body :D

Copy link
Member

Choose a reason for hiding this comment

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

Yeah was thinking the same thing, appreciate you diving in and removing body entirely since we don't really use it anymore

@machour machour changed the title fix(markdown): more markdown fixes feat(markdown): Use markdown rendered on GitHub side Sep 30, 2017
Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

It's amazing @machour 😍

image

Images and GIFs show up so nicely

@housseindjirdeh housseindjirdeh merged commit 5fb5ef6 into gitpoint:master Sep 30, 2017
@machour machour deleted the more-markdown-fixes branch September 30, 2017 23:02
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.

None yet

2 participants