Le 2011-07-08 00:02, Thilanka Kaushalya a écrit :
Hi Rouslan,
I checked out your branch and it looks good to me (for whatever it's
worth). The only few things that I noticed that might need improvement are:
- `form_name` in checkIndexName() is misleading as a variable name,
since it will contain the ID of an element (maybe form_id would be better).
- in checkIndexName() you defined:
[code] var the_idx_name = $("#input_index_name"); [/code] but then you use several times $("#input_index_name") instead of the cached `the_idx_name` variable (which BTW should be `$the_idx_name` because it's a jQuery object and not an arbitrary JS variable).
- You copied an pasted the PHPDOC header from the old version of the
checkIndexName() function, which did not take any arguments, so you are missing `@param` for the function input.
- [code] the_idx_name.attr("disabled") [/code] will never be undefined
(it's a jQuery object now). Those two times that you are comparing it against 'undefined' are pointless, so you can just remove those 'if' statements.
- missing semicolon after 'false' in this code snippet from
functions.js: [code] if ($("#"+form_name).length == 0) { return false } [/code]
I fixed those bugs and pushed to the repo.
Thilanka, looking at your repo with latest commit 526c72652fefca7025614552f5f5ad80d912462c, there is still the problem of variables which are jQuery objects and are not prefixed with $, in the checkIndexName() function.
Otherwise, the code works good and is almost ready for a merge.