Hi Marc and Rouslan,
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().
I used to call this checkIndexName() function within my "*$("#table_index tbody tr td.edit_index.ajax").live('click')*" event in tbl_structure.js file after getting the form loaded in the dialog. Then it runs at as the "onload" in non ajax scenario. Is it okay to handle that or should I rewrite the function for ajax.
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
Fixed and I think now it works.
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
Have to work with these two issues.
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 replaced the "bind" with "live" and now it seems okay. Please check that.
I updated the repo with those changes and will work with the rest tomorrow. Thank you.
Regards, Thilanka.