Would be great if users could sort the nodes using drag and drop. JQuery could be used to this.
I'm not fully entranced by the core drag & drop for tables in D6; I don't like the small drag handles nor do I like that the dragged item doesn't really follow the mouse. This is in part because it simply may not be possible -- table rows aren't super flexible. Most drag & drop is done with lists, but nodequeue is a table and it's pretty hard to do that elegantly.
For a project we're doing at the moment, I created a module to let users rearrange nodes in nodequeues using the same drag-and-drop interface which they know from the taxonomy admin pages.
Would you be interested in a patch to nodequeue.module with this functionality?
I have attached the .module and .info files for people who want to try this out. Once the module is installed, go to nodequeue_dragdrop/<qid>/<sqid, optional> to try it out.
I think this general feature should be considered an improvement, since it standardizes the interface and can reduce the size of the nodequeue codebase.
Also, it might make it easier to incorporate the views integration built for weight module (http://drupal.org/node/314248#comment-1068126) which could potentially allow queues to be resorted from inside a View. This would make nodequeue-based photo galleries more of a possibility, and could be an improvement in the photo handling space, imo.
A Drupal 5.x compatible version would be great.
When doing the drag & drop sort in Views, I found it difficult to integrate the 'add' button as well as the types of buttons that do shuffles and the like. The drag & drop code isn't very friendly to external modification of the data.
FWIW, I think this would help a lot. Standardized UI is a good thing.
Having not yet looked into it in detail could this be implemented while maintaining the "move to front/back" of queue buttons? I realize that drag + drop is cooler, but a single click seems like less work for than a drag all the way to a bottom/top of a list. It would be nice to have both types of interaction.
I've tested this briefly and I think it's a great addition to the module for the reasons mentioned above and because it seems like a general usability improvement. I would like to see us move towards using this as the default interface. I'm thinking of submitting a poll to see what module users prefer, or at least making announcement that work is being done towards integrating drag + drop.
The submit validation should probably go in #337834: Prevent bad data from being saved to nodequeue_nodes
Also, when trying to remove a node I get "warning: array_shift() [function.array-shift]: The argument should be an array in /Users/ezra/Developer/htdocs/dev/contrib/nq6/sites/all/modules/nodequeue/nodequeue.module on line 1760."
This interface does not delete all values for a subqueue when submitting the form -- I'd like all values to be deleted instead of just updating existing values since this could prevent invalid values from being saved.
I'm working on #337834, so that this interface can call a standard queue order submit function to submit the queue order.
Assigning to me and merging #337834 into this. This could also lead us to having multiple queues on one screen and dragging a node between them.
I would love to see drag and drop. Mostly because users will start expecting it since they see it at other parts of D6.x.
This module is great.
Since you're asking for feedback on this...
I hate to disagree with merlinofchaos on anything, but I have to take the other side on this one ;) I feel it would be helpful to standardize the interface as much as possible.
While that wasn't appropriate for an interface like the Views2 cockpit (which successfully allows tons of options in a very small amount of screen "real estate"), I think Nodequeue doesn't require such a customized solution. When possible, I personally feel there would be more continuity for drupal users if the row ordering matched the D6 core menu and taxonomy ajax draggable interface.
* That said, if improvements can be made to the ajax draggable rows (such as the size of the handle merlinofchaos mentioned), these could added as patches to core (for D7), so the interface would still remain consistent between core and other important contributed modules that make use of the same interface.
As a lowly n00b to contributing to Drupal projects I'd have to concur with scottrigby, standardizing on the built-in functionality of D6 is the most logical step forward, then work with core to improve the core drag'n'drop system for D7 as deemed necessary. While being able to manage them from a View2 would be pretty cool, I think that should be a v3 step, after the initial work is completed and stabilized.
As for backporting to D5, the whole purpose is to use the D6 APIs so those would have to be backported first. I think the effort would be better spent in collaboratively pushing other modules to D6 compatibility first, rather than shoe-horning yet another enhancement onto D5. That's obviously purely my opinion, but with so much of the "core" 3rd party modules now at a stable D6 release, or at the very least beta or RC, all effort should be put towards pushing on D6, including our sites =-)
Great ideas. I like this module. It solves so much for my users. I also think the proposed patch is a great addition. Thank you for your efforts and time. Bump!
This is in progress but testing and feedback are welcomed. Marking as CNR for that reason. wulff's work provided a good starting point to build off of.
@merlinofchaos: I'd like to implement this in a way that you feel good about. I preserved the current "remove" icon. Perhaps making the drag handles larger would address your concerns about their being too small. At their current core size, they _are_ larger than the nodequeue ordering buttons.
2) Restriping behavior works
- the classic Nodequeue remove icon and behavior
- callbacks for old queue manipulation form for future use by module developers
- shuffle works only on submit, meaning it saves the new order before the user can decide if she likes it.
- reverse doesn't work and should also use JS
I think the majority of people expect drag and drop in this module and are surprised when it's not there. If you think it's really a problem (beyond the coding changes) then perhaps allow people to toggle from one to the other. I suspect, however, that drag and drop is likely to be the proffered system for moving items in a queue.
I know my clients are asking for it.
I'm marking this as needs work since technically it's not finished.
Subscribing. I was surprised the rows weren't draggable for reordering.
Patch in #17 works beautifully. Here is a patch that integrates #17 with latest 2-dev branch.
Thanks for rerolling, but see my feedback in #17 under "what's left."
The attached patch adds JS reverse and shuffle support.
It still needs some polishing, but I thought I might as well share the code I have at this point.
@ezra-g would you be interested in a patch the splits the admin functions into a separate .admin.inc file? The current nodequeue.module is getting a little unwieldy.
Awesome - Thanks ! Yes, I would be interested in an admin.inc Marking as needs review.
Updated patch attached.
I'm using earlier version of this patch and there's one important feature that is missing.
While dragging its very easy to lose track of the actual order count. there should be some kind of visual aid or list the actual row numbering when you move them around.
Are you thinking of a simple indication of the current node positions as in the attached screen shots?
The behavior i was after is closer to the initial one. whether the queue is in order or is reveresd you would get row count indication from 1 to n or from n to 1. The purpose is to simplify management of longer queues when the order is important (ie. i want some node foo to be 24th in the queue).
even when its "shuffled" the row count in ui would still display 1...n
@wulff The queue position as indicated in your screenshot would be a great addition.
This re-roll is a major step forward. Thanks, wulff!
Here's a minor re-roll as a single patch. I was getting an invalid syntax sql error, which I fixed. The problem was that in nodequeue_save_subqueue_order, the INSERT part of the query was getting added twice. I also added the text, "New subqueue order not saved" to the error messages so it's clear that the subqueue order has not been changed.
@wulff, you mention in the patch that perhaps we should move some of the validation in nodequeue_save_subqueue_order to a validation function. The reason I have this code there is to prevent bad data from getting saved to the nodequeue table when passed not from a form, but from a programmatic call to this function. I think the best solution would be to replicate this validation in the form so that when dealing with forms, the validation happens in the expected place.
However, this validation doesn't really protect against invalid values in the database. Here's some weird behavior and the steps to reproduce. This was done on a clean install.
Start out with 5 nodes in a basic subqueue :
mysql> select * from nodequeue_nodes order by position asc;
| qid | sqid | nid | position | timestamp |
| 1 | 1 | 6 | 1 | 1242837499 |
| 1 | 1 | 5 | 2 | 1242837500 |
| 1 | 1 | 3 | 3 | 1242837502 |
| 1 | 1 | 2 | 4 | 1242837504 |
| 1 | 1 | 1 | 5 | 1242837506 |
Remove node in position 4, with or without js enabled.
New display order in the form (as shown with js disabled):
1, 2, 3, 1 (last one is duplicate #1)
values in database:
| qid | sqid | nid | position | timestamp |
| 1 | 1 | 6 | 1 | 1242837557 |
| 1 | 1 | 5 | 2 | 1242837557 |
| 1 | 1 | 3 | 3 | 1242837557 |
| 1 | 1 | 3 | 3 | 1242837557 |
| 1 | 1 | 1 | 5 | 1242837557 |
Duplicate nid 3 in in position 3.
We should prevent these duplicate values from being saved.
To review, the remaining to-do items as I see it:
- Prevent duplicate or otherwise invalid values from being saved to the database
- Show the node's position as in wulff's screenshots (@wulff, did you code that already?)
- Add form validation
I'll try and work through the remaining todos early next week. This weekend is DrupalCamp Copenhagen, so I probably won't have time to do any work.
I have the node position stuff in a patch somewhere, I'll be sure to add that.
wulff: any luck trying out the patch?
I have updated the patch for the latest version of nodequeue.module (1.84).
I have added a node position column at the far right of the nodequeue table. I have called this column 'Pos', but would like suggestions for the name (does it even need one?).
I'm working on form validation and duplicate/invalid values in the db this week.
The problem with duplicate/invalid values has been solved. It was caused by an array that didn't get reset in nodequeue_save_subqueue_order().
I'll upload a patch once I've added the form validation stuff and done some general cleanup.
We still have the "Undefined offset: 5" error, but I was wondering if we could get rid of that by always linking to 'admin/content/nodequeue/<qid>/view/<sqid>' instead of 'admin/content/nodequeue/<qid>'?
I've attached the latest version of my patch. There are still a couple of kinks to iron out, most notably related to node positions after one or more nodes have been removed from a queue.
this would be excellent, subscribing.
I've attached the latest version of my patch (against the current HEAD version).
This version fixes the problem with node positions after removing nodes from a queue.
Wow -- This looks very good to me. In my testing both with and without JS, I wasn't able to break it! Thanks for this great work. Functionally we seem to be there. Now, we just need a bit of cleanup.
About the "FIXME"s
On line 1795 : +// FIXME: WULFF: re-add this to the nodequeue form
Is there more work that needs to be done there?
Regarding the other FIXME's, yes, we can remove nodequeue_arrange_subqueue_entry, though we should also remove the code that calls that function. The old remove node menu callbacks can be removed as well.
I noticed you made some coding style changes to various parts of the module. Some of them improve coding standards compliance, and some of them, such as making all the keys and values of an array line up, seem to violate the coding standards (though they do make things easier to read, IMO).
In any case, they bring the patch length to ~2,700 lines, slightly longer than the entire module itself. Since this feature requires such a significant change to the code, I'd prefer to commit/review it separately so that we have a clear record of the changes for this feature.
I realize you've done a ton of work on this issue so let me know if that seems possible. I could look at some of the fixme's.
I have to agree that the patch is getting a little unwieldy. I have removed all the cosmetic changes which brings it down to a more managable 1537 lines.
This updated patch gets rid of the remaining FIXMEs and cleans up the textfield below the drag'n'drop table a bit.
The "re-add this to the nodequeue form" FIXME was related to an AJAXy way of adding nodes to the drag'n'drop table. I don't think it's necessary, so I've removed it for the time being to focus on the drag and drop functionality. We can think about adding it if we feel saucy at a later time.
I'll save the code style changes for another patch. I was thinking of incorporating them in the 'split admin pages to a separate .inc'-patch I've been talking about. That patch will mostly be moving stuff around, so cosmetic changes should be easier to incorporate there.
Thanks for the re-roll :).
Here's another one with a few changes:
- Hides the node author, post date fields when a node is restricted from a user.
- Changes the '#' column header to 'Position'
- Changes 'Date' header to 'Post Date'
Since the "Add" button is the only one that saves the subqueue immediately, I changed the text to "Add node and save subqueue".
This seems basically RTBC, but I'm throwing it out for one last review.
I'll turn to #512086: Can't add nodes using autocomplete textbox, titles display as restricted when manipulating queue for own nodes and once this patch has landed.
Very nicely done, Ezra!
Get this in so we can open new issues that don't depend on this patch!
Thanks wulff for all of the awesome work! Thanks Rob Loach for the multi-step review!
This is committed.
Automatically closed -- issue fixed for 2 weeks with no activity.
Drupal is a registered trademark of Dries Buytaert.