Forums

At the latest Open Hours the YUI dev team called for feedback on the 3.6PR3 TreeView. There doesn't appear to be an existing thread and the Github pull request isn't the place for this type of feedback.

The YUI3 TreeView is a lot simpler than YUI2 TreeView. That's fine, as long as the minimal API still supports some more complex use-cases. However...

# Dynamic loading?

No support for fetching sub-trees via ajax or datasource? That's a curious omission, even in an early release.
A big emphasis was placed on rendering performance, which is good. But a big performance gain can be won by dynamically fetching new nodes on demand rather than requiring them all up front.

# Needs to expose a lot more events

As long as there's enough events to tie into it's straightforward enough to implement dynamic loading yourself. But there's no expand or collapse events being fired. From the source, there's a 'toggleTreeState' event, but it doesn't bubble out to a subscriber on a TreeView instance.
There's 'treeview:click' and 'treeleaf:click', but there's no way to know if a treeview:click is in response to a click on the "expand" portion or on the label itself (select action).

# List item nesting

Children are inserted as sibling list-items of the TreeView's label list-item rather than as a nested list.

e.g. http://pastie.org/4211724

The way it's currently implemented results in less deeply-nested structures (rendering performance, or just a reaction to YUI2 TV's overly-nested tables?) but isn't as semantic and is, presumably, more confusing for screen reader users. It's also impossible to style sub-lists properly since children list-items are siblings of the list-item labeling the TreeView, rather than as a sub-list within the list-item.


I'd appreciate if the authors of the TreeView widget would take a look and possibly get tickets in for some of these things. YUI3 TreeView is a nice first step, but the current omissions make it difficult to implement anything more complex than a simple example.

I'd also like to see some of these issues fixed or some response from the YUI team on this.

@Satyam, do you recommend any of the gallery options?

@ZMRE:

All the gallery versions of TreeView are somewhat old. All of them would need some revamping, one of them using WidgetHTMLRenderer and many other performance improvements, specially on a NodeJS environment. None of them has this TreeView/TreeLeaf structure so, at least, that's a point in favor of any of them.

Anyway, there has been several discussions on how to improve the Gallery, the latest here: viewtopic.php?f=18&t=10066 part of which involves which components to pick and promote.

@Satyen

My main concern with this TreeView proposal is the TreeView/TreeLeaf architecture. In your reply you mention TreeView and TreeNode objects, which is how I think it should be, not TreeView/TreeLeaf which is what the proposed component has. I really don't have a particular preference as to what the TreeNode should be and I probably got carried away making them Nodes plugged with a TreeNodePlugin. Perhaps I got carried away in that, but what I wanted to really stress is that there should be one basic foundation and that would be the TreeView object, representing the whole tree and its root and then, TreeNode objects which should be much lighter. What is the right trade-off for TreeNodes, whether to make them full Widgets or plugged in Nodes, I don't know. I imagined plugged in Node instances as an extreme of lightness, but anything in between might be better.

Regarding points b) and c) it is true that people are concerned about the state of individual TreeNodes, but they usually reach them via TreeView (the root). I can't imagine anyone subscribing to a particular TreeNode click event. You usually subscribe to TreeView click events and then get the TreeNode instance in the facade of the event. In YUI2 TreeView, individual nodes didn't have events you could subscribe to (well, you have one but I don't think anyone ever cared about that one), you got all your events from the root. All YUI2 TreeView events have a node reference as one of its arguments already resolved for you. I don't recall anyone ever asking in the forums to be able to subscribe to a particular TreeNode click event.

Likewise, you usually reached the TreeNodes via the root using any of the getNodeXxxx methods. If you build your TreeView either from existing markup or a literal object, you don't get references to individual nodes. When you built your tree via JS code, you held a reference to each individual node when it was created so you could use it to provide it to its children when they were created, then you forgot about it. If you look in the examples, such reference was often called tmpNode or some equally volatile name.

So, nobody really cared about individual TreeNodes until something happened to them. Then, the event listener would be provided with a reference to the one involved. Which is good, because it means they don't even need to exist (except as markup on the screen) until needed. Moreover, markup for nodes in collapsed branches was not even rendered. Such markup can be created really fast via templates, by TreeView, with the help of a single TreeNode instance, re-used over and over, much as that tmpNode reference so often used in YUI2 TreeView.

Using WidgetParent/WidgetChild does not lead to the kind of optimizations you mention (lazy object creation or flyweight pattern), as far as I can remember. If you let a Widget extended with WidgetParent/WidgetChild run over a configuration object and it finds a children property containing an array, WidgetChild will start creating child objects of the ROOT_TYPE class for each of the children. No laziness there.

Anyway, once the TreeView/TreeLeaf architecture is dropped, then we can start talking about optimizations and improvements.

So, my main points are:

a) No TreeView/TreeLeaf.
b) Single TreeView representing the whole tree and its root and a lighter TreeNode for every other node
c) TreeNodes created on demand when needed (in response to events or getNodeXxxx methods), reused if possible (flyweight pattern or some such)
d) I don't think WidgetParent/WidgetChild can be used unless they can get 'lazy'.
e) I don't really mind if TreeNode is a full-blown Widget, if it can be created on-demand( lazy) or reused (flyweight).

Satyam

My basic point in defining what functionality the TreeNode object should have, is that the state for whether a TreeNode is expanded or collapsed, is a function of that TreeNode, or the state of the label for a TreeNode is a function of that TreeNode, not the top-level TreeView (or Root). The TreeNode encapsulates that state, and fires an event when that state changes (that responsibility is thus encapsulated and extendable). Whether people listen for that event only at the TreeView, or whether they ask for an instance and listen directly on it, is a layer above that.

As for the other stuff, I think we're saying the same thing - Lazy instantiation or flyweight doesn't exist for Parent/Child, but should. We should avoid creating full blown instances until the user needs one. Both of these things make sense in general for Parent/Child (or Model/ModelList for that matter - see below), not just TreeView.

The TreeView implementation, to the best of my knowledge already renders it's Nodes completely using strings/templates, and renders lazily (doesn't render collapsed children up front), but I'll let the TreeView owner validate that statement.

The main missing piece is lazy instantiation/flyweight (flyweight being a little more difficult). Ryan submitted a Lazy Model/ModelList impl which I've discussed with EricF as potentially looking at re-using for Parent/Child [ or a Base/BaseList if you will ], if there's nothing Model specific about what it's doing.

So, to sum up, to your points:

a) I'll let the TreeView owner comment on the choice of structure. I don't have that in my head currently, and haven't given it that much thought.

b) I think the TreeNode should be an observable/stateful thing. We just need to avoid creating 1000s of them up front (see c))

c) Agree

d) I think it's missing a 'feature' - lazy instantiation/flyweight. Using it provides a consistent API as Juan mentions (which matter not only for consistency, but also for the application of generic extensions which target the Widget API), and something to extend, to create Selectable/Editable Tree Node objects.

e) Agree

Again, if we can agree (or conclusively disagree) on whether or not these TreeNode objects, when we finally create them, need to be observable and encapsulate their own state, we can go from there. Another option, which EricF mentioned, is that Model/ModelList (with Views) has lazy instantiation, and string based rendering [ both of which we're trying to retrofit Widget and WidgetParent/Child to support - although it's more challenging there due to backcompat ]. It's another option to consider - keep in mind these would still be observable/stateful objects - but just with lazy instantiation already done. What we'd give up in the scenario is the consistent/common Widget API for extension and consistent classname generation.

Perhaps this could work:

On instantiation TreeView receives either a literal configuration object or a reference (either CSS selector or Node instance) of existing markup to parse. The object will be cloned to an internal copy, the markup parsed into a similar object.

TreeView will have a pool of TreeNode instances to make available to the developer when looking at any node in the tree. The TreeNode instances will be of the type indicated in an defaultType attribute in TreeView. This attribute will be either a string giving the name of the class under Y (Y[this.get('defaultType')]) or a reference to the class.

The configuration object will specify the structure of the tree with the children property containing specs for child nodes. The nodeType property will specify the type of TreeNode to create. If none is specified, the defaultType will be used, otherwise, an instance of the specified type would be created.

TreeView will keep one instance of each of these types (the defaultType and any other nodeTypes specified) in a pool of reusable TreeNodes.

TreeNode will have a private _node property. TreeNode (and all its derived subclasses) will always access all their internal information through this property. For example, when a node is clicked, TreeView will pick that up by delegation and locate which node it corresponds to in its internal copy of the configuration object. It will read the nodeType for that node and pick the corresponding instance from its pool of TreeNode instances. It will then set its _node property to point to the node in the configuration object.

TreeNode should have all its attributes with getters and setters so that they all get their data from the _node like:

label: {
....getter: function () {
........return this._node.label;
....},
....setter: function (value) {
........this._node.label = value;
....}
}

In this way, any TreeNode instance can be slid on top of any node in the configuration tree and work fine. Only the root attribute in TreeNode will not have such getters and setters since it is the same for all nodes.

On TreeView rendering, TreeView will read the tree configuration object and start sliding the nodeTypes (usually Y.TreeNode) on top of each of the nodes by setting their _node property and calling its render() method. TreeView won't bother to visit the children of collapsed nodes.

Whenever TreeView slides a TreeNode instance on a node, it will check it to see if it has ever been visited before (both TreeView and TreeNode will modify this internal copy of the tree configuration to add information on it, such as whether it has ever been visited) If it hasn't been visited, it will call a private method in TreeNode so that it can read all its properties and see if there are any extra properties not already declared as attributes. If any is found, TreeNode will call its own addAttr to add it to its standard attributes such as label, icon, expanded and so on. (alternatively, TreeNode could have its set and get properties overridden so that it accepts any attribute not previously declared). This is in keeping with the tradition of YUI2 TreeView which allowed extra custom properties (it didn't have attributes) on its nodes, which was a very common practice.

Since I'm thinking of BaseCore as the class TreeNode would be derived from, attribute change events should not be an issue since there are none. Anyway, if a particular TreeNode subclass were to be extended to fire attribute change events, since attributes should only be changed when the TreeNode instance is slid upon the node, it should not be a problem. If you tamper with the underlying property in this._node then you would not expect events to be fired. Such custom events should be bubbled to the root TreeView instance by using addTarget.

All events should be listened to via TreeView. TreeView would use event delegation to listen to anything happening in it or would be the target of custom TreeNode events. In all cases, TreeView would find out which node in its internal configuration object would be the originator of that event and slide the corresponding type of TreeNode on top of it and add it to the event facade.

TreeView would also have a forEachNodeBy() method which would take two functions, the first one a selector function which would receive a TreeNode instance and return a boolean to indicate whether it belongs to the set. The second function would be the function that does something about it. From this method, other helper iterators can be produced such as forEachExpanded(), forEachHighlighted(), forEachChild() which would only take a single callback function or forEachNodeByProperty (often used with custom attributes) which would take a property name, a value to match and the callback. The callback would always receive a TreeNode instance (or one of its subclasses according to its nodeType) slid on top of it.

In these callback functions, as well as in the events, the function should not keep a copy of the TreeNode instance since it will be slid on top of some other node on return. Developers can request a non-pooled TreeNode instance by using any of the getNode(s)By methods. This should not be encouraged since those copies will not be reused by TreeView though, of course, the developer can always dismiss those copies when no longer needed. Unfortunately, since JavaScript objects have no destructor the TreeNode would not be aware of it being de-referenced and would not return resources such as Node instances it might have held in the configuration object. Anyway, Node itself keeps a cache of Node instances so it probably doesn't matter.

Tree navigation via getNextSibling, getPreviousSibling, getParent would force TreeView to provide non-pooled instances for each, however, methods moveToNextSibling, moveToPreviousSibling or moveToParent would simply slide the very same node instance on top of it.

The nodeType property (or defaultNode attribute on TreeView) might also be a template. If the attribute is a string (not an object reference) and it has a curly bracket, then it is a template, if it doesn't, it is the name of a class under Y. TreeView would then use a regular TreeNode instance with the given string as its template. This would go well with the dynamic extra attributes so as to allow users to customize TreeNodes without having to create a subclass.

Some extra means of node formatting via code (not just via templates) should be provided, much as it happens with DataTable where there is a string formatter and a DOM node formatter.