Skip to content

Commit

Permalink
Overlay core service (elastic#34261) (elastic#34717)
Browse files Browse the repository at this point in the history
* Move ui/flyout to overlay core service

* Remove onClose in parameter (use FlyoutSession instead)

* Fix tests

* Remove old inspector tests

* Proper TODO message

* Convert flyout service to class

* Use correct i18n

* Resolving weird merge conflicts

* Fix panel plugin test

* Change new platform access

* Add more tests

* Remove commented tests

* Revert test fix (core is actually not fixed yet)

* Fix tests

* Expose onClose as Observable

* Use jest.doMock

* Fix typos

* Core start() -> setup()

* Remove @extends EventEmitter docs

* Refactor and test flyoutservice

* Fix comments: promise -> observable

* Fix tests

* Explicitly define OverlaySetup

* Fix OverlaySetup type signature

* Update Core API review file and docs

* Remove redudant if case

* Change FlyoutRef.onClose into a promise

* Remove redundante cleanup

* Use promise.finally

* Remove targetDomElement from openFlyout()

There's no need to support multiple targetDomElements per FlyoutService
and the current implementation handled this use case incorrectly.

Instead of adding complexity to try to support it, remove this from the
function signature.

* Fix + test to ensure child components are unmounted when a new flyover is displayed

* Wrap flyover in i18n Context component

* TSlint -> ESlint + test improvements
  • Loading branch information
rudolf authored Apr 8, 2019
1 parent 73735ab commit 8dbff4f
Show file tree
Hide file tree
Showing 29 changed files with 602 additions and 191 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ export interface CoreSetup
| [i18n](./kibana-plugin-public.coresetup.i18n.md) | <code>I18nSetup</code> | |
| [injectedMetadata](./kibana-plugin-public.coresetup.injectedmetadata.md) | <code>InjectedMetadataSetup</code> | |
| [notifications](./kibana-plugin-public.coresetup.notifications.md) | <code>NotificationsSetup</code> | |
| [overlays](./kibana-plugin-public.coresetup.overlays.md) | <code>OverlaySetup</code> | |
| [uiSettings](./kibana-plugin-public.coresetup.uisettings.md) | <code>UiSettingsSetup</code> | |

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [CoreSetup](./kibana-plugin-public.coresetup.md) &gt; [overlays](./kibana-plugin-public.coresetup.overlays.md)

## CoreSetup.overlays property

<b>Signature:</b>

```typescript
overlays: OverlaySetup;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [FlyoutRef](./kibana-plugin-public.flyoutref.md) &gt; [close](./kibana-plugin-public.flyoutref.close.md)

## FlyoutRef.close() method

Closes the referenced flyout if it's still open which in turn will resolve the `onClose` Promise. If the flyout had already been closed this method does nothing.

<b>Signature:</b>

```typescript
close(): Promise<void>;
```
<b>Returns:</b>

`Promise<void>`

24 changes: 24 additions & 0 deletions docs/development/core/public/kibana-plugin-public.flyoutref.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [FlyoutRef](./kibana-plugin-public.flyoutref.md)

## FlyoutRef class

A FlyoutRef is a reference to an opened flyout panel. It offers methods to close the flyout panel again. If you open a flyout panel you should make sure you call `close()` when it should be closed. Since a flyout could also be closed by a user or from another flyout being opened, you must bind to the `onClose` Promise on the FlyoutRef instance. The Promise will resolve whenever the flyout was closed at which point you should discard the FlyoutRef.

<b>Signature:</b>

```typescript
export declare class FlyoutRef
```

## Properties

| Property | Modifiers | Type | Description |
| --- | --- | --- | --- |
| [onClose](./kibana-plugin-public.flyoutref.onclose.md) | | <code>Promise&lt;void&gt;</code> | An Promise that will resolve once this flyout is closed.<!-- -->Flyouts can close from user interaction, calling <code>close()</code> on the flyout reference or another call to <code>openFlyout()</code> replacing your flyout. |

## Methods

| Method | Modifiers | Description |
| --- | --- | --- |
| [close()](./kibana-plugin-public.flyoutref.close.md) | | Closes the referenced flyout if it's still open which in turn will resolve the <code>onClose</code> Promise. If the flyout had already been closed this method does nothing. |

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [FlyoutRef](./kibana-plugin-public.flyoutref.md) &gt; [onClose](./kibana-plugin-public.flyoutref.onclose.md)

## FlyoutRef.onClose property

An Promise that will resolve once this flyout is closed.

Flyouts can close from user interaction, calling `close()` on the flyout reference or another call to `openFlyout()` replacing your flyout.

<b>Signature:</b>

```typescript
readonly onClose: Promise<void>;
```
2 changes: 2 additions & 0 deletions docs/development/core/public/kibana-plugin-public.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

| Class | Description |
| --- | --- |
| [FlyoutRef](./kibana-plugin-public.flyoutref.md) | A FlyoutRef is a reference to an opened flyout panel. It offers methods to close the flyout panel again. If you open a flyout panel you should make sure you call <code>close()</code> when it should be closed. Since a flyout could also be closed by a user or from another flyout being opened, you must bind to the <code>onClose</code> Promise on the FlyoutRef instance. The Promise will resolve whenever the flyout was closed at which point you should discard the FlyoutRef. |
| [ToastsSetup](./kibana-plugin-public.toastssetup.md) | |
| [UiSettingsClient](./kibana-plugin-public.uisettingsclient.md) | |

Expand All @@ -16,6 +17,7 @@
| [ChromeBrand](./kibana-plugin-public.chromebrand.md) | |
| [ChromeBreadcrumb](./kibana-plugin-public.chromebreadcrumb.md) | |
| [CoreSetup](./kibana-plugin-public.coresetup.md) | Core services exposed to the start lifecycle |
| [OverlaySetup](./kibana-plugin-public.overlaysetup.md) | |
| [Plugin](./kibana-plugin-public.plugin.md) | The interface that should be returned by a <code>PluginInitializer</code>. |
| [PluginInitializerContext](./kibana-plugin-public.plugininitializercontext.md) | The available core services passed to a <code>PluginInitializer</code> |
| [PluginSetupContext](./kibana-plugin-public.pluginsetupcontext.md) | The available core services passed to a plugin's <code>Plugin#setup</code> method. |
Expand Down
17 changes: 17 additions & 0 deletions docs/development/core/public/kibana-plugin-public.overlaysetup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [OverlaySetup](./kibana-plugin-public.overlaysetup.md)

## OverlaySetup interface


<b>Signature:</b>

```typescript
export interface OverlaySetup
```

## Properties

| Property | Type | Description |
| --- | --- | --- |
| [openFlyout](./kibana-plugin-public.overlaysetup.openflyout.md) | <code>(flyoutChildren: React.ReactNode, flyoutProps?: {`<p/>` closeButtonAriaLabel?: string;`<p/>` 'data-test-subj'?: string;`<p/>` }) =&gt; FlyoutRef</code> | |

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [OverlaySetup](./kibana-plugin-public.overlaysetup.md) &gt; [openFlyout](./kibana-plugin-public.overlaysetup.openflyout.md)

## OverlaySetup.openFlyout property

<b>Signature:</b>

```typescript
openFlyout: (flyoutChildren: React.ReactNode, flyoutProps?: {
closeButtonAriaLabel?: string;
'data-test-subj'?: string;
}) => FlyoutRef;
```
7 changes: 7 additions & 0 deletions src/core/public/core_system.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { i18nServiceMock } from './i18n/i18n_service.mock';
import { injectedMetadataServiceMock } from './injected_metadata/injected_metadata_service.mock';
import { legacyPlatformServiceMock } from './legacy/legacy_service.mock';
import { notificationServiceMock } from './notifications/notifications_service.mock';
import { overlayServiceMock } from './overlays/overlay_service.mock';
import { pluginsServiceMock } from './plugins/plugins_service.mock';
import { uiSettingsServiceMock } from './ui_settings/ui_settings_service.mock';

Expand Down Expand Up @@ -92,6 +93,12 @@ jest.doMock('./chrome', () => ({
ChromeService: ChromeServiceConstructor,
}));

export const MockOverlayService = overlayServiceMock.create();
export const OverlayServiceConstructor = jest.fn().mockImplementation(() => MockOverlayService);
jest.doMock('./overlays', () => ({
OverlayService: OverlayServiceConstructor,
}));

export const MockPluginsService = pluginsServiceMock.create();
export const PluginsServiceConstructor = jest.fn().mockImplementation(() => MockPluginsService);
jest.doMock('./plugins', () => ({
Expand Down
10 changes: 9 additions & 1 deletion src/core/public/core_system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import {
MockInjectedMetadataService,
MockLegacyPlatformService,
MockNotificationsService,
MockOverlayService,
MockPluginsService,
MockUiSettingsService,
NotificationServiceConstructor,
OverlayServiceConstructor,
UiSettingsServiceConstructor,
} from './core_system.test.mocks';

Expand Down Expand Up @@ -81,6 +83,7 @@ describe('constructor', () => {
expect(BasePathServiceConstructor).toHaveBeenCalledTimes(1);
expect(UiSettingsServiceConstructor).toHaveBeenCalledTimes(1);
expect(ChromeServiceConstructor).toHaveBeenCalledTimes(1);
expect(OverlayServiceConstructor).toHaveBeenCalledTimes(1);
});

it('passes injectedMetadata param to InjectedMetadataService', () => {
Expand Down Expand Up @@ -229,7 +232,7 @@ describe('#setup()', () => {
const root = document.createElement('div');
root.innerHTML = '<p>foo bar</p>';
await setupCore(root);
expect(root.innerHTML).toBe('<div></div><div></div>');
expect(root.innerHTML).toBe('<div></div><div></div><div></div>');
});

it('calls injectedMetadata#setup()', async () => {
Expand Down Expand Up @@ -272,6 +275,11 @@ describe('#setup()', () => {
expect(MockChromeService.setup).toHaveBeenCalledTimes(1);
});

it('calls overlays#setup()', () => {
setupCore();
expect(MockOverlayService.setup).toHaveBeenCalledTimes(1);
});

it('calls plugin#setup()', async () => {
await setupCore();
expect(MockPluginsService.setup).toHaveBeenCalledTimes(1);
Expand Down
8 changes: 8 additions & 0 deletions src/core/public/core_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { I18nService } from './i18n';
import { InjectedMetadataParams, InjectedMetadataService } from './injected_metadata';
import { LegacyPlatformParams, LegacyPlatformService } from './legacy';
import { NotificationsService } from './notifications';
import { OverlayService } from './overlays';
import { PluginsService } from './plugins';
import { UiSettingsService } from './ui_settings';

Expand Down Expand Up @@ -63,11 +64,13 @@ export class CoreSystem {
private readonly basePath: BasePathService;
private readonly chrome: ChromeService;
private readonly i18n: I18nService;
private readonly overlay: OverlayService;
private readonly plugins: PluginsService;

private readonly rootDomElement: HTMLElement;
private readonly notificationsTargetDomElement$: Subject<HTMLDivElement>;
private readonly legacyPlatformTargetDomElement: HTMLDivElement;
private readonly overlayTargetDomElement: HTMLDivElement;

constructor(params: Params) {
const {
Expand Down Expand Up @@ -101,6 +104,8 @@ export class CoreSystem {
this.http = new HttpService();
this.basePath = new BasePathService();
this.uiSettings = new UiSettingsService();
this.overlayTargetDomElement = document.createElement('div');
this.overlay = new OverlayService(this.overlayTargetDomElement);
this.chrome = new ChromeService({ browserSupportsCsp });

const core: CoreContext = {};
Expand All @@ -121,6 +126,7 @@ export class CoreSystem {
const injectedMetadata = this.injectedMetadata.setup();
const fatalErrors = this.fatalErrors.setup({ i18n });
const http = this.http.setup({ fatalErrors });
const overlays = this.overlay.setup({ i18n });
const basePath = this.basePath.setup({ injectedMetadata });
const uiSettings = this.uiSettings.setup({
notifications,
Expand All @@ -142,6 +148,7 @@ export class CoreSystem {
injectedMetadata,
notifications,
uiSettings,
overlays,
};

await this.plugins.setup(core);
Expand All @@ -153,6 +160,7 @@ export class CoreSystem {
const notificationsTargetDomElement = document.createElement('div');
this.rootDomElement.appendChild(notificationsTargetDomElement);
this.rootDomElement.appendChild(this.legacyPlatformTargetDomElement);
this.rootDomElement.appendChild(this.overlayTargetDomElement);

// Only provide the DOM element to notifications once it's attached to the page.
// This prevents notifications from timing out before being displayed.
Expand Down
7 changes: 6 additions & 1 deletion src/core/public/i18n/i18n_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { I18nService, I18nSetup } from './i18n_service';

const PassThroughComponent = ({ children }: { children: React.ReactNode }) => children;

const createSetupContractMock = () => {
const setupContract: jest.Mocked<I18nSetup> = {
Context: jest.fn(),
// By default mock the Context component so it simply renders all children
Context: jest.fn().mockImplementation(PassThroughComponent),
};
return setupContract;
};
Expand Down
4 changes: 4 additions & 0 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { HttpSetup } from './http';
import { I18nSetup } from './i18n';
import { InjectedMetadataParams, InjectedMetadataSetup } from './injected_metadata';
import { NotificationsSetup, Toast, ToastInput, ToastsSetup } from './notifications';
import { FlyoutRef, OverlaySetup } from './overlays';
import { Plugin, PluginInitializer, PluginInitializerContext, PluginSetupContext } from './plugins';
import { UiSettingsClient, UiSettingsSetup, UiSettingsState } from './ui_settings';

Expand All @@ -44,6 +45,7 @@ export interface CoreSetup {
basePath: BasePathSetup;
uiSettings: UiSettingsSetup;
chrome: ChromeSetup;
overlays: OverlaySetup;
}

export {
Expand All @@ -62,6 +64,8 @@ export {
PluginInitializerContext,
PluginSetupContext,
NotificationsSetup,
OverlaySetup,
FlyoutRef,
Toast,
ToastInput,
ToastsSetup,
Expand Down
23 changes: 23 additions & 0 deletions src/core/public/kibana.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
```ts

import * as CSS from 'csstype';
import { default } from 'react';
import { Observable } from 'rxjs';
import * as PropTypes from 'prop-types';
import * as Rx from 'rxjs';
import { Toast } from '@elastic/eui';

Expand Down Expand Up @@ -63,6 +65,8 @@ export interface CoreSetup {
// (undocumented)
notifications: NotificationsSetup;
// (undocumented)
overlays: OverlaySetup;
// (undocumented)
uiSettings: UiSettingsSetup;
}

Expand Down Expand Up @@ -93,6 +97,14 @@ export class CoreSystem {
// @public (undocumented)
export type FatalErrorsSetup = ReturnType<FatalErrorsService['setup']>;

// @public
export class FlyoutRef {
// (undocumented)
constructor();
close(): Promise<void>;
readonly onClose: Promise<void>;
}

// Warning: (ae-forgotten-export) The symbol "HttpService" needs to be exported by the entry point index.d.ts
//
// @public (undocumented)
Expand Down Expand Up @@ -152,6 +164,17 @@ export type InjectedMetadataSetup = ReturnType<InjectedMetadataService['setup']>
// @public (undocumented)
export type NotificationsSetup = ReturnType<NotificationsService['setup']>;

// @public (undocumented)
export interface OverlaySetup {
// Warning: (ae-forgotten-export) The symbol "React" needs to be exported by the entry point index.d.ts
//
// (undocumented)
openFlyout: (flyoutChildren: React.ReactNode, flyoutProps?: {
closeButtonAriaLabel?: string;
'data-test-subj'?: string;
}) => FlyoutRef;
}

// @public
export interface Plugin<TSetup, TDependencies extends Record<string, unknown> = {}> {
// (undocumented)
Expand Down
3 changes: 3 additions & 0 deletions src/core/public/legacy/legacy_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ import { httpServiceMock } from '../http/http_service.mock';
import { i18nServiceMock } from '../i18n/i18n_service.mock';
import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock';
import { notificationServiceMock } from '../notifications/notifications_service.mock';
import { overlayServiceMock } from '../overlays/overlay_service.mock';
import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock';
import { LegacyPlatformService } from './legacy_service';

Expand All @@ -159,6 +160,7 @@ const i18nSetup = i18nServiceMock.createSetupContract();
const injectedMetadataSetup = injectedMetadataServiceMock.createSetupContract();
const notificationsSetup = notificationServiceMock.createSetupContract();
const uiSettingsSetup = uiSettingsServiceMock.createSetupContract();
const overlaySetup = overlayServiceMock.createSetupContract();

const defaultParams = {
targetDomElement: document.createElement('div'),
Expand All @@ -176,6 +178,7 @@ const defaultSetupDeps = {
basePath: basePathSetup,
uiSettings: uiSettingsSetup,
chrome: chromeSetup,
overlays: overlaySetup,
};

afterEach(() => {
Expand Down
Loading

0 comments on commit 8dbff4f

Please sign in to comment.