Skip to content

Commit

Permalink
[Serverless] Changes to TopNavMenu placement for serverless projects. (
Browse files Browse the repository at this point in the history
…#158053)

## Summary

Part of #158034

This PR moves the TopNavMenu placement to below the breadcrumbs bar for
the serverless project chrome layout.

The app toolbar must appear below the breadcrumbs bar in the layout,
when the app has TopNavMenu items registered with the navigation
service.

To prevent the layout from rendering an empty container, I had to create
a new hook, `useHeaderActionMenuMounter`, that extracts the state from
HeaderActionMenu. The state (`mounter: MountPoint | undefined`) is
hoisted above so that the header can check if it is empty before
rendering the container.

_Future work is still needed to achieve the end goal of the design._
Apps must be able to define custom layouts, which commonly require grid
and spacer components. The TopNavMenu needs augmentation to support the
new API. See the project document from the issue.

## Screenshots
**App showing the toolbar, with the container shown below breadcrumbs**

![image](https://github.com/elastic/kibana/assets/908371/a6f49772-2f0b-4e4a-9a32-7b02398bd0b2)

**App without the toolbar and no empty container**

![image](https://github.com/elastic/kibana/assets/908371/df361b26-1bb0-430a-98c1-863de4cde2da)
  • Loading branch information
tsullivan authored May 22, 2023
1 parent 344adf2 commit dba3c03
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { HeaderBreadcrumbs } from './header_breadcrumbs';
import { HeaderHelpMenu } from './header_help_menu';
import { HeaderLogo } from './header_logo';
import { HeaderNavControls } from './header_nav_controls';
import { HeaderActionMenu } from './header_action_menu';
import { HeaderActionMenu, useHeaderActionMenuMounter } from './header_action_menu';
import { HeaderExtension } from './header_extension';
import { HeaderTopBanner } from './header_top_banner';
import { HeaderMenuButton } from './header_menu_button';
Expand Down Expand Up @@ -92,6 +92,7 @@ export function Header({
const [isNavOpen, setIsNavOpen] = useState(false);
const [navId] = useState(htmlIdGenerator()());
const breadcrumbsAppendExtension = useObservable(breadcrumbsAppendExtension$);
const headerActionMenuMounter = useHeaderActionMenuMounter(application.currentActionMenu$);

if (!isVisible) {
return (
Expand Down Expand Up @@ -235,7 +236,7 @@ export function Header({

<EuiHeaderSection side="right">
<EuiHeaderSectionItem border="none">
<HeaderActionMenu actionMenu$={application.currentActionMenu$} />
<HeaderActionMenu mounter={headerActionMenuMounter} />
</EuiHeaderSectionItem>
</EuiHeaderSection>
</EuiHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { mount, ReactWrapper } from 'enzyme';
import { BehaviorSubject } from 'rxjs';
import { act } from 'react-dom/test-utils';
import type { MountPoint, UnmountCallback } from '@kbn/core-mount-utils-browser';
import { HeaderActionMenu } from './header_action_menu';
import { HeaderActionMenu, useHeaderActionMenuMounter } from './header_action_menu';

type MockedUnmount = jest.MockedFunction<UnmountCallback>;

Expand All @@ -26,7 +26,7 @@ describe('HeaderActionMenu', () => {
});

const refresh = () => {
new Promise(async (resolve, reject) => {
new Promise((resolve, reject) => {
try {
if (component) {
act(() => {
Expand All @@ -53,26 +53,35 @@ describe('HeaderActionMenu', () => {
return unmount;
};

it('mounts the current value of the provided observable', async () => {
component = mount(<HeaderActionMenu actionMenu$={menuMount$} />);
it('mounts the current value of the provided observable', () => {
const TestComponent = () => {
const mounter = useHeaderActionMenuMounter(menuMount$);
return <HeaderActionMenu mounter={mounter} />;
};
component = mount(<TestComponent />);

act(() => {
menuMount$.next(createMountPoint('FOO'));
});
await refresh();
refresh();

expect(component.html()).toMatchInlineSnapshot(
`"<div data-test-subj=\\"headerAppActionMenu\\"><div>FOO</div></div>"`
);
});

it('clears the content of the component when emitting undefined', async () => {
component = mount(<HeaderActionMenu actionMenu$={menuMount$} />);
it('clears the content of the component when emitting undefined', () => {
const TestComponent = () => {
const mounter = useHeaderActionMenuMounter(menuMount$);
return <HeaderActionMenu mounter={mounter} />;
};

component = mount(<TestComponent />);

act(() => {
menuMount$.next(createMountPoint('FOO'));
});
await refresh();
refresh();

expect(component.html()).toMatchInlineSnapshot(
`"<div data-test-subj=\\"headerAppActionMenu\\"><div>FOO</div></div>"`
Expand All @@ -81,20 +90,24 @@ describe('HeaderActionMenu', () => {
act(() => {
menuMount$.next(undefined);
});
await refresh();
refresh();

expect(component.html()).toMatchInlineSnapshot(
`"<div data-test-subj=\\"headerAppActionMenu\\"></div>"`
);
});

it('updates the dom when a new mount point is emitted', async () => {
component = mount(<HeaderActionMenu actionMenu$={menuMount$} />);
it('updates the dom when a new mount point is emitted', () => {
const TestComponent = () => {
const mounter = useHeaderActionMenuMounter(menuMount$);
return <HeaderActionMenu mounter={mounter} />;
};
component = mount(<TestComponent />);

act(() => {
menuMount$.next(createMountPoint('FOO'));
});
await refresh();
refresh();

expect(component.html()).toMatchInlineSnapshot(
`"<div data-test-subj=\\"headerAppActionMenu\\"><div>FOO</div></div>"`
Expand All @@ -103,28 +116,32 @@ describe('HeaderActionMenu', () => {
act(() => {
menuMount$.next(createMountPoint('BAR'));
});
await refresh();
refresh();

expect(component.html()).toMatchInlineSnapshot(
`"<div data-test-subj=\\"headerAppActionMenu\\"><div>BAR</div></div>"`
);
});

it('calls the previous mount point `unmount` when mounting a new mount point', async () => {
component = mount(<HeaderActionMenu actionMenu$={menuMount$} />);
it('calls the previous mount point `unmount` when mounting a new mount point', () => {
const TestComponent = () => {
const mounter = useHeaderActionMenuMounter(menuMount$);
return <HeaderActionMenu mounter={mounter} />;
};
component = mount(<TestComponent />);

act(() => {
menuMount$.next(createMountPoint('FOO'));
});
await refresh();
refresh();

expect(Object.keys(unmounts)).toEqual(['FOO']);
expect(unmounts.FOO).not.toHaveBeenCalled();

act(() => {
menuMount$.next(createMountPoint('BAR'));
});
await refresh();
refresh();

expect(Object.keys(unmounts)).toEqual(['FOO', 'BAR']);
expect(unmounts.FOO).toHaveBeenCalledTimes(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ import { Observable } from 'rxjs';
import type { MountPoint, UnmountCallback } from '@kbn/core-mount-utils-browser';

interface HeaderActionMenuProps {
actionMenu$: Observable<MountPoint | undefined>;
mounter: { mount: MountPoint | undefined };
}

export const HeaderActionMenu: FC<HeaderActionMenuProps> = ({ actionMenu$ }) => {
export const useHeaderActionMenuMounter = (
actionMenu$: Observable<MountPoint<HTMLElement> | undefined>
) => {
// useObservable relies on useState under the hood. The signature is type SetStateAction<S> = S | ((prevState: S) => S);
// As we got a Observable<Function> here, React's setState setter assume he's getting a `(prevState: S) => S` signature,
// therefore executing the mount method, causing everything to crash.
Expand All @@ -29,6 +31,10 @@ export const HeaderActionMenu: FC<HeaderActionMenuProps> = ({ actionMenu$ }) =>
return () => s.unsubscribe();
}, [actionMenu$]);

return mounter;
};

export const HeaderActionMenu: FC<HeaderActionMenuProps> = ({ mounter }) => {
const elementRef = useRef<HTMLDivElement>(null);
const unmountRef = useRef<UnmountCallback | null>(null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { Observable } from 'rxjs';
import { MountPoint } from '@kbn/core-mount-utils-browser';
import { InternalApplicationStart } from '@kbn/core-application-browser-internal';
import { HeaderBreadcrumbs } from '../header/header_breadcrumbs';
import { HeaderActionMenu } from '../header/header_action_menu';
import { HeaderActionMenu, useHeaderActionMenuMounter } from '../header/header_action_menu';
import { HeaderHelpMenu } from '../header/header_help_menu';
import { HeaderNavControls } from '../header/header_nav_controls';
import { ProjectNavigation } from './navigation';
Expand Down Expand Up @@ -69,6 +69,8 @@ export const ProjectHeader = ({
/>
);

const headerActionMenuMounter = useHeaderActionMenuMounter(observables.actionMenu$);

return (
<>
<EuiHeader position="fixed" data-test-subj="kibanaProjectHeader">
Expand Down Expand Up @@ -119,15 +121,25 @@ export const ProjectHeader = ({
navigateToUrl={application.navigateToUrl}
/>
</EuiHeaderSectionItem>
<EuiHeaderSectionItem border="left">
<HeaderActionMenu actionMenu$={observables.actionMenu$} />
</EuiHeaderSectionItem>

<EuiHeaderSectionItem>
<HeaderNavControls navControls$={observables.navControlsRight$} />
</EuiHeaderSectionItem>
</EuiHeaderSection>
</EuiHeader>
{headerActionMenuMounter.mount && (
<EuiHeader data-test-subj="kibanaProjectHeaderActionMenu">
{/* TODO: This puts a group of nav menu items on the right edge of the screen,
but it should be possible for apps customize the layout in a grid and use spacers between items.
https://github.com/elastic/kibana/issues/158034 */}
<EuiHeaderSection />
<EuiHeaderSection side="right">
<EuiHeaderSectionItem>
<HeaderActionMenu mounter={headerActionMenuMounter} />
</EuiHeaderSectionItem>
</EuiHeaderSection>
</EuiHeader>
)}
</>
);
};

0 comments on commit dba3c03

Please sign in to comment.