mar10/fancytree

getNodeByKey issue

jvowles opened this issue · 5 comments

Hey. Found an issue where ActivateKey isn't working because GetNodeByKey can't distinguish between keys in two separate trees.

I know element IDs should be unique, but in this case they aren't. And I think the issue is obvious enough that it might be better fixed in the library, though I have a viable work-around.

Expected and Actual Behavior

I have two trees (both of them using table mode, but that's likely unimportant). In my case these represent large hierarchical structures, where one is a more limited view of the other, so specific items appear in both trees.

Each tree has a search/filter function that works great at finding the correct item within their respective trees. However, I'm returning the results in a popup, and clicking a result activates the corresponding node so it scrolls up into view. That relies on a click handler that reads some embedded data elements in the link to do this: [myTree].activateKey(myKey)

Let's call the trees $tree1 and $tree2.
$tree1 is a view that contains some elements of $tree2; $tree2 is the parent from which $tree1 is built. Thus, elements exist with the same ID in each tree, and that ID is used to build the tree's key (and that anchors a ton of other stuff, so I can't easily change the key pattern without a ripple effect for other functionality).

The problem is that when I call $tree2.activateKey(myIDvalue), it isn't confining its search to just $tree2. In the fancytree library, under "getNodeByKey", it's using document.getElementById to get that node, which finds the first instance of that ID in the entire document, rather than just within the context of $tree2. So it is activating the element in $tree1, rather than the $tree2 I called it against.

My work-around is to do a manual visit within the nodes of $tree2, until I find the correct node, and activate it from there. In fact, that's what you do when "searchRoot" is provided already, but NOT if it can find the node on the page. Thus, if I search for something that happens visible in the other tree, it picks that up rather than the one in the tree I'm trying to search.

It seems to me that if you're invoking a function against a tree, it should only apply against that tree. Here's the block of library code where this is occurring.

image

Steps to Reproduce the Problem

  1. create two trees that have duplicate keys between them, ideally a level or two in. ($tree1, $tree2)
  2. invoke "$tree2.getNodeByKey(myKey)", against values that you expect to find in tree 2, but which also exist in tree 1. Repeat with tree1 expanded, but not tree2. It will return the value from $tree1, instead of the one in $tree2.

Environment

  • Browser type and version: (chrome 109.0.5414.120 , but irrelevant)
  • jQuery and jQuery UI versions: 3.6.0, and 1.13.1
  • Fancytree version: 2.8.1
    enabled/affected extensions: table, glyph, dnd5, multi. (i use persist in other versions as well)
mar10 commented

Hi Jim,
does setting different values for the idPrefix option help? (https://wwwendt.de/tech/fancytree/doc/jsdoc/global.html#FancytreeOptions)

I suspect they would, because the IDs would be different. Ideally you wouldn't have any repeated IDs, but in this case it is necessary and I can envision similar cases where it would be possible to have non-unique keys to search on. Which wouldn't be an issue if the function restricted itself to the proper context. In this case, these are sometimes immense (thousands of entries, a dozen layers deep) accounting structures that are involved, and one tree is built as a manageable view of the other.

Unfortunately, in our case, there's a ton of automation that depends on a consistent and predictable prefix structure, and changing it would be a huge lift.

What isn't clear, and is the source of my woes, is that I'm basically using a few pattern matching cues to automatically detect and populate trees/grids on page load from ajax sources, and hinging all the functionality off the concept that the keys are based on the actual IDs. it's pretty seamless about showing form elements, hooking up any necessary lookups, and so forth, and we've extended the heck out of it with a ton of custom code. And with automation, it's entirely possible that the two tables would generate with similar keys but refer to different data sources.

All was well until I had to retool to support more than one grid on a page. And at this point I've got dozens of grids that can auto-generate buttons and connect them, handle custom server-side ajax and excel webservices, etc. and automatically keep things restricted to their respective contexts -- I'm even wrapping the trees in simple jq.datatables for scrolling and auto-sizing functionality. But it all hinges on keeping my existing patterns for naming conventions.

Like I said, I have a work-around that basically skips the "if you spot it in using document.getlementbyID, great, if not, use a Visit); I've tried passing the tree, the tree's root node, and "true" as the searchnode, and the behavior remains unchanged. Whereas if the function restricted the match to the tree you call if from, it would work fine and no work-arounds needed.

Appreciate the responsiveness -- I noticed a few to-dos in that function so I suspected it wasn't quite 100%. If you do end up tweaking it, great! If not I understand, but I do think it's implicit in the way you call it that it should be restricted to the table you're referencing.

mar10 commented

I admit that I don't fully understand the circumstances, and why having a name-prefix per tree does not work for you.
Since Fancytree renders nodes only on demand, relying on the DOM Attributs might be a problem too?

Anyway, 'The id defines an identifier (ID) which must be unique in the whole document'.
The name prefix only applies to the rendered element attribute, but tree nodes can still have identical ids stored in node.key. You can use code like this to access it in event handlers:

tree.$container.on("mousedown.contextMenu", function (event) {
    var node = $.ui.fancytree.getNode(event);
    var id = node.key;

I've tried passing the tree, the tree's root node, and "true" as the searchnode, and the behavior remains unchanged.

Did you really try to pass tree.rootNode as in tree.getNodeByKey(KEY, tree.rootNode)? I havn't tested, but looking at the code I would think it should work.

Appreciate the responsiveness -- I noticed a few to-dos in that function so I suspected it wasn't quite 100%. If you do end up tweaking it, great! If not I understand, but I do think it's implicit in the way you call it that it should be restricted to the table you're referencing.

Currently I am hesitant changing this working code, but try to remember this, when I come to this code again.

I take your point about the IDs not being unique enough -- it could be that I'm misreading it and it's just highlighting key ft_123 by the node.key, and because that node.key exists in two structures, it highlights both.

The issue for this screen (and others) is that the keys are meaningful -- they correspond to specific items in the DB, and the source is the same for both trees. The left tree is a view you're building from pieces you copy from the right tree (the parent view).

I suppose I could try changing the key prefix rules to use something like ft_accountsCurrent_ and ft_accountsParent_ (kind of like what I'm doing for the persistence cookie) and see if that causes problems.

Thanks again.

This issue has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.