mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
[8.18] Fix toggling of the secondary panel for non-landing page nav item buttons (#211852) (#213172)
# Backport This will backport the following commits from `main` to `8.18`: - [Fix toggling of the secondary panel for non-landing page nav item buttons (#211852)](https://github.com/elastic/kibana/pull/211852) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Tim Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-03-04T15:05:26Z","message":"Fix toggling of the secondary panel for non-landing page nav item buttons (#211852)\n\n## Summary\n\nCloses https://github.com/elastic/kibana-team/issues/1514\n\n**Release note:** Fixed an issue with the side navigation of solution\nprojects where clicking the nav item label would open but not close the\nsecondary navigation panel.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"52bbc243875d29a08cd648ac06fd0d9a4da2adc3","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:SharedUX","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"Fix toggling of the secondary panel for non-landing page nav item buttons","number":211852,"url":"https://github.com/elastic/kibana/pull/211852","mergeCommit":{"message":"Fix toggling of the secondary panel for non-landing page nav item buttons (#211852)\n\n## Summary\n\nCloses https://github.com/elastic/kibana-team/issues/1514\n\n**Release note:** Fixed an issue with the side navigation of solution\nprojects where clicking the nav item label would open but not close the\nsecondary navigation panel.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"52bbc243875d29a08cd648ac06fd0d9a4da2adc3"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/211852","number":211852,"mergeCommit":{"message":"Fix toggling of the secondary panel for non-landing page nav item buttons (#211852)\n\n## Summary\n\nCloses https://github.com/elastic/kibana-team/issues/1514\n\n**Release note:** Fixed an issue with the side navigation of solution\nprojects where clicking the nav item label would open but not close the\nsecondary navigation panel.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"52bbc243875d29a08cd648ac06fd0d9a4da2adc3"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
This commit is contained in:
parent
4a0f8937ed
commit
a215ddc2d2
4 changed files with 143 additions and 34 deletions
|
@ -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;
|
||||
|
|
|
@ -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();
|
||||
|
@ -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 (
|
||||
|
|
|
@ -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<Element | null>;
|
||||
/** Handler to retrieve the component to render in the panel */
|
||||
getContent: () => React.ReactNode;
|
||||
}
|
||||
|
@ -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);
|
||||
},
|
||||
|
@ -67,6 +77,7 @@ export const PanelProvider: FC<PropsWithChildren<Props>> = ({
|
|||
|
||||
const close = useCallback(() => {
|
||||
setActiveNode(null);
|
||||
selectedNodeEl.current = null;
|
||||
setIsOpen(false);
|
||||
setSelectedNode?.(null);
|
||||
}, [setSelectedNode]);
|
||||
|
@ -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>;
|
||||
|
|
|
@ -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) => {
|
||||
if (!target) {
|
||||
close();
|
||||
return;
|
||||
}
|
||||
|
||||
let doClose = true;
|
||||
|
||||
if (target) {
|
||||
// Only close if we are not clicking on the currently selected nav node
|
||||
const testSubj =
|
||||
getTargetTestSubj(target) ?? getTargetTestSubj((target as HTMLElement).parentNode);
|
||||
|
||||
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();
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue