Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serverless] Changes to TopNavMenu placement for serverless projects. #158053

Merged
merged 7 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor cleanup: I've removed the unnecessary await statements, as they create warnings in my editor.

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>
)}
</>
);
};