Currently, this widget loads all of the terms at once. For large trees, this might prove to be impractical. A solution would be to load subtrees dynamically via AJAX when they are expanded for the first time.
I'll remove the other 2 patches as they contain issues.

Comments

sdrycroft’s picture

+1 for this one. I'm happy to help you work on this if you would like - I was in the process of writing a similar module when I discovered your module.

bartk’s picture

Welcome aboard! :)

I haven't started on this particular feature, so if it's something you'd like to work on, go for it.

Thanks!

Bart

sdrycroft’s picture

Wow, thanks very much Bart. I'm a little busy this week with meetings, but should hopefully get some work done towards this.

bartk’s picture

Just wanted to make a note here. I'm currently in the middle of a major project that this isn't strictly necessary for, so I don't have time to work on it. sdrycroft, feel free to poke at it if you'd like. :)

sdrycroft’s picture

This is coming on quite well thanks Bart, although I have made a little mess with the current code. I'll report back here when it's working properly (currently having a little issue with setting values the correct "Drupal" way).

bartk’s picture

Glad to hear it. :)

Just a word of warning -- the code has undergone some significant changes in the beta version -- making in theme-able involved rewriting a lot of code from the ground up.

Anyway, if you'd like another set of eyes, check a new branch into the git repo and I'll take a look at it and maybe make some suggestions.

Bart

sdrycroft’s picture

I've added a branch to the repo (dynamic_tree_load). I'm afraid I've torn apart a lot of the code, so it may not be obvious which bits I've changed, and which bits are new. I'd certainly appreciate you taking a look at it, and seeing what you think. Note, you will need to uninstall the previous version before installing this (to ensure the menu is rebuilt for the AJAX/AHAH callback).

I was also struggling with themeing the buttons nicely, and integrating them properly into the form, so have rewritten all of the CSS - I'm happy for this to be changed back, especially as I've removed the dotted lines (personally I don't think they're necessary, but if you'd rather have them in, then we can add them back).

Things still to do:

  • Add back the filter by view option, which sadly doesn't yet work (although I think this shouldn't be too tricky to fix)
  • Add access control to the callback. I'm not sure how the access control should be handled, so at the moment all calls to the callback URL are permitted.
  • Add a second module which will replace the TinyTax module that I maintain. This could be as an additional project, or simply as a second module within this project. The theme functions will require a little tweaking to do this, but otherwise this should be pretty easy.

Cheers, Simon.

sdrycroft’s picture

Have you had a chance to look at this Bart?

bartk’s picture

Not yet. I'm trying to get the other stuff stable and working first before I merge it in. I was also sick all of last week, which set me back a bit. :)

bartk’s picture

Okay, time to look at this now. At cursory glance, this looks like it's going to take a lot of work. I'll get back to you if I run into any problems.

Bart

mikgreen’s picture

Any movement with this? I might be able to contribute work to this effort.

BartK, did you take a look at branch sdrycroft made? Are latest changes to code significate enough and it should be started from scratch?

I don't believe there is another way to handle large taxonomies out there?

bartk’s picture

Yes, I've looked at the branch, and unfortunately the changes were made before I did a major overhaul to the code, and thus the code is very divergent. I can't make heads or tails of the merge. :(

rhylos’s picture

While this is def getting there if the items are not expanded upon expansion anything below does not slide down on the page. The terms tend to overlay content such as the default image upload article option (tested with D7 default theme).

Update (Disregard this is the same as issue: http://drupal.org/node/1277268
)

nags338228’s picture

Any movement with this? eagerly waiting for this feature. And in BEF in Views also....

nchar’s picture

Issue summary: View changes
StatusFileSize
new24.42 KB

I have worked on this feature, as it was crucial for one of my projects. In this project I had a huge taxonomy tree with hundreds of items and the page was responding too slow both in the frontend and in the backend.

The following patch adds a new setting in every "term reference tree" field to load subtree on click with ajax. I tested with all features of the module and it work fine so far.

bartk’s picture

I'll keep an eye on this. I don't have the time to test it myself extensively, but if some people try it out and it works without any issues, I'll merge it in and put out a new version.

metammar’s picture

+1 Thanks a lot! works like a charm..

But after applying the patch i got a js error in console because of "on" function. i upgraded my jquery version in admin to 1.10 and error has been gone and all of my problems solved!

nchar’s picture

StatusFileSize
new24.42 KB

Yes I missed that. I fixed the patch.

hgoto’s picture

Status: Active » Needs work

Thank you for the patch!

The patch #18 works well for me. I think it's almost OK but found one point to fix.

If the ajax function is disabled after the patch is applied, an error shows up in my browser console.

Uncaught TypeError: Cannot read property 'prototype' of undefined at term_reference_tree.js:3

The exact line in the js file is the following one.

Drupal.ajax.prototype.commands.term_reference_tree_append_children = function(ajax, response, status) {

The steps to reproduce is as following.

  1. Enable the module.
  2. Apply the patch #18.
  3. Create a field with the Term reference tree widget.
  4. Check off the checkbox "Load children with ajax" in the field setting page.
  5. Open an edit page for the content with the field and you can see the error log in your browser console.

I think the error happens because the file ajax.js in the core is not loaded but the line expects that the file is loaded and that the Drupal.ajax already exists there.

sano’s picture

After applying this patch the dynamic loading works, but the term labels no longer show in the language set in the user profile (they are always shown in the default language). The i18n is broken regardless of whether ajax loading is turned on or off.

herved’s picture

Hello everyone,

Thank you very much for this patch! Great stuff.

In reply to #20 indeed the labels translation were removed.

+++ b/term_reference_tree.widget.inc
@@ -663,7 +703,7 @@ function _term_reference_tree_build_item($element, $term, $form_state, $value, $
+  if (module_exists('locale') && !empty($term->tid) && FALSE) {

I started #2936277: Performance improvements with i18n some time ago to improve the widget process function but found out that the rendering was taking a long time as well on the website I'm working on. So ajax would be the best to improve the render part.
So here is a patch with both ajax and process improvements.

The process function in dev branch is currently very slow because of the taxonomy_term_load() calls used just to get the label.
It also triggers multiple queries to build the tree. The new patch here aims to fix this.
I added some hard caching for the trees mainly to cache the term labels.
If you are on a multilingual website or if you have a specific label callback you'll still have to do this in a custom module:

/**
 * Implements hook_term_reference_tree_taxonomy_get_tree_alter().
 */
function mymodule_term_reference_tree_taxonomy_get_tree_alter($vid, &$terms, $hierarchy, $langcode) {
  // Example with i18n.
  foreach ($terms as $term) {
    $term->name = i18n_taxonomy_term_name($term);
  }

  // If using the title module I think simply this should work.
  $field = field_info_field('name_field');
  field_attach_load('taxonomy_term', $terms, FIELD_LOAD_CURRENT, array('field_id' => $field['id']));
}

I also fixed a bug where if term C is selected but terms A and B are not then the widget would not be expanded as it was checking for direct children only:
- A
-- B
--- C

herved’s picture

Status: Needs work » Needs review

Switching to needs review. Any feedback is greatly appreciated.

sano’s picture

really busy at the moment, but looking forward to check your solution asap.

sano’s picture

Looking good. The i18n problem is fixed with a custom module utilizing your code. This is what I found:

1. editing a node with TTRT widget does not show the previously-made selection. In other words, the tree is collapsed, with no indication as to which branch/leaf is selected. Not sure if this is possible with the ajaxified version, but if I remember correctly I was able to do it using the older patch
2. default value can't be set - clicking the tree root does not expand into branches.

thank you

herved’s picture

Hi sano,

You're right. These render arrays are so confusing...
I'm working on a new patch

herved’s picture

Status: Needs review » Needs work
herved’s picture

Here is an updated patch which should solve both issues.
Any feedback is greatly appreciated ;)

herved’s picture

Status: Needs work » Needs review
sano’s picture

I see the issue #1 is fixed, but see no change in #2. I still can't set a default value. If too much trouble, the ajax version of the widget could be turned off there and replaced with - say - a multi-select text field.

thank you

herved’s picture

Ok bear with me :)
There was a small issue in the ajax callback - term_reference_tree_children_callback() - and issues with clearing the hard caching.

Here is I think my final take on this.
I removed the hard caching as the perf gain was not that high and I don't think that would ever be merged to develop as it prevents the dynamic loading on translations.

Here are the changes I did against #18:
1. Fixed can't set default value - issue in term_reference_tree_children_callback()
2. Refactored the _term_reference_tree_get_term_hierarchy() function. Retrieves the term hierarchy in 1 query only. Uses static caching (useful if several widgets use the same taxonomy and possibly different parents)
3. Moved the max_depth logic in the process function
4. Restored the the label resolution for multilingual websites and moved it in the process function + possibility to override it with a variable (useful for performance in order not to load the whole entity).
To use it with e.g. i18n: drush vset term_reference_tree_taxonomy_resolve_label_callback "mymodule_term_reference_tree_taxonomy_resolve_label_callback"

/**
 * Callback to resolve labels for term_reference_tree.
 */
function mymodule_term_reference_tree_taxonomy_resolve_label_callback($term) {
  // Resolve labels with i18n.
  return i18n_taxonomy_term_name($term);
}

Possible improvements:
- The label resolution is heavy for multilingual websites as it loads full entities one by one then calls entity_label().
To improve this we could possibly flatten the array returned by _term_reference_tree_get_term_hierarchy(), pass terms in a drupal_alter() and then if some property is not set, resolve all labels (or tokens if used) using taxonomy_term_load_multiple() and entity_label().

herved’s picture

I eventually implemented my suggestion for possible improvements.
Final-final patch containing big changes is here.
But any suggestion is of course welcome.

Changes against #30:
- Improvements to the label resolution for multilingual websites and/or if using tokens.
- Added alter hook: hook_term_reference_tree_process_terms_alter().

If you are on a multilingual website labels will be resolved using taxonomy_term_load() and entity_label() which is the most reliable.
As you know loading full terms is rather heavy.
If using i18n or ET and title module, and to avoid loading terms we can now use the alter hook I added like this (No variable set, and possibility to load all terms at once):

/**
 * Implements hook_term_reference_tree_process_terms_alter().
 */
function mymodule_term_reference_tree_process_terms_alter(&$terms, $element) {
  // Resolve labels with i18n.
  foreach ($terms as $term) {
    $term->term_reference_tree_label = i18n_taxonomy_term_name($term);
  }
}
herved’s picture

AAAh I messed up the info file...
New patch here.

herved’s picture

I didn't realize but we need to keep the logic about max_depth, use_ajax and expanded in the process function as well as in the _term_reference_tree_build_level() and _term_reference_tree_build_item() functions...
This is really confusing.

Probably the best would be to build the render arrays as part of _term_reference_tree_get_term_hierarchy()...
Anyway I hope this patch has no issues. I think so. We'll see.
Sorry for the mess here...

sano’s picture

Looking good. I tested it with English and Slovak - two languages my site is using atm. Setting default value works, selecting multiple values works, creating a new term/translating it and subsequently selecting works as well.

Great job. :)

Thank you

herved’s picture

Great to hear sano ;)

On my side I can definitely see a performance improvement between patch #18 (/!\ with restored label translation) and #33.
This is even more noticeable if I use several fields using the same taxonomy but different parents.

However I'm still not 100% happy with the whole process function and I think it could be improved even more.
If I still find some time I might provide a new patch.

But if anyone wants to dive in or simply review what I did so far here's my logic:
1. Retrieve the whole tree (in 1 query, terms and hierarchy cached statically)
2. Filter terms to get only the ones about to be rendered (based on filter_view, max_depth, default, use_ajax, opened,...)
3. Provide an alter hook on the terms - provide all terms at once for perf (useful for example to resolve labels faster than using taxonomy_term_load() + entity_label(), maybe unset some terms, etc).
4. Resolve tokens and labels (if not resolved already).
5. Finally build the render arrays.

herved’s picture

Minor fixes here:
1. Fix issue reported in #19 by hgoto
2. Fix throbber-active.gif path for websites with sub-directory url
3. Some functions doc fixes

herved’s picture

New patch that disables the row when we click on a "+" while ajax is loading.
Thanks to my colleague @pgingao ;)

Delphine Lepers’s picture

Status: Needs review » Reviewed & tested by the community
sano’s picture

installed patch #37. I see a speed improvement from #33. No issues to report. Thank you

troybthompson’s picture

#37 has made a huge difference in loading time for me using a large taxonomy for three fields on the same edit page. Thank you for this patch.

herved’s picture

Glad to hear ;)

If you are on a multilingual website (locale module installed) I highly suggest to use the snippet I posted in #31 which uses i18n and avoids taxonomy_term_load() calls.

If you use entity_translation something like this should work (untested):

/**
 * Implements hook_term_reference_tree_process_terms_alter().
 */
function mymodule_term_reference_tree_process_terms_alter(&$terms, $element) {
  $instance = field_info_instance('taxonomy_term', 'name_field', $element['#vocabulary']->machine_name)
  if (module_exists('title') && $instance) {
    // Resolve labels with title.
   field_attach_load('taxonomy_term', $terms, FIELD_LOAD_CURRENT, array('field_id' => $instance['field_id']));
   foreach ($terms as $term) {
     $term->term_reference_tree_label = $term->name;
   }
  }
  elseif (module_exists('i18n_taxonomy')) {
    // Resolve labels with i18n.
    foreach ($terms as $term) {
      $term->term_reference_tree_label = i18n_taxonomy_term_name($term);
    }
  }
}
kyuubi’s picture

Hi guys,
What's the status of this patch? This one is critical for us (we have thousands of terms) and it's working great but it no longer applies to the most recent version of the module.
Any chance to get this committed soon?
Cheers,

sano’s picture

I am running the the patch on the 7.x-1.11 version and see no problems. What version do you have installed?

kyuubi’s picture

You're right, don't know what happened there.
+1 for me, let's get this committed!
Cheers

yogeshmpawar’s picture

Looks good to me, +1 for RTBC.

alex_optim’s picture

Version: 7.x-1.0-alpha1 » 7.x-1.x-dev
Assigned: bartk » Unassigned
Status: Reviewed & tested by the community » Needs work

Need to upgrade the patch for version 7.x-1.x-dev

eduardo morales alberti’s picture

Status: Needs work » Needs review
StatusFileSize
new36.85 KB

Rerroll patch to apply with the last version.
Not posible to do the interdiff.

alex_optim’s picture

Status: Needs review » Patch (to be ported)
alex_optim’s picture

Status: Patch (to be ported) » Fixed
alex_optim’s picture

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

Issue tags: +dckyiv2019
arefen’s picture

Hi. I check D8 version of this module and see doesn't this feature on it. Are you any plan to support in D8?