Skip to content

提高gitment的浏览器兼容性 #52

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.

Sign up for GitHub

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

Open
GeekaholicLin wants to merge 10 commits into
base: master
Choose a base branch
from
Open

提高gitment的浏览器兼容性 #52

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bd261e7
browser compatibility
GeekaholicLin Jul 21, 2017
aa7c305
fix config error && add npm script
GeekaholicLin Jul 21, 2017
ffa970e
fix browser compatibility for IE11/10 etc.
GeekaholicLin Jul 21, 2017
9fdfef5
support uglifying js
GeekaholicLin Jul 21, 2017
bdd45fe
remove console.log
GeekaholicLin Jul 21, 2017
1c68090
fix bugs using instance && do not change api as posible
GeekaholicLin Jul 22, 2017
54af3cb
format
GeekaholicLin Jul 22, 2017
2f9a7ba
format
GeekaholicLin Jul 22, 2017
f0e2a0b
api break && webpack config and npm script optimize
GeekaholicLin Jul 22, 2017
50ff9dd
fix toString.call not working in IE && recover origin api
GeekaholicLin Jul 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/theme/default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ function render(state, instance) {
const container = document.createElement('div')
container.lang = "en-US"
container.className = 'gitment-container gitment-root-container'
container.appendChild(instance.theme.renderHeader(state, instance))
container.appendChild(instance.theme.renderComments(state, instance))
container.appendChild(instance.theme.renderEditor(state, instance))
container.appendChild(instance.theme.renderFooter(state, instance))
container.appendChild(instance.renderHeader(state, instance))
container.appendChild(instance.renderComments(state, instance))

@GeekaholicLin GeekaholicLin Jul 22, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

找到IE的问题所在了。。确实是API的问题。我们简单地走一下流程。当调用gitment.render(container)的时候，实际上是调用instance.render(container)，而在instance.render(container)中获取了instance.theme[renderer]也就是this.theme.render(state, instance)，是渲染的总入口(囊括了Header等)，但是在this.theme.render(state, instance)中调用，比如默认的this.theme.render(state, instance)是调用以下：

  container.appendChild(instance.renderHeader(state, instance))
  container.appendChild(instance.renderComments(state, instance))
  container.appendChild(instance.renderEditor(state, instance))
  container.appendChild(instance.renderFooter(state, instance))

问题来了，instance.renderHeader(state, instance)的使用是错误的，签名对应不上，正确的参数列表应该是(container)，因为instance.renderHeader接收的是一个element或字符串。而实际上传进去的state和instance，那container是一个state对象，正确应该是使用instance.theme.renderxxxx(state, instance)（只渲染不操作，因为操作已经在container.appendChild有了呀），不过并无大影响，因为你的isString()已经帮你过滤了不正确的参数,粗略看了一下，不过是外加了一个div。所以本来改了API后又改了回来

但是对于IE不通过的原因是，toString不兼容。。所以加了一个空对象。

@tiexo tiexo Jul 29, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

完善的如何了，最新的引用地址是哪个？

@GeekaholicLin GeekaholicLin Jul 29, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

非官方的引用地址可以看上面的记录，等作者有空了并且review通过了应该就可以merge了。

http://blog.geekaholic.cn/js/thirdParty/gitment.browser.js
http://blog.geekaholic.cn/js/thirdParty/gitment.browser.min.js

@tiexo tiexo Jul 29, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好的，先用着你的...

@asdf2014 asdf2014 Jun 21, 2018 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi, @GeekaholicLin . 大佬，刚看到你博客了，有兴趣互相添加友链否？宇宙湾（https://yuzhouwan.com）

container.appendChild(instance.renderEditor(state, instance))
container.appendChild(instance.renderFooter(state, instance))
return container
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { LS_ACCESS_TOKEN_KEY } from './constants'

export const isString = s => toString.call(s) === '[object String]'
export const isString = s => ({}).toString.call(s) === '[object String]'

export function getTargetContainer(container) {
let targetContainer
Expand Down