Skip to content
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

[Feature Request] [TreeView] Dynamic TreeView width #16935

Closed
1 task done
spfennell opened this issue Aug 8, 2019 · 5 comments · Fixed by #17046
Closed
1 task done

[Feature Request] [TreeView] Dynamic TreeView width #16935

spfennell opened this issue Aug 8, 2019 · 5 comments · Fixed by #17046
Labels
component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@spfennell
Copy link

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

I would like an option for the width of the TreeView to rely on the current expansion of the tree. Currently the TreeView width is based on the fully expanded tree.

Motivation 🔦

My app can have really deep trees that won't necessarily ever be expanded fully. I would like the collapsed tree to take up less space in my UI and expand dynamically as needed.

@spfennell spfennell changed the title Dynamic TreeView width [Feature Request] Dynamic TreeView width Aug 8, 2019
@spfennell spfennell changed the title [Feature Request] Dynamic TreeView width [Feature Request] [TreeView] Dynamic TreeView width Aug 8, 2019
@oliviertassinari oliviertassinari added the component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! label Aug 8, 2019
@oliviertassinari
Copy link
Member

@spfennell Interesting, the following would implement the expected outcome (I believe). It would also help with performance:

diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js
index f89049246..64c79c18a 100644
--- a/packages/material-ui-lab/src/TreeItem/TreeItem.js
+++ b/packages/material-ui-lab/src/TreeItem/TreeItem.js
@@ -252,7 +252,7 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
         <Typography className={classes.label}>{label}</Typography>
       </div>
       {children && (
-        <TransitionComponent className={classes.group} in={expanded} component="ul" role="group">
+        <TransitionComponent unmountOnExit className={classes.group} in={expanded} component="ul" role="group">
           {children}
         </TransitionComponent>
       )}

On the benchmarking side, we have the following that render lazy.

Not all of the solutions I could benchmark do, but most. I would propose that we make it the default behavior. cc @joshwooding.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 9, 2019
@joshwooding
Copy link
Member

@oliviertassinari Sounds like a great idea! @spfennell Do you want to give this a go?

@Shubhamchinda
Copy link
Contributor

Can I take this?

@joshwooding
Copy link
Member

@Shubhamchinda Sure :)

@mbrookes
Copy link
Member

This should hopefully also solve the problem of screen readers reading the children when an item is selected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants