[DashboardLayout] Show nested navigation in mini-drawer #4276

apedroferreira · 2024-10-18T17:35:45Z

Initially was just trying to refactor the sidebar navigation in DashboardLayout to use the same navigation cache and utilities as the PageContainer component.

But this required adding nested navigation to the sidebar for consistency with the warnings we show when there are duplicated navigation items, because the navigation links should match in all variants of the sidebar:

Screen.Recording.2025-03-13.at.18.08.48.mov

Also improved selected navigation item tests to cover an extra case.

Inspiration:

https://minimals.cc/dashboard

douglaszaltron · 2024-11-08T20:06:08Z

Nice!

…cted-item

mui-bot · 2025-01-16T11:53:16Z

Netlify deploy preview

https://deploy-preview-4276--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against 145f76b

…cted-item

bharatkashyap

Looks really good! Great addition to the feature-set. Some small comments on code.

bharatkashyap · 2025-01-17T08:55:39Z

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

            sx={{
              py: 0,
              px: 1,
              overflowX: 'hidden',
            }}
          >
            <NavigationListItemButton
-              selected={isSelected && (!navigationItem.children || isMini)}
+              selected={


Might be helpful to add some comments to explain the thinking behind the really long ternaries

bharatkashyap · 2025-01-17T08:57:36Z

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

-
-        if (isSelected && !selectedItemId) {
-          selectedItemId = navigationItemId;
+        let nestedNavigationCollapseSx: SxProps<Theme> = { display: 'none' };


Seems like this could be a useMemo to prevent having to recalculate on every render

bharatkashyap · 2025-01-17T08:59:17Z

packages/toolpad-core/src/shared/navigation.tsx

@@ -67,7 +25,9 @@ function buildItemToPathMap(navigation: Navigation): Map<NavigationPageItem, str

  const visit = (item: NavigationItem, base: string) => {
    if (isPageItem(item)) {
-      const path = `${base}${item.segment ? `/${item.segment}` : ''}` || '/';
+      const path =
+        `${base.startsWith('/') ? base : `/${base}`}${base && base !== '/' && item.segment ? '/' : ''}${item.segment || ''}` ||


Similar comment about comments being helpful on really long ternaries. Even though this one seems relatively straightforward, a comment helps bolt down the original intent instead of having to guess/derive

bharatkashyap · 2025-01-17T09:06:31Z

packages/toolpad-core/src/shared/navigation.tsx

+        return hasSelectedNavigationChildren(navigation, nestedItem, activePagePath);
+      }
+
+      return activePagePath === getItemPath(navigation, nestedItem);


To be sure, does this function work in cases where the nested navigation items are supplied via regex patterns? I might be missing something, but seems like the earlier function was checking for a navigationItem.pattern case and testing the path against the regex pattern in case it existed but I can't seem to find an analogous case here

I believe the activePagePath that goes here as a parameter already accounts for pattern matching with the logic in the useActivePage hook. I'll check again just to be sure.

So the behavior should be that pattern matching in children items works, but only under the segments in the parent items. If the parent also has a pattern, that pattern in the parent will probably be ignored in these cases and I think that's probably the way it should work.

bharatkashyap · 2025-01-17T09:09:05Z

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

+                <Box
+                  sx={{
+                    position: 'fixed',
+                    left: 62,


Is there no avoidable way to hard code the left and pl values here? I could be wrong but this might create a future bug if we make the size of the mini sidebar configurable - perhaps we can link these values to that constant (if indeed they derive from it)

royhuntley · 2025-02-06T15:00:14Z

Related a little to this item...

If you have a navigation structure that a PAGE item also has children, when the navigation drawer is open (Wide), you cant actually navigate to the page, because clicking on it expands to make the children visible. However, when the navigation drawer is closed (Narrow), you get taken to the page when you click on the item.

{
segment: 'test',
title: 'Test',
children: [ {
segment: 'orders',
title: 'Orders',
icon: ,
},
{
kind: 'divider'
},
{
segment: 'orders',
title: 'Orders',
icon: ,
},

apedroferreira · 2025-02-07T12:31:17Z

Related a little to this item...

If you have a navigation structure that a PAGE item also has children, when the navigation drawer is open (Wide), you cant actually navigate to the page, because clicking on it expands to make the children visible. However, when the navigation drawer is closed (Narrow), you get taken to the page when you click on the item.

{ segment: 'test', title: 'Test', children: [ { segment: 'orders', title: 'Orders', icon: , }, { kind: 'divider' }, { segment: 'orders', title: 'Orders', icon: , },

Hey, I know, that behavior will change once this PR is merged.

…cted-item

apedroferreira · 2025-03-13T18:41:18Z

Didn't want to abandon this old PR so addressed review suggestions + improved mini bar design as the previous one here wasn't great.
Updated the video above with the new mini bar design.

royhuntley · 2025-03-13T21:09:01Z

What needs to happen to get this change released - its good work - the menu bar is so much more functional when minimised!On 13 Mar 2025, at 18:41, Pedro Ferreira ***@***.***> wrote: Didn't want to abandon this old PR so fixed what made sense from the review suggestions + improved mini bar design as the previous one here wasn't great. Updated the video above with the new mini bar design.—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***> apedroferreira left a comment (mui/toolpad#4276) Didn't want to abandon this old PR so fixed what made sense from the review suggestions + improved mini bar design as the previous one here wasn't great. Updated the video above with the new mini bar design. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>

apedroferreira · 2025-03-18T17:10:45Z

I'll merge this one soon if it's good with everyone!

bharatkashyap

Looks good!

DaliborHomola · 2025-03-19T09:02:26Z

I'll merge this one soon if it's good with everyone!

Yes, great

Use navigation utils for sidebar selected item

e3b6f68

apedroferreira added the enhancement This is not a bug, nor a new feature label Oct 18, 2024

apedroferreira self-assigned this Oct 18, 2024

apedroferreira changed the title ~~Use navigation utils for sidebar selected item~~ [DashboardLayout] Refactor sidebar selected item logic Oct 18, 2024

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 18, 2024

apedroferreira added 6 commits October 22, 2024 17:17

Change props order

6adbbd1

Missing full stop

10670e9

Fix false warnings

998c968

Temporary fix for mini-drawer selected item

c0165b9

Mini-drawer popovers without animations

3d05f7c

Remove Grow component

4a82b51

apedroferreira changed the title ~~[DashboardLayout] Refactor sidebar selected item logic~~ [DashboardLayout] Show nested navigation in mini-drawer Oct 28, 2024

oliviertassinari added component: DashboardLayout component: layout This is the name of the generic UI component, not the React module! labels Nov 8, 2024

apedroferreira added 2 commits November 8, 2024 17:21

Use animated popover for nested menus on hover

228d610

Show tooltips for folder items too

c26bb41

prakhargupta1 removed the component: DashboardLayout label Nov 11, 2024

smol refactor

8d6a960

This comment was marked as resolved.

Sign in to view

Merge remote-tracking branch 'upstream/master' into refactor-nav-sele…

ee59d32

…cted-item

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 16, 2025

apedroferreira added 2 commits January 16, 2025 13:59

Add a test and final fixes

9acf943

Merge remote-tracking branch 'upstream/master' into refactor-nav-sele…

98e83dc

…cted-item

apedroferreira marked this pull request as ready for review January 16, 2025 14:03

apedroferreira requested a review from a team January 16, 2025 14:03

bharatkashyap approved these changes Jan 17, 2025

View reviewed changes

Review suggestions + improve mini sidebar look

f4f8a35

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 13, 2025

Remove stuff from testing

17ee76f

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 13, 2025

Merge remote-tracking branch 'upstream/master' into refactor-nav-sele…

145f76b

…cted-item

apedroferreira requested a review from a team March 13, 2025 18:41

douglaszaltron approved these changes Mar 18, 2025

View reviewed changes

bharatkashyap approved these changes Mar 19, 2025

View reviewed changes

apedroferreira merged commit 2bf6a0b into mui:master Mar 19, 2025
15 checks passed

apedroferreira deleted the refactor-nav-selected-item branch March 19, 2025 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DashboardLayout] Show nested navigation in mini-drawer #4276

[DashboardLayout] Show nested navigation in mini-drawer #4276

apedroferreira commented Oct 18, 2024 •

edited

Loading

douglaszaltron commented Nov 8, 2024

This comment was marked as resolved.

mui-bot commented Jan 16, 2025 •

edited

Loading

bharatkashyap left a comment

bharatkashyap Jan 17, 2025

bharatkashyap Jan 17, 2025

bharatkashyap Jan 17, 2025

bharatkashyap Jan 17, 2025

apedroferreira Mar 12, 2025 •

edited

Loading

bharatkashyap Jan 17, 2025

royhuntley commented Feb 6, 2025

apedroferreira commented Feb 7, 2025

apedroferreira commented Mar 13, 2025 •

edited

Loading

royhuntley commented Mar 13, 2025 via email

apedroferreira commented Mar 18, 2025

bharatkashyap left a comment

DaliborHomola commented Mar 19, 2025

[DashboardLayout] Show nested navigation in mini-drawer #4276

[DashboardLayout] Show nested navigation in mini-drawer #4276

Conversation

apedroferreira commented Oct 18, 2024 • edited Loading

douglaszaltron commented Nov 8, 2024

This comment was marked as resolved.

mui-bot commented Jan 16, 2025 • edited Loading

Netlify deploy preview

bharatkashyap left a comment

Choose a reason for hiding this comment

bharatkashyap Jan 17, 2025

Choose a reason for hiding this comment

bharatkashyap Jan 17, 2025

Choose a reason for hiding this comment

bharatkashyap Jan 17, 2025

Choose a reason for hiding this comment

bharatkashyap Jan 17, 2025

Choose a reason for hiding this comment

apedroferreira Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

bharatkashyap Jan 17, 2025

Choose a reason for hiding this comment

royhuntley commented Feb 6, 2025

apedroferreira commented Feb 7, 2025

apedroferreira commented Mar 13, 2025 • edited Loading

royhuntley commented Mar 13, 2025 via email

apedroferreira commented Mar 18, 2025

bharatkashyap left a comment

Choose a reason for hiding this comment

DaliborHomola commented Mar 19, 2025

apedroferreira commented Oct 18, 2024 •

edited

Loading

mui-bot commented Jan 16, 2025 •

edited

Loading

apedroferreira Mar 12, 2025 •

edited

Loading

apedroferreira commented Mar 13, 2025 •

edited

Loading