Drag and drop of table rows (e.g. block admin)

Nice, it changes everything when you don't actually pull the row out of the table. I'd always found this sort of things to be very difficult when you pull the table row out of the table.

It might not be too late for Drag and Drop in D6. After the helpful example was posted by fajerstarter, the ability to sort our weights via drag and drop became closer than I'd ever thought. I set down to implement this feature for D6, here's the first go. It's rough but I want to get feedback and thoughts about AHAH vs. Drag and Drop.

Here's a video sampling the new behavior: http://quicksketch.org/node/746

I mention some problems in the video, which I'll repeat here:
- Do we want the selects for both weight and region? Should we hide weights entirely (or both)?
- Should we show all the regions available all the time, then let users drag into them?
- Asterisk stars... should we add them to changed content?

Right now the code for adding d-n-d to the page looks like this:

  foreach ($block_regions as $region => $title) {
    // Add drag and drop to the table with the ID 'blocks'.
    //drupal_add_tabledrag($table_id, $group, $action, $relationship, $target_classname); // Optional $source_classname at end.
    drupal_add_tabledrag('blocks', 'block-region', 'match', 'sibling', 'block-region-'. $region);
    drupal_add_tabledrag('blocks', 'block-weight', 'order', 'sibling', 'block-weight-'. $region);
  }

With special classes added to each select box, such as 'block-region-left'. The drag and drop is attached to rows containing these special elements. When a table row is dropped, the fields inside that row compare themselves to the fields around them. Depending on the action ('match' or 'order'), they will change their values or the values of fields in their same group.

Right now the only relationship is 'sibling', but the goal is to have this work for menus also. The declaration for menus would look like this:

  //drupal_add_tabledrag($table_id, $group, $action, $relationship, $target_classname, $source_classname);
  drupal_add_tabledrag('menu', 'menu-pid', 'match', 'parent', 'input.pid', 'input.mid');

Please try out the patch and give me as much feedback as possible.

After applying the patch, the part of the table that takes care of the right region had vanished! Using Safari 3.03 on Mac OS X.

I think drag 'n drop makes the AHAH functionality (where you select the weight/region and it then moves the block to the right section) pretty much obsolete. Obsolete might not be the right word though. It's just that having both drag and drop *and* select and update doesn't make much sense. IMHO it has to be either, but not both.

The asterisks most definitely have to go if you go for drag and drop: you've just moved an item, so you *know* which one that was. It doesn't make sense to highlight it.
And I think that we should indeed leave out both the region and weight selects.

I've attached a UI suggestion: all disabled blocks on the left, all regions on the right, no more region/weights at all, and only configure links when the block has been put in a certain region.

I like the drag and drop icon thing. But I don't like that it's always being displayed, it distracts too much. I think it should be displayed just in front of the row (to the left), and only when you hover over a row. That's quite simple to do in CSS.

I agree that, for a generalized solution, only a limited part of the row should be selectable for the drag and drop. Otherwise you may accidentally drag and drop, or not be able to use any of the form elements inside the row (as it currently is in Safari 3), although that may be fixable.

fajerstarter, if you'd like a reminder to save the form, then that can be done in a more elegant way (drupal_set_message()). If you're heavily rearranging your blocks, it's a bit silly IMO to see the asterisk move around all the time.

I agree with Wim Leers that it doesn't really make sense to have both weights *and* drag and drop, and we can simplify the interface by removing the weight drop-downs when JS is enabled.

However, we need the asterisks. Otherwise the second I move *another* row, I forget what I've changed on this screen. Maybe remove it on mouse-down and don't add it again until mouse-up?

And we can't auto-save, because that would make the change live immediately. So if you're just playing around with how blocks ought to look, your users see the login block on one side of the page one page load, the other side of the page another page load, and then in the footer the next one. :P

Oh, also +1 to the idea of those little drag/drop arrow thingies (assuming that's a standard UI widget), but only on hover and mousedown.

I like the screenshot by fajerstarter in #4 best. I agree that we should go with either DnD or with AHAH -- mixing both doesn't seem to make much sense.

If you can make this work for menus, I'm going to kiss your bare feet. ;-)

I came to the bottom of this thread to comment... but the man beat me to it. This would be so awesome on menu admin.

The drag icon is #4 might be improved by omitting the horizontal arrow in cases like this where only vertical drag is allowed. Looking good ...

I like the drag and drop icon in #4 as it is a convention for many systems. I think that it is too large however. Attached is screenshot from Basecamp as they use a small version of this icon. If this patch makes D6 (fingers crossed, it is a usability fix) I see the sizing as setting president for other areas of the interface that may get "iconified".

Thanks for the fantastic feedback. Here's another video you can review and post new thoughts, the code is posted also.

http://quicksketch.org/node/747

What's changed:
- There are now 2 javascript files involved. Because of the desire to maintain the select list for region, there needs to be something that enables the region list to move the current row. The new file is only 50 lines though, and leverages tabledrag.js for re-striping, changing the weights, etc.

- I've added a permanent 'handle' to the row. Making the entire thing draggable broke any form elements in the row for IE and Safari. The handle icon is always visible to indicate you need to grab that explicitly to move the row. Making it appear on hover of the row made it seem like the entire row could be dragged, when this is not the case.

- The handle is the 4 way directional arrow, simply because we'll want the same icon used on the menu and book pages, where the horizontal axis will actually make a difference when it comes to parenting.

- The asterisk is back for all changed rows. It's themable using the new javascript theme functions. Should we put the message at the top of the page initially that they need to save their changes?

- The sort order on blocks has changed. Has anyone ever noticed that the regions are displayed alphabetically in the table, but the select list arranges them according to the region list in the .info file (previously template.php)? After discovering the inconsistencies, I changed the sort function to use the same order as the .info file. This prevents regions like 'footer' coming before regions like 'header' in the table.

The javascript is getting more jQuerified, but it's still a bit sloppy in places. I'll continue to refine the code quality as we improve the UI.

In case you're curious about the significantly larger patch size, this version removes all the AHAH-related PHP code from the blocks page, as well as some crufty CSS that's no longer necessary.

Interface suggestion....

1) Remove the drop-downs as with the drag and drop they're not necessary and slightly confusing (weight AND region)...
2) Get the blocks page to pre-render ALL region areas (even if initially empty) so it can be purely drag and drop...

Does that sounds sensible?

I agree with both of Nicholas Thompson's comments in #15.

Also I assume the old behaviour is maintained if no js?

Looks really, really, nice.

In regards to catch's comment... I'd leave the elements ON the page (part of the row?) but have their display as "none". Non JS clients simply display:block and the old method is resumed.

Re: #15: I thought about removing all select fields, in fact it's only a 2 line difference to remove (hide) both columns and prerender all the regions. However, I found the plethora of empty regions to be a little daunting at first (shot attached). It also makes it painful to enable a block with a large list of disabled blocks. Say you have 50-100 disabled blocks, you'd have to drag a long ways to get it into the Left Sidebar. On the other hand, drag-only would be the least confusing during usage.

Re #17: No elements are ever removed from the page, the table cells are set to display: none similar to as you suggest. Disabled javascript reverts to the old behavior yes.

I'm keeping at code needs review to keep people looking at it, though clearly it's still needed work from the original patch.

It also makes it painful to enable a block with a large list of disabled blocks. Say you have 50-100 disabled blocks, you'd have to drag a long ways to get it into the Left Sidebar.

This is as good a reason as any to keep it (looks at his long disabled blocks list)

In that case though, how about only showing the select in the disable blocks list? I dunno if it'd be straightforward but it could be the best of both worlds.

Thanks fajerstarter for the image and better margins. They do indeed increase the grippable area quite nicely. I've expanded it a bit further in this patch, to ensure the handle takes up the entire row.

Changes in this version:
- Table header cells are now entirely removed by javascript to fix the layout issues in Safari. Colspans are now automatically adjusted also, so the colspan doesn't accidentally overshoot the table.
- Major code cleanup and jQuerification. Formatting is much closer to other javascript files in Drupal core.
- Fixed browser compatibility issues. Now tested and working in Safari 2 (Mac), Firefox 1.5/2.0 (Win, Mac, Ubuntu), IE 6/7.

IE6 has an bit of an issue updating the table rows. The screen can flicker pretty badly, but the functionality is just as good as other browsers.

Attached is the draggable.png file I've been using. It's dimensions are the same as throbber.gif for consistency, since it seems that is the other image used as a background in two positions.

I forgot to test Opera in the previous post. Opera 9 works fine on Ubuntu, Mac, and Windows.

Tested in Safari 3 (Mac) and works perfectly.

I think we should add a message at the top of the table as soon as a first change has been made that the user needs to save his change(s)?

Yeah, we could do that and make it part of the tablesort.js. Since it's the one adding the asterisk.

Thanks again. Here's a complete re-roll with 1-3 of fajerstarter's changes above. Leaving the highlight row behind is a little confusing, but maybe as this develops further it'll turn out to be better than my initial experience. I doubt many users will be switching around as many blocks as we're doing during testing.

This patch begins to incorporate parent relationships in addition to the sibling relationship used on the blocks page. This patch will continue to evolve as the menu patch progresses.

Other changes in this version:
- Much faster table striping and row updating. I'll work on speeding things up further as the architecture becomes clear.
- All the form elements that receive sorting behavior now require 2 classes to operate, a group class (such as block-weight) and a class for the subset of related fields (block-weight-[region name]). The group class relates the fields to one another, it also allows some of the new optimization. I was doing this before as good practice, but now it's a requirement.

Oh, I forgot to mention the menu patch: http://drupal.org/node/181126

I'm posting patches there also for menu.module only, which enable the drag and drop on the menu overview page.

Marking CNW per fajerstarter's review.

After having webchick do a quick review of the patch, she suggested again that all regions should be visible, even if they have no blocks in them. The drop-down select is still maintained though.

I'm not sure one way or the other which would be best (aesthetic versus functional). Could we have a look at:
http://quicksketch.org/sites/quicksketch.org/files/FirefoxScreenSnapz003...

And tell me if this method is better than hiding regions when they're empty?

That's much better. Having the empty regions visible means you can easily add blocks to them.

Yeah it's nice to have the empty regions there.

Yes, it is nice.

But I see something weird in the video. The block title jumps sometime. The title height of empty block gets higher when you drop something in it, and changes back when you release the mouse button.
It can be seen clearly when you dropped "navigation" item form "right sidebar" into "content" block.

Much better indeed!

Awesome, thanks guys. I think it was a 10 to 0 vote among people I asked to keep the regions there at all times. I'm not sure what's going on with the sizing of table rows. I think it's just Firefox trying to keep up with all the switching classes. This is one time when Safari and Opera do a significantly better job of updating the page.

I've got the menu tree sorting 90% done and fully functional and we should see a patch later today. I'm working on making the page scroll for you when you get to the top or bottom.

- Tree relationships
Here's the new version which now has complete support for treed parent relationships. Functionally, it is 100% complete for use on the menu page, so yes you can now sort the entire menu structure via drag and drop!

- Regions always visible
This patch makes changes on the blocks page also. All the regions are now always visible so you can drag things into them as you please. The region select list is still there to make enabling a buried block easier.

- The highlighting of the dropped row.
The '.highlight' class is used after a row has been changed, the '.drag' class is used while the row is being dragged.

- Auto scroll on top and bottom of page.

- Flicker "fix" for IE6.
In IE6 you can't do hardly anything to select lists without causing rampant flickering on the screen (it's literally everything flickering). To work around it, select lists are hidden when the drag is started and shown again when done. This is only for IE6.

I've tested this patch in IE6/7, Safari 2 (Mac), Opera 9 (Mac), Firefox 2 (Mac/Win). So far, so good on compatibility!

Tree image (it makes sense when you see it in use).

To summarize the files need to be placed in /misc, here's a list:

- draggable.png from #26
- tree.png from #39
- tree-bottom.png from #40

And if all this sounds like a lot of work, here's a video of the menu page to prove that this is definitely worth it.
http://quicksketch.org/sites/quicksketch.org/files/FirefoxScreenSnapz004...

Bump to start it for core consideration.

I can confirm fajerstarter's report of auto-scroll not working at the top of the viewport (Safari 3). Other than that, works great.

Nice work, Nathan!

auto-scroll works for me, for both top and bottom (Opera 9).

I find this JS more comfortable to use than the AHAH solution (which I considered a usability nightname, because I'm using an old computer and waiting twice for a server reply (region+weight) wasn't fun at all).

(Tiny ones: (1) jQuery already ensures there's an event.pageX/Y; (2) thanks to jQuery event = event || window.evet too isn't necessary; (3) getPosition()'s parameter should be named "elem", not "event"; (4) jQuery 1.2 actually provides an offset() method but your getPosition() is faster.)

It looks great, but I'd like to see us add some more PHPdoc. For example, the different CSS classes (i.e. 'highlight') need to be documented better, IMO. Let's document this properly so people will actually be able to use and understand this. :)

For example, the different CSS classes (i.e. 'highlight') need to be documented better,

BTW, 'highlight' is mostly a presentational name. Maybe a more semantic name could be found --and this could make its documentation unnecessary.

Marking to needs work for fajerstarter's review and Dries phpdoc request.

Thanks for all the great feedback. I love fajerstarter's idea of making the handles tabbable links. Adding key handlers doesn't *seem* like it'd be difficult.

The scrolling up was working before. I must've made a change right before rolling the patch that broke it.

Thanks for the javascript fixes fajerstarter and moofie, I'll roll those in and repost. So it sounds like using the .drag class is fine for the lighter yellow when dragging, but people are questioning .highlight. I'll try to find an existing class that might work or come up with a more suitable name.

Testing #38-40.

Blocks page Firefox/Linux: All functionality seems to work for me, as near as I can tell, except that the drag icon doesn't appear. The mouse pointer switches to a hand and I can drag successfully, but the icon itself is blank.

Blocks page in Konqueror 3.5.8: The weight column is hidden, and the move-on-select functionality works, but there is no grippie, even a hidden one. That means it's impossible to reorder blocks. :-(

Navigation page in Firefox/Linux and Konqueror: There is no change. (Not sure if there's supposed to be; I thought I saw a separate issue for that.)

As with the teaser splitter, I am OK if we have to just disable this feature in Konqueror if we can't get it to work, as KHTML/WebKit browsers are a PITA to write Javascript for. As of right now, though, the net effect is that Konqueror can't change weights. :-)

Oh yes, Nathan, please add me to the list of people who will love you forever if you can get this in. :-)

Oh Konqueror... we'll see what we can do about you. :D

Crell, did you rename the draggable_1.png to draggable.png? I should have made it clear that this needs to be done. Otherwise it's probably just a CSS issue that needs to be worked out.

Ah, no I hadn't. Trying again and renaming the file this time, and it works fine on the blocks page in Firefox. Still no dragging love in Konqueror, though. :-(

Wow, thanks again for the suggestions. This is rocking so much I can hardly contain myself. This patch addresses almost all concerns, with some sweet new keyboard support for accessibility. You can now organize the entire menu or blocks page by tabbing to the handle and using the arrow keys. I've tested it working in Opera 9 (Mac/Win/Ubuntu, though you have to use Q and A keys to rotate between links, not tab), Safari 2 (Mac), IE 6 (though it's painfully slow) and IE7, Firefox 1.5/2.0 (Mac/Win/Ubuntu).

I certainly *tried* to get Konqueror working, but even with debugging and javascript errors turned on, Konqueror just fails silently on the menu page, not a hint at what might be wrong. On the blocks page it nicely tells me that my (correct) javascript in block.js is incorrect. Bah.

Concerns addressed:
#42

1. The auto-scroll only works for the bottom of the screen in my tests.
2. Add cursor:move to the new .drag class to make it more likely that the cursor stays like a move cursor when dragging (could be a Windows only thing?).
3. Add title attribute to the drag handler (see my mockup in #4).

1. Auto-scroll is fixed, I had a backwards greater-than sign changed at the last minute before my last patch.
2. I added a 'drag' class to the body of the document when dragging begins. In the CSS body.drag has the hand cursor.
3. I added the title to the drag handle. Thanks for the help.

#44

(Tiny ones: (1) jQuery already ensures there's an event.pageX/Y; (2) thanks to jQuery event = event || window.evet too isn't necessary; (3) getPosition()'s parameter should be named "elem", not "event"; (4) jQuery 1.2 actually provides an offset() method but your getPosition() is faster.)

1/2. Thanks I took out the event || window.event. I removed it in other places but this one slipped through.
3. Thanks, that's definitely right.
4. I haven't experimented with $.offset, but if it's even a tiny bit slower we should avoid it since getPosition is called many times every mouse movement during a drag.

#45

I was thinking of keyboard support: Wrapping the tab-handler in an a-tag would allow tabbing. Arrow up/down/left/right could be used for moving a row.

Thanks again fajerstarter for this awesome suggestion! I think you'll be pleased with the implementation.

Because of the abstraction needed to work with either keyboard or mouse, this version is actually MUCH cleaner. It grew a bit in size, but a large part of that is the breaking into new functions and objects allowed me to insert bigger blocks of documentation without interrupting the flow when reading the code. With every new patch I can feel this code getting substantially better.

I still haven't figured out the .highlight class issue. Should we just call it 'drag-previous' and be done with it?

Well I found the basic reason why Konqueror doesn't work (for nearly anything it seems). Take your copy of Konqueror over to http://jquery.com/test/. Konqueror makes it down to #16 of 155, if you're lucky. The second time I tried it failed on #11.

If Konqueror can't adequately run jQuery, I don't see how we can possibly support it. Fortunately I found a fix that makes it so Konqueror will fail silently which I'll roll into the next patch.

Konqueror can't handle jQueries with this syntax:

$('div.class-im-looking-for', dom_object_to_look_in);

Which is a serious problem, which will need to be fixed in Konqueror (preferably) or in jQuery. I'm not sure there's much we can do here other than make it degrade nicely, which it does adequately after we prevent the current error messages from popping up.

fajerstarter, thanks I had that return false in keydown originally but I can't remember why I took it out. It may have been causing problems in Safari. Speaking of, you might need to turn on highlight links in the Safari preferences, under the Advanced tab. If that option isn't there, option-tabbing is the default on Macs, you might try alt-tab and see if that works.

On #2 above. Moving the code to keyup sounds like a fantastic idea. Our goal: to use every event handler in existence on one link. :D

Ouch. That sounds like a rather serious problem. :-(

I believe any $(selector, object) syntax can be rewritten as object.find(selector). So it sounds like we have to either only use the latter or check for Konqueror and disable the dragging entirely to avoid rendering the UI unusable. Blargh. jQuery, AFAIK, doesn't directly support Konqueror, but there are bugs filed against it in their bug tracker. I don't see this one there, so it may be a good idea to file given how, er, critical that syntax is.

More concerns addressed:

(1) IE needs return false on the keydown event as well (it's a shame IE doens't support keypress event on a link element).
(2) I would move most of the table update code to the keyup event to speed up things when the user holds down the key.
(3) Also, it's now possible to move a row above the first region header.
(4) Lastly, it seems like block.js was not updated to the new structure.

1. I have the keydown event returning false now if an arrow key was pressed. Seems that Safari will also cancel window scrolling from the keydown event, so I removed it's special keyCodes from keypress.
2. I move the fieldUpdate to keyup, though only a few browsers will be able to leverage this it seems. I had luck holding down the down arrow in Safari (Mac), IE, and Firefox (Win). Other browsers I tested didn't continue moving down while the key was held. I know the cause for this is the loss (and re-addition) of focus on the link. The only way to correct would be to move the trees around the dragged rows, rather than moving the group of rows in the tree. For the negative performance impact and minimal gains, I'm planning on leaving it as is.
3. Now we're calling isInvalidSwap() on linear lists also to keep this from happening.
4. It should be updated now to properly add the asterisk and changed rows.

I've changed the name of 'highlight' to 'drag-previous'. Now there shouldn't be much question about where the class should be used. This version should also fail "silently" without throwing any javascript errors in Konqueror. Konqueror will fall back to the degraded version of both pages.

This version fixes the two minor issues fajerstarter reported in #59. I've tested it quite a bit in every browser with javascript compression turned on and off. In preparation for core, I created or extended a significant amount of function documentation, including 100 lines of PHPdoc for the drupal_add_tabledrag() function. The documentation borders a bit on the menu patch, as it refers to it as an example.

I consider this version of the patch "Release candidate 1". So please test the crap out of it and let's get it in!

About to review - anyone else doing the same thing, don't forget the images in #40 and to rename draggable_1.png to draggable.png

Fresh tarball, included the images in /misc. I'm on XP here, with firefox 2.0.0.x, IE7, IE6, Opera and Safari 3 beta.

Firefox: Wow. Dragging and dropping is ultra smooth. The cursor change works great, and trying to drag on the edges of blocks always selected the right one (and the highlighting on click means if you mis-aimed you'd notice quite easily). Definitely nice to have the regions visible even if empty. Scrolling is ultra fast, no conflict with tableheader.js. I also tried keystrokes, found it very, very fast to use with this as well. Tried it with javascript disabled and it degrades gracefully to the old interface, only problem with js disabled was I missed the drag and drop ;)

IE7. Drag and drop works fine, a touch slower than firefox when dropping blocks into new positions but very much ok. keystrokes also worked great. Again the targeting of rows is very accurate and no unexpected behaviour (although you have to be a tiny bit more patient dragging to empty regions). IE7 probably the slowest of all the browsers, but I imagine people who use it regularly are accustomed to that and it's not a normal use case comparing browsers side by side like this.

IE6: Still fast, and worked much better than I expected - i.e. no issues I could see. The select boxes disappear when dragging, which is a minor visual querk but no big deal (and they reappear when you mouse up so still useable when they need to be). Key strokes again fine.

Opera: again, dragging is smooth and fast. scrolling presents no issues. I couldn't tab through the links but that's probably because I never use opera and don't know how.

Safari: see opera, no appreciable difference.

Massive, massive +1 to this. It's very nice indeed.

Beautiful.

A few usability suggestions:

Add a mouse-over message to the red asterisk: "This block has changed. Use 'Save blocks' to apply (confirm?) the changes."
and/or
An explanation of the red asterisk just above the Save blocks button: "One or more blocks have changed. Use 'Save blocks' to apply the changes."

I'm on Firefox 2.0.0.8, Ubuntu, Garland. It is difficult to move a block to an empty region. Aiming at the Right sidebar I often end up in the Content region below. That's probably because a region TD has a white 18px top border, so you can't see where one region ends and the next one starts.
How to solve this? I'dd say either enlarge an empty region with whitespace, or add an empty row to an empty region (this row could contain the word "empty".)

Add a mouse-over message to the red asterisk: "This block has changed. Use 'Save blocks' to apply (confirm?) the changes."
and/or An explanation of the red asterisk just above the Save blocks button: "One or more blocks have changed. Use 'Save blocks' to apply the changes."

I'll add a title attribute to the asterisk which will give it a hover (in Firefox). The message sounds like a good idea, though I hesitate to include large amounts of markup in the javascript (even if they are themable now). We'll try to find a compromise.

It is difficult to move a block to an empty region. Aiming at the Right sidebar I often end up in the Content region below. That's probably because a region TD has a white 18px top border, so you can't see where one region ends and the next one starts.

Your suggestion on fixing this turns out to work quite well. Since this problem is caused by the Garland CSS, it's not hard to counter balance this in Garland's style.css:

tr.region-empty td {
  border-bottom: 1.5em solid #fff;
}

This seems to make the empty regions much easier to hit, because they're taking up more space than before. If this doesn't cause any problems, I'll roll it into the next version.

Thank you quicksketch.

The CSS fix doesn't seem to work for me, though (Firefox 2 and Opera 9.24 on Ubuntu). Adding the border-bottom just decreases the TD height with the same amount of pixels, resulting in exactly the same lay-out.

(border-top + padding-top + height + padding-bottom + border-bottom was 18 + 3 + 22 + 3 + 1; after the fix it is 18 + 3 + 5 + 3 + 18)

After applying the CSS change the empty rows seemed noticeably easier for me to hit. Like you say, the layout appears exactly the same visually, but the height of the empty region is actually increased:

18 + 3 + 22 + 3 + 1; after the fix it is 18 + 3 + 5 + 3 + 18

That is, the bottom border is extended. I'll double check to see if these changes make the difference I experienced on all browsers.

Major kudos quicksketch, you've hit all the right usability concerns. Sorry for coming late into this issue, will test and report back. I hope we can get this in D6.

Sorry I meant to say above:
I'll double check to see if these changes make the difference I experienced [in Firefox 2 on a Mac, actually makes a difference] on all browsers.

I didn't mean that I experienced it making it better in all browsers, that's what I need to check.

I haven't tested the latest version yet, but one thing has me worried here. We're modifying Garland in order to make the drop targets a convenient size. Do I understand correctly that *every theme would then need to specifically have rules in order to make the drop target work?* Remember, the block page is used by every theme, regardless of whether it's an admin theme or not. Any custom CSS needed to keep this page from breaking needs to be in core, not the responsibility of the theme. If it's the responsibility of the theme, we will render most themes incapable of supporting the blocks page unless the themer jumps through hoops. That's not a good thing.

If my concerns aren't warranted, please explain why. :-)

That's a valid concern, but I don't think it'll be necessary in other themes. The problems gaele reported are largely caused by Garland's CSS itself, so other themes won't need to have a similar fix. As far as I know, not many themes do the work of theming things in the admin area like the blocks page. But Garland being the default goes the extra mile.

The problem is table rows get swapped when you're about halfway over the next row. Since Garland adds a signficant white border to region headers, the rows get swapped sooner than a user might expect when moving a block downward. So we'll probably need to compensate for that somehow when the regions are empty and the target area for dropping a block is fairly small. I attached a screenshot with the explanation (and potential fix).

Ah, OK. Spiffy. We may still want to document it somewhere for other admin-safe themes, just as a caveat. "If you're doing X, be careful about the blocks admin page for this reason."

This patch adds the missing warning message to the bottom of the table. I've found putting it at the bottom of the table will be the most reusable solution, as some forms will likely use a table in the middle of the form, rather than being the only thing on the page. The asterisk convention we're using also naturally means "see below for a message".

Second, this patch attempts to solve empty regions being difficult to drop a block in. I used the approach depicted in #70 of making the empty message an entire row. In the end, I didn't make any CSS changes to Garland to compensate. The additional table row helps significantly enough that I don't think we need to worry about it.

Sorry, #72 isn't patching for me. I keep getting an error that a reverse patch is detected, even after I wipe and re-update my repository. :-( It looks like it's just affecting teaser.js for some reason.

Despite the error I was still able to apply and test it, using Garland only. In Firefox/Linux, everything seems to work correctly, including drag and drop to empty regions. The only problem I see is that it's not clear that the row will be moving or where it's moving to until it jumps into place. Could there be some sort of indicator for that? A little line between rows for where the dragged row would land, perhaps? Or would that complicate the code too much?

In Konqueror, it is improving! I was able to drag and drop rows, but only within a region. Dragging to an empty region did not move the row, but the row never "undragged" and I was unable to move anything after that. The dragged row, and anything else I tried to drag, just highlights and doesn't change.

I'm curious why the "not yet saved" message is now at the bottom of the page instead of the top, as before?

Thanks Crell, looks like the patch had become out of date. Here's a re-roll up-to-date with HEAD.

The only problem I see is that it's not clear that the row will be moving or where it's moving to until it jumps into place. Could there be some sort of indicator for that? A little line between rows for where the dragged row would land, perhaps? Or would that complicate the code too much?

The rows are already being updated as soon as the row is in a valid location. I assume that the bigger problem here is the menu page, rather than the blocks page, where there are places where you're not allowed to drag. Putting a line in-between table rows, while possible, might give users the wrong impression that they can actually drop the row somewhere, where nothing will actually occur. Open to thoughts on this though.

In Konqueror, it is improving! I was able to drag and drop rows, but only within a region. Dragging to an empty region did not move the row, but the row never "undragged" and I was unable to move anything after that. The dragged row, and anything else I tried to drag, just highlights and doesn't change.

Really, that's great news! My Konqueror doesn't work at all, I'll try to get an updated version and see if it's possible to get it working completely.

I'm curious why the "not yet saved" message is now at the bottom of the page instead of the top, as before?

The drag-and-drop table is now a much more reusable solution, rather than a special case for the blocks page. Placing it below makes more sense if the table were used in the middle of a larger form (which is likely in places like poll.module and CCK). Plus the asterisk notation traditionally means there's a message below, rather than above.

Good news Crell! After upgrading my Ubuntu to 7.10 (I was using 6.06 previously) and installing KDE, Konqueror did indeed seem much more stable than before (though it still fails the jQuery unit tests). The Konqueror version is still 3.5.8, so I'm wondering why it's behaving so much better. Konqueror surprisingly only had one issue preventing it from working immediately with dragtable.js. Konqueror gives the focus (little dotted outline) to the drag handle even though it should not. The drag and drop worked fine as long as you clicked off the handle after every drag to safely remove the focus before selecting another row.

This patch includes a 3 line fix, documented as a fix for Konqueror right below the fix for IE6.

I'm a little discouraged by the lack of reviews on this patch (and the short deadline). I think the patch is RTBC, but I'd like to have others give it another thorough review. Please test!

A preemptive patch fix before webchick posts her review. When doing a clean install, all the blocks start out in the left sidebar when you visit admin/build/block. This fixes the changed line in _block_rehash() that loads the regions for the first time, properly putting them in Disabled if there's no region specified.

So #78 didn't seem to correct the problem when starting from a completely fresh database. Here's another one that sets the blocks default to the more logical disabled, rather than left sidebar.

Yep, I can confirm that #79 does indeed fix the errors I was experiencing.

I tested this with all three browsers I have access to currently: Firefox 2.0.0.9, Opera 9.24, and Safari 2.0.4 on Mac OS 10.4, and all are working properly. I know for a fact that quicksketch tested in a million other ones (even Konqueror ;)), so I'm not too worried about compatibility.

I also threw in a couple curve balls like enabling the Throttle module, and confirmed that the locations of the checkboxes are remembered. On visual inspection, the patch is well-documented and conforms to coding standards. It's hard to find much to nit-pick. :)

The one thing that I find slightly odd is the location of the message, although this was covered in #72. Nate and I discussed some alternatives briefly in IRC, and this does seem the better option.

Therefore, I'm marking this RTBC.

ahem. ;)

I found a few bugs dealing with keyboard support that'll need to have a little extra testing before release. This version corrects a javascript error when using the keyboard at the top and bottom of the table.

Please review and test exhaustively!

Needs review, not work :)

2 hunks failed in block.module. :-( Looks like it just needs a quick reroll.

Forgot to mention - this is a re-roll but it doesn't work

@quicksketch - help!!!

Here's a new patch that thanks to the block.module changes, has actually gotten shorter! This patch also reuses the dropRow() method for both the keyboard support and mouse support, since they are so similar in this regard. One more additional change: when using the keyboard fields are now updated on blur, rather than mouseup. This added a significant speed boost to Firefox and Opera in my testing.

Though I don't think there have been any changes to the mouse support in the last few iterations of the patch, this version seems very smooth using both mouse and keyboard in Opera, Firefox, and Safari. Windows users: could we get a test?

Steef had a few concerns on IRC that are addressed here:

- The default message text is too "techie". Because of the nice theming options javascript allows us, I've over-ridden the message on the blocks page to use this text instead: "The changes to these blocks will not be saved until the Save blocks button is clicked.". The previous message of "Changes made in this table will not be saved until the form is submitted." will still be used in other places where dragtable.js is used.

- The font-size should not be reduced on the empty region text "Currently no blocks in this region.". Rather, they are wrapped in an EM tag.

This needs some cor committers attention, so please Dries or Goba review this patch please.

Things are going pretty smooth, and a usability boost like this is going to give us really needs some comments from your hand..

Let's try to get this in, this is sitting much to long in the issue queue..

wow this is amazing.

i tested the patch on OSX and Windows using all the major browsers I can think of, and it works pretty good and smooth.

Now, let's get a core committer on this and commit the patch...

Tested Successfully in all cases for me:

Windows: FF 2.0, IE 6, Opera 9.24
Ubuntu: FF 2.0, Epiphany (gnome web browser 2.18.1)

Typo: drupal_add_tableDrag() and drupal_add_dragtable() used in code comments.

Fixed the camelCase occurrence in the comments to be lowercase.

Opera has problem when holding down key up to move a row up-wards. The page scrolls without the row moving. Down-wards works better.

This seemed to happen when moving a row up from one region to another one. There was a race condition where tabledrag.js was swapping the row up but blocks.js was swapping the row down. After fixing this, the keyboard speed in IE when moving a block upwards sped up quite a bit also.

Is the use of   really needed in tabledrag.js? Removing it fixes a CSS issue in IE6. Is it for validation you are using it?

Unfortunately Opera will not give focus to an emtpy anchor tag, even if it contains another empty tag. Without it Opera doesn't seem to work. It's also valid HTML as you say.

I'm still curious to know how snappy the dragging is on Safari Mac, compared to e.g. Firefox Mac. To me the speed is the biggest usability concern here. I noticed that IE7 slows down considerably when dragging from one region to another. Still the speed is ok (and Opera and Safari for Windows which are the slowest are still quite rare browsers), and not a showstopper for me.

Speed between platforms seems pretty consistent in Firefox (Mac is about equivalent to Windows). In my testing, Safari on Mac is the fastest one of all surprisingly. It's significantly smoother feeling than Firefox Mac. Between platforms it's hard for me to say whether IE6 or Safari is faster, but I'd probably give the edge to Safari here also.

Code style wise I personally prefer prepending jQuery objects with "$", e.g $field, $handle etc. But I don't think it's a coding standard (yet).

I agree here, but I don't really want to re-roll this patch again. I made a pitch for doing $variables when we created ahah.js, but it was shot down there. Maybe after we get this in we can do a series of patches to update all the .js files in core.

I'm going to bump back up to RTBC. Gabor or Dries, we could really use the review.

The patch no longer applies and needs a quick reroll.

It's a lot of new code but it is properly documented. Thanks for that.

My only concern is that it really implements two different paths, and that sometimes, we could try to merge both better. Specifically, there is a lot of JS-script overriding going on which might not be strictly necessary.

Example: "Changes made in this table will not be saved until the form is submitted." is overwritten to "The changes to these blocks will not be saved until the Save blocks button is clicked.". I don't see why the latter can't be used in both scenario's.

Nothing major though. Let's re-roll for starters. :)

Reroll to keep with HEAD.

Committed to CVS HEAD. Incredible job! Respect.

Hooray!!!!!! Way to GO, Nate!!! :D

Thank you all. Nice job!

ROCK! Thanks so much Dries, fajerstarter, Stefan, catch, Wim... everyone! I feel like I just won a grammy :D

Let's shift focus to the menu sorting patch: http://drupal.org/node/181126, which currently needs a reroll but is still quite exciting.

Congrats Nate :) Drupal's UI is starting to kick ass now!

First of all, excellent work!

I have one small question: while trying to port this over to Drupal 5 (I have no idea whether this will work), I noticed these comments lines at the beginning of the patch

$form['my_elements'][$delta]['weight']['attributes']['class'] = "my-elements-weight";
...
$form['my_elements'][$region][$delta]['weight']['attributes']['class'] = "my-elements-weight my-elements-weight-". $region;

Is it possible that ['attributes'] should be ['#attributes'] instead?

Man, this is so cool.

Wow. This is really amazing.

UPDATE: I tried to reply to an early comment, and since this can't be deleted, then it'll have to stay. But I was just pointing to Tao Starbow hierarchical Drag & Drop menu sorting with his Quick Admin Menu module.

I did a 50-second demo video of the basic tree sorting here: http://blip.tv/file/get/Kentbye_tech-QuickAdminMenuDemo680.mov -- (included as part of this BADCamp Usability thread.)

UPDATE: Just cross posted this info into this relevant thread: http://drupal.org/node/181126
This is certainly something for end users to get excited about.

This is fantastic stuff. Really enjoyed the dojo presentation too.

Is this the right place to suggest usability enhancements? Maybe these have already been considered?

• While dragging an item, it's not that easy to see which one you have hold of - the very pale yellow doesn't show up well on my LCD. Suggest this should be darker (or a stronger colour) than the yellow used at the end of a drap operation to show which item you changed
• How to cancel a drag operation? ESC or dragging outside the window would be useful ways of doing this
• Also it would be useful if, when you grab an item, the original row in the table stayed put but greyed out, so you can "put it back" there if you change your mind or realise you grabbed the wrong item.
• Another way of showing what's going on while you are dragging would be to have the entire item drag with the cursor (instead of just the "drag" icon) and the new position would be shown by a dark line moving up and down the table at the insert point. I've seen this approach in other situations and it was pretty intuitive

Just a few thoughts aimed at making this already great usability enhancement even better. :)

@gpk, I'd recommend creating a separate issue to discuss your enhancement ideas and linking to it from this thread. Generally, we try and leave it one issue per bug fix/enhancement and the enhancement from this issue is already done and in core. :)

Even better would be if you could mock-up some of your proposed changes in HTML, so you could for example pick colours that look good, etc. But if that's not possible, a simple feature request will do.

Thanks webchick, I've put the first 2 in a new issue here http://drupal.org/node/201876.

Sorry for commenting in a close issue, but I'd really like to get drag and drop functionality for at least the block admin interface in Drupal 5. I love this feature and I would use D6 if there was an e-commerce solution for it. How did the backport go toemaz??? Any progress?? I'll be willing to help out with this port, I'd love to see how far you got.

I am sure that new features for Drupal 5 will not be accepted..
And don't worry, there will be an e-commerce solution for D6.

HI, i really need to order table rows of views in Drupal 5.10, if any patches could work with the 5.10 version??

then you really need to upgrade to Drupal 6.
new features will not be added to stable versions.

The other way would be to implment the JS tabledrag functionality yourself in 5.x via a custom module. http://drupal.org/project/draggableviews appears to do something a bit like this, but only for a 6.x (so Drupal's tabledrag js is already available). However it might just be simpler to do the upgrade as per #114.

Hi, well, I'm going to use Drupal 6.x version, I have no choices ^^