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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dmitrig01’s picture

FileSize
5.63 KB

I've already implemented the AHAH API in book module where javascript was already (the admin page isn't done).

dmitrig01’s picture

Assigned: Unassigned » dmitrig01
Category: feature » task
Priority: Normal » Critical
Status: Active » Needs review
webchick’s picture

Priority: Critical » Normal

Sorry, but there is absolutely no way in the world this is critical. :)

dmitrig01’s picture

FileSize
9.41 KB

@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

quicksketch’s picture

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!

dmitrig01’s picture

FileSize
9.41 KB

and the re-roll..

quicksketch’s picture

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:

    '#ahah' => array(
      'path' => 'book/js',
      'wrapper' => 'edit-book-plid-wrapper',
      'method' => 'replace',
      'effect' => 'slide',
      'event' => 'change', 
    )

to just

    '#ahah' => array(
      'path' => 'book/js',
      'wrapper' => 'edit-book-plid-wrapper',
      'effect' => 'slide',
    )
dmitrig01’s picture

FileSize
9.35 KB

re-rolled for quicksketch's suggestions

Stefan Nagtegaal’s picture

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.

quicksketch’s picture

FileSize
11.46 KB

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.

quicksketch’s picture

FileSize
12.38 KB

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.

catch’s picture

Status: Needs review » Needs work

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!

dmitrig01’s picture

Status: Needs work » Reviewed & tested by the community

@catch - you need to rebuild the menu to get this to work

I've extensively tested this and it passed with flying colors!

catch’s picture

dmitrig01 is right, oops. Works great, and the little asterixes are loveky.

RTBC!

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Tried the patch, and everything seemed to be right. So looked over the code and all seemed fine, so committed. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)
beginner’s picture

Assigned: dmitrig01 » Unassigned
Category: task » bug
Priority: Normal » Critical
Status: Closed (fixed) » Active

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.

Gábor Hojtsy’s picture

Category: bug » task
Priority: Critical » Normal
Status: Active » Closed (fixed)

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.

beginner’s picture

Thanks.
Happy New Year, Gábor.