Expanding/collapsing nested terms is broken under certain conditions

[a-roussos]

When viewing the details page of a term (e.g. vocab/index.php?tema=123) that has one or more narrower terms under it, TemaTres groups the blocks of narrower terms under HTML <ul> tags. These are used when clicking on the blue arrow buttons next to a term, in order to expand or collapse the hierarchy.

We recently came across a case where this does not work as intended.

When the same term appears more than once in the page, the aforementioned <ul> tags do not have unique identifiers (HTML id property) across the DOM. In such a scenario, only the first expand/collapse button (blue arrow) for the term that appears multiple times will operate normally; the remaining expand/collapse buttons (for the same term) will misbehave, since clicking on them causes the first instance of the term to expand/collapse. Here's a screen capture showing this in action (the problematic term is "theodicy"):

To fix this issue, I modified the HTMLverTE() function so that the depth level of the current term ($i) is appended to the term's id ($tema_id):

--- fun.html.php.2022-02-23.AR 2022-02-08 11:39:25.455402512 +0200
+++ fun.html.php               2022-02-24 17:07:58.957357996 +0200
@@ -1109,7 +1109,7 @@ function HTMLverTE($tema_id, $i_profundi

     global $CFG;
     $sql=SQLverTerminosE($tema_id);
-    $rows='<ul id="masTE'.$tema_id.'"  style="list-style:none; display: none">';
+    $rows='<ul id="masTE'.$tema_id.$i.'"  style="list-style:none; display: none">';
     //Contador de profundidad de TE desde la raíz
     $i_profundidad=($i_profundidad==0) ? 1 : $i_profundidad;
     $i_profundidad=++$i_profundidad;
@@ -1119,7 +1119,7 @@ function HTMLverTE($tema_id, $i_profundi
     while ($array=$sql->FetchRow()) {
         if ($array["id_te"]) {
             if ($i<CFG_MAX_TREE_DEEP) {
-                $link_next='  <a href="javascript:expand(\''.$array["id_tema"].'\')" title="'.LABEL_verDetalle.' '.$array["tema"].' ('.TE_termino.')" ><span id ="expandTE'.$array["id_tema"].'">&#x25ba;</span><span id ="contraeTE'.$array["id_tema"].'" style="display: none">&#x25bc;</span></a> ';
+                $link_next='  <a href="javascript:expand(\''.$array["id_tema"].$i.'\')" title="'.LABEL_verDetalle.' '.$array["tema"].' ('.TE_termino.')" ><span id ="expandTE'.$array["id_tema"].$i.'">&#x25ba;</span><span id ="contraeTE'.$array["id_tema"].$i.'" style="display: none">&#x25bc;</span></a> ';
                 $link_next.=HTMLverTE($array["id_tema"], $i_profundidad, $i);
             } else {
                 $link_next='&nbsp; <a title="'.LABEL_verDetalle.TE_termino.' '.$array["tema"].'" href="'.URL_BASE.'index.php?tema='.$array["id_tema"].'">&#x25ba;</a>';

This resulted in <ul> identifiers that seemed unique, and for a moment I thought the issue was solved. However, I then realised that if the same term appears more than once under the same depth level, the <ul> ids will not be unique and the problem re-appears. See, for example, what happens when I try to expand the second instance of the term "meditation":

So, given that my initial approach was not working 100%, I reverted the changes I'd made in the HTMLverTE() function and decided to re-work the internals of the JavaScript expand() function. Here's what I've come up with:

--- js.php.2022-02-24.AR 2021-12-06 14:14:03.000000000 +0200
+++ js.php               2022-02-25 08:41:06.060776783 +0200
@@ -152,9 +152,10 @@ if ($_SESSION[$_SESSION["CFGURL"]]["ssus
 ?>

 function expand( id ) {
-    var details = document.getElementById('masTE' + id );
-    var enlaceMas= document.getElementById( 'expandTE' + id );
-    var enlaceMenos= document.getElementById( 'contraeTE' + id );
+    var current = document.activeElement;
+    var details = current.nextElementSibling;
+    var enlaceMas= current.childNodes[0];
+    var enlaceMenos= current.childNodes[1];
     details.style.display = ( details.style.display == 'block' ) ? 'none' : 'block';
     enlaceMas.style.display = ( details.style.display == 'block' ) ? 'none' : 'inline';
     enlaceMenos.style.display = ( details.style.display == 'block' ) ? 'inline' : 'none';

This seems to have done the trick -- I can now expand and collapse the "theodicy" and "meditation" terms and it works fine.

I'm a bit hesitant to raise a PR, though, because in the long term my JS fix may not be enough (e.g. if the HTML layout of the details page changes in the future).

@tematres: I'd value your opinion on this! Do you think the JS fix is the best way forward? Do you have any ideas about how to produce 100% unique ids across the page?

Thank you! 👍

[tematres]

Hi @a-roussos :)! sorry for delay in response :/ (and please sorry about my english :/) I check the 2 alternatives and I think the best way to solve the problem is change the JS, because in the other way we need to anticipate the dom ID for all the

div and rewrite them. This way is more complex. We adopt your propouse in our sandbox, you can try it here (the login is open): https://r020.com.ar/tematres/demo/index.php Thank you very much! :))

[tematres]

@a-roussos , if you agree, please go ahead with the PR :)

[a-roussos]

Fixed in 96e0805 with a small follow-up in 8153e2f. I'm closing this issue now 😄 Thanks for all your efforts, @tematres!