Closed (fixed)
Project:
Form Builder
Version:
7.x-1.x-dev
Component:
Form Builder Core
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
30 Dec 2010 at 20:14 UTC
Updated:
14 May 2011 at 05:41 UTC
Jump to comment: Most recent file
Comments
Comment #1
james.elliott commentedComment #2
quicksketchThis seems mostly okay with me. I tend to use the raw DOM objects when possible because it makes for easier comparisons and it is sometimes more efficient when pulling out raw attribute (i.e. element.id or element.className), but for the items you've selected it may make sense. However before making this change I'd like to be consistent about our variable names. It's a common jQuery convention to name jQuery object variables with a $ prefix, so instead of element, if it's a jQuery object it should be $element. Then you know the difference between variables that are jQuery objects and those that are not.
Comment #3
james.elliott commentedThe $ prefix is a convention that some adopt. I personally don't use it because I like to keep a clear separation when I'm working with variables in JS or PHP, lest I fall into the habit of not using var for JS variable declarations. However, I don't feel strongly about it either way and am happy to use the $ prefix.
Comment #4
quicksketchThis patch had some pretty major issues (lots of undefined variables, using jQuery methods on non-jQuery objects). Rerolled with consistent $-prefixing of jQuery objects and corrected all JS errors. Same patch applied to both branches.