[TreeView] Fix the height of the customization demo #16874
Conversation
mbrookes
added
component: tree view
|
No bundle size changes comparing 14e595f...dd41c0c |
| @@ -54,40 +70,44 @@ TransitionComponent.propTypes = { | |||
| in: PropTypes.bool, | |||
| }; | |||
|
|
|||
| const StyledTreeItem = withStyles(theme => ({ | |||
There was a problem hiding this comment.
I think that we should use withStyles here. It's the approach we document for customizing components. They might be a case or two where we don't do it, that was because I couldn't get TypeScript to accept it.
But no matter the API, we shouldn't share the same style sheet.
There was a problem hiding this comment.
It's the approach we document for customizing components
Perhaps we should update the documentation then? The hook method is much cleaner.
we shouldn't share the same style sheet
I did it separately originally, and thought you'd say to combine them! 😆
What's the main concern?
There was a problem hiding this comment.
What's the main concern?
I think that people should be able to copy & paste the customized tree item and to move it into its own file without having to worry about the styles of the overall tree view.
Perhaps we should update the documentation then? The hook method is much cleaner.
I believe that we document makeStyles for page layout styles (but can often be replaced with the Box) and for new components creation. I believe that we also document withStyles for customizing existing components. Migrating to styled-components, we will have to remove all the hook styles.
There was a problem hiding this comment.
move it into its own file without having to worry about the styles
Okay, that's what I guessed.
There was a problem hiding this comment.
Migrating to styled-components, we will have to remove all the hook styles.
Sounds like a migration nightmare for users.
There was a problem hiding this comment.
I think that the API should still work, my comment was targeted at what we use in the demos.
Just to make it match the basic demo.
Also capitalised the labels, although we could probably do with finding labels that match a real-world use-case as our other demos do...