Opened 11 years ago

Closed 11 years ago

#5719 closed enhancement (fixed)

[patch] Tree: factory for nodes

Reported by: guest Owned by: Robert Coup
Priority: high Milestone: 1.2
Component: Dijit Version: 1.0
Keywords: dijit tree create custom node style Cc:
Blocked By: Blocking:

Description (last modified by bill)

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.

Attachments (3)

tree_custom_nodetype.3.patch (3.8 KB) - added by Robert Coup 11 years ago.
tree_custom_nodetype.4.patch (3.7 KB) - added by Robert Coup 11 years ago.
5719.patch (4.6 KB) - added by bill 11 years ago.
similar patch to above, with createNode() and updateNode() methods in Tree (untested)

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by bill

Owner: set to bill

Sounds reasonable.

comment:2 Changed 11 years ago by bill

Summary: Access to tree nodes in dijit.Tree needed[patch] Access to tree nodes in dijit.Tree needed

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.

comment:3 Changed 11 years ago by bill

Summary: [patch] Access to tree nodes in dijit.Tree needed[patch] Tree: factory for nodes

See also dup #5912.

comment:4 Changed 11 years ago by 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?

comment:5 Changed 11 years ago by 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.

comment:6 Changed 11 years ago by Robert Coup

Owner: changed from bill to 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.

Changed 11 years ago by Robert Coup

Changed 11 years ago by Robert Coup

comment:7 Changed 11 years ago by Robert Coup

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

Changed 11 years ago by bill

Attachment: 5719.patch added

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

comment:8 Changed 11 years ago by 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.

comment:9 Changed 11 years ago by bill

Description: modified (diff)
Resolution: fixed
Status: newclosed

[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.

Note: See TracTickets for help on using tickets.