feat(Tree): control activeItemIds through expanded prop of items #2061
Conversation
silviuaavram
requested review from
dvdzkwsk,
layershifter,
levithomason,
miroslavstastny and
mnajdova
as code owners
silviuaavram
changed the title
|
Let's add an example of how the user can control the expended prop |
| @@ -117,7 +117,7 @@ class TreeItem extends UIComponent<WithAsProp<TreeItemProps>, TreeItemState> { | |||
| onFocusFirstChild: PropTypes.func, | |||
| onFocusParent: PropTypes.func, | |||
| onSiblingsExpand: PropTypes.func, | |||
| open: PropTypes.bool, | |||
| expanded: PropTypes.bool, | |||
There was a problem hiding this comment.
I think this should be treated as auto controlled prop, by defining defaultExpanded and onExpandedChange. With this we will allow the user to define only the initial state, if they want that, or listen to the onExpandedChange and change the expanded prop by themselves.
There was a problem hiding this comment.
we are not having any state in TreeItem. All expanded stateful logic is in Tree, since TreeItems can be unmounted on virtualization. This is just a prop. The user can control it as it is.
There was a problem hiding this comment.
I am not sure about this one. I understand that it is just a prop, but all other similar props are treated as autocontrolled. @miroslavstastny thoughts on this?
| _.reduce( | ||
| items, | ||
| (acc, item) => { | ||
| if (item['expanded'] && acc.indexOf(item['id']) === -1) { |
There was a problem hiding this comment.
The items could be defined as render functions too, we should handle that case, or at least check whether it is an object.
There was a problem hiding this comment.
Currently it won't work well with render functions. We are calculating some props that we pass to TreeItem from Tree in getItemsForRender. However we probably can support this scenario with render prop as well. We just call the children function with the props from itemsForRender so the child component has access to them as well.
I would consider working on this in a separate PR. And just perform here the object check as per suggestion.
What do you think @mnajdova @jurokapsiar @miroslavstastny ?
silviuaavram
changed the title
User passing
expandedprop to items will allow him to control the expanded state of these items. Renamedopentoexpanded.Controlling the Tree's
activeItemIdswill have precedence over this individual control method.Added also missing tests for Tree, subcomponents and behaviors. Added / edited test definitions for behavior tests.