[Phpmyadmin-devel] AJAXify phpMyAdmin Interface - GSOC 2011 - Table->Structure ->Column->Change

[Marc Delisle]

Le 2011-07-19 14:10, Rouslan Placella a écrit :
> On Tue, 2011-07-19 at 23:02 +0530, Thilanka Kaushalya wrote:
>> Hi Marc,
>>
>> Thilanka,
>>> yes, try that.
>>>
>>> P.S. I suggest renaming initToolTips() to a better name like
>>> PMA_convertFootnotesToTooltips().
>>>
>>>
>> I did the necessary changed and committed to the repo. please check that.
>> Thank you.
>
> Looks good to me, for whatever that's worth. The only things that I
> noticed that you might want to have a look at are:
>
>   * Since PMA_convertFootnotesToTooltips() now takes a parameter,
>     you might want to add a PHPDOC header to it to explain what
>     that parameter is for and that it can be omitted.
>   * The 'span_id' variable in PMA_convertFootnotesToTooltips() is
>     now misleading, since it stores a class, not an id.

Thilanka,

1. please follow Rouslan's suggestions; however, the variable, after the 
split, really contains a span id (to find the other footnote span)

2. please add a comment just before the line containing the split, 
explaining that this variable contains two classes at this point (for 
example "footnotemarker footnote_1_1")

3. also add a comment, using for example footnote_2_3 and that after the 
split, the variable contains 2, so will find the footnote #2

I find it a bit hazardous to use this split on a string which contains 
two classes. You never know, another class having an underscore could be 
added to this element in the future.

Another thing I find unclear: in PMA_showHint() and 
PMA_convertFootnotesToTooltips()  you have kept the principle of using 
the instance of the footnote, even though the goal of the instance was 
only to have unique ids. IMO you should get rid of the instance.

-- 
Marc Delisle
http://infomarc.info