[DashboardLayout] Add hideNavigation prop #4231

vikasgurjar · 2024-10-08T16:08:14Z

I've read and followed the contributing guide on how to create great pull requests.
I've updated the relevant documentation for any new or updated feature.
I've linked relevant GitHub issue with "Closes #".
I've added a visual demonstration in the form of a screenshot or video.

https://deploy-preview-4231--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout

…o feat/hide-navbar

apedroferreira

Looks perfect, thank you! And that's exactly what I meant with the demo, good job finding it! :)
Just a few really minor changes suggested.

Also for the docs changes you can run:
pnpm proptypes to generate proptypes the be read by the API documentation
pnpm docs:build:api to build the API docs
pnpm docs:typescript:formatted to format all the docs files based on the .tsx file

apedroferreira · 2024-10-08T17:16:06Z

packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx

-      >
-        {getDrawerContent(isDesktopMini, 'Desktop')}
-      </Drawer>
+      {!hideNavigation && <React.Fragment>


Can we write it like {!hideNavigation ? ... : null}? Just because we've been following that convention in the project.

apedroferreira · 2024-10-08T17:11:46Z

packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx

@@ -360,6 +360,11 @@ export interface DashboardLayoutProps {
    toolbarActions?: {};
    toolbarAccount?: AccountProps;
  };
+  /**
+  * If the navigation bar and button should to be hidden


Can we rephrase it to Whether the navigation bar and menu icon should be hidden?

apedroferreira · 2024-10-08T17:15:27Z

packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx

@@ -460,6 +465,11 @@ function DashboardLayout(props: DashboardLayoutProps) {

  const getMenuIcon = React.useCallback(
    (isExpanded: boolean) => {
+
+      if (hideNavigation) {


There's also a marginRight on the boxes around these icons that we should not have if they're not shown.
So instead on line 566 we can probably have something kind of like:

{!hideNavigation ? <React.Fragment><Box sx={{ mr: { sm: disableCollapsibleSidebar ? 0 : 1 }, display: { md: 'none' }, }} > {getMenuIcon(isMobileNavigationExpanded)} </Box> <Box sx={{ display: { xs: 'none', md: disableCollapsibleSidebar ? 'none' : 'block' }, mr: disableCollapsibleSidebar ? 0 : 1, }} > {getMenuIcon(isDesktopNavigationExpanded)} </Box></React.Fragment> : null}

apedroferreira · 2024-10-08T17:16:30Z

packages/toolpad-core/src/DashboardLayout/DashboardLayout.test.tsx

+    const desktopNavigation = screen.queryByRole('navigation', { name: 'Desktop' });
+    const navigationToggle = screen.queryByLabelText('Collapse menu');
+
+    // Expect that the navigation and toggle button is not rendered


Suggested change

// Expect that the navigation and toggle button is not rendered

// Expect that the navigation and toggle button are not rendered

apedroferreira · 2024-10-08T17:17:49Z

docs/data/toolpad/core/components/dashboard-layout/dashboard-layout.md

@@ -109,6 +109,12 @@ The layout sidebar is collapsible to a mini-drawer (with icons only) in desktop

 {{"demo": "DashboardLayoutNoMiniSidebar.js", "height": 400, "iframe": true}}

+### Hiding the sidebar
+
+The layout sidebar can be hidden if needed using the `hideNavigation` prop.


Suggested change

The layout sidebar can be hidden if needed using the `hideNavigation` prop.

The layout sidebar can be hidden if needed with the `hideNavigation` prop.

apedroferreira · 2024-10-08T17:29:27Z

docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutSidebarHidden.js

+import Typography from '@mui/material/Typography';
+import { createTheme } from '@mui/material/styles';
+
+const NAVIGATION = [];


We can just not pass this as it defaults to []

apedroferreira · 2024-10-08T17:30:09Z

docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutSidebarHidden.js

@@ -0,0 +1,88 @@
+import * as React from 'react';
+


docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutSidebarHidden.js

+
+  const [pathname, setPathname] = React.useState('/dashboard');
+
+  const router = React.useMemo(() => {


apedroferreira · 2024-10-08T17:32:34Z

docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutSidebarHidden.js

+  const demoWindow = window !== undefined ? window() : undefined;
+
+  return (
+    // preview-start


You can remove // preview-start and // preview-end here and the code preview will show only the DashboardLayout which is the most important information.

…o feat/hide-navbar

vikasgurjar · 2024-10-09T06:14:31Z

@apedroferreira I will need your help with the test_static build issue. Due to some conf mismatch I'm not able to fix the above issue. Can you guide on how to fix it?

apedroferreira · 2024-10-09T10:43:27Z

@apedroferreira I will need your help with the test_static build issue. Due to some conf mismatch I'm not able to fix the above issue. Can you guide on how to fix it?

Can you run pnpm docs:typescript:formatted from the project root? That should do it.
Everything looks great btw, thanks for the changes!

vikasgurjar · 2024-10-09T12:50:25Z

@apedroferreira I will need your help with the test_static build issue. Due to some conf mismatch I'm not able to fix the above issue. Can you guide on how to fix it?

Can you run pnpm docs:typescript:formatted from the project root? That should do it. Everything looks great btw, thanks for the changes!

I have done that multiple times now. I'm running it on windows, not really what's wrong here, can it be new line char issue or formatter issue?

apedroferreira · 2024-10-09T13:49:48Z

@apedroferreira I will need your help with the test_static build issue. Due to some conf mismatch I'm not able to fix the above issue. Can you guide on how to fix it?

Can you run pnpm docs:typescript:formatted from the project root? That should do it. Everything looks great btw, thanks for the changes!

I have done that multiple times now. I'm running it on windows, not really what's wrong here, can it be new line char issue or formatter issue?

I see, when I run pnpm docs:typescript:formatted (docs/scripts/formattedTSDemos script) on my Macbook there's a difference in the DashboardLayoutSidebarHidden.tsx.preview file compared to your current PR... Might be an OS thing, maybe line endings.

@mui/docs-infra @Janpot any knowledge on this issue? Is it an issue with the script? We're generally using the same script as https://github.com/mui/material-ui/blob/master/docs/scripts/formattedTSDemos.js.

Janpot · 2024-10-09T14:17:13Z

The line endings don't match. Maybe you hit "save" on the preview and the auto-formatter kicked in?

vikasgurjar · 2024-10-09T14:36:22Z

The line endings don't match. Maybe you hit "save" on the preview and the auto-formatter kicked in?

There was no need to hit the save. Seems like some other issue. Even your commit didn't help here

Janpot · 2024-10-09T15:30:09Z

It did fix it, CI just found another issue, still had to run pnpm prettier.

vikasgurjar · 2024-10-09T15:31:53Z

It did fix it, CI just found another issue, still had to run pnpm prettier.

Great, Thanks! I was setting up WSL in my windows machine, to try fixing this

apedroferreira · 2024-10-09T16:10:30Z

Ok, let me know if you ever find what the issue was!
Just added some small demo changes and will merge :)

vikasgurjar added 4 commits October 8, 2024 21:35

add hideNavigation prop

61eda69

add hideNavigation prop

5dae712

Merge branch 'master' into feat/hide-navbar

b12100c

Merge branch 'feat/hide-navbar' of github.com:vikasgurjar/toolpad int…

f93556e

…o feat/hide-navbar

vikasgurjar changed the title ~~Add hideNavigation prop~~ [core] Add hideNavigation prop Oct 8, 2024

vikasgurjar changed the title ~~[core] Add hideNavigation prop~~ [component: DashboardLayout] Add hideNavigation prop Oct 8, 2024

vikasgurjar changed the title ~~[component: DashboardLayout] Add hideNavigation prop~~ [DashboardLayout] Add hideNavigation prop Oct 8, 2024

apedroferreira added the feature: Components Button, input, table, etc. label Oct 8, 2024

apedroferreira requested changes Oct 8, 2024

View reviewed changes

apedroferreira reviewed Oct 8, 2024

View reviewed changes

docs/data/toolpad/core/components/dashboard-layout/DashboardLayoutSidebarHidden.js

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

const router = React.useMemo(() => {

This comment was marked as outdated.

Sign in to view

apedroferreira reviewed Oct 8, 2024

View reviewed changes

vikasgurjar requested a review from apedroferreira October 8, 2024 18:44

Address review comments

bb41a09

vikasgurjar force-pushed the feat/hide-navbar branch from 215a0d1 to bb41a09 Compare October 8, 2024 20:18

vikasgurjar added 4 commits October 9, 2024 11:03

Run pnpm docs:typescript:formatted

0717262

add hideNavigation prop

91e1e65

Address review comments

d3f7ee8

Run pnpm docs:typescript:formatted

d958950

vikasgurjar force-pushed the feat/hide-navbar branch from 0717262 to d958950 Compare October 9, 2024 05:34

vikasgurjar added 2 commits October 9, 2024 11:14

Fix formatting issues

001646b

Merge branch 'feat/hide-navbar' of github.com:vikasgurjar/toolpad int…

8a6c2f4

…o feat/hide-navbar

vikasgurjar added 2 commits October 9, 2024 18:07

Fix formatting issues

8d57c95

Merge branch 'master' into feat/hide-navbar

dcca231

Update DashboardLayoutSidebarHidden.tsx.preview

8b0866a

run prettier

ab66296

Small demo adjustments

c5d7e26

apedroferreira approved these changes Oct 9, 2024

View reviewed changes

apedroferreira merged commit 06ca8df into mui:master Oct 9, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DashboardLayout] Add hideNavigation prop #4231

[DashboardLayout] Add hideNavigation prop #4231

vikasgurjar commented Oct 8, 2024 •

edited by apedroferreira

Loading

apedroferreira left a comment •

edited

Loading

This comment was marked as resolved.

apedroferreira Oct 8, 2024 •

edited

Loading

apedroferreira Oct 8, 2024

apedroferreira Oct 8, 2024 •

edited

Loading

apedroferreira Oct 8, 2024

apedroferreira Oct 8, 2024

apedroferreira Oct 8, 2024 •

edited

Loading

apedroferreira Oct 8, 2024

This comment was marked as outdated.

apedroferreira Oct 8, 2024

vikasgurjar commented Oct 9, 2024

apedroferreira commented Oct 9, 2024 •

edited

Loading

vikasgurjar commented Oct 9, 2024

apedroferreira commented Oct 9, 2024 •

edited

Loading

Janpot commented Oct 9, 2024

vikasgurjar commented Oct 9, 2024

Janpot commented Oct 9, 2024

vikasgurjar commented Oct 9, 2024

apedroferreira commented Oct 9, 2024 •

edited

Loading

	// Expect that the navigation and toggle button is not rendered
	// Expect that the navigation and toggle button are not rendered

	The layout sidebar can be hidden if needed using the `hideNavigation` prop.
	The layout sidebar can be hidden if needed with the `hideNavigation` prop.


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

		const router = React.useMemo(() => {

[DashboardLayout] Add hideNavigation prop #4231

[DashboardLayout] Add hideNavigation prop #4231

Conversation

vikasgurjar commented Oct 8, 2024 • edited by apedroferreira Loading

apedroferreira left a comment • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

apedroferreira Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

apedroferreira Oct 8, 2024

Choose a reason for hiding this comment

apedroferreira Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

apedroferreira Oct 8, 2024

Choose a reason for hiding this comment

apedroferreira Oct 8, 2024

Choose a reason for hiding this comment

apedroferreira Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

apedroferreira Oct 8, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

apedroferreira Oct 8, 2024

Choose a reason for hiding this comment

vikasgurjar commented Oct 9, 2024

apedroferreira commented Oct 9, 2024 • edited Loading

vikasgurjar commented Oct 9, 2024

apedroferreira commented Oct 9, 2024 • edited Loading

Janpot commented Oct 9, 2024

vikasgurjar commented Oct 9, 2024

Janpot commented Oct 9, 2024

vikasgurjar commented Oct 9, 2024

apedroferreira commented Oct 9, 2024 • edited Loading

vikasgurjar commented Oct 8, 2024 •

edited by apedroferreira

Loading

apedroferreira left a comment •

edited

Loading

apedroferreira Oct 8, 2024 •

edited

Loading

apedroferreira Oct 8, 2024 •

edited

Loading

apedroferreira Oct 8, 2024 •

edited

Loading

apedroferreira commented Oct 9, 2024 •

edited

Loading

apedroferreira commented Oct 9, 2024 •

edited

Loading

apedroferreira commented Oct 9, 2024 •

edited

Loading