#13699 closed enhancement (fixed)
Convert dojo/fx to AMD with minimal deps
Reported by: | Chris Mitchell | Owned by: | Chris Mitchell |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Core | Version: | 1.6.1 |
Keywords: | Cc: | Colin Snover | |
Blocked By: | Blocking: |
Description
Convert dojo/fx to AMD with minimal deps
Change History (13)
comment:1 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 Changed 9 years ago by
This patch violates JavaScript (& Dojo) code conventions. Uppercase first letters are reserved for constructors (dom, array, etc. are not constructors) and uppercase letters in other places in variables are reserved for delineating word breaks in variable names (i.e. thisDomThing not thisDOMThing). Please fix at your earliest convenience. Thanks!
Also, the return value change doesn’t make sense; it should be building an object and assigning the object to dojo.fx and returning the same object, otherwise people can’t hot patch properly.
Finally as I mentioned in another ticket, as pointed out by kgf, your change to remove “dojo.” from the comment on line 20 probably breaks the doc parser.
comment:6 Changed 9 years ago by
Pretty sure I've addressed the above issues. Please review and let me know if there is anything else you see. Thanks!
comment:7 Changed 9 years ago by
Cc: | Colin Snover added |
---|
comment:9 Changed 9 years ago by
I'm still seeing hundreds of references to dojox in dojo/fx. I guess that's not covered by this ticket?
Oops, nevermind, I was looking at dojox/fx and this is about dojo/fx.js.
comment:10 Changed 9 years ago by
I've got a good start at dojox.fx conversion locally. Once I get past the last few probs in gfx3d, I'll continue with the modules under dojox/fx. -Chris
comment:11 Changed 9 years ago by
Priority: | normal → high |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
After these changes the Toggler test (in fx.html) is failing. Ran test on IE8.
comment:12 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
In [26235]: