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

Add props to control sidebar's collapsed state in DashboardLayout component #4406

Open
Mohammad-210 opened this issue Nov 12, 2024 · 12 comments · May be fixed by #4516
Open

Add props to control sidebar's collapsed state in DashboardLayout component #4406

Mohammad-210 opened this issue Nov 12, 2024 · 12 comments · May be fixed by #4516
Labels
component: layout This is the name of the generic UI component, not the React module! customization: logic Logic customizability scope: toolpad-core Abbreviated to "core" waiting for 👍 Waiting for upvotes

Comments

@Mohammad-210
Copy link

Mohammad-210 commented Nov 12, 2024

Summary

This feature request is to enhance the DashboardLayout component by adding two new props:

collapsed: A boolean prop that controls the initial state of the sidebar (collapsed or expanded).
onToggleSidebar: A function prop that allows toggling the sidebar’s collapsed state from outside the DashboardLayout component.
This setup will make the sidebar’s state externally controllable, enabling more dynamic and flexible UI behaviors in various responsive scenarios.

Examples

import React, { useState } from 'react';
import DashboardLayout from './DashboardLayout';

const AppProvider = () => {
const [isSidebarCollapsed, setIsSidebarCollapsed] = useState(false);

const handleToggleSidebar = (collapsed) => {
setIsSidebarCollapsed(collapsed);
};

return (

{/* Application Content */}

);
};

export default AppProvider;

Motivation

This feature will improve DashboardLayout’s flexibility and control by allowing parent components to manage and toggle the sidebar’s collapsed state. With responsive design requirements, many apps need to control the sidebar's visibility dynamically based on user interactions or screen size. This enhancement will make DashboardLayout more adaptable for complex UI requirements and enhance user experience across devices. The issue i face is i need a state to ensure the sidebar is collapsed or not to manage the responsiveness of design of rendered children comp

Search keywords: sidebar toggle, sidebar collapsed state, DashboardLayout, Material-UI, Toolpad sidebar, responsive sidebar, toggle sidebar from parent, sidebar props

@Mohammad-210 Mohammad-210 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 12, 2024
@bharatkashyap
Copy link
Member

Agree with providing a way to access the state of the sidebar, and also imperatively setting it. An alternative way of doing this is through a useSidebar hook:

const { collapsed, setCollapsed } = useSidebar();

const onClick = () => { if(!collapsed) { setCollapsed(); } 

This will enable components across the app to be able to access (and imperatively set) the sidebar's state instead of having to share props.

@bharatkashyap bharatkashyap added waiting for 👍 Waiting for upvotes scope: toolpad-core Abbreviated to "core" component: layout This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 12, 2024
@vikasgurjar
Copy link
Contributor

vikasgurjar commented Nov 13, 2024

@Mohammad-210 are you planning to work on this and create a PR? If not, I can help with the PR

@Janpot
Copy link
Member

Janpot commented Nov 13, 2024

a controlled prop makes sense to me. a hook will be constraint to descendants only, which is probably too limiting.

@Mohammad-210
Copy link
Author

@vikasgurjar I'm swamped with other tasks, I won't get to it.But I'm planning to fix it, but I haven't started yet. Feel free to create the PR!. if help needed let me know ..!

@vikasgurjar
Copy link
Contributor

@Janpot from what I understand, controlled prop will have to be passed down to every child that needs it. Also even in that case these props will be constraint to descendants only

@vikasgurjar
Copy link
Contributor

vikasgurjar commented Nov 14, 2024

@bharatkashyap I'm thinking of using useNavigation instead of useSidebar for consistency as we already have a prop called 'hideNavigation'

const { collapsed, setCollapsed } = hideNavigation();

const onClick = () => { if(!collapsed) { setCollapsed(); } 

@bharatkashyap
Copy link
Member

@bharatkashyap I'm thinking of using useNavigation instead of useSidebar for consistency as we already have a prop called 'hideNavigation'

const { collapsed, setCollapsed } = hideNavigation();

const onClick = () => { if(!collapsed) { setCollapsed(); } 

I discussed this with @mnajdova and we came to the conclusion that a controlled prop makes sense to begin with, while a hook/context can be implemented by users themselves if they need it as long as the prop is present. We could make an issue to collect upvotes on the hook feature request, to see if there is enough interest in providing that out of the box.

Let's focus on adding a controlled prop as part of this issue 👍🏻

@apedroferreira
Copy link
Member

apedroferreira commented Nov 18, 2024

A controlled prop as suggested sounds good!
sidebarCollapsed and onToggleSidebar sound good as possible prop names for DashboardLayout.
Feel free to create a PR, or we can do it for the next release.

@vikasgurjar
Copy link
Contributor

@apedroferreira I think eventually sidebarCollapsed can replace defaultSidebarCollapsed in future. Do you think we will deprecate the prop? If so, should we add the warning for the same?

@apedroferreira
Copy link
Member

apedroferreira commented Nov 29, 2024

@apedroferreira I think eventually sidebarCollapsed can replace defaultSidebarCollapsed in future. Do you think we will deprecate the prop? If so, should we add the warning for the same?

I think they would both stay with different purposes: sidebarCollapsed if you want that state to be controlled, and defaultSidebarCollapsed just to set the initial value if you want to leave it uncontrolled.
sidebarCollapsed would always take priority if set.

@oliviertassinari oliviertassinari added the customization: logic Logic customizability label Dec 4, 2024
@vikasgurjar
Copy link
Contributor

@apedroferreira @bharatkashyap I got some time to work on this. I was wondering what's the use of onToggleSidebar prop? Will it act as just a callback for whenever there is a state change for sidebarCollapsed state?

@apedroferreira
Copy link
Member

@apedroferreira @bharatkashyap I got some time to work on this. I was wondering what's the use of onToggleSidebar prop? Will it act as just a callback for whenever there is a state change for sidebarCollapsed state?

Yes, it should run when the menu button on the top-right is clicked and by default toggle the sidebarCollapsed state.

@vikasgurjar vikasgurjar linked a pull request Dec 6, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: layout This is the name of the generic UI component, not the React module! customization: logic Logic customizability scope: toolpad-core Abbreviated to "core" waiting for 👍 Waiting for upvotes
Projects
Status: Backlog
6 participants