#format wiki
||<tablestyle="float:right; font-size: 0.9em; width:40%; background:#F1F1ED; margin: 0 0 1em 1em;" style="padding:0.5em;"><<TableOfContents>>||

= JavaScript Review Notes =

These are items that any [[ReviewerSchedule|code reviewer]] should keep in mind when reviewing JavaScript.

<<TableOfContents()>>

== CRITICAL ==

 * '''ALL JAVASCRIPT FILES MUST BE LINT-FREE'''
   * Note though that you can sometimes confuse XmlLint [1]
 * '''NO SCRIPT FILES SHOULD BE PULLED IN BY THE YAHOO CDN''' (use [[JavaScriptBuildSystem|these instructions]] to add the scripts properly)
 * '''ANY DISPLAY OF TEXT INPUT MUST BE TESTED FOR PROPER ESCAPING.'''  If this isn't tested, we don't know that we're safe against HTML injections.
 * '''NEVER DO YUI().use IN LAUNCHPAD TEMPLATES.''' Always use LPS and do LPS.use in Launchpad templates.
    * LPS is an instance of YUI that has been initialized in the base template and passed YUI config parameters.  Using LPS ensures we use a YUI object consistently. You should never need a YUI().use block before LPS has been initialized; if you think you do, you '''must''' restructure your code.
 * '''DON'T PERFORM JS LOGIC IN TEMPLATES''' Always include a module of JS that is testable and performs the logic. At most, the template file should be running a run() or init() method.

== Building ==

To build the javascript run:

{{{
make jsbuild
}}}

More info: [[JavaScriptBuildSystem | JavaScript Build System]]


== Plain Old JavaScript ==

 * JavaScript blocks should be indented by 4 spaces.
 * If 3 or more lines of strings are to be joined together, use the `[str1, str2, ...].join("")` form.
 * 1 and 2 character variable names are to be avoided.
 * Regular expression literals should be avoided.
 * All public or protected functions, methods, and object properties should be described with [[http://developer.yahoo.com/yui/yuidoc|JSDoc comment strings]] and [[http://developer.yahoo.com/yui/yuidoc#tags|tags]].
   * Module-level private variables don't need full JSDoc tags.
 * C++ style comments ("`//`") should only be used inside function bodies.
 * Per-line `var` statements are preferred inside function bodies.
 * As `var`s are [[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/var#var_hoisting|hoisted]], declare `var`s at the top of function or global scope.
 * Function, method, and variable names should follow the [[http://www.python.org/dev/peps/pep-0008/|PEP 0008]] style guide (and no, this is not how the rest of the world does it).
 * Any string literal used more than once in a file should be assigned to a CONSTANT (a variable, really) so that the minifier can compile it to a short name.
 * functions should always return `null` not `undefined`.
 * To make conditionals explicit but readable, use the [[http://developer.yahoo.com/yui/3/api/Lang.html|Y.Lang]].isXXX methods: Y.Lang.isArray, Y.Lang.isObject, Y.Lang.isValue, and others.


== XHTML, CSS, and Styling ==

 * CSS files should come before JavaScript files in the document head.
 * CSS `url()` directives should have the file name surrounded by single-quotes: `url('spinner.png')`
 * XHTML attributes should be surrounded by double-quotes: `<input type="text"/>`
 * Setting a node's style explicitly with `setStyle()` should be avoided.
 * To set a node's style, it is preferable to add a CSS class to the node, and let the stylesheet handle the presentation. [2]
 * Please use the [[http://validator.w3.org|W3C markup validation service]] to check what you write, and to check what the DOM looks like after AJAX inserts new elements.


== Event Handling ==

 * "`e`" should be used as the event object variable name in event handlers:  `function onclick(e) {...}`
 * `event.halt()` should be preferred over `event.stopPropagation()`, `event.preventDefault()`, or `e.returnValue = false`
 * Document-wide `keypress` handlers are dangerous, and should be detached as soon as they are no longer needed.
 * Key event handling ''sucks''.  The best cross-browser reference we've found to date is at http://unixpapa.com/js/key.html and for an example in our code, see `lib/lp/registry/templates/product-new.pt`.

=== Javascript Module formatting ===

 * Modules should have a JSDoc string with the `@module` or `@submodule` tag.
 * Module-body content does not have to be indented.
 * Module-wide constants should go at the top of the file, after the module JSDoc string.
 * Multi-line `var` statements may be used in the module-wide constants block.
 * Script tags included for local development should be wrapped in <tal:devmode condition="devmode"></tal:devmode> tags so that un-minified scripts are only loaded for dev usage.

=== Naming Javascript Modules and Namespaces ===
 * The file path, the module name, and the namespace should correspond to each other.
   * For example:
      * File path: lib/lp/'''code/javascript/subscription'''.js
      * Module name: Y.add(''' "lp.code.subscription" ''', ...
      * Namespace: var module = Y.namespace(''' "lp.code.subscription" ''');
 * A module's public functions should be attached to the namespace variable and '''not''' be attached directly to the `Y.namespace_name` variable.
   * Do this: {{{
var module = Y.namespace("lp.code.subscription");
module.my_public_function = function() {};
var my_private_function = function() {};
}}}
   * Don't do this, since Y.namespace() will create a new Y.lp object and Y.lp.calendar object if they don't already exist. {{{
Y.lp.calendar = Y.namespace("lp.code.subscription");
}}}
   * Don't do this, since it will be easier to change the module name and namespace, if you only have to do it in one place in the module. {{{
var module = Y.namespace("lp.code.subscription");
Y.lp.calendar.my_public_function = function() {}; // Should use `module` variable here.
}}}
 * It will then be possible to use module functions like this: {{{
Y.use("lp.code.subscription", function() {
    Y.lp.code.subscription.my_public_function();
});
}}}

== Widgets and Plugins ==

 * Protected and private method names should be prefixed with an underscore.
 * All protected methods should have the `@protected` JSDoc tag, even ones like `initializer()` and `renderUI()`.
 * Event handler method names should be prefixed with `_on` or `_after`.
 * Custom events should be documented on the line immediately preceding the line where they are published.
 * All JSDoc strings for attributes should have `@attribute`, `@type`, and `@default` tags.
 * Objects such as `Node` instances should not be instantiated in the widget's `ATTRS` hash using `value:`, use `valueFn:` instead.
 * All Widgets should have a functional example page.
 * ''(Not implemented)'' Widget example pages should follow the Example page Template.

== Tests ==

 * The unit test suite should leave a clean DOM tree in between each test case.

== Footnotes ==

[1] ''It looks like literal regexps confuse it'', but OTOH, we recommend against using them.
<<BR>>'''Wrong''', XmlLint rightly complains there is an unescaped less-than (<) in a document. Use `!= 0` or `!= 1` if you must embed script, but the right thing to do is to extract the script to a separate file.

{{{
== XmlLint notices ==

lib/lp/registry/templates/product-new.pt
    110: parser error : StartTag: invalid element name
    if (steps.search(/hidesearch/) < 0) {
    ^
}}}

[2] For example, if you want to toggle an element's visibility using POJS, use our `unseen` class:

{{{
    // Make this element invisible.
    element1.addClass('unseen');
    // Make this element visible again.
    element2.removeClass('unseen');
}}}

----
CategoryJavaScript CategoryJavaScript