[Phpmyadmin-devel] About the AJAXification of the index editor

[Marc Delisle]

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.


-- 
Marc Delisle
http://infomarc.info