TreeView and TreeItem components

[estruyf]

Category

[x] Enhancement
[ ] Bug
[ ] Question

New component(s)

TreeView and TreeItem, should be used as follows:

<TreeView>
  <TreeItem key="1" label="Group header">
    <TreeItem key="2" label="Item 1" />
    <TreeItem key="3" label="Item 2" />
  </TreeItem>
</TreeView>

[ghost]

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

[nanddeepn]

Hi @estruyf ,
Me and @siddharth-vaghasia will be happy to take this.

[nanddeepn]

Hi @estruyf

Can you please help to define the end user usage for this control?

We propose to have usage something like below:
< TreeView items={treeitems} >
< /TreeView >

where, treeitems will be an array of JSON with hierarchical structure of TreeItems

[AJIXuMuK]

Hi @nanddeepn!

I think hierarchical JSON is ok.

But I would also think more broadly as this kind of control must be pretty flexible.
If you even look at TaxonomyPicker where we have tree inside, you'll see we need:

1. IconProps for items. And different items can have different icons
2. SelectionMode : none, single, multi (similar to DetailsList)
3. Some API to provide additional items to the component. Like, vertical three dots with contextual menu
4. We definitely need onSelect, onExpand
5. It would be also great to have ability to provide custom rendering, something like onRenderNode(node: ITreeItem)
6. I would include data property to the TreeItem props and ITreeItem interface to contain any data related to the node
7. Maybe consider having parent reference in ITreeItem - it may be helpful in many scenarios

@estruyf - any additions?

[nanddeepn]

Hi @AJIXuMuK
Thanks for your detailed response.

Unfortunately, my previous comment discarded html format of proposed usage. Just adding it here again for your review.
<TreeView items={treeitems}> </TreeView>

Please share your view.

[AJIXuMuK]

Hey @nanddeepn,
I would expect something like

<TreeView
  items={treeItems}
  onSelect={(treeItem: ITreeItem, ...) => {...}}
  onExpand={(treeItem: ITreeItem, ...) => {...}}
  onRenderItem={(treeItem: ITreeItem, ...) => {...}}
  selectionMode={SelectionMode.None/Single/Multi} />

And each treeItem is of ITreeItem type:

interface ITreeItem {
iconProps?: IIconProps;
disabled?: boolean;
data?: any;
key: string;
label: string;
subLabel?: string; //maybe
actions?: IContextualMenuItem[]; // or similar interface
}

Something like that on top of my head.

[AJIXuMuK]

and as a future improvement - lazy loading. Probably something that can be achieve with onExpand

[siddharth-vaghasia]

@AJIXuMuK ...thanks for your valuable suggestion...we have started working on this and will share updates soon... meanwhile, can you help with below confirmation.

what will be the schema of 'treeItems' array which will be passed by the end developer while using this control?

As of now, we have written logic to convert flat structure to hierarchy structure
Like user has to pass below object (flat hierarchy)

This will be converted to below object

and then we are processing this object to create an actual tree view.

So do we need to force the user to pass flat structure and we do the processing to create a hierarchy OR We need to ask user to provide a hierarchy?

Please advise. hope this question is making sense :P

[AJIXuMuK]

Hi @siddharth-vaghasia,
I would expect a hierarchy... Similarly to Context Menu in Office UI Fabric

[{
  "key": "key",
  "text": "text",
  ...
  "children": [...]
}[

[siddharth-vaghasia]

yes, we also thought so..... thanks...

[nanddeepn]

Hi @AJIXuMuK ,
One question, should TreeView support multiple root nodes?

[AJIXuMuK]

Hi @nanddeepn,
I think yes. It’s a natural scenario for folders, navigation

[nanddeepn]

Hi @AJIXuMuK

We are thinking of implementing indeterminate state for the parent checkbox (as in image here)

Indeterminate state property is supported in Office UI Fabric V7.105.3 (latest version)
Reference: https://docs.microsoft.com/en-us/javascript/api/examples/checkbox?view=office-ui-fabric-react-latest

However Office UI Fabric version supported by SPFx v1.10.0 is 6.189.2, which does not support Indeterminate state property.

Questions:

1. Is it advisable to upgrade to Office UI Fabric V7.105.3? But, this might not work fluently unless individual upgrades the Office UI Fabric to V7.105.3 in their SPFx solutions.
2. Or should we target this (indeterminate state) functionality for checkbox in the future release?

Please advise.

Also, when you get time, please check the implementation of this control so far here

[AJIXuMuK]

Hi @nanddeepn
The Reusable Controls are based on v 5.131.0 of OUIFR.
We will update to v 7 when SPFx adds support of it.

[nanddeepn]

Thanks @AJIXuMuK
Please have a quick review the implementation of control
https://github.com/nanddeepn/code-samples/tree/master/SPFx/TreeView

If you are fine with the implementation, we will raise PR.
Thank you.

[siddharth-vaghasia]

@AJIXuMuK Sending link again as it is broken in above message

https://github.com/nanddeepn/code-samples/tree/master/SPFx/TreeView

[AJIXuMuK]

Hi @siddharth-vaghasia, @nanddeepn!
Great job guys! It will be really great addition to the library!

I have few comments though.

1. I like the idea of having treeItemActions in the TreeView. But it looks like in that case you can't have different actions for different tree items. They will be the same for all items. I think actions property in ITreeItem is enough. And we can use the same function to define the same set of actions to each item if needed.
2. treeItemActionsDisplayStyle: TreeItemActionsDisplayStyle.textAndIcon - I think we can just check if we have iconProps and text for the action. For example, one item can have both, another - text only. See ContextualMenu.
3. Please, use Capitalized values in enums. Buttons instead of buttons.
4. TreeItemActionsDisplayMode.dropdown - I would name it TreeItemActionsDisplayMode.ContextualMenu.
5. Could you please use MoreVertical instead of ChevronDown for the actions?
6. After 5 is done we can use Chevron instead of custom icons for expand/collapse state of the tree items. Similarly how it's done in Nav component of Office UI Fabric.
7. I personally don't like that createChildNodes is passed as props to the TreeItem. I think TreeItem should be more "encapsulated" and this function should be implemented inside TreeItem itself.
8. I would reduce spacing between label and sub-label.
9. I would also think of making checkbox visibility optional. I see that it is hidden when the selection is set to None. But even when we have selection set to Single or Multiple it might be helpful to use just styling instead of rendering a checkbox.
10. And it would be also great to specify if item can't be selected. Example : PropertyFieldTermPicker. Term Sets can't be selected
11. iconProps?: IIconProps; - These properties should not be used as a checkmark of a checkbox. They should be used to render custom icon in the tree item. Again, example - PropertyFieldTermPicker.
12. Selection of child nodes when selecting a parent should be probably optional as well. I agree that this behavior is pretty natural. But, again, in situation with Term Picker we can have Term nested into another Term. And if we select parent Term we don't want the child Term to be selected.

Sorry for the large list. Let me know if you have questions or do not agree with any of the comments.

[siddharth-vaghasia]

@AJIXuMuK ...thanks for the review and your valuable comments... we will go through in detail and get back to you if any queries..

[siddharth-vaghasia]

@AJIXuMuK ...we have started working on these comments and have some queries/thoughts. Can you please check below and provide your comments.

I would also think of making checkbox visibility optional. I see that it is hidden when the selection is set to None. But even when we have selection set to Single or Multiple it might be helpful to use just styling instead of rendering a checkbox.

When user chooses to keep visibility to false, dp we allow them to select tree items? if we allow selecting tree node, we might have to change the background of a label to feel that item is selected. ?

And it would be also great to specify if item can't be selected. Example : PropertyFieldTermPicker. Term Sets can't be selected

Do you mean if term item is disabled, display tool tip or something on hover that item can't be selected?

I personally don't like that createChildNodes is passed as props to the TreeItem. I think TreeItem should be more "encapsulated" and this function should be implemented inside TreeItem itself.

We tried this without passing parameter, bu when we want to show/hide all child controls on click of parent one....state object of all child components has to be changed which can only be changed from treeview component. We would need a separate component that should handle rendering of child component(tree item). This might not be possible from tree item. But still if you can look at code and help us understand what approach might work, that would be helpful.

[AJIXuMuK]

Hi @siddharth-vaghasia,

When user chooses to keep visibility to false, dp we allow them to select tree items? if we allow selecting tree node, we might have to change the background of a label to feel that item is selected. ?

Yes. I would expect it to look similar to Nav component in Office UI Fabric.

Do you mean if term item is disabled, display tool tip or something on hover that item can't be selected?

No. I mean that there are Term Sets and Terms. Term Sets cannot be selected. Terms can be selected. It means that we need an option to specify if this specific Tree Item is selectable. Disabled in a separate setting :)

We tried this without passing parameter, bu when we want to show/hide all child controls on click of parent one....state object of all child components has to be changed which can only be changed from treeview component. We would need a separate component that should handle rendering of child component(tree item). This might not be possible from tree item. But still if you can look at code and help us understand what approach might work, that would be helpful.

I don't quite get why this method can't be move to the TreeItem.tsx itself... It is called inside TreeItem only.
Could you please explain what state can't be changed?

[siddharth-vaghasia]

@AJIXuMuK ....thanks...we have incorporated all the changes as request in the comment here

I don't quite get why this method can't be move to the TreeItem.tsx itself... It is called inside TreeItem only.
Could you please explain what state can't be changed?

Also on the above - True what you said, it is possible....I was stuck in one place, and being lazy came to an early conclusion that it might not be possible due to the nature of loop we need... but my dear @nanddeepn came to rescue with an idea....we worked it out together :)

Can you please check again and let us know if anything.

[nanddeepn]

Hi @AJIXuMuK ,

We have also prepared documentation here

Please have your review for code and documentation.
Let us know, if we are good to raise the PR.

[AJIXuMuK]

Hi @siddharth-vaghasia, @nanddeepn!
I'm working on it :) Don't have too much time these days.

Sorry for the delay.

[AJIXuMuK]

Hey @siddharth-vaghasia, @nanddeepn!

I've submitted a small PR with some updates to the component PR-14.

Please, align the documentation with the changes and I think we'll be good to go!

Thank you again for the great contribution and effort and time you put into it!

[siddharth-vaghasia]

@AJIXuMuK ...thanks for the changes....totally make sense and we have merged it to master branch...

I found one issue where " selectionMode: SelectionMode.None," and showcheckbox is false.... it was still allowing the user to select items.... this is fixed now...

You can review changes here

[AJIXuMuK]

Hi @nanddeepn, @siddharth-vaghasia!
There is one small comment to remove debugger. And feel free to create a PR! Great job!

For the documentation - please, move all interfaces in Implementation section. Now you describe all additional interfaces in How to use, and the main props in Implementation.

Looking forward to seeing the PR! :)

[nanddeepn]

Hi @AJIXuMuK
We have implemented your review comments and have raised a PR #536
Please help to get it merged.

Thank you again for your review comments and support to get this done! Hope to see this control useful for the community.

[AJIXuMuK]

The PR has been merged to dev branch.

[djmrky]

Hi everyone,

I'm trying to implement the TreeView control, but I find that when the TreeView is rendered, the property "defaultExpanded" of the tree item is hardcoded to "true". I'd like to be able to control the expanded property through the ITreeItem props. There is this property in the state of the TreeItem (ITreeItemState)

  items.map((treeNodeItem, index) => (
            <TreeItem
              treeItem={treeNodeItem}
              leftOffset={20}
              isFirstRender={true}
        defaultExpanded={true}
              selectionMode={selectionMode}
              activeItems={this.state.activeItems}
              parentCallbackExpandCollapse={this.handleTreeExpandCollapse}
              parentCallbackOnSelect={this.handleOnSelect}
              onRenderItem={onRenderItem}
              showCheckboxes={showCheckboxes}
              treeItemActionsDisplayMode={treeItemActionsDisplayMode}
            />
          ))

How can I control the expanded property programatically?

Thanks

[nanddeepn]

Hi @djmrky
By default all first level nodes of tree are expanded (default behavior of tree).
Use the property onExpandCollapse to observe the expanded/collapsed tree items, as Expand/Collapse are end-user actions on tree.
Use the property defaultSelectedKeys to specify keys of items to be selected by default.

[djmrky]

Hi, yes, I was able to solve the issue of first level nodes (I just added a "dummy" top/root element) and I'm using the defaultSelectedKeys to preselect some node.

But what I would like to do is when I set the defaultSelectedKeys (let's say I have single key and the node is on the 4th level) and the tree is rendered, I'd like this node to show as expanded, so it's immediately visible what node is selected.

Is this supported right now?

[nanddeepn]

Hi @djmrky,
Expanding a specific path to tree Item is not supported. However set the TreeView property defaultExpanded to true in order to expand all nodes.

[AJIXuMuK]

Great discussion @djmrky and @nanddeepn.
I think we definitely need to support "expand to selected" as well as collapsing the first level of the tree by default (enterprise scenarios might need that).
So, I've added two enhancements and one issue related to that discussion: #559 #560 #561

[djmrky]

@AJIXuMuK That would be a great enhancement ... I was actually able to change some parts of the code to make this work somewhat, just as a proof of concept. I just added a boolean prop in ITreeItem that says if this item is rendered as expanded or collapsed, and then pass that to the defaultExpanded prop of the TreeItem component. I just supply this data in the model.

It would be much better to get this functionality in the package :-)

[nanddeepn]

Hi @djmrky , Thanks for reporting.
Me and @siddharth-vaghasia will take care of the reported issues and enhancements.