Drag and drop enable book outline
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | book.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
The book outline (content/book/x) has now been updated for drag and drop also. It's glorious.
This patch is dependent on http://drupal.org/node/181126, which makes a few minor tweaks to the drag and drop javascript. It'll work without it, but the other patch will correct a few odd behaviors.
The only significant change required in book.module was not listing the book itself in the outline view. Exposing the book item itself as the first item in the list would erroneously let the user drag an item outside of the book by moving it all the way to the left. I don't think this is at all undesirable though. Listing the book itself in the outline is similar to having the title of a (real paper) book as Chapter 1 in the table of contents then everything underneath it as a sub-chapter. From an outline sense, it's not logical to include the book itself as the top item.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| book_drag_and_drop.patch | 7.86 KB | Ignored | None | None |
#1
Again this looks great. I agree with the changes to the book title listing, didn't miss it at all.
Same browsers, same issues as #25 on the block drag and drop, so CNW unfortunately but these must be pretty minor.
#2
I'm going to need help on this issue for sure. If pwolanin or chx could look over the code that is saving menu items in book.module I would greatly appreciate it.
Here's the problem. Drag and drop is operating fine, but book.module is not setup to handle mass-reparenting of items. Frequently when arranging a small book, the parenting behavior will work fine on the page, but when I save the changes, several of the items disappear from the book.
Here's a video of this nasty behavior http://quicksketch.org/sites/quicksketch.org/files/book_eating_items.mov
To help reproduce you can download this D6 dump: http://quicksketch.org/sites/quicksketch.org/files/drupalhead.sql
user: admin
pass: admin
Follow the same steps as in the video and you'll reproduce. Here's an updated patch that is the code I'm currently using.
#3
Ahhh. I found the cause for the weirdness. I explain it more in-depth at the menu issue, but I'll post the solution here also.
Basically, we can't save children in the hierarchy before their parents. Because we had been saving them in the order set in the $form array, often times children were being saved before their parents. To fix this I'm now saving the elements according to the new order on the page, by checking the order passed in by the POST array. Note I'm not actually using any post values, just checking the order in which they come in.
<?php// Save elements in the same order as defined in post rather than the form.
// This ensures parents are updated before their children, preventing orphans.
$order = array_flip(array_keys($form['#post']['table']));
$form['table'] = array_merge($order, $form['table']);
?>
After applying #34 from the menu patch, this patch works great!
One more change of significance: we no longer do a node_load/save for each item moved in the tree. This is only done if giving an item a new title. This speeds things up quite a bit when rearranging a large number of items all at once.
#4
This patch adds a little more documentation and references the very similar menu_overview_form_submit() function. The parent item field is now hidden again.
#5
subscribing to remind myself to review
#6
quicksketch just keeps on sketching. subscribing!
#7
I don't get any indentation in FF2 or IE7. Everything else smooth and snappy but CNW.
#8
Garland:
Indentation looks fine up to level Four - then the text box shifts to the line below the drag icon.
Bluemarine:
Arrow is indented properly, but text box is shifted to the next line for all levels.
(FF2/WinXP)
#9
It also appears that it's allowing me to attempt to move a page to depth 10 (> than Max_depth). The error that should be appearing appears to be suppressed.
I also somehow managed to lose the titles and depths of a bunch of items...
While doing further testing - it appears that the indentation is now working under Firefox on bluemarine after making some changes...
#10
What do we think about removing the text box entirely for titles? The width of the text box causes wrapping when it's not necessary when the title is short, and for really long titles cuts off the title. Just a thought.
Fortunately we're not going to have to deal with indentation not showing up any more now that the menu patch has gone in (which this patch was dependent on). Without the new theme_indentation() function, no indents would show up. Now that it's in reviewers won't be having this problem any more.
#11
Removing the textfield for titles seems like a good idea. Apologies for #7, was in a hurry and forgot the menu patch. I now get the indentation, but I'm losing it on save (using the tarball from mid-day though and have the same issue with menus - is there another dependency?).
#12
The ability to change titles seems like a key feature of this admin interface - I think it would be a regression to remove this. Is it just a visual problem? Is there some way with the JS to make the the title appear normal and then switch to presenting the textbox when clicked?
#13
Applying the patch in #4 works fine with the same layout breaking problems in Garland as were mentioned above. Only the first two levels are shown as intended at 800px and 4 at 1080px width of window. I'd hate to see the ability edit titles inline go but the drag and drop functionality seems to be more important. One possible solution might be to apply less indentation to differentiate levels. The suggestion by pwolanin for inline editing would be ideal but not worth it if it would hold up other things. I remember that inline editing was one of the items on Dries' agenda for D7, though. It would be great to see it across the board but right now the Book Outline manager is the only place it exists so removing it until a more systemic solution can be found would probably not be such a great loss.
#14
Well after tinkering a bit I think I'd agree that saving the ability to rename titles is desirable, and if it's easy enough to keep then we shouldn't take it out. This patch takes a very simple approach and sets the table width to 56em, preventing any wrapping at up to 7 parents (the maximum amount).
It also makes the maximum depth a setting passed into tabledrag.js. The menu outline is set to 8 parents max, which books get set to 7, since the book itself is a parent that's not shown on the page.
#15
Flawless with IE7/FF2/Safari3/IE6 on XP. Also tried w/o javascript on a couple of browsers and it degrades cleanly. Should be RTBC but could do with a review on another platform.
#16
This works perfect on MAC OS X in all possible browsers. Great work!
This is definatly RTBC...
One question though, Nate why do we still display the "Weight" columns in the JS-versions?
#17
Indeed, why do we display the weight? (This is understandable with the input format dragger for example, as there is some information in the default weights). Why is it displayed here?
#18
The weight field *shouldn't* be visible, that's strange. I double checked with a brand new Drupal install and checked in Opera 9, Safari 3, and Firefox 2 on Mac. I don't have my Windows hard drives available to check those, but with no JS changes that affect those areas, there shouldn't be browser differences.
#19
Not sure wha went wrong, but I also did a new install of drupal and applied the patch. The weight column isn't visible..
No code remarks at all...
Let's get this in, Goba/Dries?
#20
Committed to CVS HEAD. Thanks a bunch guys! :)
#21
Woot! Taxonomy stands alone :)
http://drupal.org/node/193333
#22
#23
I'm still finding that the 'Operations' columns slide under the right sidebar with any horizontal resolution under 1160px. Removing the 100% width from the id 'book-outline' fixes this but reintroduces the problem with wrapping. Am I doing something wrong?
#24
Automatically closed -- issue fixed for two weeks with no activity.
#25
The drag and drop layout of the book outline breaks Gardland in default install of RC2, again. I thought the patch in #14 fixed this but it doesn't seem to have made it into RC2. In my view, this is close to a critical error, since the layout is broken 'out of the box' with the default theme.
#26
Please open a new issue for your problem. This issue is closed.