diff --git a/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx b/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx index c2458d8bf1a73..15baf09d04dc3 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx +++ b/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(); diff --git a/packages/shared-ux/chrome/navigation/__jest__/utils.tsx b/packages/shared-ux/chrome/navigation/__jest__/utils.tsx index 21ac68ae06c0f..9f603709c8efa 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/utils.tsx +++ b/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, diff --git a/packages/shared-ux/chrome/navigation/mocks/storybook.ts b/packages/shared-ux/chrome/navigation/mocks/storybook.ts index 6ae9fd24ef8f6..47d0bc342a7bd 100644 --- a/packages/shared-ux/chrome/navigation/mocks/storybook.ts +++ b/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; +export type Params = Pick; export class StorybookMock extends AbstractStorybookMock<{}, NavigationServices> { propArguments = {}; serviceArguments = { - navIsOpen: { + isSideNavCollapsed: { control: 'boolean', - defaultValue: true, + defaultValue: false, }, }; diff --git a/packages/shared-ux/chrome/navigation/src/services.tsx b/packages/shared-ux/chrome/navigation/src/services.tsx index 8f9a1915e3c4d..e7d7f30886b2e 100644 --- a/packages/shared-ux/chrome/navigation/src/services.tsx +++ b/packages/shared-ux/chrome/navigation/src/services.tsx @@ -44,7 +44,6 @@ export const NavigationKibanaProvider: FC; - navIsOpen: boolean; navigateToUrl: NavigateToUrlFn; activeNodes$: Observable; isSideNavCollapsed: boolean; diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_section_ui.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_section_ui.tsx index fde5cf16d0315..6235d30bcf0a9 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_section_ui.tsx +++ b/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['renderItem'] = - (navNode, { spaceBefore }) => - () => { - const { title } = navNode; - const dataTestSubj = getTestSubj(navNode); - return ( - { - return { - marginTop: spaceBefore ? euiTheme.size[spaceBefore] : undefined, - paddingBlock: euiTheme.size.xs, - paddingInline: euiTheme.size.s, - }; - }} - > -
{title}
-
- ); - }; + navNode: ChromeProjectNavigationNode +) => Required['renderItem'] = (navNode) => () => { + const { title, spaceBefore } = navNode; + const dataTestSubj = getTestSubj(navNode); + return ( + { + return { + marginTop: spaceBefore ? euiTheme.size[spaceBefore] : undefined, + paddingBlock: euiTheme.size.xs, + paddingInline: euiTheme.size.s, + }; + }} + > +
{title}
+
+ ); +}; const renderGroup = ( navGroup: ChromeProjectNavigationNode, - groupItems: Array, - { spaceBefore = DEFAULT_SPACE_BETWEEN_LEVEL_1_GROUPS }: { spaceBefore?: EuiThemeSize | null } = {} + groupItems: Array ): Required['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: () => , + renderItem: () => , }); } @@ -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 => { 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; 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 = React.memo(({ navNode: _navNode }) => { const { activeNodes } = useNavigation(); - const { navigateToUrl, eventTracker, basePath } = useServices(); + const { navigateToUrl, eventTracker, basePath, isSideNavCollapsed } = useServices(); const [items, setItems] = useState(); - 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(); diff --git a/packages/shared-ux/chrome/navigation/src/ui/hooks/use_accordion_state.ts b/packages/shared-ux/chrome/navigation/src/ui/hooks/use_accordion_state.ts index 03ded17ee4097..8ebf91a949db2 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/hooks/use_accordion_state.ts +++ b/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 = { + buttonProps: { 'data-test-subj': 'accordionToggleBtn' }, ..._accordionProps, arrowProps, isCollapsible, diff --git a/x-pack/plugins/observability_solution/observability/public/navigation_tree.ts b/x-pack/plugins/observability_solution/observability/public/navigation_tree.ts index b2964648f8d02..e632cee3d732c 100644 --- a/x-pack/plugins/observability_solution/observability/public/navigation_tree.ts +++ b/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',