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

[Rouslan Placella]

On Mon, 2011-07-04 at 20:46 +0530, Thilanka Kaushalya wrote:
> Hi Rouslan,
> 
> On Mon, Jul 4, 2011 at 8:04 PM, Rouslan Placella <rouslan at placella.com>wrote:
> 
> > Hi Thilanka (and everyone else),
> >
> > You filed bug #3350790 on the tracker, which I looked into and fixed
> > earlier. This also gave me a chance to look at the AJAXification of the
> > index editor, which as I can see from your blog you have deemed
> > completed. Anyway, I found some issues in your implementation and
> > thought you might want to know about them.
> >
> > First, the JS errors from bug #3350790 are gone, but the functionality
> > from checkIndexName() function in indexes.js only works in the QA_3_4
> > branch. This is due to the AJAXification that you did. That function was
> > written in plain JS, when there was no jQuery or AJAX. Line 44 in that
> > file should have been a dead giveaway that the function was about to
> > break (onload = checkIndexName;), since a browser does not fire an
> > onload event for AJAX requests. Anyway, I think that the
> > checkIndexName() function should be rewritten in jQuery and then called
> > inside the AJAX request from tbl_structure.js. Also there is some inline
> > JS in tbl_indexes.php, which you may want to rewrite in jQuery if you
> > will be rewriting checkIndexName().
> >
> > Now for the fun stuff...
> >
> >
> > Try this:
> > * go to `sakila` db
> > * go to `actor` table
> > * go to structure tab
> > * click on "Edit" link for the primary key
> > * in the ajax dialog click 'Cancel'
> > * click again on "Edit" link for the primary key
> > * in the dialog click on "Go" (to add more rows to the table)
> > Expected result:
> > * 1 row is added to the table of indices
> > Actual result:
> > * The table of indices is cloned and 1 row is added to the first table
> >
> >
> > Another thing is that the AJAX dialog should really be modal (you can
> > make the dialog modal by adding "modal: true," to the dialog options in
> > jQuery).
> > Try this:
> > * go to `sakila` db
> > * go to `actor` table
> > * go to structure tab
> > * click on "Edit" link for the primary key
> > * without closing the AJAX dialog, move it out of the way
> > * click on "Edit" link for the `idx_actor_last_name` index
> > * inside this new dialog click "Save"
> > Expected result:
> > * the dialog for `idx_actor_last_name` is submitted
> > Actual result:
> > * the dialog for the primary key is submitted
> >
> >
> > Another bug:
> > * go to `sakila` db
> > * go to `actor` table
> > * go to structure tab
> > * click on "Edit" link for the primary key
> > * set all rows in the table to "--ignore--"
> > * click "Save"
> > Expected result:
> > * An error message
> > Actual result:
> > * An empty AJAX message
> >
> >
> > Another thing is that the new function that you wrote to handle the
> > SPATIAL indices doesn't actually work. The problem is that its event is
> > attached using jQuery's 'bind' method, which is unsuitable for your
> > purposes. I think that you should use the 'live' method that allows for
> > late binding. After you do that, you will notice that the function still
> > doesn't work as expected. This is because in this function you assume
> > that you will always have one instance of each element that you
> > reference by ID. However once the DOM has registered an element with a
> > particular ID (open a dialog), even if you destroy that element and
> > recreate it (close dialog/ open new dialog), the DOM will still
> > reference the original, and now non-existent, element. The best way to
> > deal with this is to use classes, then search for objects inside some
> > common parent element.
> >
> > I guess that this is quite a mouthful of an email, hope it helps.
> >
> 
> This is very interesting and thanks for reporting. I'll quickly go through
> these facts and update you.

OK, great.

BTW, before I forget, personally I'm not a big fan of the "'Save' Or Add
to index 'X' column(s)" part in the fieldset footer. I think that the
"Save" button should go into the bottom part of the dialog, near the
"Cancel" button.

Rouslan