Skip to content

Commit

Permalink
Project Side Navigation: Use EuiCollapsibleNavBeta component (#164910)
Browse files Browse the repository at this point in the history
## Summary

Closes #162507
Relates to #166545
^ additional IA-related tasks - related to the alignment discussions -
can be found here

## Work for next steps
In this PR, some work items are being saved for a next PR:
1. _Only affects Search solution_: Navigation "group titles" do not
create a breadcrumb item, as sub-items in the group are not
hierarchically under the title. To address this, group titles may be
going away from the design.
#167323
2. _Only affects Observability solution_: Navigation accordions can not
be collapsed and do not show arrow icons. To address this, in a later PR
we will add internal state management for the open/closed state of each
accordion. #167328
3. _Affects all solutions:_ The "collapsed" state of the side nav should
show a docked view with icons-only. To address this, in later PRs we
will bring Security solution into the unified nav components.
4. #167326
5. #167330
6. #167332

### Recordings
These videos show a before-and-after with the new UI. 
| project | old | new |
|--|--|--|
|observability|
https://github.com/elastic/kibana/assets/908371/663765a3-4e4b-416e-b7d5-7d87eece83e8
| <img width="298" alt="CleanShot 2023-09-22 at 14 20 48@2x"
src="https://github.com/elastic/kibana/assets/446285/d61f6fe0-a6a9-4806-bc27-08b0ff2afb49">
|
|search|
https://github.com/elastic/kibana/assets/908371/f383773e-27a8-4485-8289-274d8231b960
| <img width="281" alt="CleanShot 2023-09-22 at 14 18 43@2x"
src="https://github.com/elastic/kibana/assets/446285/901b8fc1-9945-4cee-b566-5e4539f08043">
|
|security|
https://github.com/elastic/kibana/assets/908371/481f4533-64e5-41db-bc8e-5012f82c188a
| *will change to the new style after this PR and the flyout/panel
support are completed |

### Checklist

- [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: Sébastien Loix <[email protected]>
  • Loading branch information
tsullivan and sebelga authored Sep 27, 2023
1 parent 9fba1e3 commit 0c680d7
Show file tree
Hide file tree
Showing 28 changed files with 763 additions and 1,112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import React from 'react';
import { HeaderActionMenu } from '../header/header_action_menu';

interface AppMenuBarProps {
isOpen: boolean;
headerActionMenuMounter: { mount: MountPoint<HTMLElement> | undefined };
}
export const AppMenuBar = ({ headerActionMenuMounter }: AppMenuBarProps) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { EuiHeader } from '@elastic/eui';
import { applicationServiceMock } from '@kbn/core-application-browser-mocks';
import { docLinksServiceMock } from '@kbn/core-doc-links-browser-mocks';
import { fireEvent, render, screen } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import React from 'react';
import * as Rx from 'rxjs';
import { ProjectHeader, Props as ProjectHeaderProps } from './header';
Expand Down Expand Up @@ -45,43 +45,18 @@ describe('Header', () => {
</ProjectHeader>
);

expect(await screen.findByTestId('toggleNavButton')).toBeVisible();
expect(await screen.findByTestId('euiCollapsibleNavButton')).toBeVisible();
expect(await screen.findByText('Hello, world!')).toBeVisible();
});

it('can collapse and uncollapse', async () => {
render(
<ProjectHeader {...mockProps}>
<EuiHeader>Hello, goodbye!</EuiHeader>
</ProjectHeader>
);

expect(await screen.findByTestId('toggleNavButton')).toBeVisible();
expect(await screen.findByText('Hello, goodbye!')).toBeVisible(); // title is shown

const toggleNav = async () => {
fireEvent.click(await screen.findByTestId('toggleNavButton')); // click

expect(await screen.findByText('Hello, goodbye!')).not.toBeVisible();

fireEvent.click(await screen.findByTestId('toggleNavButton')); // click again

expect(await screen.findByText('Hello, goodbye!')).toBeVisible(); // title is shown
};

await toggleNav();
await toggleNav();
await toggleNav();
});

it('displays the link to projects', async () => {
render(
<ProjectHeader {...mockProps}>
<EuiHeader>Hello, world!</EuiHeader>
</ProjectHeader>
);

const projectsLink = await screen.getByTestId('projectsLink');
const projectsLink = screen.getByTestId('projectsLink');
expect(projectsLink).toHaveAttribute('href', '/projects/');
expect(projectsLink).toHaveTextContent('My Project');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ import {
EuiHeaderLogo,
EuiHeaderSection,
EuiHeaderSectionItem,
EuiHeaderSectionItemButton,
EuiIcon,
EuiLoadingSpinner,
htmlIdGenerator,
useEuiTheme,
EuiThemeComputed,
} from '@elastic/eui';
Expand All @@ -35,8 +32,7 @@ import { MountPoint } from '@kbn/core-mount-utils-browser';
import { i18n } from '@kbn/i18n';
import { RedirectAppLinks } from '@kbn/shared-ux-link-redirect-app';
import { Router } from '@kbn/shared-ux-router';
import React, { createRef, useCallback, useState } from 'react';
import useLocalStorage from 'react-use/lib/useLocalStorage';
import React, { useCallback } from 'react';
import useObservable from 'react-use/lib/useObservable';
import { debounceTime, Observable, of } from 'rxjs';
import { useHeaderActionMenuMounter } from '../header/header_action_menu';
Expand Down Expand Up @@ -66,13 +62,6 @@ const getHeaderCss = ({ size }: EuiThemeComputed) => ({
top: 2px;
`,
},
nav: {
toggleNavButton: css`
border-right: 1px solid #d3dae6;
margin-left: -1px;
padding-right: ${size.xs};
`,
},
projectName: {
link: css`
/* TODO: make header layout more flexible? */
Expand Down Expand Up @@ -123,7 +112,6 @@ export interface Props {
prependBasePath: (url: string) => string;
}

const LOCAL_STORAGE_IS_OPEN_KEY = 'PROJECT_NAVIGATION_OPEN' as const;
const LOADING_DEBOUNCE_TIME = 80;

type LogoProps = Pick<Props, 'application' | 'homeHref$' | 'loadingCount$' | 'prependBasePath'> & {
Expand Down Expand Up @@ -186,9 +174,6 @@ export const ProjectHeader = ({
docLinks,
...observables
}: Props) => {
const [navId] = useState(htmlIdGenerator()());
const [isOpen, setIsOpen] = useLocalStorage(LOCAL_STORAGE_IS_OPEN_KEY, true);
const toggleCollapsibleNavRef = createRef<HTMLButtonElement & { euiAnimate: () => void }>();
const headerActionMenuMounter = useHeaderActionMenuMounter(observables.actionMenu$);
const projectsUrl = useObservable(observables.projectsUrl$);
const projectName = useObservable(observables.projectName$);
Expand All @@ -210,34 +195,9 @@ export const ProjectHeader = ({
<div id="globalHeaderBars" data-test-subj="headerGlobalNav" className="header__bars">
<EuiHeader position="fixed" className="header__firstBar">
<EuiHeaderSection grow={false}>
<EuiHeaderSectionItem css={headerCss.nav.toggleNavButton}>
<Router history={application.history}>
<ProjectNavigation
isOpen={isOpen!}
closeNav={() => {
setIsOpen(false);
if (toggleCollapsibleNavRef.current) {
toggleCollapsibleNavRef.current.focus();
}
}}
button={
<EuiHeaderSectionItemButton
data-test-subj="toggleNavButton"
aria-label={headerStrings.nav.closeNavAriaLabel}
onClick={() => setIsOpen(!isOpen)}
aria-expanded={isOpen!}
aria-pressed={isOpen!}
aria-controls={navId}
ref={toggleCollapsibleNavRef}
>
<EuiIcon type={isOpen ? 'menuLeft' : 'menuRight'} size="m" />
</EuiHeaderSectionItemButton>
}
>
{children}
</ProjectNavigation>
</Router>
</EuiHeaderSectionItem>
<Router history={application.history}>
<ProjectNavigation>{children}</ProjectNavigation>
</Router>

<EuiHeaderSectionItem>
<Logo
Expand Down Expand Up @@ -297,7 +257,7 @@ export const ProjectHeader = ({
</header>

{headerActionMenuMounter.mount && (
<AppMenuBar isOpen={isOpen ?? false} headerActionMenuMounter={headerActionMenuMounter} />
<AppMenuBar headerActionMenuMounter={headerActionMenuMounter} />
)}
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,55 +6,25 @@
* Side Public License, v 1.
*/

import { EuiCollapsibleNavBeta } from '@elastic/eui';
import React from 'react';
import { css } from '@emotion/react';
import { EuiCollapsibleNav, EuiCollapsibleNavProps } from '@elastic/eui';
import useLocalStorage from 'react-use/lib/useLocalStorage';

const SIZE_EXPANDED = 248;
const SIZE_COLLAPSED = 0;
const LOCAL_STORAGE_IS_COLLAPSED_KEY = 'PROJECT_NAVIGATION_COLLAPSED' as const;

export interface ProjectNavigationProps {
isOpen: boolean;
closeNav: () => void;
button: EuiCollapsibleNavProps['button'];
}

export const ProjectNavigation: React.FC<ProjectNavigationProps> = ({
children,
isOpen,
closeNav,
button,
}) => {
const collabsibleNavCSS = css`
border-inline-end-width: 1,
display: flex,
flex-direction: row,
`;

const DOCKED_BREAKPOINT = 's' as const;
const isVisible = isOpen;
export const ProjectNavigation: React.FC = ({ children }) => {
const [isCollapsed, setIsCollapsed] = useLocalStorage(LOCAL_STORAGE_IS_COLLAPSED_KEY, false);
const onCollapseToggle = (nextIsCollapsed: boolean) => {
setIsCollapsed(nextIsCollapsed);
};

return (
<>
{
/* must render the tree to initialize the navigation, even if it shouldn't be visible */
!isOpen && <div hidden>{children}</div>
}
<EuiCollapsibleNav
className="projectLayoutSideNav"
css={collabsibleNavCSS}
isOpen={isVisible /* only affects docked state */}
showButtonIfDocked={true}
onClose={closeNav}
isDocked={true}
size={isVisible ? SIZE_EXPANDED : SIZE_COLLAPSED}
hideCloseButton={false}
dockedBreakpoint={DOCKED_BREAKPOINT}
ownFocus={false}
button={button}
>
{isOpen && children}
</EuiCollapsibleNav>
</>
<EuiCollapsibleNavBeta
initialIsCollapsed={isCollapsed}
onCollapseToggle={onCollapseToggle}
css={isCollapsed ? { display: 'none;' } : {}}
>
{children}
</EuiCollapsibleNavBeta>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { ComponentType } from 'react';
import type { Location } from 'history';
import { EuiAccordionProps } from '@elastic/eui';
import type { AppId as DevToolsApp, DeepLinkId as DevToolsLink } from '@kbn/deeplinks-devtools';
import type {
AppId as AnalyticsApp,
Expand Down Expand Up @@ -68,6 +70,8 @@ export interface ChromeProjectNavigationNode {
deepLink?: ChromeNavLink;
/** Optional icon for the navigation node. Note: not all navigation depth will render the icon */
icon?: string;
/** Optional flag to indicate if the node must be treated as a group title */
isGroupTitle?: boolean;
/** Optional children of the navigation node */
children?: ChromeProjectNavigationNode[];
/**
Expand All @@ -88,6 +92,8 @@ export interface ChromeProjectNavigationNode {
* @default 'visible'
*/
breadcrumbStatus?: 'hidden' | 'visible';

accordionProps?: Partial<EuiAccordionProps>;
}

/** @public */
Expand Down Expand Up @@ -139,7 +145,12 @@ export interface NodeDefinition<
cloudLink?: CloudLinkId;
/** Optional icon for the navigation node. Note: not all navigation depth will render the icon */
icon?: string;
/** Optional children of the navigation node */
/**
* Optional flag to indicate if the node must be treated as a group title.
* Can not be used with `children`
*/
isGroupTitle?: boolean;
/** Optional children of the navigation node. Can not be used with `isGroupTitle` */
children?: NonEmptyArray<NodeDefinition<LinkId, Id, ChildrenId>>;
/**
* Use href for absolute links only. Internal links should use "link".
Expand All @@ -155,6 +166,8 @@ export interface NodeDefinition<
* @default 'visible'
*/
breadcrumbStatus?: 'hidden' | 'visible';

accordionProps?: Partial<EuiAccordionProps>;
}

/**
Expand Down
19 changes: 7 additions & 12 deletions packages/default-nav/analytics/default_navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,13 @@ export const defaultNavigation: AnalyticsNodeDefinition = {
icon: 'stats',
children: [
{
id: 'root',
children: [
{
link: 'discover',
},
{
link: 'dashboards',
},
{
link: 'visualize',
},
],
link: 'discover',
},
{
link: 'dashboards',
},
{
link: 'visualize',
},
],
};
25 changes: 10 additions & 15 deletions packages/default-nav/devtools/default_navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,16 @@ export const defaultNavigation: DevToolsNodeDefinition = {
icon: 'editorCodeBlock',
children: [
{
id: 'root',
children: [
{
link: 'dev_tools:console',
},
{
link: 'dev_tools:searchprofiler',
},
{
link: 'dev_tools:grokdebugger',
},
{
link: 'dev_tools:painless_lab',
},
],
link: 'dev_tools:console',
},
{
link: 'dev_tools:searchprofiler',
},
{
link: 'dev_tools:grokdebugger',
},
{
link: 'dev_tools:painless_lab',
},
],
};
8 changes: 1 addition & 7 deletions packages/default-nav/management/default_navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,7 @@ export const defaultNavigation: ManagementNodeDefinition = {
icon: 'gear',
children: [
{
id: 'root',
title: '',
children: [
{
link: 'monitoring',
},
],
link: 'monitoring',
},
{
id: 'integration_management',
Expand Down
14 changes: 4 additions & 10 deletions packages/default-nav/ml/default_navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,10 @@ export const defaultNavigation: MlNodeDefinition = {
icon: 'machineLearningApp',
children: [
{
title: '',
id: 'root',
children: [
{
link: 'ml:overview',
},
{
link: 'ml:notifications',
},
],
link: 'ml:overview',
},
{
link: 'ml:notifications',
},
{
title: i18n.translate('defaultNavigation.ml.anomalyDetection', {
Expand Down
4 changes: 2 additions & 2 deletions packages/shared-ux/chrome/navigation/mocks/src/storybook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { NavigationServices } from '../../types';
type Arguments = NavigationServices;
export type Params = Pick<
Arguments,
'navIsOpen' | 'recentlyAccessed$' | 'navLinks$' | 'onProjectNavigationChange'
'navIsOpen' | 'recentlyAccessed$' | 'activeNodes$' | 'navLinks$' | 'onProjectNavigationChange'
>;

export class StorybookMock extends AbstractStorybookMock<{}, NavigationServices> {
Expand Down Expand Up @@ -43,7 +43,7 @@ export class StorybookMock extends AbstractStorybookMock<{}, NavigationServices>
recentlyAccessed$: params.recentlyAccessed$ ?? new BehaviorSubject([]),
navLinks$: params.navLinks$ ?? new BehaviorSubject([]),
onProjectNavigationChange: params.onProjectNavigationChange ?? (() => undefined),
activeNodes$: new BehaviorSubject([]),
activeNodes$: params.activeNodes$ ?? new BehaviorSubject([]),
cloudLinks: {
billingAndSub: {
title: 'Billing & Subscriptions',
Expand Down
Loading

0 comments on commit 0c680d7

Please sign in to comment.