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 render for thread author #3

Merged
merged 6 commits into from
Jun 22, 2020
Merged

feat: markdown render for thread author #3

merged 6 commits into from
Jun 22, 2020

Conversation

panda2134
Copy link

  • math not supported, will add KaTeX support later
  • not tested

@Yixuan-Wang
Copy link

Tested on pkuhole community fork, not working😢 because the text is rendered as text, not HTML.

@panda2134
Copy link
Author

panda2134 commented Jun 22, 2020

Tested on pkuhole community fork, not working😢 because the text is rendered as text, not HTML.

we then need some "dangerouslySetInnerHTML " thing...
working on a fix

@panda2134
Copy link
Author

Sorry for the bug, I'm familiar with Vuejs, but not so familiar with React

@Yixuan-Wang
Copy link

之前修了一下,但是修改在Common.js,除了洞主的内容,回复也一并渲染了……在这里贴了一张图。

@panda2134
Copy link
Author

拿react-markdown重新弄了下,兼容了原来的几条渲染规则

@Yixuan-Wang 能否帮忙测试一下?我这边没有测试的环境 应该洞主渲染是正常的了

@Yixuan-Wang
Copy link

Yixuan-Wang commented Jun 22, 2020

简单测试了一下显示效果:

效果图

遇到的问题:

  1. 链接可能意外包含更多内容。

    效果图

  2. 图片超出边框。

  3. 补充 树洞洞号内链失效。

@Yixuan-Wang
Copy link

另外,我这边测试也不太方便🤦,两边代码库和代码风格差异太大,冲突太多……

@panda2134
Copy link
Author

感谢您的测试,洞号内链我大概知道是什么问题了,等会来修;图片可否考虑先禁止掉?毕竟已经有图片类型树洞了

@panda2134
Copy link
Author

1,3均已经修复

@panda2134
Copy link
Author

图片
图片

@panda2134
Copy link
Author

出于布局考虑,是否应该把markdown 中的h1,h2也渲染为h3?

@panda2134
Copy link
Author

图片

禁用了markdown图片语法,不予渲染,以避免可能的攻击风险

@panda2134
Copy link
Author

图片

@panda2134 panda2134 changed the title [WIP] feat: markdown render for thread author feat: markdown render for thread author Jun 22, 2020
@panda2134
Copy link
Author

panda2134 commented Jun 22, 2020

出于布局考虑,是否应该把markdown 中的h1,h2也渲染为h3?

44a54bf

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

image
现存置顶洞的换行都没了。。。
理想情况应该是:
image

@panda2134
Copy link
Author

估计是过于严格地遵守了md规范

应该改一个比较宽松的格式

@panda2134
Copy link
Author

panda2134 commented Jun 22, 2020 via email

@panda2134
Copy link
Author

图片

@thuhole thuhole merged commit 197dfa5 into treehollow:master Jun 22, 2020
@panda2134
Copy link
Author

坏了坏了,网站挂了,建议立刻revert

@panda2134
Copy link
Author

应该是动了packages.json导致改变了cdn路径,但是travis ci没有对应改过来

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

抱歉刚刚紧急revert了

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

我CDN的部署非常的tricky,可以看.travis.yml

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

package.json里的CDN也是非标准操作

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

我来手动merge吧

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

@panda2134 react-markdown的依赖应该可以去掉吧?

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

成功了。下一步可能就要加markdown preview了哈哈哈哈。。又多了个锅

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

顺便,应该没有xss注入问题吧?

@panda2134
Copy link
Author

panda2134 commented Jun 22, 2020 via email

@panda2134
Copy link
Author

panda2134 commented Jun 22, 2020 via email

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

已经有用户要求加预览了hhhh
我感觉预览做成弹窗比较方便

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

[图片] 可否显示链接?

@panda2134
Copy link
Author

[图片] 可否显示链接?

可以倒是可以,但是为啥不直接用链接呢hhh

anyway我来加上吧 顺便做一下预览

@panda2134
Copy link
Author

我新开一个pr 过去说吧

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

@panda2134 算了,我觉得这样也可以,做个文档说明一下就行了。

@thuhole
Copy link
Collaborator

thuhole commented Jun 22, 2020

@panda2134 其实最迫切需要的还是preview哈哈哈哈。。明天再说吧。

@thuhole
Copy link
Collaborator

thuhole commented Sep 5, 2020

图片

禁用了markdown图片语法,不予渲染,以避免可能的攻击风险

这个禁用得好啊,不知道为什么 waylonflinn/markdown-it-katex#26 到现在都还open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants