Add a test and final fixes

mui · Jan 16, 2025 · 9acf943 · 9acf943
1 parent ee59d32
commit 9acf943
Show file tree

Hide file tree

Showing 7 changed files with 3,525 additions and 10,017 deletions.
diff --git a/docs/translations/api-docs/dashboard-layout/dashboard-layout.json b/docs/translations/api-docs/dashboard-layout/dashboard-layout.json
@@ -10,7 +10,7 @@
       "description": "Whether the sidebar should not be collapsible to a mini variant in desktop and tablet viewports."
     },
     "hideNavigation": {
-      "description": "Whether the navigation bar and menu icon should be hidden"
+      "description": "Whether the navigation bar and menu icon should be hidden."
     },
     "navigation": { "description": "Navigation definition for the dashboard." },
     "sidebarExpandedWidth": { "description": "Width of the sidebar when expanded." },

diff --git a/packages/toolpad-core/src/DashboardLayout/DashboardLayout.test.tsx b/packages/toolpad-core/src/DashboardLayout/DashboardLayout.test.tsx
@@ -207,13 +207,18 @@ describe('DashboardLayout', () => {
         icon: <ShoppingCartIcon />,
       },
       {
-        segment: 'dynamic',
+        segment: 'dynamic/override',
+        title: 'Dynamic Override',
+        icon: <BarChartIcon />,
+      },
+      {
+        segment: 'dynamicMoreOnly',
         title: 'Dynamic',
         icon: <BarChartIcon />,
         pattern: 'dynamic/:dynamicId',
       },
       {
-        segment: 'optional',
+        segment: 'optionalMoreOnly',
         title: 'Optional',
         pattern: 'optional{/:optionalId}?',
       },
@@ -268,11 +273,23 @@ describe('DashboardLayout', () => {
     expect(within(desktopNavigation).getByRole('link', { name: 'Dynamic' })).toHaveClass(
       'Mui-selected',
     );
+    expect(
+      within(desktopNavigation).getByRole('link', { name: 'Dynamic Override' }),
+    ).not.toHaveClass('Mui-selected');
     rerender(<AppWithPathname pathname="/dynamic/123/456" />);
     expect(within(desktopNavigation).getByRole('link', { name: 'Dynamic' })).not.toHaveClass(
       'Mui-selected',
     );
 
+    // Does not show multiple selected items if a dynamic segment is overridden by a more specific segment
+    rerender(<AppWithPathname pathname="/dynamic/override" />);
+    expect(within(desktopNavigation).getByRole('link', { name: 'Dynamic Override' })).toHaveClass(
+      'Mui-selected',
+    );
+    expect(within(desktopNavigation).getByRole('link', { name: 'Dynamic' })).not.toHaveClass(
+      'Mui-selected',
+    );
+
     rerender(<AppWithPathname pathname="/optional" />);
     expect(within(desktopNavigation).getByRole('link', { name: 'Optional' })).toHaveClass(
       'Mui-selected',

diff --git a/packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx b/packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx
@@ -80,15 +80,15 @@ export interface DashboardLayoutProps {
    */
   navigation?: Navigation;
   /**
-   * Whether the sidebar should not be collapsible to a mini variant in desktop and tablet viewports.
+   * Whether the sidebar should start collapsed in desktop size screens.
    * @default false
    */
-  disableCollapsibleSidebar?: boolean;
+  defaultSidebarCollapsed?: boolean;
   /**
-   * Whether the sidebar should start collapsed in desktop size screens.
+   * Whether the sidebar should not be collapsible to a mini variant in desktop and tablet viewports.
    * @default false
    */
-  defaultSidebarCollapsed?: boolean;
+  disableCollapsibleSidebar?: boolean;
   /**
    * Whether the navigation bar and menu icon should be hidden.
    * @default false
@@ -492,7 +492,7 @@ DashboardLayout.propTypes /* remove-proptypes */ = {
    */
   disableCollapsibleSidebar: PropTypes.bool,
   /**
-   * Whether the navigation bar and menu icon should be hidden
+   * Whether the navigation bar and menu icon should be hidden.
    * @default false
    */
   hideNavigation: PropTypes.bool,

diff --git a/packages/toolpad-core/src/DashboardLayout/DashboardSidebarSubNavigation.tsx b/packages/toolpad-core/src/DashboardLayout/DashboardSidebarSubNavigation.tsx
@@ -122,10 +122,10 @@ function DashboardSidebarSubNavigation({
     <List
       sx={{
         padding: 0,
-        mt: isPopover ? 0.5 : 0,
+        mt: isPopover && depth === 1 ? 0.5 : 0,
         mb: depth === 0 && !isPopover ? 4 : 0.5,
-        pl: isPopover ? 0 : 2 * depth,
-        minWidth: isPopover ? 240 : 'auto',
+        pl: (isPopover ? 1 : 2) * (isPopover ? depth - 1 : depth),
+        minWidth: isPopover && depth === 1 ? 240 : 'auto',
       }}
     >
       {subNavigation.map((navigationItem, navigationItemIndex) => {
@@ -189,7 +189,7 @@ function DashboardSidebarSubNavigation({
             fontSize: 18,
             position: 'absolute',
             top: '50%',
-            right: '-2px',
+            right: '-1px',
             transform: 'translateY(-50%) rotate(-90deg)',
           };
         } else if (!isMini && isFullyExpanded) {
@@ -331,6 +331,7 @@ function DashboardSidebarSubNavigation({
                   subNavigation={navigationItem.children}
                   depth={depth + 1}
                   onLinkClick={onLinkClick}
+                  isPopover={isPopover}
                 />
               </Collapse>
             ) : null}

diff --git a/packages/toolpad-core/src/shared/navigation.tsx b/packages/toolpad-core/src/shared/navigation.tsx
@@ -71,16 +71,15 @@ function buildItemLookup(navigation: Navigation) {
         console.warn(`Duplicate path in navigation: ${path}`);
       }
 
+      map.set(path, item);
+      if (item.pattern) {
+        const basePath = item.segment ? path.slice(0, -item.segment.length) : path;
+        map.set(pathToRegexp(basePath + item.pattern), item);
+      }
       if (item.children) {
         for (const child of item.children) {
           visit(child);
         }
-      } else {
-        map.set(path, item);
-        if (item.pattern) {
-          const basePath = item.segment ? path.slice(0, -item.segment.length) : path;
-          map.set(pathToRegexp(basePath + item.pattern), item);
-        }
       }
     }
   };

diff --git a/playground/vite/src/App.tsx b/playground/vite/src/App.tsx
@@ -13,44 +13,6 @@ const NAVIGATION: Navigation = [
   {
     title: 'Dashboard',
     icon: <DashboardIcon />,
-    children: [
-      {
-        title: 'Dashboard',
-        icon: <DashboardIcon />,
-      },
-      {
-        segment: 'orders',
-        title: 'Orders',
-        icon: <ShoppingCartIcon />,
-      },
-    ],
-  },
-  {
-    segment: 'orders',
-    title: 'Orders',
-    icon: <ShoppingCartIcon />,
-    children: [
-      {
-        title: 'Dashboard',
-        icon: <DashboardIcon />,
-      },
-      {
-        segment: 'orders',
-        title: 'Orders',
-        icon: <ShoppingCartIcon />,
-        children: [
-          {
-            title: 'Dashboard',
-            icon: <DashboardIcon />,
-          },
-          {
-            segment: 'orders',
-            title: 'Orders',
-            icon: <ShoppingCartIcon />,
-          },
-        ],
-      },
-    ],
   },
   {
     segment: 'orders',