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

Add setHeaderActionMenu API to AppMountParameters #75422

Merged
merged 23 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
96df7a9
add `setHeaderActionMenu` to AppMountParameters
pgayvallet Aug 19, 2020
4556772
allow to remove the current menu by calling handler with undefined
pgayvallet Aug 19, 2020
96d3494
update generated doc
pgayvallet Aug 19, 2020
eb70d9c
updating snapshots
pgayvallet Aug 19, 2020
e595519
fix legacy tests
pgayvallet Aug 19, 2020
403a5f1
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 20, 2020
22fef27
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 20, 2020
d3a2c8e
call renderApp with params
pgayvallet Aug 20, 2020
ea91712
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 20, 2020
f777721
Merge branch 'master' into kbn-68524-header-app-menu-api
elasticmachine Aug 23, 2020
ab75d27
rename toMountPoint component file for consistency
pgayvallet Aug 24, 2020
20685b3
add the MountPointPortal utility component
pgayvallet Aug 24, 2020
6405ab5
adapt TopNavMenu to add optional `setMenuMountPoint` prop
pgayvallet Aug 24, 2020
70dad1d
add kibanaReact as required bundle.
pgayvallet Aug 24, 2020
bdbaa6a
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 26, 2020
0d02a2c
use innerHTML instead of textContent for portal tests
pgayvallet Aug 26, 2020
530e031
add error boundaries to portal component
pgayvallet Aug 26, 2020
e34f402
improve renderLayout readability
pgayvallet Aug 26, 2020
950b2a2
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 26, 2020
89e3363
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 26, 2020
7ee6b10
duplicate wrapper in portal mode to avoid altering styles
pgayvallet Aug 26, 2020
339ed72
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Aug 31, 2020
5759f93
Merge remote-tracking branch 'upstream/master' into kbn-68524-header-…
pgayvallet Sep 1, 2020
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 @@ -19,4 +19,5 @@ export interface AppMountParameters<HistoryLocationState = unknown>
| [element](./kibana-plugin-core-public.appmountparameters.element.md) | <code>HTMLElement</code> | The container element to render the application into. |
| [history](./kibana-plugin-core-public.appmountparameters.history.md) | <code>ScopedHistory&lt;HistoryLocationState&gt;</code> | A scoped history instance for your application. Should be used to wire up your applications Router. |
| [onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md) | <code>(handler: AppLeaveHandler) =&gt; void</code> | A function that can be used to register a handler that will be called when the user is leaving the current application, allowing to prompt a confirmation message before actually changing the page.<!-- -->This will be called either when the user goes to another application, or when trying to close the tab or manually changing the url. |
| [setHeaderActionMenu](./kibana-plugin-core-public.appmountparameters.setheaderactionmenu.md) | <code>(menuMount: MountPoint &#124; undefined) =&gt; void</code> | A function that can be used to set the mount point used to populate the application action container in the chrome header.<!-- -->Calling the handler multiple time will erase the current content of the action menu with the mount from the latest call. Calling the handler with <code>undefined</code> will unmount the current mount point. Calling the handler after the application has been unmounted will have no effect. |

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [AppMountParameters](./kibana-plugin-core-public.appmountparameters.md) &gt; [setHeaderActionMenu](./kibana-plugin-core-public.appmountparameters.setheaderactionmenu.md)

## AppMountParameters.setHeaderActionMenu property

A function that can be used to set the mount point used to populate the application action container in the chrome header.

Calling the handler multiple time will erase the current content of the action menu with the mount from the latest call. Calling the handler with `undefined` will unmount the current mount point. Calling the handler after the application has been unmounted will have no effect.

<b>Signature:</b>

```typescript
setHeaderActionMenu: (menuMount: MountPoint | undefined) => void;
```

## Example


```ts
// application.tsx
import React from 'react';
import ReactDOM from 'react-dom';
import { BrowserRouter, Route } from 'react-router-dom';

import { CoreStart, AppMountParameters } from 'src/core/public';
import { MyPluginDepsStart } from './plugin';

export renderApp = ({ element, history, setHeaderActionMenu }: AppMountParameters) => {
const { renderApp } = await import('./application');
const { renderActionMenu } = await import('./action_menu');
setHeaderActionMenu((element) => {
return renderActionMenu(element);
})
return renderApp({ element, history });
}

```

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/core/public/application/application_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { History } from 'history';
import { BehaviorSubject, Subject } from 'rxjs';

import type { MountPoint } from '../types';
import { capabilitiesServiceMock } from './capabilities/capabilities_service.mock';
import {
ApplicationSetup,
Expand Down Expand Up @@ -87,6 +88,7 @@ const createInternalStartContractMock = (): jest.Mocked<InternalApplicationStart
applications$: new BehaviorSubject<Map<string, PublicAppInfo | PublicLegacyAppInfo>>(new Map()),
capabilities: capabilitiesServiceMock.createStartContract().capabilities,
currentAppId$: currentAppId$.asObservable(),
currentActionMenu$: new BehaviorSubject<MountPoint | undefined>(undefined),
getComponent: jest.fn(),
getUrlForApp: jest.fn(),
navigateToApp: jest.fn().mockImplementation((appId) => currentAppId$.next(appId)),
Expand Down
42 changes: 37 additions & 5 deletions src/core/public/application/application_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs';
import { map, shareReplay, takeUntil, distinctUntilChanged, filter } from 'rxjs/operators';
import { createBrowserHistory, History } from 'history';

import { MountPoint } from '../types';
import { InjectedMetadataSetup } from '../injected_metadata';
import { HttpSetup, HttpStart } from '../http';
import { OverlayStart } from '../overlays';
Expand Down Expand Up @@ -90,6 +91,11 @@ interface AppUpdaterWrapper {
updater: AppUpdater;
}

interface AppInternalState {
leaveHandler?: AppLeaveHandler;
actionMenu?: MountPoint;
}

/**
* Service that is responsible for registering new applications.
* @internal
Expand All @@ -98,8 +104,9 @@ export class ApplicationService {
private readonly apps = new Map<string, App<any> | LegacyApp>();
private readonly mounters = new Map<string, Mounter>();
private readonly capabilities = new CapabilitiesService();
private readonly appLeaveHandlers = new Map<string, AppLeaveHandler>();
private readonly appInternalStates = new Map<string, AppInternalState>();
private currentAppId$ = new BehaviorSubject<string | undefined>(undefined);
private currentActionMenu$ = new BehaviorSubject<MountPoint | undefined>(undefined);
private readonly statusUpdaters$ = new BehaviorSubject<Map<symbol, AppUpdaterWrapper>>(new Map());
private readonly subscriptions: Subscription[] = [];
private stop$ = new Subject();
Expand Down Expand Up @@ -293,12 +300,14 @@ export class ApplicationService {
if (path === undefined) {
path = applications$.value.get(appId)?.defaultPath;
}
this.appLeaveHandlers.delete(this.currentAppId$.value!);
this.appInternalStates.delete(this.currentAppId$.value!);
this.navigate!(getAppUrl(availableMounters, appId, path), state, replace);
this.currentAppId$.next(appId);
}
};

this.currentAppId$.subscribe(() => this.refreshCurrentActionMenu());

return {
applications$: applications$.pipe(
map((apps) => new Map([...apps.entries()].map(([id, app]) => [id, getAppInfo(app)]))),
Expand All @@ -310,6 +319,10 @@ export class ApplicationService {
distinctUntilChanged(),
takeUntil(this.stop$)
),
currentActionMenu$: this.currentActionMenu$.pipe(
distinctUntilChanged(),
takeUntil(this.stop$)
),
history: this.history,
registerMountContext: this.mountContext.registerContext,
getUrlForApp: (
Expand Down Expand Up @@ -338,6 +351,7 @@ export class ApplicationService {
mounters={availableMounters}
appStatuses$={applicationStatuses$}
setAppLeaveHandler={this.setAppLeaveHandler}
setAppActionMenu={this.setAppActionMenu}
setIsMounting={(isMounting) => httpLoadingCount$.next(isMounting ? 1 : 0)}
/>
);
Expand All @@ -346,15 +360,32 @@ export class ApplicationService {
}

private setAppLeaveHandler = (appId: string, handler: AppLeaveHandler) => {
this.appLeaveHandlers.set(appId, handler);
this.appInternalStates.set(appId, {
...(this.appInternalStates.get(appId) ?? {}),
leaveHandler: handler,
});
};

private setAppActionMenu = (appId: string, mount: MountPoint | undefined) => {
this.appInternalStates.set(appId, {
...(this.appInternalStates.get(appId) ?? {}),
actionMenu: mount,
});
this.refreshCurrentActionMenu();
};

private refreshCurrentActionMenu = () => {
const appId = this.currentAppId$.getValue();
const currentActionMenu = appId ? this.appInternalStates.get(appId)?.actionMenu : undefined;
this.currentActionMenu$.next(currentActionMenu);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not worth doing until after we complete #74911, but it feels like there's a lot of state & update juggling going on in this service now. Seems like it would be better to have a single source of truth that everything else responds to. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, and I also think we should wait until legacy mode has been removed to cleanup that thing

};

private async shouldNavigate(overlays: OverlayStart): Promise<boolean> {
const currentAppId = this.currentAppId$.value;
if (currentAppId === undefined) {
return true;
}
const action = getLeaveAction(this.appLeaveHandlers.get(currentAppId));
const action = getLeaveAction(this.appInternalStates.get(currentAppId)?.leaveHandler);
if (isConfirmAction(action)) {
const confirmed = await overlays.openConfirm(action.text, {
title: action.title,
Expand All @@ -372,7 +403,7 @@ export class ApplicationService {
if (currentAppId === undefined) {
return;
}
const action = getLeaveAction(this.appLeaveHandlers.get(currentAppId));
const action = getLeaveAction(this.appInternalStates.get(currentAppId)?.leaveHandler);
if (isConfirmAction(action)) {
event.preventDefault();
// some browsers accept a string return value being the message displayed
Expand All @@ -383,6 +414,7 @@ export class ApplicationService {
public stop() {
this.stop$.next();
this.currentAppId$.complete();
this.currentActionMenu$.complete();
this.statusUpdaters$.complete();
this.subscriptions.forEach((sub) => sub.unsubscribe());
window.removeEventListener('beforeunload', this.onBeforeUnload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import { MockLifecycle } from '../test_types';
import { overlayServiceMock } from '../../overlays/overlay_service.mock';
import { AppMountParameters } from '../types';
import { ScopedHistory } from '../scoped_history';
import { Observable } from 'rxjs';
import { MountPoint } from 'kibana/public';

const flushPromises = () => new Promise((resolve) => setImmediate(resolve));

Expand Down Expand Up @@ -309,4 +311,189 @@ describe('ApplicationService', () => {
expect(history.entries[1].pathname).toEqual('/app/app1');
});
});

describe('registering action menus', () => {
const getValue = (obs: Observable<MountPoint | undefined>): Promise<MountPoint | undefined> => {
return obs.pipe(take(1)).toPromise();
};

const mounter1: MountPoint = () => () => undefined;
const mounter2: MountPoint = () => () => undefined;

it('updates the observable value when an application is mounted', async () => {
const { register } = service.setup(setupDeps);

register(Symbol(), {
id: 'app1',
title: 'App1',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
setHeaderActionMenu(mounter1);
return () => undefined;
},
});

const { navigateToApp, getComponent, currentActionMenu$ } = await service.start(startDeps);
update = createRenderer(getComponent());

expect(await getValue(currentActionMenu$)).toBeUndefined();

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);
});

it('updates the observable value when switching application', async () => {
const { register } = service.setup(setupDeps);

register(Symbol(), {
id: 'app1',
title: 'App1',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
setHeaderActionMenu(mounter1);
return () => undefined;
},
});
register(Symbol(), {
id: 'app2',
title: 'App2',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
setHeaderActionMenu(mounter2);
return () => undefined;
},
});

const { navigateToApp, getComponent, currentActionMenu$ } = await service.start(startDeps);
update = createRenderer(getComponent());

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);

await act(async () => {
await navigateToApp('app2');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter2);
});

it('updates the observable value to undefined when switching to an application without action menu', async () => {
const { register } = service.setup(setupDeps);

register(Symbol(), {
id: 'app1',
title: 'App1',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
setHeaderActionMenu(mounter1);
return () => undefined;
},
});
register(Symbol(), {
id: 'app2',
title: 'App2',
mount: async ({}: AppMountParameters) => {
return () => undefined;
},
});

const { navigateToApp, getComponent, currentActionMenu$ } = await service.start(startDeps);
update = createRenderer(getComponent());

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);

await act(async () => {
await navigateToApp('app2');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBeUndefined();
});

it('allow applications to call `setHeaderActionMenu` multiple times', async () => {
const { register } = service.setup(setupDeps);

let resolveMount: () => void;
const promise = new Promise((resolve) => {
resolveMount = resolve;
});

register(Symbol(), {
id: 'app1',
title: 'App1',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
setHeaderActionMenu(mounter1);
promise.then(() => {
setHeaderActionMenu(mounter2);
});
return () => undefined;
},
});

const { navigateToApp, getComponent, currentActionMenu$ } = await service.start(startDeps);
update = createRenderer(getComponent());

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);

await act(async () => {
resolveMount();
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter2);
});

it('allow applications to unset the current menu', async () => {
const { register } = service.setup(setupDeps);

let resolveMount: () => void;
const promise = new Promise((resolve) => {
resolveMount = resolve;
});

register(Symbol(), {
id: 'app1',
title: 'App1',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
setHeaderActionMenu(mounter1);
promise.then(() => {
setHeaderActionMenu(undefined);
});
return () => undefined;
},
});

const { navigateToApp, getComponent, currentActionMenu$ } = await service.start(startDeps);
update = createRenderer(getComponent());

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);

await act(async () => {
resolveMount();
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBeUndefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('AppRouter', () => {
mounters={mockMountersToMounters()}
appStatuses$={mountersToAppStatus$()}
setAppLeaveHandler={noop}
setAppActionMenu={noop}
setIsMounting={noop}
/>
);
Expand Down
Loading