-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TreeView] Add selectItem
and getItemDOMElement
methods to the public API
#13485
[TreeView] Add selectItem
and getItemDOMElement
methods to the public API
#13485
Conversation
@@ -56,9 +56,10 @@ Use the `setItemExpansion` API method to change the expansion of an item. | |||
apiRef.current.setItemExpansion( | |||
// The DOM event that triggered the change | |||
event, | |||
// The ID of the item to expand or collapse | |||
// The id of the item to expand or collapse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to unify a bit the wording for those boolean parameters throughout the Tree View codebase
And to always have the same descriptions for the params in the JSDoc and in the doc section
Deploy preview: https://deploy-preview-13485--material-ui-x.netlify.app/ Updated pages: |
@flaviendelangle This DX looks perfectly agreeable to me 👍🏻 |
newSelected = [itemId].concat(cleanSelectedItems); | ||
} else { | ||
newSelected = cleanSelectedItems; | ||
} | ||
} else { | ||
// eslint-disable-next-line no-lonely-if | ||
if (newValue === false) { | ||
if (isSelected === false || (isSelected == null && instance.isItemSelected(itemId))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small fix: in single selection, if isSelected
is not defined and the item is selected we should de-select.
I fixed the usage of this method in useTreeItemState
and useTreeItem2Utils
to keep the same behavior on the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice addition 👌 The DX is good, and the docs look nice. Thanks for adding this 🎉 Left a little nitpick, but looks good to me otherwise
Closes #10113
selectItem doc preview
getItemDOMElement doc preview
@bharatkashyap I propose to expose a method that allow you to easily retrieve the DOM element rather than a method that allow you to scroll.
I first tried to implement something like
apiRef.current.scrollToItem(itemId)
, but I was basically copy-pasting the entire API of thescrollIntoView
method.Your use case can be achieved as follow:
And if you are using multi selection you might prefer:
I'll add some tests once we agree on the DX