[Stateful sidenav] Fix collapsed menu for panels with no landing pages (#195904)

packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx

@@ -114,7 +114,7 @@ describe('builds navigation tree', () => {
 
       const accordionToggleButton = await findByTestId(/nav-item-group1\s/);
       accordionToggleButton.click();
-      expect(navigateToUrl).not.toHaveBeenCalled();
+      expect(navigateToUrl).not.toHaveBeenCalled(); // Should not navigate to the href
       unmount();
     }
 
@@ -138,6 +138,85 @@ describe('builds navigation tree', () => {
     }
   });
 
+  test('should render panel opener groups as accordion when the sideNav is collapsed', async () => {
+    const panelOpenerNode: ChromeProjectNavigationNode = {
+      id: 'nestedGroup1',
+      title: 'Nested Group 1',
+      path: 'group1.nestedGroup1',
+      renderAs: 'panelOpener', // Should be converted to accordion when sideNav is collapsed
+      children: [
+        {
+          id: 'item1',
+          title: 'Item 1',
+          href: 'https://foo',
+          path: 'group1.item1',
+        },
+      ],
+    };
+
+    const nodes: ChromeProjectNavigationNode = {
+      id: 'group1',
+      title: 'Group 1',
+      path: 'group1',
+      children: [panelOpenerNode],
+    };
+
+    {
+      // Side nav is collapsed
+      const { queryAllByTestId, unmount } = renderNavigation({
+        navTreeDef: of({
+          body: [nodes],
+        }),
+        services: { isSideNavCollapsed: true },
+      });
+
+      const accordionButtonLabel = queryAllByTestId('accordionToggleBtn').map((c) => c.textContent);
+      expect(accordionButtonLabel).toEqual(['Group 1', 'Nested Group 1']); // 2 accordion buttons
+
+      unmount();
+    }
+
+    {
+      // Side nav is not collapsed
+      const { queryAllByTestId, unmount } = renderNavigation({
+        navTreeDef: of({
+          body: [nodes],
+        }),
+        services: { isSideNavCollapsed: false }, // No conversion to accordion
+      });
+
+      const accordionButtonLabel = queryAllByTestId('accordionToggleBtn').map((c) => c.textContent);
+
+      expect(accordionButtonLabel).toEqual(['Group 1']); // Only 1 accordion button (top level)
+      unmount();
+    }
+
+    {
+      // Panel opener with a link
+      const { queryAllByTestId, unmount } = renderNavigation({
+        navTreeDef: of({
+          body: [
+            {
+              ...nodes,
+              children: [
+                {
+                  ...panelOpenerNode,
+                  href: '/foo/bar', // Panel opener with a link should not be converted to accordion
+                },
+              ],
+            },
+          ],
+        }),
+        services: { isSideNavCollapsed: true }, // SideNav is collapsed
+      });
+
+      const accordionButtonLabel = queryAllByTestId('accordionToggleBtn').map((c) => c.textContent);
+
+      expect(accordionButtonLabel).toEqual(['Group 1']); // Only 1 accordion button (top level)
+      unmount();
+    }
+  });
+
   test('should track click event', async () => {
     const navigateToUrl = jest.fn();
     const reportEvent = jest.fn();

packages/shared-ux/chrome/navigation/__jest__/utils.tsx

@@ -34,7 +34,6 @@ export const getServicesMock = (): NavigationServices => {
   return {
     basePath,
     recentlyAccessed$,
-    navIsOpen: true,
     navigateToUrl,
     activeNodes$: of(activeNodes),
     isSideNavCollapsed: false,

packages/shared-ux/chrome/navigation/mocks/storybook.ts

@@ -14,15 +14,15 @@ import { EventTracker } from '../src/analytics';
 import { NavigationServices } from '../src/types';
 
 type Arguments = NavigationServices;
-export type Params = Pick<Arguments, 'navIsOpen' | 'recentlyAccessed$' | 'activeNodes$'>;
+export type Params = Pick<Arguments, 'isSideNavCollapsed' | 'recentlyAccessed$' | 'activeNodes$'>;
 
 export class StorybookMock extends AbstractStorybookMock<{}, NavigationServices> {
   propArguments = {};
 
   serviceArguments = {
-    navIsOpen: {
+    isSideNavCollapsed: {
       control: 'boolean',
-      defaultValue: true,
+      defaultValue: false,
     },
   };
 

packages/shared-ux/chrome/navigation/src/services.tsx

@@ -44,7 +44,6 @@ export const NavigationKibanaProvider: FC<PropsWithChildren<NavigationKibanaDepe
       basePath,
       recentlyAccessed$: chrome.recentlyAccessed.get$(),
       navigateToUrl,
-      navIsOpen: true,
       activeNodes$,
       isSideNavCollapsed,
       eventTracker: new EventTracker({ reportEvent: analytics.reportEvent }),

packages/shared-ux/chrome/navigation/src/types.ts

@@ -34,7 +34,6 @@ export type NavigateToUrlFn = ApplicationStart['navigateToUrl'];
 export interface NavigationServices {
   basePath: BasePathService;
   recentlyAccessed$: Observable<ChromeRecentlyAccessedHistoryItem[]>;
-  navIsOpen: boolean;
   navigateToUrl: NavigateToUrlFn;
   activeNodes$: Observable<ChromeProjectNavigationNode[][]>;
   isSideNavCollapsed: boolean;

packages/shared-ux/chrome/navigation/src/ui/components/navigation_section_ui.tsx

@@ -55,12 +55,48 @@ const itemIsVisible = (item: ChromeProjectNavigationNode) => {
   return false;
 };
 
-const getRenderAs = (navNode: ChromeProjectNavigationNode): RenderAs => {
+const getRenderAs = (
+  navNode: ChromeProjectNavigationNode,
+  { isSideNavCollapsed }: { isSideNavCollapsed: boolean }
+): RenderAs => {
+  if (isSideNavCollapsed && navNode.renderAs === 'panelOpener' && !nodeHasLink(navNode))
+    return 'accordion'; // When the side nav is collapsed, we render panel openers as accordions if they don't have a landing page
   if (navNode.renderAs) return navNode.renderAs;
   if (!navNode.children) return 'item';
   return DEFAULT_RENDER_AS;
 };
 
+const getSpaceBefore = (
+  navNode: ChromeProjectNavigationNode,
+  {
+    isSideNavCollapsed,
+    treeDepth,
+    parentNode,
+  }: { isSideNavCollapsed: boolean; treeDepth: number; parentNode?: ChromeProjectNavigationNode }
+): EuiThemeSize | null | undefined => {
+  const hasChildren = nodeHasChildren(navNode);
+  const isItem = navNode.renderAs === 'item';
+
+  if (navNode.spaceBefore === undefined && treeDepth === 1 && hasChildren && !isItem) {
+    // For groups at level 1 that don't have a space specified we default to add a "m"
+    // space. For all other groups, unless specified, there is no vertical space.
+    return DEFAULT_SPACE_BETWEEN_LEVEL_1_GROUPS;
+  }
+
+  if (
+    isSideNavCollapsed &&
+    navNode.renderAs === 'block' &&
+    !!navNode.title &&
+    parentNode?.renderAs === 'accordion'
+  ) {
+    // When the side nav is collapsed we control the spacing between groups inside accordions
+    // for consistency and don't allow custom spacing to be set.
+    return DEFAULT_SPACE_BETWEEN_LEVEL_1_GROUPS;
+  }
+
+  return navNode.spaceBefore;
+};
+
 const getTestSubj = (navNode: ChromeProjectNavigationNode, isActive = false): string => {
   const { id, path, deepLink } = navNode;
   return classnames(`nav-item`, `nav-item-${path}`, {
@@ -70,20 +106,33 @@ const getTestSubj = (navNode: ChromeProjectNavigationNode, isActive = false): st
   });
 };
 
-const serializeNavNode = (navNode: ChromeProjectNavigationNode) => {
+const serializeNavNode = (
+  navNode: ChromeProjectNavigationNode,
+  {
+    isSideNavCollapsed,
+    treeDepth,
+    parentNode,
+  }: { isSideNavCollapsed: boolean; treeDepth: number; parentNode?: ChromeProjectNavigationNode }
+) => {
   const serialized: ChromeProjectNavigationNode = {
     ...navNode,
-    children: navNode.children?.filter(itemIsVisible),
   };
 
-  serialized.renderAs = getRenderAs(serialized);
+  serialized.renderAs = getRenderAs(serialized, { isSideNavCollapsed });
+  serialized.spaceBefore = getSpaceBefore(serialized, {
+    isSideNavCollapsed,
+    treeDepth,
+    parentNode,
+  });
+  serialized.children = navNode.children?.filter(itemIsVisible).map((child) =>
+    serializeNavNode(child, {
+      isSideNavCollapsed,
+      treeDepth: treeDepth + 1,
+      parentNode: serialized,
+    })
+  );
 
-  return {
-    navNode: serialized,
-    hasChildren: nodeHasChildren(serialized),
-    hasLink: nodeHasLink(serialized),
-    isItem: serialized.renderAs === 'item',
-  };
+  return serialized;
 };
 
 const isEuiCollapsibleNavItemProps = (
@@ -95,41 +144,38 @@ const isEuiCollapsibleNavItemProps = (
 };
 
 const renderBlockTitle: (
-  navNode: ChromeProjectNavigationNode,
-  { spaceBefore }: { spaceBefore: EuiThemeSize | null }
-) => Required<EuiCollapsibleNavSubItemProps>['renderItem'] =
-  (navNode, { spaceBefore }) =>
-  () => {
-    const { title } = navNode;
-    const dataTestSubj = getTestSubj(navNode);
-    return (
-      <EuiTitle
-        size="xxxs"
-        className="eui-textTruncate"
-        data-test-subj={dataTestSubj}
-        css={({ euiTheme }: any) => {
-          return {
-            marginTop: spaceBefore ? euiTheme.size[spaceBefore] : undefined,
-            paddingBlock: euiTheme.size.xs,
-            paddingInline: euiTheme.size.s,
-          };
-        }}
-      >
-        <div>{title}</div>
-      </EuiTitle>
-    );
-  };
+  navNode: ChromeProjectNavigationNode
+) => Required<EuiCollapsibleNavSubItemProps>['renderItem'] = (navNode) => () => {
+  const { title, spaceBefore } = navNode;
+  const dataTestSubj = getTestSubj(navNode);
+  return (
+    <EuiTitle
+      size="xxxs"
+      className="eui-textTruncate"
+      data-test-subj={dataTestSubj}
+      css={({ euiTheme }: any) => {
+        return {
+          marginTop: spaceBefore ? euiTheme.size[spaceBefore] : undefined,
+          paddingBlock: euiTheme.size.xs,
+          paddingInline: euiTheme.size.s,
+        };
+      }}
+    >
+      <div>{title}</div>
+    </EuiTitle>
+  );
+};
 
 const renderGroup = (
   navGroup: ChromeProjectNavigationNode,
-  groupItems: Array<EuiCollapsibleNavItemProps | EuiCollapsibleNavSubItemPropsEnhanced>,
-  { spaceBefore = DEFAULT_SPACE_BETWEEN_LEVEL_1_GROUPS }: { spaceBefore?: EuiThemeSize | null } = {}
+  groupItems: Array<EuiCollapsibleNavItemProps | EuiCollapsibleNavSubItemPropsEnhanced>
 ): Required<EuiCollapsibleNavItemProps>['items'] => {
   let itemPrepend: EuiCollapsibleNavItemProps | EuiCollapsibleNavSubItemProps | null = null;
+  const { spaceBefore } = navGroup;
 
   if (!!navGroup.title) {
     itemPrepend = {
-      renderItem: renderBlockTitle(navGroup, { spaceBefore }),
+      renderItem: renderBlockTitle(navGroup),
     };
   } else if (spaceBefore) {
     itemPrepend = {
@@ -147,11 +193,9 @@ const renderGroup = (
 const renderPanelOpener = (
   navGroup: ChromeProjectNavigationNode,
   {
-    spaceBefore,
     navigateToUrl,
     activeNodes,
   }: {
-    spaceBefore?: EuiThemeSize | null;
     navigateToUrl: NavigateToUrlFn;
     activeNodes: ChromeProjectNavigationNode[][];
   }
@@ -168,9 +212,9 @@ const renderPanelOpener = (
     },
   ];
 
-  if (spaceBefore) {
+  if (navGroup.spaceBefore) {
     items.unshift({
-      renderItem: () => <EuiSpacer size={spaceBefore!} />,
+      renderItem: () => <EuiSpacer size={navGroup.spaceBefore!} />,
     });
   }
 
@@ -178,7 +222,7 @@ const renderPanelOpener = (
 };
 
 const getEuiProps = (
-  _navNode: ChromeProjectNavigationNode,
+  navNode: ChromeProjectNavigationNode,
   deps: {
     navigateToUrl: NavigateToUrlFn;
     closePanel: PanelContext['close'];
@@ -194,7 +238,6 @@ const getEuiProps = (
   isSelected: boolean;
   isItem: boolean;
   dataTestSubj: string;
-  spaceBefore?: EuiThemeSize | null;
 } & Pick<EuiCollapsibleNavItemProps, 'linkProps' | 'onClick'> => {
   const {
     navigateToUrl,
@@ -205,7 +248,8 @@ const getEuiProps = (
     eventTracker,
     basePath,
   } = deps;
-  const { navNode, isItem, hasChildren, hasLink } = serializeNavNode(_navNode);
+  const hasLink = nodeHasLink(navNode);
+  const isItem = navNode.renderAs === 'item';
   const { path, href, onClick: customOnClick, isCollapsible = DEFAULT_IS_COLLAPSIBLE } = navNode;
 
   const isAccordion = isAccordionNode(navNode);
@@ -225,13 +269,6 @@ const getEuiProps = (
 
   const dataTestSubj = getTestSubj(navNode, isSelected);
 
-  let spaceBefore = navNode.spaceBefore;
-  if (spaceBefore === undefined && treeDepth === 1 && hasChildren && !isItem) {
-    // For groups at level 1 that don't have a space specified we default to add a "m"
-    // space. For all other groups, unless specified, there is no vertical space.
-    spaceBefore = DEFAULT_SPACE_BETWEEN_LEVEL_1_GROUPS;
-  }
-
   const subItems: EuiCollapsibleNavItemProps['items'] | undefined = isItem
     ? undefined
     : navNode.children
@@ -303,7 +340,6 @@ const getEuiProps = (
     subItems,
     isSelected,
     isItem,
-    spaceBefore,
     dataTestSubj,
     linkProps,
     onClick,
@@ -328,9 +364,11 @@ function nodeToEuiCollapsibleNavProps(
   items: Array<EuiCollapsibleNavItemProps | EuiCollapsibleNavSubItemPropsEnhanced>;
   isVisible: boolean;
 } {
-  const { navNode, subItems, dataTestSubj, isSelected, isItem, spaceBefore, linkProps, onClick } =
-    getEuiProps(_navNode, deps);
-  const { id, path, href, renderAs, isCollapsible } = navNode;
+  const { navNode, subItems, dataTestSubj, isSelected, isItem, linkProps, onClick } = getEuiProps(
+    _navNode,
+    deps
+  );
+  const { id, path, href, renderAs, isCollapsible, spaceBefore } = navNode;
 
   if (navNode.renderItem) {
     // Leave the rendering to the consumer
@@ -343,7 +381,7 @@ function nodeToEuiCollapsibleNavProps(
   if (renderAs === 'panelOpener') {
     // Render as a panel opener (button to open a panel as a second navigation)
     return {
-      items: [...renderPanelOpener(navNode, { spaceBefore, ...deps })],
+      items: [...renderPanelOpener(navNode, deps)],
       isVisible: true,
     };
   }
@@ -351,7 +389,7 @@ function nodeToEuiCollapsibleNavProps(
   if (renderAs === 'block' && deps.treeDepth > 0 && subItems) {
     // Render as a group block (bold title + list of links underneath)
     return {
-      items: [...renderGroup(navNode, subItems, { spaceBefore: spaceBefore ?? null })],
+      items: [...renderGroup(navNode, subItems)],
       isVisible: subItems.length > 0,
     };
   }
@@ -399,16 +437,19 @@ interface Props {
 
 export const NavigationSectionUI: FC<Props> = React.memo(({ navNode: _navNode }) => {
   const { activeNodes } = useNavigation();
-  const { navigateToUrl, eventTracker, basePath } = useServices();
+  const { navigateToUrl, eventTracker, basePath, isSideNavCollapsed } = useServices();
   const [items, setItems] = useState<EuiCollapsibleNavSubItemProps[] | undefined>();
 
-  const { navNode } = useMemo(
+  const navNode = useMemo(
     () =>
-      serializeNavNode({
-        renderAs: _navNode.children ? 'accordion' : 'item', // Top level nodes are either item or accordion
-        ..._navNode,
-      }),
-    [_navNode]
+      serializeNavNode(
+        {
+          renderAs: _navNode.children ? 'accordion' : 'item', // Top level nodes are either item or accordion
+          ..._navNode,
+        },
+        { isSideNavCollapsed, treeDepth: 0 }
+      ),
+    [_navNode, isSideNavCollapsed]
   );
   const { close: closePanel } = usePanel();
 

packages/shared-ux/chrome/navigation/src/ui/hooks/use_accordion_state.ts

@@ -108,6 +108,7 @@ export const useAccordionState = ({ navNode }: { navNode: ChromeProjectNavigatio
       };
 
       const updated: Partial<EuiAccordionProps & { isCollapsible?: boolean }> = {
+        buttonProps: { 'data-test-subj': 'accordionToggleBtn' },
         ..._accordionProps,
         arrowProps,
         isCollapsible,

x-pack/plugins/observability_solution/observability/public/navigation_tree.ts

@@ -356,7 +356,6 @@ export function createNavTree(pluginsStart: ObservabilityPublicPluginsStart) {
               defaultMessage: 'Other tools',
             }),
             renderAs: 'panelOpener',
-            icon: 'editorCodeBlock',
             children: [
               {
                 link: 'logs:stream',