Opened 6 years ago

Closed 6 years ago

#16690 closed defect (fixed)

dijit/Tree documentation problem with mayHavChildren()

Reported by: spaquette Owned by: bill
Priority: undecided Milestone: 1.9
Component: Documentation Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

You can see this in action in the 1.8 documentation example of a (DnD) dijit/Tree connected to a store:

http://dojotoolkit.org/documentation/tutorials/1.8/store_driven_tree/demo/demo.php

If you select 'House of Representatives', then click on the 'Add new child to selected item' button below the tree widget, the 'House' leaf will not become a branch with a new leaf node named 'New Child'. However, this addition does work with some other nodes--eg., President.

In looking at Tree, the problem seems to be that model.getChildren is only ever called in _expandNode, and not when a node is added to the tree. If no attempt is ever made to expand a node, then there will be no observation of its children, so if it is a leaf it will not convert into a branch when children are added via store.put(). During Tree loading, it looks like any leaf which is a child of a branch that is expanded at loading will have _expandNode run on it. In a few tests, I found that if I left a branch collapsed at load, none of its leaves could be converted to branches.

Current workaround: After using store.put() to load new data into the store, call Tree._expandNode for the destination/parent node. (With criteria to limit when it does this to prevent unnecessary or invalid expansions).

OS: Windows 7, 64-bit

Browsers tested: Chrome 24, Firefox 18

Change History (11)

comment:1 Changed 6 years ago by noxryan

There does appears to be issues with the 1.8 dndTree example. If you drag an item into its current parent it disappears. This is especially a problem if you have a betweenThreshold that allows the re-ordering of items within the same parent.

Here are some others that have reported the issue:

http://dojo.6188.n7.nabble.com/v1-8-dijit-Tree-DnD-of-nodes-makes-change-invisible-until-subsequent-DnD-action-td8630.html

http://dojo-toolkit.33424.n3.nabble.com/dijit-Tree-DND-betweenThreshold-bug-please-help-td3992612.html

comment:2 Changed 6 years ago by spaquette

Added note, if a tree isn't DnD, the leaf-doesn't-become-a-branch problem still holds for store.put(). You can test this by disabling the DnD in the demo.

An additional thing, I was testing the demo again in Firefox, and it looks like you can't convert *any* leaves to branches using the 'Add new...' button in Firefox, but in Chrome you can do it to some of them. I'm guessing this is a difference in behavior of the tree's loading per browser.

comment:3 Changed 6 years ago by bill

Thanks for the report. You might be talking about a bunch of separate issues, in which case they should be separate tickets.

As for the original issue, at first glance it looks like the bug is in the demo, in the mayHaveChildren() method. It's supposed to return false only if the element doesn't have and never will have children.

comment:4 Changed 6 years ago by noxryan

I have moved the issue I reported into a separate ticket:

http://bugs.dojotoolkit.org/ticket/16692

comment:5 Changed 6 years ago by spaquette

Regarding the mayHaveChildren aspect: if I'm reading what you've said correctly, this implies if any node *could* have children, regardless of whether or not it has them right now, then one would need to have mayHaveChildren just return true.

This introduces a new problem, though. If mayHaveChildren always returns true, all nodes, including leaves, show an expando icon. (They also default to folder icons, but you can override getIconClass to deal with that.) In the case of a 'leaf' folder, when someone clicks on it, the expando icon vanishes. You can't override the presence of the expando icon and its click action, so there's no way to stop this behavior. There's a use-case for having all leaves behave and look like leaves until they have children, rather than as 'potential branches' ala exploring files and folders whose content status is unknown.

If mayHaveChildren is intended to return true if a node may ever have children, can we have a way to also override the presence of the expando icon, as is done with getIconClass?

(This is looking like it's related to #9413.)

(Edited so what I'm getting at is more clear.)

Last edited 6 years ago by spaquette (previous) (diff)

comment:6 Changed 6 years ago by bill

Agreed about the relation to #9413.

Saying that "one would need to have mayHaveChildren just return true" is an exaggeration. For example, in a Tree of artists, albums, and songs, the songs will never have children, so mayHaveChildren() should return false for songs.

comment:7 Changed 6 years ago by bill

Component: DijitDocumentation
Milestone: tbd1.9
Status: newassigned
Summary: dijit/Tree: Adding a child to a childless node sometimes fails to display folder w/expansiondijit/Tree tutorial problem with mayHavChildren() implementation

Anyway I will fix the tutorial.

comment:8 Changed 6 years ago by spaquette

Artists and songs is definitely a case where leaves will only ever be leaves. I'm thinnking about a use-case where there isn't that kind of situation for a leaf, though. For example, the demo shows government positions, i.e. an org chart--at any time, a leaf on that org chart might be assigned a subordinate, and now it needs to become a branch, but until then, it should behave visually as a leaf. The node icon can be handled with getIconClass, but the expando icon can't be.

Or is there a way to accomplish that which I'm missing, aside from the workaround listed above, or recreating the widget any time a leaf is converted to a branch (which is how it's done in some other libraries)?

Last edited 6 years ago by spaquette (previous) (diff)

comment:9 Changed 6 years ago by bill

Unfortunately, I don't think you are missing anything. #9413 just needs to be fixed.

comment:10 Changed 6 years ago by bill

In [31273]:

clarify description of mayHaveChildren(), refs #16690 !strict

comment:11 Changed 6 years ago by bill

Resolution: fixed
Status: assignedclosed
Summary: dijit/Tree tutorial problem with mayHavChildren() implementationdijit/Tree documentation problem with mayHavChildren()

Reference doc updated in https://github.com/dojo/docs/commit/d7cbb4e3f98652d8d26a04acb59a518fc60f1bbb.

I also fixed the tutorial.

Note: See TracTickets for help on using tickets.