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.

Sign up for GitHub

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

Improve DashboardLayout navigation docs #3864

Merged
merged 5 commits into from
Aug 5, 2024
Merged

Improve DashboardLayout navigation docs #3864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d34cc79
Improve DashboardLayout docs
apedroferreira Jul 30, 2024
8a9e976
Adjustments
apedroferreira Jul 30, 2024
7f09562
Fix initially expanded navigation items
apedroferreira Jul 31, 2024
268f3d2
Switch things a bit in one demo
apedroferreira Aug 1, 2024
8c39c58
Merge remote-tracking branch 'upstream/master' into dashboard-layout-…
apedroferreira Aug 5, 2024
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
2 changes: 1 addition & 1 deletion docs/data/toolpad/core/components/app-provider/AppProviderBasic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ DemoPageContent.propTypes = {
function AppProviderBasic(props) {
const { window } = props;

const [pathname, setPathname] = React.useState('page');
const [pathname, setPathname] = React.useState('/page');

const router = React.useMemo(() => {
return {
Expand Down
2 changes: 1 addition & 1 deletion docs/data/toolpad/core/components/app-provider/AppProviderBasic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ interface DemoProps {
export default function AppProviderBasic(props: DemoProps) {
const { window } = props;

const [pathname, setPathname] = React.useState('page');
const [pathname, setPathname] = React.useState('/page');

const router = React.useMemo<Router>(() => {
return {
Expand Down
6 changes: 3 additions & 3 deletions docs/data/toolpad/core/components/app-provider/AppProviderTheme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ const NAVIGATION = [
title: 'Main items',
},
{
segment: '/page',
segment: 'page',
title: 'Page',
icon: <DashboardIcon />,
},
{
segment: '/page-2',
segment: 'page-2',
title: 'Page 2',
icon: <TimelineIcon />,
},
Expand Down Expand Up @@ -78,7 +78,7 @@ DemoPageContent.propTypes = {
function AppProviderTheme(props) {
const { window } = props;

const [pathname, setPathname] = React.useState('page');
const [pathname, setPathname] = React.useState('/page');

const router = React.useMemo(() => {
return {
Expand Down
6 changes: 3 additions & 3 deletions docs/data/toolpad/core/components/app-provider/AppProviderTheme.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ const NAVIGATION: Navigation = [
title: 'Main items',
},
{
segment: '/page',
segment: 'page',
title: 'Page',
icon: <DashboardIcon />,
},
{
segment: '/page-2',
segment: 'page-2',
title: 'Page 2',
icon: <TimelineIcon />,
},
Expand Down Expand Up @@ -82,7 +82,7 @@ interface DemoProps {
export default function AppProviderTheme(props: DemoProps) {
const { window } = props;

const [pathname, setPathname] = React.useState('page');
const [pathname, setPathname] = React.useState('/page');

const router = React.useMemo<Router>(() => {
return {
Expand Down
2 changes: 1 addition & 1 deletion docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutBasic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ DemoPageContent.propTypes = {
function DashboardLayoutBasic(props) {
const { window } = props;

const [pathname, setPathname] = React.useState('dashboard');
const [pathname, setPathname] = React.useState('/dashboard');

const router = React.useMemo(() => {
return {
Expand Down
2 changes: 1 addition & 1 deletion docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutBasic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ interface DemoProps {
export default function DashboardLayoutBasic(props: DemoProps) {
const { window } = props;

const [pathname, setPathname] = React.useState('dashboard');
const [pathname, setPathname] = React.useState('/dashboard');

const router = React.useMemo<Router>(() => {
return {
Expand Down
2 changes: 1 addition & 1 deletion docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutBranding.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ DemoPageContent.propTypes = {
function DashboardLayoutBranding(props) {
const { window } = props;

const [pathname, setPathname] = React.useState('dashboard');
const [pathname, setPathname] = React.useState('/dashboard');

const router = React.useMemo(() => {
return {
Expand Down
2 changes: 1 addition & 1 deletion docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutBranding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ interface DemoProps {
export default function DashboardLayoutBranding(props: DemoProps) {
const { window } = props;

const [pathname, setPathname] = React.useState('dashboard');
const [pathname, setPathname] = React.useState('/dashboard');

const router = React.useMemo<Router>(() => {
return {
Expand Down
86 changes: 43 additions & 43 deletions docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutNavigationActions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,25 @@ DemoPageContent.propTypes = {
pathname: PropTypes.string.isRequired,
};

const CALLS_NAVIGATION = [
{
segment: '/made',
title: 'Made',
icon: <CallMadeIcon />,
action: <Chip label={12} color="success" size="small" />,
},
{
segment: '/received',
title: 'Received',
icon: <CallReceivedIcon />,
action: <Chip label={4} color="error" size="small" />,
},
];

function DashboardLayoutNavigationActions(props) {
const { window } = props;

const [pathname, setPathname] = React.useState('contacts');
const [pathname, setPathname] = React.useState('/contacts');

const router = React.useMemo(() => {
return {
Expand Down Expand Up @@ -78,60 +93,45 @@ function DashboardLayoutNavigationActions(props) {
// Remove this const when copying and pasting into your project.
const demoWindow = window !== undefined ? window() : undefined;

const popoverMenuAction = (
Copy link
Member

@bharatkashyap bharatkashyap Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would recommend using tabs on this demo since it becomes very long to read through if expanded.

Edit: In fact, most of the demos on this page could do with separating the style, and additional components into separate files for ease of reading when expanded. You could perhaps benefit from some code sharing as well.

Copy link
Member

Choose a reason for hiding this comment

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

You could perhaps benefit from some code sharing as well.

I'm not sure demos should do much code sharing. Ideally they're self-contained units. And as simple as possible. i.e. as simple as can be to convey the aspect that is being demoed.

Copy link
Member

Choose a reason for hiding this comment

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

Agree with ideally them being self-contained, but perhaps we need to optimise for readability if they grow too long?

Copy link
Member

Choose a reason for hiding this comment

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

Advantage of single file demoes is that they can be simply copy+pasted from in one action (explains some of the popularity of shadcn). Splitting them across multiple files requires more context switching on behalf of the user to get the example integrated in their own code base.
I disagree that splitting up in multiple files increases readability, to me it mostly creates extra friction to navigate the code. Instead we should think more in terms of "what can we remove from the code to get to the essence?" and focus on that to increase readability.
I believe the feature of multiple files in demoes should be used very sparingly. i.e. only when the split is mandated by the runtime (_document.js, _app.js) or perhaps to hold a demo dataset.

Copy link
Member

Choose a reason for hiding this comment

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

I think if we're optimising for copy and paste-ability, we should ideally: advertise it, and make the demo code blocks much larger. Given the size of our demo code blocks, I think the sense that this is meant to be copy and pasted doesn't immediately come out. (For instance, https://ui.shadcn.com/blocks as a benchmark has "Copy and Paste" as a verb in the header, and a very large code block).

Shadcn Blocks Us
Screenshot 2024-08-01 at 5 58 57 PM Screenshot 2024-08-01 at 5 58 26 PM

One possible solution is to come up with a flag that can render the MUI Docs Layout with navigation hidden, so that code can be much wider.

Within the context of our docs, I think that longer code can become harder for skimming. This is my opinion, okay to drop this if single files being okay for readability is the more popular opinion 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

About this in specific, I tried to make the preview content as short as possible while still showcasing each scenario, but when intentionally expanded I feel like it's ok for things to be longer as long as most of the code is relevant to the example.

Copy link
Member Author

Choose a reason for hiding this comment

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

One possible solution is to come up with a flag that can render the MUI Docs Layout with navigation hidden, so that code can be much wider.

Yeah, seems like in some of these pages we might benefit from taking the full-width of the page as shadcn does, but so far I guess it hasn't been super necessary.

Copy link
Member

Choose a reason for hiding this comment

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

I think if we're optimising for copy and paste-ability, we should ideally: advertise it, and make the demo code blocks much larger.

We don't optimize for copy and paste-ability in an absolute sense. Definitely not the demo previews. They are too small for most demos to achieve that, they are to help the user decide whether the particular demo from a code point of view is going to be applicable to their problem. You could say the previews are mostly optimized for scanneability, that's why they should be short, to the point and not scrollable. If such a preview is not possible, then there should be likely just no preview. The expanded version should be more optimized for copy-pasteability.

shadcn is different a different case, they basically act as a snippets library. Our demos are not snippets, we show the integration of our libraries. When I copy+paste from our docs I usually only copy portions, not the whole demo.

<React.Fragment>
<IconButton aria-describedby={popoverId} onClick={handlePopoverButtonClick}>
<MoreHorizIcon />
</IconButton>
<Menu
id={popoverId}
open={isPopoverOpen}
anchorEl={popoverAnchorEl}
onClose={handlePopoverClose}
anchorOrigin={{
vertical: 'bottom',
horizontal: 'right',
}}
disableAutoFocus
disableAutoFocusItem
>
<MenuItem onClick={handlePopoverClose}>New call</MenuItem>
<MenuItem onClick={handlePopoverClose}>Mark all as read</MenuItem>
</Menu>
</React.Fragment>
);

return (
// preview-start
<AppProvider
navigation={[
{
segment: '/contacts',
segment: 'contacts',
title: 'Contacts',
icon: <PersonIcon />,
action: <Chip label={7} color="primary" size="small" />,
},
{
segment: '/calls',
segment: 'calls',
title: 'Calls',
icon: <CallIcon />,
action: (
<React.Fragment>
<IconButton
aria-describedby={popoverId}
onClick={handlePopoverButtonClick}
sx={{ ml: 1 }}
>
<MoreHorizIcon />
</IconButton>
<Menu
id={popoverId}
open={isPopoverOpen}
anchorEl={popoverAnchorEl}
onClose={handlePopoverClose}
anchorOrigin={{
vertical: 'bottom',
horizontal: 'right',
}}
disableAutoFocus
disableAutoFocusItem
>
<MenuItem onClick={handlePopoverClose}>New call</MenuItem>
<MenuItem onClick={handlePopoverClose}>Mark all as read</MenuItem>
</Menu>
</React.Fragment>
),
children: [
{
segment: '/made',
title: 'Made',
icon: <CallMadeIcon />,
action: <Chip label={12} color="success" size="small" />,
},
{
segment: '/received',
title: 'Received',
icon: <CallReceivedIcon />,
action: <Chip label={4} color="error" size="small" />,
},
],
action: popoverMenuAction,
children: CALLS_NAVIGATION,
},
]}
router={router}
Expand Down
87 changes: 44 additions & 43 deletions docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutNavigationActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import CallMadeIcon from '@mui/icons-material/CallMade';
import CallReceivedIcon from '@mui/icons-material/CallReceived';
import { AppProvider, Router } from '@toolpad/core/AppProvider';
import { DashboardLayout } from '@toolpad/core/DashboardLayout';
import type { Navigation } from '@toolpad/core';

const demoTheme = extendTheme({
breakpoints: {
Expand Down Expand Up @@ -42,6 +43,21 @@ function DemoPageContent({ pathname }: { pathname: string }) {
);
}

const CALLS_NAVIGATION: Navigation = [
{
segment: '/made',
title: 'Made',
icon: <CallMadeIcon />,
action: <Chip label={12} color="success" size="small" />,
},
{
segment: '/received',
title: 'Received',
icon: <CallReceivedIcon />,
action: <Chip label={4} color="error" size="small" />,
},
];

interface DemoProps {
/**
* Injected by the documentation to work in an iframe.
Expand All @@ -53,7 +69,7 @@ interface DemoProps {
export default function DashboardLayoutNavigationActions(props: DemoProps) {
const { window } = props;

const [pathname, setPathname] = React.useState('contacts');
const [pathname, setPathname] = React.useState('/contacts');

const router = React.useMemo<Router>(() => {
return {
Expand Down Expand Up @@ -82,60 +98,45 @@ export default function DashboardLayoutNavigationActions(props: DemoProps) {
// Remove this const when copying and pasting into your project.
const demoWindow = window !== undefined ? window() : undefined;

const popoverMenuAction = (
<React.Fragment>
<IconButton aria-describedby={popoverId} onClick={handlePopoverButtonClick}>
<MoreHorizIcon />
</IconButton>
<Menu
id={popoverId}
open={isPopoverOpen}
anchorEl={popoverAnchorEl}
onClose={handlePopoverClose}
anchorOrigin={{
vertical: 'bottom',
horizontal: 'right',
}}
disableAutoFocus
disableAutoFocusItem
>
<MenuItem onClick={handlePopoverClose}>New call</MenuItem>
<MenuItem onClick={handlePopoverClose}>Mark all as read</MenuItem>
</Menu>
</React.Fragment>
);

return (
// preview-start
<AppProvider
navigation={[
{
segment: '/contacts',
segment: 'contacts',
title: 'Contacts',
icon: <PersonIcon />,
action: <Chip label={7} color="primary" size="small" />,
},
{
segment: '/calls',
segment: 'calls',
title: 'Calls',
icon: <CallIcon />,
action: (
<React.Fragment>
<IconButton
aria-describedby={popoverId}
onClick={handlePopoverButtonClick}
sx={{ ml: 1 }}
>
<MoreHorizIcon />
</IconButton>
<Menu
id={popoverId}
open={isPopoverOpen}
anchorEl={popoverAnchorEl}
onClose={handlePopoverClose}
anchorOrigin={{
vertical: 'bottom',
horizontal: 'right',
}}
disableAutoFocus
disableAutoFocusItem
>
<MenuItem onClick={handlePopoverClose}>New call</MenuItem>
<MenuItem onClick={handlePopoverClose}>Mark all as read</MenuItem>
</Menu>
</React.Fragment>
),
children: [
{
segment: '/made',
title: 'Made',
icon: <CallMadeIcon />,
action: <Chip label={12} color="success" size="small" />,
},
{
segment: '/received',
title: 'Received',
icon: <CallReceivedIcon />,
action: <Chip label={4} color="error" size="small" />,
},
],
action: popoverMenuAction,
children: CALLS_NAVIGATION,
},
]}
router={router}
Expand Down
Loading
Loading