Skip to content

Commit

Permalink
Fix toggling of the secondary panel for non-landing page nav item but…
Browse files Browse the repository at this point in the history
…tons (#211852)

## Summary

Closes elastic/kibana-team#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 52bbc24)

# Conflicts:
#	src/platform/packages/shared/shared-ux/chrome/navigation/src/ui/navigation.stories.tsx
  • Loading branch information
tsullivan committed Mar 4, 2025
1 parent 029cde7 commit 5bd4cce
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,21 @@ export const NavigationItemOpenPanel: FC<Props> = ({ 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();
Expand All @@ -123,9 +126,12 @@ export const NavigationItemOpenPanel: FC<Props> = ({ 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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import React, {
useState,
ReactNode,
useEffect,
useRef,
} from 'react';
import type { ChromeProjectNavigationNode, PanelSelectedNode } from '@kbn/core-chrome-browser';

Expand All @@ -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<Element | null>;
/** Handler to retrieve the component to render in the panel */
getContent: () => React.ReactNode;
}
Expand All @@ -51,14 +54,21 @@ export const PanelProvider: FC<PropsWithChildren<Props>> = ({
}) => {
const [isOpen, setIsOpen] = useState(false);
const [selectedNode, setActiveNode] = useState<PanelSelectedNode | null>(selectedNodeProp);
const selectedNodeEl = useRef<Element | null>(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);
},
Expand All @@ -67,6 +77,7 @@ export const PanelProvider: FC<PropsWithChildren<Props>> = ({

const close = useCallback(() => {
setActiveNode(null);
selectedNodeEl.current = null;
setIsOpen(false);
setSelectedNode?.(null);
}, [setSelectedNode]);
Expand Down Expand Up @@ -110,9 +121,10 @@ export const PanelProvider: FC<PropsWithChildren<Props>> = ({
open,
close,
selectedNode,
selectedNodeEl,
getContent,
}),
[isOpen, toggle, open, close, selectedNode, getContent]
[isOpen, toggle, open, close, selectedNode, selectedNodeEl, getContent]
);

return <Context.Provider value={ctx}>{children}</Context.Provider>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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();
Expand Down

0 comments on commit 5bd4cce

Please sign in to comment.