[patch] Tree: factory for nodes

[guest]

This relates to bug #4817. I have a problem where I need to change the style of the text in the tree nodes. I can't use CSS classes because the style is interpreted from remote data. Specifically, I need to be able to make the background and font color of the tree nodes to be anything my server tells it to. I can't realistically make 256*256*256 CSS classes--one for each color I want to accommodate, so I really need to access the style object for the labelNode in dijit._TreeNode directly.

The fix for this is very simple and I have tested it thoroughly. In dijit/Tree.js, add this method in the dijit.Tree class (I added it after getItemChildren):

createChildNode: function(params){

return new dijit._TreeNode(params);

},

In the _setChildren(childrenArray) method in dijit._TreeNode, replace the line

var child = new dijit._TreeNode(dojo.mixin({

with

var child = this.tree.createChildNode(dojo.mixin({

and in dijit._TreeNode._addChildren, replace

var child = new dijit._TreeNode(

with

var child = this.tree.createChildNode(

This fix changes no functionality in the default implementation, but it allows custom extensions of dijit.Tree to override createChildNode and supply custom dijit._TreeNodes such as a custom implementation as I have. It just allows a programmer easy access to the tree nodes without adding any burden on users who don't require this access.

[bill]

Sounds reasonable.

[bill]

Although note that this solution is incomplete because it doesn't address updates to items in the store... and update to an item might cause the corresponding TreeNode? to require a change in it's CSS.

[bill]

See also dup #5912.

[Robert Coup]

Aha, missed this one in my search :)

Although note that this solution is incomplete because it doesn't address updates to items in the store

ok, so currently there's a TreeNode.updateItemClasses() method, which is called from Tree._onSetItem(). It is used for updating the CSS classes at the moment, but it could be used by a custom Node to do anything useful when an item changes.

imho we should rename updateItemClasses() to updateItem(). dijit._TreeNode is private, so there's no API breakage (but we could alias to deprecate).

comments on that before i prepare a patch?

[bill]

I agree, probably updateItem() is better than updateItemClasses() but my point was that the createChildNode() factory might create different classes depending on the item (ex: EmployeeTreeNode?, ManagerTreeNode?), and thus if an item changed (ex: an employee was updated to a manager), then would need to destroy the EmployeeTreeNode? and new() the ManagerTreeNode?.

But I guess we shouldn't worry about that for now.... let's not try to boil the ocean.

[Robert Coup]

If someone wants to be that clever its a caveat emptor situation imho, but we can warn about that in the comments/docs. I'll prepare a patch for your perusal.

[Robert Coup]

.4.patch makes updateItem() just an extension point, not mixing it with _updateItemClasses() which is also used for expand/collapse behaviours.

[bill]

similar patch to above, with createNode() and updateNode() methods in Tree (untested)

[bill]

I've looked over this, and even applied a modified version of the patch rcoup attached (my modified code is attached as 5719.patch) I could add this functionality to Tree by ISTM that what people really want to override (or what people mean by "node") is just the (folder) icon and the label, not the expando (+/-) icon and/or the children container for child nodes. So I think this deserves some thought.

[bill]

[Temporarily] fixed by [13932]. As listed above I'd like to have a better fix someday, where a widget can be specified just for the item itself.