From 5bd4cce40b49ac123a1cf9e8c9f87d5a208fd33c Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Tue, 4 Mar 2025 08:05:26 -0700 Subject: [PATCH] Fix toggling of the secondary panel for non-landing page nav item buttons (#211852) ## Summary Closes https://github.com/elastic/kibana-team/issues/1514 **Release note:** Fixed an issue with the side navigation of solution projects where clicking the nav item label would open but not close the secondary navigation panel. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 52bbc243875d29a08cd648ac06fd0d9a4da2adc3) # Conflicts: # src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/navigation.stories.tsx --- .../chrome/navigation/__jest__/panel.test.tsx | 101 +++++++++++++++++- .../components/navigation_item_open_panel.tsx | 28 +++-- .../src/ui/components/panel/context.tsx | 18 +++- .../ui/components/panel/navigation_panel.tsx | 30 ++---- 4 files changed, 143 insertions(+), 34 deletions(-) diff --git a/src/platform/packages/shared/shared-ux/chrome/navigation/__jest__/panel.test.tsx b/src/platform/packages/shared/shared-ux/chrome/navigation/__jest__/panel.test.tsx index 19254d1cfcfc5..3b1d899852c0e 100644 --- a/src/platform/packages/shared/shared-ux/chrome/navigation/__jest__/panel.test.tsx +++ b/src/platform/packages/shared/shared-ux/chrome/navigation/__jest__/panel.test.tsx @@ -8,8 +8,11 @@ */ import './setup_jest_mocks'; + +import { userEvent } from '@testing-library/user-event'; import React from 'react'; import { BehaviorSubject, of } from 'rxjs'; + import type { ChromeProjectNavigationNode, NavigationTreeDefinitionUI, @@ -110,9 +113,105 @@ describe('Panel', () => { expect(queryByTestId(/panelOpener-root.group2/)).toBeVisible(); }); + describe('toggle the panel open and closed', () => { + let navigationTree: NavigationTreeDefinitionUI; + beforeAll(() => { + navigationTree = { + id: 'es', + body: [ + { + id: 'root', + title: 'Root', + path: 'root', + isCollapsible: false, + children: [ + { + id: 'group1', + title: 'Group 1', + path: 'root.group1', + renderAs: 'panelOpener', + children: [ + { id: 'item1', title: 'Item 1', href: '/app/item1', path: 'root.group1.item1' }, + ], + }, + ], + }, + ], + }; + }); + + test('should allow button to toggle', async () => { + const { findByTestId, queryByTestId } = renderNavigation({ + navTreeDef: of(navigationTree), + }); + + // open the panel + (await findByTestId(/nav-item-id-group1/)).click(); + expect(queryByTestId(/sideNavPanel/)).toBeVisible(); + + // close the panel + await userEvent.click(await findByTestId(/nav-item-id-group1/)); + expect(queryByTestId(/sideNavPanel/)).toBeNull(); + }); + + test('should allow the button label to toggle', async () => { + const { findByTestId, queryByTestId, container } = renderNavigation({ + navTreeDef: of(navigationTree), + }); + + // open the panel via the button + (await findByTestId(/nav-item-id-group1/)).click(); + expect(queryByTestId(/sideNavPanel/)).toBeVisible(); + + // click the label element + const buttonLabel = container.querySelectorAll('span span')[0]; + expect(buttonLabel).toBeInTheDocument(); + await userEvent.click(buttonLabel!); + + expect(queryByTestId(/sideNavPanel/)).toBeNull(); + }); + + test('should allow the button icon to toggle', async () => { + const { findByTestId, queryByTestId, container } = renderNavigation({ + navTreeDef: of(navigationTree), + }); + + // open the panel via the button + (await findByTestId(/nav-item-id-group1/)).click(); + expect(queryByTestId(/sideNavPanel/)).toBeVisible(); + + // click the label element + const buttonIcon = container.querySelectorAll('span span')[1]; + expect(buttonIcon).toBeInTheDocument(); + await userEvent.click(buttonIcon!); + + expect(queryByTestId(/sideNavPanel/)).toBeNull(); + }); + + test('should allow outside click to close the panel', async () => { + const { findByTestId, queryByTestId } = renderNavigation({ + navTreeDef: of(navigationTree), + }); + + // open the panel via the button + (await findByTestId(/nav-item-id-group1/)).click(); + expect(queryByTestId(/sideNavPanel/)).toBeVisible(); + + // click an element outside of the panel + const navRoot = await findByTestId(/nav-item-id-root/); + expect(navRoot).toBeInTheDocument(); + + const navRootParent = navRoot!.parentNode! as Element; + expect(navRootParent).toBeInTheDocument(); + await userEvent.click(navRootParent); + + expect(queryByTestId(/sideNavPanel/)).toBeNull(); + }); + }); + describe('custom content', () => { test('should render custom component inside the panel', async () => { - const panelContentProvider: PanelContentProvider = (id) => { + const panelContentProvider: PanelContentProvider = (_id) => { return { content: ({ closePanel, selectedNode, activeNodes }) => { const [path0 = []] = activeNodes; diff --git a/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx b/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx index dfe1d26873560..9531f5f12a794 100644 --- a/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx +++ b/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx @@ -102,18 +102,21 @@ export const NavigationItemOpenPanel: FC = ({ item, navigateToUrl, active [`panelOpener-deepLinkId-${deepLink?.id}`]: !!deepLink, }); - const togglePanel = useCallback(() => { - if (selectedNode?.id === item.id) { - closePanel(); - } else { - openPanel(item); - } - }, [selectedNode?.id, item, closePanel, openPanel]); + const togglePanel = useCallback( + (target: EventTarget) => { + if (selectedNode?.id === item.id) { + closePanel(); + } else { + openPanel(item, target as Element); + } + }, + [selectedNode?.id, item, closePanel, openPanel] + ); const onLinkClick = useCallback( (e: React.MouseEvent) => { if (!href) { - togglePanel(); + togglePanel(e.target); return; } e.preventDefault(); @@ -123,9 +126,12 @@ export const NavigationItemOpenPanel: FC = ({ item, navigateToUrl, active [closePanel, href, navigateToUrl, togglePanel] ); - const onIconClick = useCallback(() => { - togglePanel(); - }, [togglePanel]); + const onIconClick = useCallback( + (e: React.MouseEvent) => { + togglePanel(e.target); + }, + [togglePanel] + ); if (!hasLandingPage) { return ( diff --git a/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx b/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx index 917862b08b2b4..96dfa32aa38a1 100644 --- a/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx +++ b/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx @@ -16,6 +16,7 @@ import React, { useState, ReactNode, useEffect, + useRef, } from 'react'; import type { ChromeProjectNavigationNode, PanelSelectedNode } from '@kbn/core-chrome-browser'; @@ -25,10 +26,12 @@ import { ContentProvider } from './types'; export interface PanelContext { isOpen: boolean; toggle: () => void; - open: (navNode: PanelSelectedNode) => void; + open: (navNode: PanelSelectedNode, openerEl: Element | null) => void; close: () => void; /** The selected node is the node in the main panel that opens the Panel */ selectedNode: PanelSelectedNode | null; + /** Reference to the selected nav node element in the DOM */ + selectedNodeEl: React.MutableRefObject; /** Handler to retrieve the component to render in the panel */ getContent: () => React.ReactNode; } @@ -51,14 +54,21 @@ export const PanelProvider: FC> = ({ }) => { const [isOpen, setIsOpen] = useState(false); const [selectedNode, setActiveNode] = useState(selectedNodeProp); + const selectedNodeEl = useRef(null); const toggle = useCallback(() => { setIsOpen((prev) => !prev); }, []); const open = useCallback( - (navNode: PanelSelectedNode) => { + (navNode: PanelSelectedNode, openerEl: Element | null) => { setActiveNode(navNode); + + const navNodeEl = openerEl?.closest(`[data-test-subj~=nav-item]`); + if (navNodeEl) { + selectedNodeEl.current = navNodeEl; + } + setIsOpen(true); setSelectedNode?.(navNode); }, @@ -67,6 +77,7 @@ export const PanelProvider: FC> = ({ const close = useCallback(() => { setActiveNode(null); + selectedNodeEl.current = null; setIsOpen(false); setSelectedNode?.(null); }, [setSelectedNode]); @@ -110,9 +121,10 @@ export const PanelProvider: FC> = ({ open, close, selectedNode, + selectedNodeEl, getContent, }), - [isOpen, toggle, open, close, selectedNode, getContent] + [isOpen, toggle, open, close, selectedNode, selectedNodeEl, getContent] ); return {children}; diff --git a/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx b/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx index f991dd6a30714..bb0fc69f5126b 100644 --- a/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx +++ b/src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx @@ -32,15 +32,9 @@ const getTestSubj = (selectedNode: PanelSelectedNode | null): string | undefined }); }; -const getTargetTestSubj = (target: EventTarget | null): string | undefined => { - if (!target) return; - - return (target as HTMLElement).dataset.testSubj; -}; - export const NavigationPanel: FC = () => { const { euiTheme } = useEuiTheme(); - const { isOpen, close, getContent, selectedNode } = usePanel(); + const { isOpen, close, getContent, selectedNode, selectedNodeEl } = usePanel(); // ESC key closes PanelNav const onKeyDown = useCallback( @@ -54,26 +48,24 @@ export const NavigationPanel: FC = () => { const onOutsideClick = useCallback( ({ target }: Event) => { - let doClose = true; + if (!target) { + close(); + return; + } - if (target) { - // Only close if we are not clicking on the currently selected nav node - const testSubj = - getTargetTestSubj(target) ?? getTargetTestSubj((target as HTMLElement).parentNode); + let doClose = true; - if ( - testSubj?.includes(`nav-item-${selectedNode?.path}`) || - testSubj?.includes(`panelOpener-${selectedNode?.path}`) - ) { - doClose = false; - } + // Do not close if clicking on the button (or child of the button) of the currently selected node, + // because we must defer to allowing the button's click handler to manage toggling. + if (selectedNodeEl && selectedNodeEl.current?.contains(target as Node)) { + doClose = false; } if (doClose) { close(); } }, - [close, selectedNode] + [close, selectedNodeEl] ); const panelWrapperClasses = getPanelWrapperStyles();