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

[core] Agnostic DashboardLayout component #3831

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion docs/pages/toolpad/core/api/dashboard-layout.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
{
"props": { "children": { "type": { "name": "node" }, "required": true } },
"props": {
"children": { "type": { "name": "node" }, "required": true },
"branding": {
"type": { "name": "shape", "description": "{ logo?: node, title?: string }" },
"default": "null"
},
"navigation": {
"type": {
"name": "arrayOf",
"description": "Array&lt;{ children?: Array&lt;object<br>&#124;&nbsp;{ kind: 'header', title: string }<br>&#124;&nbsp;{ kind: 'divider' }&gt;, icon?: node, kind?: 'page', segment: string, title?: string }<br>&#124;&nbsp;{ kind: 'header', title: string }<br>&#124;&nbsp;{ kind: 'divider' }&gt;"
},
"default": "[]"
},
"paletteMode": { "type": { "name": "enum", "description": "'dark'<br>&#124;&nbsp;'light'" } },
"setPaletteMode": { "type": { "name": "func" } },
"window": { "type": { "name": "object" }, "default": "window" }
},
"name": "DashboardLayout",
"imports": [
"import { DashboardLayout } from '@toolpad-core/DashboardLayout';",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
{
"componentDescription": "",
"propDescriptions": { "children": { "description": "The content of the dashboard." } },
"propDescriptions": {
"branding": { "description": "Branding options for the layout." },
"children": { "description": "The content of the dashboard." },
"navigation": { "description": "Navigation definition for the layout." },
"paletteMode": { "description": "Active palette mode in theme." },
"setPaletteMode": { "description": "Function to run when the theme switcher is toggled." },
"window": {
"description": "The window where the layout is rendered. This is needed when rendering the layout inside an iframe, for example."
}
},
"classDescriptions": {}
}
119 changes: 109 additions & 10 deletions packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import * as React from 'react';
import PropTypes from 'prop-types';
import { styled, useTheme } from '@mui/material';
import { PaletteMode, styled, useTheme } from '@mui/material';
import MuiAppBar from '@mui/material/AppBar';
import Box from '@mui/material/Box';
import Collapse from '@mui/material/Collapse';
Expand Down Expand Up @@ -34,7 +34,7 @@ import {
RouterContext,
WindowContext,
} from '../shared/context';
import type { Navigation, NavigationPageItem } from '../AppProvider';
import type { AppProviderProps, Navigation, NavigationPageItem } from '../AppProvider';
import { ToolpadLogo } from './ToolpadLogo';
import { getItemTitle, isPageItem } from '../shared/navigation';

Expand Down Expand Up @@ -78,11 +78,20 @@ const NavigationListItemButton = styled(ListItemButton)(({ theme }) => ({
},
}));

function ThemeSwitcher() {
interface ThemeSwitcherProps {
value?: PaletteMode;
onChange?: (mode: PaletteMode) => void;
}

function ThemeSwitcher({ value, onChange }: ThemeSwitcherProps) {
const isSsr = useSsr();
const theme = useTheme();

const { paletteMode, setPaletteMode, isDualTheme } = React.useContext(PaletteModeContext);
const paletteModeContext = React.useContext(PaletteModeContext);

const paletteMode = value ?? paletteModeContext.paletteMode;
const setPaletteMode = onChange ?? paletteModeContext.setPaletteMode;
const isDualTheme = !!onChange ?? paletteModeContext.isDualTheme;

const toggleMode = React.useCallback(() => {
setPaletteMode(paletteMode === 'dark' ? 'light' : 'dark');
Expand Down Expand Up @@ -301,6 +310,30 @@ export interface DashboardLayoutProps {
* The content of the dashboard.
*/
children: React.ReactNode;
/**
* Branding options for the layout.
* @default null
*/
branding?: AppProviderProps['branding'];
/**
* Navigation definition for the layout.
* @default []
*/
navigation?: AppProviderProps['navigation'];
/**
* Active palette mode in theme.
*/
paletteMode?: PaletteMode;
Copy link
Member

@Janpot Janpot Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe the state for paletteMode and setPaletteMode (we use a different naming convention for controlled props anyway) should be owned by DashboardLayout. It's owned by the theme and if the user wants to manipulate it useColorScheme should be used. For the classic theme, I believe we should create a useColorScheme shim (i.e. rewrite useStandardPaletteMode with the same signature as useColorScheme and export from the library).

Copy link
Member Author

@apedroferreira apedroferreira Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it was just a first idea and I wasn't sure about the names for these 2 props either.
We can go with that, the only disadvantage I guess is that it will only work with MUI theming (nevermind, the theme is strongly connected to the component anyway so this should be more automatic, sounds good!)

Copy link
Member Author

@apedroferreira apedroferreira Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking a bit more into this and not sure about the approach to take.

  1. User is using DashboardLayout inside an AppProvider with any of the 3 theming methods we support (https://mui.com/toolpad/core/react-app-provider/#theming) => theme switcher already works
  2. User uses DashboardLayout without AppProvider around it => we can make things work automatically if they're using a CSS vars theme by simply using the methods from useColorScheme, otherwise if they're using a standard theme it can only have one mode unless they're managing the active theme state themselves somehow, in which case we can't support it automatically?

About the shim, what scenario are we trying to solve with it? For internal components we have this context https://github.com/mui/mui-toolpad/blob/ad9bddb719a81cd113b73e82d304ae5ead1a0e8c/packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx#L81

Should I make an abstraction around the context as a hook that users can import called useColorScheme?

Copy link
Member

@Janpot Janpot Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we could make it a dumb controlled property. But name it colorScheme, onColorSchemeChange to match naming of the cssvar theme, and also we always use onXyzChange for controlled prop xyz, not setXyz

/**
* Function to run when the theme switcher is toggled.
*/
setPaletteMode?: (theme: PaletteMode) => void;
/**
* The window where the layout is rendered.
* This is needed when rendering the layout inside an iframe, for example.
* @default window
*/
window?: AppProviderProps['window'];
}

/**
Expand All @@ -314,11 +347,22 @@ export interface DashboardLayoutProps {
* - [DashboardLayout API](https://mui.com/toolpad/core/api/dashboard-layout)
*/
function DashboardLayout(props: DashboardLayoutProps) {
const { children } = props;

const branding = React.useContext(BrandingContext);
const navigation = React.useContext(NavigationContext);
const appWindow = React.useContext(WindowContext);
const {
children,
paletteMode,
setPaletteMode,
branding: brandingProp,
navigation: navigationProp,
window: windowProp,
} = props;

const brandingContext = React.useContext(BrandingContext);
const navigationContext = React.useContext(NavigationContext);
const windowContext = React.useContext(WindowContext);

const branding = brandingProp ?? brandingContext;
const navigation = navigationProp ?? navigationContext;
const appWindow = windowProp ?? windowContext;

const [isMobileNavigationOpen, setIsMobileNavigationOpen] = React.useState(false);

Expand Down Expand Up @@ -396,7 +440,7 @@ function DashboardLayout(props: DashboardLayoutProps) {
</a>
</Box>
<Box sx={{ flexGrow: 1 }} />
<ThemeSwitcher />
<ThemeSwitcher value={paletteMode} onChange={setPaletteMode} />
</Toolbar>
</AppBar>
<Drawer
Expand Down Expand Up @@ -449,10 +493,65 @@ DashboardLayout.propTypes /* remove-proptypes */ = {
// │ These PropTypes are generated from the TypeScript type definitions. │
// │ To update them, edit the TypeScript types and run `pnpm proptypes`. │
// └─────────────────────────────────────────────────────────────────────┘
/**
* Branding options for the layout.
* @default null
*/
branding: PropTypes.shape({
logo: PropTypes.node,
title: PropTypes.string,
}),
/**
* The content of the dashboard.
*/
children: PropTypes.node,
/**
* Navigation definition for the layout.
* @default []
*/
navigation: PropTypes.arrayOf(
PropTypes.oneOfType([
PropTypes.shape({
children: PropTypes.arrayOf(
PropTypes.oneOfType([
PropTypes.object,
PropTypes.shape({
kind: PropTypes.oneOf(['header']).isRequired,
title: PropTypes.string.isRequired,
}),
PropTypes.shape({
kind: PropTypes.oneOf(['divider']).isRequired,
}),
]).isRequired,
),
icon: PropTypes.node,
kind: PropTypes.oneOf(['page']),
segment: PropTypes.string.isRequired,
title: PropTypes.string,
}),
PropTypes.shape({
kind: PropTypes.oneOf(['header']).isRequired,
title: PropTypes.string.isRequired,
}),
PropTypes.shape({
kind: PropTypes.oneOf(['divider']).isRequired,
}),
]).isRequired,
),
/**
* Active palette mode in theme.
*/
paletteMode: PropTypes.oneOf(['dark', 'light']),
/**
* Function to run when the theme switcher is toggled.
*/
setPaletteMode: PropTypes.func,
/**
* The window where the layout is rendered.
* This is needed when rendering the layout inside an iframe, for example.
* @default window
*/
window: PropTypes.object,
} as any;

export { DashboardLayout };
4 changes: 2 additions & 2 deletions playground/nextjs/src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ const NAVIGATION: Navigation = [
title: 'Main items',
},
{
slug: '',
segment: '',
title: 'Dashboard',
icon: <DashboardIcon />,
},
{
slug: 'orders',
segment: 'orders',
title: 'Orders',
icon: <ShoppingCartIcon />,
},
Expand Down
Loading