-
-
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] Clean label editing code #14264
[TreeView] Clean label editing code #14264
Conversation
8e3ea4f
to
4484573
Compare
4484573
to
cdab9e0
Compare
Deploy preview: https://deploy-preview-14264--material-ui-x.netlify.app/ |
@@ -1,8 +1,12 @@ | |||
export interface TreeItem2LabelInputProps extends React.InputHTMLAttributes<HTMLInputElement> { |
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.
We should avoid to add props we don't actually use to the typing of our slots.
That way people implementing custom slots only need to support the props we actually need and not all the props we could theoretically use on our default slot component.
UseTreeViewLabelSignature, | ||
} from './useTreeViewLabel.types'; | ||
|
||
export const isAndroid = () => navigator.userAgent.toLowerCase().includes('android'); |
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.
Never used
@@ -76,5 +76,10 @@ export type UseTreeViewLabelSignature = TreeViewPluginSignature<{ | |||
experimentalFeatures: 'labelEditing'; | |||
dependencies: [UseTreeViewItemsSignature]; | |||
}>; | |||
export interface UseTreeItem2LabelInputSlotPropsFromItemsReordering |
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.
Typo in the name and the props were never added to the typing of UseTreeItem2LabelInputSlotProps
const editable = instance.isItemEditable(itemId); | ||
|
||
if (!editable) { | ||
return {}; | ||
} | ||
|
||
const handleKeydown = ( |
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.
Move those two callbacks here, that way useTreeItem2
is totally agnostic of anything regarding this prop
We have some infos about label editing in other slots but nothing too important
e883c8a
to
5070e7d
Compare
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.
Thanks for taking care of this! 🎉
No description provided.