TreeView usage
#14451
Replies: 1 comment
-
@emyarod I'm curious your thoughts on this as you established the bulk of the TreeView API. Are some of these intentional? |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
I had to implement the TreeView component recently and ran into a few issues regarding the usage and wanted to discuss what of it is intentional and what are bugs. Also, can they be fixed or would they technically cause breaking changes?
1. No auto-rerender on state prop change
Updating
props.active
andprops.selected
onTreeView
does not render automatically. A force re-render (for example through setting akey
is needed to apply these changes.Proposed action
This can probably be addressed by using the
useControllableState
hook, but would it technically be a breaking change since it modifies the behavior or is this considered the expected interaction and just bugged?2. Unclear usage of
props.active
andprops.selected
onTreeNode
The usage of
props.active
andprops.selected
is unclear at first as they exist on bothTreeView
andTreeNode
. Based on other Carbon components I was expected to setprops.active = true
on a givenTreeNode
before realizing these would be overwritten by the parentTreeView
. Same forprops.selected
.Proposed action
Can be handled by documentation
3. Misleading documentation around
props.active
andprops.selected
I found it misleading to read the
props.active
prop definition:This suggests that whatever I pass as
props.value
would be used to set the state. In fact though, theTreeNode
's id is used.Proposed action
Either documentation update to reflect that the IDs are used or make the component respect
props.value
instead (technically a breaking change without further actions though).4. Undocumented
props.id
onTreeNode
See 3. above. Each
TreeNode
is given a unique ID if none is passed via the props. These IDs are then used forprops.selected
andprops.active
. Support forprops.id
is undocumented though and can only be found by looking at the source code.Proposed action
Goes hand in hand with point 3. Either expose
props.id
or useprops.value
as the internal id.I could imagine that in order to avoid a breaking change,
props.id
could be deprecated (although never officially exposed) and if noprops.id
but aprops.value
is passed, to useprops.value
as the internal id. Alternatively the other way around (deprecatingprops.value
and exposingprops.id
).I'd be curious to hear which points you see as bugs that can be fixed, which ones are blocked and which ones are just based on wrong assumptions from my side.
Beta Was this translation helpful? Give feedback.
All reactions