组件code review - 远尘 about dnd HOT 14 CLOSED

没publish是因为乔花没给权限 80 + 1 = 81

from dnd.

哦... 是我的错. 加下权限

from dnd.

from @saiyagg

• 事件绑定时，事件名称用空格分隔
• 建议 data-api 上不要存放实例，建议把实例放在全局的一个管理对象上，data-api 上存放实例的 id
• containment 默认改为 document
• isContain 代码重复
• open， close 方法 放到类静态方法中
• css({}) 代替 css(propertie, value)
• 变量命名，在考虑下
• 代码结构上，只暴露几个关键方法。
• 触发元素如何指定的问题。
• prox 样式问题，样式有可能和父节点有关，使用时有哪些限制
• revert 可增加到上一次定位的位置

from dnd.

• 所有 attrs 的有效性检查，建议使用 setter 的方式完成（格式不正确就无法被成功设置，自然 get 的时候拿到的就是合法的值）

  https://github.com/aralejs/dnd/blob/master/src/dnd.js#L442 开始的那些处理，都可以交由 setter 完成。否则即浪费性能又使代码看着不舒服。

• jquery 的一些使用细节：

  • 多次使用某个 jquery 对象用局部变量缓存：https://github.com/aralejs/dnd/blob/master/src/dnd.js#L72
  • 这里的offset对象也可以缓存：https://github.com/aralejs/dnd/blob/master/src/dnd.js#L504

from dnd.

多谢 @lianqin7 的 review. @ustccjw 看着修改下~~

from dnd.

好的 虽然回学校了 但是我还是会关注arale的 谢谢各位的review

from dnd.

👍

from dnd.

• 排版
  • 存在空格和tab 混存的现象, 最好统一.
  • 分号之前为啥要有一个空格?
• api 设计
  • 87-91 可以优化为 element.data('style', element.attr('style') || '')
  • executeDragEnterLeaveOver 和 executeDrop 方法中关于 drop 的判断可以改下, 这样可以减少后面的代码缩进.
    if (drop === null) {return}

from dnd.

@leoner
感谢这么细致的review
空格和tab已经统一，主要是回来vim的设置没弄好
分号前的空格属于自己的习惯，如果不是太不和谐，可能就不修改了

api设计提议已经修改，之前主要是纠结要不要把var定义移到if (drop === null) {return}后，但是又想统一把var放在函数最前面，你有啥看法呢

from dnd.

@ustccjw

var 哪里需要哪里定义 玉伯比较好这口，个人也喜好这种方式 ：）

from dnd.

虽然说可以那里用那里定义，不过在if 里var定义变量不推荐

在 2013-9-11，下午5:33，lianqin7 [email protected] 写道：

@ustccjw

var 哪里需要哪里定义 玉伯比较好这口，个人也喜好这种方式 ：）

—
Reply to this email directly or view it on GitHub.

from dnd.

分号前的空格的确比较独特。。。

from dnd.

@afc163 也不知道啥时养成这个习惯，之前", ;"的前后空格老是混用，最后就决定','后加空格，";"前加空格。。。

from dnd.

规范这一块，我们推崇入乡随俗，但这个「俗」，尽量还是一批人的共同习惯，这才能叫「俗」

太个性的东西，不能称之为「俗」的，还是得能避免就避免掉。

2013/9/11 ustccjw [email protected]

@afc163 https://github.com/afc163 也不知道啥时养成这个习惯，之前",
;"的前后空格老是混用，最后就决定','后加空格，";"前加空格。。。

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-24225390
.

王保平 / 玉伯（射雕）
送人玫瑰手有余香

from dnd.