-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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] Improve overrides story #20159
Conversation
Details of bundle changes.Comparing: c776d34...878fca2 Details of page changes
|
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 looking at the problem :). I have noticed that same issue with the "Default theme" tree view of the documentation. I think that we can take a step back and consider how we could change the implementation to make such demo easier to write.
@@ -41,7 +41,7 @@ const useTreeItemStyles = makeStyles((theme: Theme) => | |||
color: 'var(--tree-view-color)', | |||
}, | |||
'&:focus > $content $label, &:hover > $content $label, &$selected > $content $label': { |
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 think that we should keep iterate on the CSS override story. A selector of this complexity isn't OK. It will require a breaking change on the component.
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.
Could this pull be added to v5 milestone?
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 don't think that we need to wait, TreeView won't make it to the core in v5.
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.
Great. I'll will work on a proposal.
@oliviertassinari What do you think about moving the |
The usage of |
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.
Would be nice to add explicit documentation for the variants. Right now I don't understand what the names of these variants mean.
@eps1lon I changed my approach for one that doesn't require new props. |
@m4theushw Thanks for the help. We've taken a different approach to improve the override story. |
Fixes #20125