optimize: update modal toast #26
Conversation
src/component/toast.ts
Outdated
| const nodes = document.querySelectorAll('.page-spy-toast'); | ||
| if (nodes.length) { | ||
| [...nodes].forEach((n) => { | ||
| if (n) { |
There was a problem hiding this comment.
这里的n判断目的是? nodes里面是有可能为null?
There was a problem hiding this comment.
原理上 nodes 是全部存在的，这里属于做点防御性
There was a problem hiding this comment.
我个人是不建议这么做. 因为这里的写法作用你是知道用来防御的, 但是后面的人不知道. 所以你可以删, 但是后面的人是一定不敢删的. 如果你们团队有防御写法的习惯, 那也不是什么大问题.
There was a problem hiding this comment.
判断条件写错了，换成 document.contains 理解起来顺畅些
src/component/toast.ts
Outdated
| node.classList.add('page-spy-toast'); | ||
| node.textContent = String(text); | ||
| document.documentElement.appendChild(node); | ||
| Toast.timer = window.setTimeout(() => { |
There was a problem hiding this comment.
window挂载的方法, 没必要写window. 如果要写, 那就是全局规范. 根据之前的alert都没写window, 这里应该也没必要写吧.
src/component/toast.ts
Outdated
| static message(text: string) { | ||
| const node = document.createElement('div'); | ||
| node.classList.add('page-spy-toast'); | ||
| node.textContent = String(text); |
There was a problem hiding this comment.
textContent在IE8中不支持. innerHTML 会不会更好点?
src/component/toast.ts
Outdated
| node.textContent = String(text); | ||
| document.documentElement.appendChild(node); | ||
| Toast.timer = window.setTimeout(() => { | ||
| if (node) { |
There was a problem hiding this comment.
什么情况下 node 会不存在? 这里不是一定会create的吗
There was a problem hiding this comment.
这里应该判断 node 是否在 DOM 上，已修改
There was a problem hiding this comment.
调用 Toast.message 多次后，执行 Toast.destroy 会删除所有 .page-spy-toast 节点和最新的 Toast.timer，之前 的 timer 里的闭包在执行时拿到的 node 已经不存在于 DOM 上了。
|
没问题了, 可以合并. |
No description provided.