This patch will change the taxonomy term and vocabulary listing pages into forms. These forms will allow you to reorder taxonomy terms or vocabularies by drag and drop. This could be used as a springboard for adding an inline form for adding terms or vocabs.

This issue ties in to these two issues:
Usability: Improve add vocabulary form: http://drupal.org/node/192242
UI simplification for Vocabulary addition: http://drupal.org/node/192696

Despite how appealing and useful drag and drop could potentially be on single and flat hierarchies of a relatively small size, there are many problems to consider when dealing with multiple hierarchies or free-tagging. For these cases, I don't think drag and drop is appropriate and would probably avoid any implementation for them.

I'd greatly appreciate a review of this functionality and suggestions for where drag and drop is appropriate for taxonomy (if at all). Please watch this video and tell me what you think.

http://quicksketch.org/sites/quicksketch.org/files/taxonomy-dnd.mov

For those too hurried to watch a movie, here are the two main problems:
- Free-tagging vocabs can (and probably) contain 100s or 1000s of terms. Moving one item in the list would cause the entire vocabulary to become sorted 0 to x. Causing a loss of alphabetical order and a very long processing time.
- Drag and drop does not make sense in a mulitple-heirarchy vocabulary, because an item can exist several places in the tree, but the user can only drag one item at a time. Updating all the instances of a term on drop would likely be more confusing than helpful.

CommentFileSizeAuthor
#33 forum_alpha.patch590 bytescatch
#31 Screenshot_1.png27.8 KBgábor hojtsy
#31 Screenshot_2.png65.43 KBgábor hojtsy
#29 taxonomy_drag_and_drop2.patch2.51 KBquicksketch
#24 taxonomy_drag_and_drop.patch56.01 KBquicksketch
#23 taxonomy_drag_and_drop_6.patch51.36 KBrobloach
#22 taxonomy_drag_and_drop_5.patch48.82 KBrobloach
#17 taxonomy_drag_and_drop.patch52.29 KBquicksketch
#16 forums-reset.png14.02 KBcatch
#13 taxonomy_drag_and_drop.patch52.29 KBquicksketch
#10 taxonomy_drag_and_drop.patch52.61 KBquicksketch
#8 taxonomy_drag_and_drop.patch34.93 KBquicksketch
#6 taxonomy_drag_and_drop.patch46.72 KBquicksketch
#3 taxonomy_drag_and_drop.patch34.63 KBquicksketch

Comments

bonobo’s picture

bonobo commented

We have some code that does this already in 5.x -- feel free to take what you need, or to disregard entirely :)

http://drupal.org/project/nodeorder

Cheers,

Bill

catch’s picture

catch
he/him
Primary language English
commented

subscribing.

quicksketch’s picture

quicksketch
he/him
commented
Priority: Normal » Critical
Status: Active » Needs review
StatusFileSize
new taxonomy_drag_and_drop.patch34.63 KB

Here's the patch for taxonomy. The changes that went into cleaning up the vocabulary add form have some lingering cleanup that are addressed in this patch.

This patch makes the default hierarchy for a taxonomy 'none', then dynamically updates the vocabulary as the user defines tree relationships or flattens a tree into a flat vocabulary. The user is never bothered what kind of vocabulary they're working with, unless they attempt to add multiple parents to a single term (see screenshot).

Drag and drop is added for flat hierarchies and single parenting. It is not enabled on free tagging vocabs or multiple parent hierarchies (hence the warning when a user tries to add another parent, this only occurs the first time they add multiple parents).

This patch is also the first instance of a multiple-page drag and drop. If the user has a taxonomy with several hundred terms, the drag and drop pages nicely. The last element on the page will be included as the first item on the next page, with a visual cue from a dashed line.

Please test thoroughly and as quickly as possible. We're very short on time to finish up drag and drop implementations.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

Ok drag and drop works fine in itself, seems to be a depth limit of ten - is that enforced already or just in drag and drop? Seems like plenty anyway :)

admin/content/taxonomy I get:

notice: Undefined variable: type in drupal6/modules/node/node.module on line 368.

Drag and drop on a single page is fine on FF2, IE7, Safari 3 (XP) including key strokes. Seems faster/smoother if anything.
IE6 it works too (although with the shadowing of the image still as reported on menu issue)

Saving is fine when terms are on one page.

However,

I made enough terms to have two pages via some freetagging typing, then switched back to non-tags for drag and drop. Moved a bunch of terms around on the first page

On saving page 1, I got:
Fatal error: Cannot use object of type stdClass as array in /modules/taxonomy/taxonomy.admin.inc on line 425

Page 2, I did less drag and dropping, got this:

Undefined index: parent in drupal6/modules/taxonomy/taxonomy.admin.inc on line 384.
then dozens of these: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 381.

When I view /admin/content/taxonomy the type is also: "0, 1" for the one I've edited.

So drag and drop itself is flawless, but the saving across pages needs work.

Multiple parents warning - this looks fine to me. I also tested setting it back to a single parent, and that immediately re-enabled drag and drop. nice work!

catch’s picture

catch
he/him
Primary language English
commented

I'm reluctant to post this since it's probably out of scope, but is the admin/content/forum page similar enough that it could be included in this issue? If not will open a new one.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_drag_and_drop.patch46.72 KB

Here's an updated patch that should fix all the type errors. Stupid taxonomy module and using arrays and objects for the same data >:(

I think we'll worry about forums after taxonomy (or create a separate issue). Since it's literally the same form with a different link, we'll be able to simply include the taxonomy form inside of forums and change the link. I wouldn't want to write two separate submit handlers like this anyway.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

will test a bit later but

+  drupal_set_message('<pre>'. check_plain(print_r($var, true)) .'</pre>');
quicksketch’s picture

quicksketch
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_drag_and_drop.patch34.93 KB

Sorry I didn't cleanup my patch after creation. Here's the same patch cleaned up to only include the taxonomy changes.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

We need some PHPDoc for the following functions:

theme_taxonomy_overview_terms
theme_taxonomy_overview_vocabularies
taxonomy_form_term_validate

The theme functions need @ingroup themeable so they show up at http://api.drupal.org/api/group/themeable/6.

There are also some // FIXMEs that should probably be, well, fixed. ;)

When I click "list terms" on a vocabulary with no terms, I get a screen full of these notices, and then an empty table row.

    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 294.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 294.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 295.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 295.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 299.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 302.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 308.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 312.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 312.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 317.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 321.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.admin.inc on line 324.

And then somewhere in there (I think after creating a new term in an empty vocabulary), my "Types" associated with that vocabulary turned into "0, 1" rather than "Page, Story"..?? And that then results in 

notice: Undefined variable: type in /Users/webchick/Sites/head/modules/node/node.module on line 368.

..on admin/content/taxonomy.

So.. yeah. A little buggy still. :)

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_drag_and_drop.patch52.61 KB

This patch should clear up any errors reported in the above responses. Thanks for the extensive testing!

- Added documentation for functions, put theme function ingroup themeable and form functions ingroup forms.
- Added support for drupal_set_error(), which will come into play when we do the drag and drop forum patch.
- tabledrag.js was modified to take another $relationship value, 'group'. This updates the entire group of dragged rows, rather than just the main parent being dragged. This was needed to update the 'depth' option on all children when moved. The change to tabledrag.js looks substantial since it split out a function into two, but other than indentation changes only about 10 lines were changed.
- Reset to Alphabetical button was added.

I tested this version pretty well. An extensive code review or functionality testing would be appreciated.

quicksketch’s picture

quicksketch
he/him
commented

To test the validation of a drag and drop form, you can try out the dependent forums patch: http://drupal.org/node/195250 and try creating a forum 'container' and put it under a normal 'forum' term.

robloach’s picture

robloach
he/him
commented
Status: Needs review » Needs work

Some hunk failures and offsets when applying. This looks very promising though!

quicksketch’s picture

quicksketch
he/him
commented
StatusFileSize
new taxonomy_drag_and_drop.patch52.29 KB

Updated for HEAD. Thanks Rob!

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs work » Needs review
catch’s picture

catch
he/him
Primary language English
commented
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work
StatusFileSize
new forums-reset.png14.02 KB

OK I'll only mention bits that were broken before or I have questions about to save going "this is great!" on yet another issue followup. Also any cross-browser issues have been dealt with as other patches have gone in (although I tested in FF2 and IE6 just in case).

Saving across multiple pages now works, no flaws I could find.

The programmatic enabling and disabling of drag and drop when multiple parents is selected makes complete sense to me, and also works very smoothly as parents are added and removed from multiple terms.

There seems to be a hard-coded depth limit of ten in the drag and drop, but afaik taxonomy module has no such restriction. Also if I manually select a parent at a lower depth from the term edit form, I can get 11, 12 levels or more - and these are correctly displayed on the terms list after saving the form. I can then drag them to back up the hierarchy, but not down again. More than ten levels of hierarchy is a pretty rare edge case but there doesn't seem a particular reason to limit it unless I've missed something. By no means a showstopper but worth mentioning.

One thing I think reckon should be changed is that the Submit and "Reset to Alphabetical" buttons should be reversed. It's trivial, but my mouse hovered over the reset button a couple too many times when I went to save the form. This would also make it consistent with reset buttons elsewhere in core (screenshot of the forum settings page attached) Otherwise this is RTBC.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_drag_and_drop.patch52.29 KB

Thanks catch. Fortunately the depth limit was fixed just this morning also in the book patch (http://drupal.org/node/192736). After updating to the latest CVS, the depth limit should be removed from the taxonomy pages since tabledrag.js will now default to no limit.

About button placement, I'm fine either way. I switched it in this version. Drupal seems to be a little inconsistent with where the submit button lives. Try a node edit form: [ Preview ] [ Save ] [ Delete ], hmmm, why is the one I want to hit in the *middle* of all places? Thanks for the screenshot, that sets a nice precedent and I think it makes it easier also.

This switches the Submit button with reset and is another re-roll for head to line up the line numbers since the book patch.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

Quick review of the updated patch and all of my concerns are dealt with. This is ready!!

I'll open an issue for the node form, that seem wrong.

edit: oops, Rob Loach is right. Vocabulary ordering no longer works.

robloach’s picture

robloach
he/him
commented
Status: Needs review » Needs work

Awesome work! Looks very nice. I've noticed that setting the weights in the vocabulary page (admin/content/taxonomy) doesn't work. The weight values arn't sent through taxonomy_overview_vocabularies_submit's form_state, which is weird. Everything else seems to be working on this end though! This'll be so slick.

Stefan Nagtegaal’s picture

Stefan Nagtegaal commented

This is RTBC! Tested on Win/Mac with all availlable browsers..
Nate, you rock man! You definatly rock...

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Heh, Stefan is as excited as me, but back to CNW for the vocabularies.

robloach’s picture

robloach
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_drag_and_drop_5.patch48.82 KB

After talking with quicksketch and taking a closer look, it seems that $form was missing #tree. Here's a rerolled patch with that attribute. $form = array('#tree' => TRUE); did it.

robloach’s picture

robloach
he/him
commented
StatusFileSize
new taxonomy_drag_and_drop_6.patch51.36 KB

Oops, was missing taxonomy.js and taxonomy.css.

quicksketch’s picture

quicksketch
he/him
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new taxonomy_drag_and_drop.patch56.01 KB

Missing taxonomy.js and taxonomy.css files from the last patch. Here we are.

UPDATE: I'd delete this comment if I could. Both patches are identical but rolled with different flags. (This one has -p).

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Yep that fixes it. Back to RTBC.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Fixed

Great, committed. Thanks. Good work.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Add drag and drop to taxonomy » Taxonomy drag and drop bugs
Category: feature » bug
Status: Fixed » Active

Hm, a few usability problems I noticed with this patch after commit:

- "No terms available" is shown in the *vocabulary* table when there are no vocabularies.
- Also there is a submit button displayed in this case.
- Drag and drop is activated even if I only have one vocabulary (that's not the case with terms).

Seems like these transitional states were not properly tested. Let's fixed these!

catch’s picture

catch
he/him
Primary language English
commented

"No terms available" wasn't introduced by this patch, it was probably a bad string in one of my taxonomy cleanup patches if not there from earlier. Needs fixing though, probably just to "No vocabularies available".

It seems we missed the other two completely, apologies!

quicksketch’s picture

quicksketch
he/him
commented
Status: Active » Needs review
StatusFileSize
new taxonomy_drag_and_drop2.patch2.51 KB

Yup, I even saw "No terms available", but I didn't realize it was such a recent change (Taxonomy terminology patch). This changes it to "No vocabularies available" and disables the submit button and drag and drop when there is only one vocabulary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Taxonomy drag and drop bugs » Add drag and drop to taxonomy
Category: bug » feature
Status: Needs review » Fixed

Thanks, committed. Setting back title and category for future reference.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Add drag and drop to taxonomy » Taxonomy drag and drop broke forums
Category: feature » bug
Priority: Critical » Normal
Status: Fixed » Active
StatusFileSize
new Screenshot_2.png65.43 KB
new Screenshot_1.png27.8 KB

This broke forums. See the two images attached. First, it does not make much sense to restore forums and containers to alphabetical order, right? Second, when that button is pressed, there are all kinds of misbehaviors. The content of the table goes blank, and we have quite a few error messages. This needs to be fixed.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

On a related note, as the alphabetical reset button submits to the same URL, the help text is also displayed on the confirm page, but this is misleading.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new forum_alpha.patch590 bytes

This patch unsets the alphabetical reset button in the forum overview page. Simply doesn't need to be there.

catch’s picture

catch
he/him
Primary language English
commented

hmm, I'm getting a bunch of notices as well:



    * notice: Undefined index: in drupal6/modules/taxonomy/taxonomy.module on line 987.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 254.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 278.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 292.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 384.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 384.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 384.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 384.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 411.
    * notice: Trying to get property of non-object in drupal6/modules/taxonomy/taxonomy.admin.inc on line 411.

Same as reported by webchick in #9, but I'm convinced those got fixed in subsequent iterations.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

#32 is also a valid problem still, on the terms pages too, not only on the forums!

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Hey, can we get these issues fixed *soon*?

catch’s picture

catch
he/him
Primary language English
commented

http://drupal.org/node/201333#comment-663138 was duplicate.

So I stick by this button being simply unnecesary (and potentially nightmarish) on the forum admin page - so what do we need, seperate confirm page to avoid the help text duplication?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Yes, it looks like we need separate URL to avoid the help text showing up on the confirm form.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

@Gabor: I think #32 is a separate issue since it equally applies for deletion from the comment admin form and potentially others in core - so I've opened a new issue for it to standardise all confirm forms to separate pages. http://drupal.org/node/202061

#33 deals with the specific issue of forum breakage (which ought to be a release blocker, I'm not sure if confirmation forms also should be), so marking back to review for that.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Fixed

Well, all the confirm forms in taxonomy got broken with this drag and drop patch, but we can fix them just as well elsewhere.
Committed #33.

catch’s picture

catch
he/him
Primary language English
commented
Title: Taxonomy drag and drop broke forums » Add drag and drop to taxonomy
Category: bug » feature

Resetting title for future reference.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.