AHAH enable book module
quicksketch - October 5, 2007 - 17:13
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | book.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Book module has some obvious places where AHAH can now be added (similar to the blocks page).
- On the node edit form, the Book outline fieldset uses javascript from book.js. This should be able to be removed and use ahah.js instead.
- The book structuring form (admin/content/book/x) can be updated dynamically similar to the blocks page.
#1
I've already implemented the AHAH API in book module where javascript was already (the admin page isn't done).
#2
#3
Sorry, but there is absolutely no way in the world this is critical. :)
#4
@webchick - we're duplicating code we don't need to, look how much code this patch removes! :)
but anyway, here's a stab at the admin page. It doesn't work yet
#5
dmitrig01 is rocking this patch so fast I don't dare try to re-roll. Here's a helpful bit of CSS that will eliminate the 'jump' you get when new content appears on the page. It occurs because the top and bottom margins need to be adjusted after they are fully placed on the page, but we can remove these margins and the effect is much smoother with very little visible difference:
.book-outline-form .form-item {margin-top: 0;
margin-bottom: 0;
}
It's looking great! The code the for the node page works amazing... and we can really eliminate the 1 remaining line of code in book.js by a drupal_add_js() inline. One more .js file down! Woot!
#6
and the re-roll..
#7
Btw, this patch is another one that will not work quite right in Safari until the AHAH bug fixes go in from http://drupal.org/node/181741.
I also like it a lot more with the throbber version, which is another effect of that patch. Neato.
Some more code comments, this declaration can be shortened because of the defaults set for #ahah:
<?php'#ahah' => array(
'path' => 'book/js',
'wrapper' => 'edit-book-plid-wrapper',
'method' => 'replace',
'effect' => 'slide',
'event' => 'change',
)
?>
to just
<?php'#ahah' => array(
'path' => 'book/js',
'wrapper' => 'edit-book-plid-wrapper',
'effect' => 'slide',
)
?>
#8
re-rolled for quicksketch's suggestions
#9
Nice! Seems to work on Macintosh with FF > 2, Opera > 9, Safari > 2..
Let's get some guys to test this on Windows and set this RTBC..
Remember that this patch relies on http://drupal.org/node/181741, so first apply that patch to the trunk.
#10
Here's an update that gets us a little further along. It:
- Hides the button on the node edit form (seems that $(element).hide() didn't work in the header like that).
- Adds classes and asterisks to changed rows, similar to the blocks page.
- Updates the form cache and renders the updated version properly.
What needs to be done:
- The actual sorting of the weights via some PHP function. Due to the data structures for book module, this isn't a trivial task. All the current sorting is done via SQL query, we need to find a way to generate a new in-order list from the new POST values we're given.
#11
Here's a final patch that adds proper sorting of form elements. It bumps up the patch a bit in size, since book module doesn't actually have any code for sorting the book tree structure other than an ORDER BY clause in SQL. This adds the sort function and utilizes it on the AHAH callback after updating the tree with new weight values.
#12
I applied this on fresh dev tarball, tested on firefox and IE7 on XP.
In both browsers, when changing the weight of a node at admin/content/book/n, I get the spinner icon, but it just keeps spinning with no update.
Everything else I tried was very nice though!
#13
@catch - you need to rebuild the menu to get this to work
I've extensively tested this and it passed with flying colors!
#14
dmitrig01 is right, oops. Works great, and the little asterixes are loveky.
RTBC!
#15
Tried the patch, and everything seemed to be right. So looked over the code and all seemed fine, so committed. Thanks.
#16
#17
I just spent 12 hours trying to understand how book.js works. I just couldn't get it.
I was getting more and more convinced that there was a bug somewhere.
Then I found this issue. The patch that was applied calls for the suppression of book.js. However the patch was not fully applied and book.js is still part of core, and part of a normal D6 install.
To test, simply delete book.js, make sure you don't have cached .js, and the ahah call still works.
#18
Oh, this was overlooked for sure. Sorry for the confusion, and for your wasted hours. The patch does include a hunk to remove the book.js but I did not properly commit this. Now done. Setting back status to as it was before.
#19
Thanks.
Happy New Year, Gábor.