From 49655764b8b0e7d5f783e3213ba9a34ba90abb11 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 14 Sep 2018 22:13:12 -0700 Subject: [PATCH 1/5] [core/public/banners] migrate ui/notify/banners --- .../banners/banners_service.test.tsx | 168 ++++++++ .../notifications/banners/banners_service.tsx | 88 +++++ .../banners/global_banner_list.css} | 0 .../banners/global_banner_list.test.tsx | 69 ++++ .../banners/global_banner_list.tsx | 77 ++++ .../public/notifications/banners/index.ts} | 3 +- .../notifications/notifications_service.ts | 17 +- src/ui/public/chrome/directives/kbn_chrome.js | 19 +- src/ui/public/notify/banners.test.ts | 62 +++ .../notify/{banners/banners.js => banners.ts} | 94 +---- src/ui/public/notify/banners/BANNERS.md | 360 ------------------ .../global_banner_list.test.js.snap | 22 -- src/ui/public/notify/banners/banners.test.js | 161 -------- .../notify/banners/global_banner_list.js | 85 ----- .../notify/banners/global_banner_list.test.js | 65 ---- src/ui/public/notify/index.js | 2 +- 16 files changed, 499 insertions(+), 793 deletions(-) create mode 100644 src/core/public/notifications/banners/banners_service.test.tsx create mode 100644 src/core/public/notifications/banners/banners_service.tsx rename src/{ui/public/notify/banners/global_banner_list.less => core/public/notifications/banners/global_banner_list.css} (100%) create mode 100644 src/core/public/notifications/banners/global_banner_list.test.tsx create mode 100644 src/core/public/notifications/banners/global_banner_list.tsx rename src/{ui/public/notify/banners/index.js => core/public/notifications/banners/index.ts} (89%) create mode 100644 src/ui/public/notify/banners.test.ts rename src/ui/public/notify/{banners/banners.js => banners.ts} (51%) delete mode 100644 src/ui/public/notify/banners/BANNERS.md delete mode 100644 src/ui/public/notify/banners/__snapshots__/global_banner_list.test.js.snap delete mode 100644 src/ui/public/notify/banners/banners.test.js delete mode 100644 src/ui/public/notify/banners/global_banner_list.js delete mode 100644 src/ui/public/notify/banners/global_banner_list.test.js diff --git a/src/core/public/notifications/banners/banners_service.test.tsx b/src/core/public/notifications/banners/banners_service.test.tsx new file mode 100644 index 0000000000000..aed772b822260 --- /dev/null +++ b/src/core/public/notifications/banners/banners_service.test.tsx @@ -0,0 +1,168 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { unmountComponentAtNode } from 'react-dom'; +import { BannersService } from './banners_service'; + +describe('start.add()', () => { + it('renders the component in the targetDomElement', () => { + const targetDomElement = document.createElement('div'); + const start = new BannersService({ targetDomElement }).start(); + start.add('foo'); + expect(targetDomElement).toMatchInlineSnapshot(` +
+
+
+ foo +
+
+
+`); + }); + + it('renders higher-priority banners abover lower-priority ones', () => { + const targetDomElement = document.createElement('div'); + const start = new BannersService({ targetDomElement }).start(); + start.add('100', 100); + start.add('1', 1); + start.add('200', 200); + expect(targetDomElement).toMatchInlineSnapshot(` +
+
+
+ 200 +
+
+ 100 +
+
+ 1 +
+
+
+`); + }); +}); + +describe('start.remove()', () => { + it('removes the component from the targetDomElement', () => { + const targetDomElement = document.createElement('div'); + const start = new BannersService({ targetDomElement }).start(); + const id = start.add('foo'); + start.remove(id); + expect(targetDomElement).toMatchInlineSnapshot(`
`); + }); + + it('does nothing if the id is unknown', () => { + const targetDomElement = document.createElement('div'); + const start = new BannersService({ targetDomElement }).start(); + start.add('foo'); + start.remove('something random'); + expect(targetDomElement).toMatchInlineSnapshot(` +
+
+
+ foo +
+
+
+`); + }); +}); + +describe('start.replace()', () => { + it('replaces the banner with the matching id', () => { + const targetDomElement = document.createElement('div'); + const start = new BannersService({ targetDomElement }).start(); + const id = start.add('foo'); + start.replace(id, 'bar'); + expect(targetDomElement).toMatchInlineSnapshot(` +
+
+
+ bar +
+
+
+`); + }); + + it('adds the banner if the id is unknown', () => { + const targetDomElement = document.createElement('div'); + const start = new BannersService({ targetDomElement }).start(); + start.add('foo'); + start.replace('something random', 'bar'); + expect(targetDomElement).toMatchInlineSnapshot(` +
+
+
+ foo +
+
+ bar +
+
+
+`); + }); +}); + +describe('stop', () => { + it('unmounts the component from the targetDomElement', () => { + const targetDomElement = document.createElement('div'); + const service = new BannersService({ targetDomElement }); + service.start(); + service.stop(); + expect(unmountComponentAtNode(targetDomElement)).toBe(false); + }); + + it('cleans out the content of the targetDomElement', () => { + const targetDomElement = document.createElement('div'); + const service = new BannersService({ targetDomElement }); + service.start().add('foo-bar'); + service.stop(); + expect(targetDomElement).toMatchInlineSnapshot(`
`); + }); +}); diff --git a/src/core/public/notifications/banners/banners_service.tsx b/src/core/public/notifications/banners/banners_service.tsx new file mode 100644 index 0000000000000..249f29b964d35 --- /dev/null +++ b/src/core/public/notifications/banners/banners_service.tsx @@ -0,0 +1,88 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { render, unmountComponentAtNode } from 'react-dom'; +import * as Rx from 'rxjs'; +import { map } from 'rxjs/operators'; + +import { GlobalBannerList } from './global_banner_list'; + +export interface Banner { + readonly id: string; + readonly component: React.ReactChild; + readonly priority: number; +} + +export type Banners = Banner[]; + +interface Params { + targetDomElement: HTMLElement; +} + +export class BannersService { + constructor(private readonly params: Params) {} + + public start() { + let uniqueId = 0; + const banners$ = new Rx.BehaviorSubject([]); + const sortedBanners$ = banners$.pipe( + map(banners => banners.slice().sort((a, b) => b.priority - a.priority)) + ); + + render(, this.params.targetDomElement); + + return { + /** + * Add a component that should be rendered at the top of the page along with an optional priority. + */ + add: (component: React.ReactChild, priority = 0) => { + const id = `${++uniqueId}`; + banners$.next([...banners$.getValue(), { id, component, priority }]); + return id; + }, + + /** + * Remove a component from the top of the page. + */ + remove: (id: string) => { + banners$.next(banners$.getValue().filter(banner => banner.id !== id)); + }, + + /** + * Replace a banner component with a new one, potentially with a new priority + */ + replace: (id: string, component: React.ReactChild, priority = 0) => { + const newId = `${++uniqueId}`; + banners$.next([ + ...banners$.getValue().filter(banner => banner.id !== id), + { id: newId, component, priority }, + ]); + return newId; + }, + }; + } + + public stop() { + unmountComponentAtNode(this.params.targetDomElement); + this.params.targetDomElement.textContent = ''; + } +} + +export type BannersStartContract = ReturnType; diff --git a/src/ui/public/notify/banners/global_banner_list.less b/src/core/public/notifications/banners/global_banner_list.css similarity index 100% rename from src/ui/public/notify/banners/global_banner_list.less rename to src/core/public/notifications/banners/global_banner_list.css diff --git a/src/core/public/notifications/banners/global_banner_list.test.tsx b/src/core/public/notifications/banners/global_banner_list.test.tsx new file mode 100644 index 0000000000000..298d88f7d8a9c --- /dev/null +++ b/src/core/public/notifications/banners/global_banner_list.test.tsx @@ -0,0 +1,69 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { shallow } from 'enzyme'; +import React from 'react'; +import * as Rx from 'rxjs'; +import { GlobalBannerList } from './global_banner_list'; + +it('renders nothing without banners', () => { + const component = shallow(); + expect(component).toMatchInlineSnapshot(`""`); +}); + +it('renders banners in the order received', () => { + const component = shallow( + first
, + priority: Math.random(), + }, + { + id: '456', + component:
second
, + priority: Math.random(), + }, + ])} + /> + ); + expect(component).toMatchInlineSnapshot(` +
+
+
+ first +
+
+
+
+ second +
+
+
+`); +}); diff --git a/src/core/public/notifications/banners/global_banner_list.tsx b/src/core/public/notifications/banners/global_banner_list.tsx new file mode 100644 index 0000000000000..e1ce6afcc3496 --- /dev/null +++ b/src/core/public/notifications/banners/global_banner_list.tsx @@ -0,0 +1,77 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import * as Rx from 'rxjs'; + +import { Banners } from './banners_service'; +import './global_banner_list.css'; + +interface Props { + banners$: Rx.Observable; +} + +interface State { + error?: Error; + banners?: Banners; +} + +export class GlobalBannerList extends React.Component { + public state: State = {}; + private subscription?: Rx.Subscription; + + public componentDidMount() { + this.subscription = this.props.banners$.subscribe({ + next: banners => { + this.setState({ banners }); + }, + error: error => { + this.setState({ error }); + }, + }); + } + + public componentWillUnmount() { + if (this.subscription) { + this.subscription.unsubscribe(); + } + } + + public render() { + const { error, banners } = this.state; + + if (error) { + throw error; + } + + if (!banners || !banners.length) { + return null; + } + + return ( +
+ {banners.map(({ id, component, priority, ...rest }) => ( +
+ {component} +
+ ))} +
+ ); + } +} diff --git a/src/ui/public/notify/banners/index.js b/src/core/public/notifications/banners/index.ts similarity index 89% rename from src/ui/public/notify/banners/index.js rename to src/core/public/notifications/banners/index.ts index b4d2a11f61003..81f4e0e751821 100644 --- a/src/ui/public/notify/banners/index.js +++ b/src/core/public/notifications/banners/index.ts @@ -17,5 +17,4 @@ * under the License. */ -export { GlobalBannerList } from './global_banner_list'; -export { banners } from './banners'; \ No newline at end of file +export { Banners, Banner, BannersService, BannersStartContract } from './banners_service'; diff --git a/src/core/public/notifications/notifications_service.ts b/src/core/public/notifications/notifications_service.ts index b1f50497aec70..738c9e6549f32 100644 --- a/src/core/public/notifications/notifications_service.ts +++ b/src/core/public/notifications/notifications_service.ts @@ -17,6 +17,7 @@ * under the License. */ +import { BannersService } from './banners'; import { ToastsService } from './toasts'; interface Params { @@ -25,26 +26,36 @@ interface Params { export class NotificationsService { private readonly toasts: ToastsService; + private readonly banners: BannersService; private readonly toastsContainer: HTMLElement; + private readonly bannersContainer: HTMLElement; constructor(private readonly params: Params) { this.toastsContainer = document.createElement('div'); this.toasts = new ToastsService({ targetDomElement: this.toastsContainer, }); + + this.bannersContainer = document.createElement('div'); + this.banners = new BannersService({ + targetDomElement: this.bannersContainer, + }); } public start() { this.params.targetDomElement.appendChild(this.toastsContainer); + const toasts = this.toasts.start(); + + this.params.targetDomElement.appendChild(this.bannersContainer); + const banners = this.banners.start(); - return { - toasts: this.toasts.start(), - }; + return { toasts, banners }; } public stop() { this.toasts.stop(); + this.banners.stop(); this.params.targetDomElement.textContent = ''; } diff --git a/src/ui/public/chrome/directives/kbn_chrome.js b/src/ui/public/chrome/directives/kbn_chrome.js index 31bdf62646a7f..d29defba76e67 100644 --- a/src/ui/public/chrome/directives/kbn_chrome.js +++ b/src/ui/public/chrome/directives/kbn_chrome.js @@ -17,8 +17,6 @@ * under the License. */ -import React from 'react'; -import ReactDOM from 'react-dom'; import $ from 'jquery'; import './kbn_chrome.less'; @@ -27,11 +25,7 @@ import { getUnhashableStatesProvider, unhashUrl, } from '../../state_management/state_hashing'; -import { - notify, - GlobalBannerList, - banners, -} from '../../notify'; +import { notify } from '../../notify'; import { SubUrlRouteFilterProvider } from './sub_url_route_filter'; export function kbnChromeProvider(chrome, internals) { @@ -86,17 +80,6 @@ export function kbnChromeProvider(chrome, internals) { // Notifications $scope.notifList = notify._notifs; - // Non-scope based code (e.g., React) - - // Banners - ReactDOM.render( - , - document.getElementById('globalBannerList') - ); - return chrome; } }; diff --git a/src/ui/public/notify/banners.test.ts b/src/ui/public/notify/banners.test.ts new file mode 100644 index 0000000000000..b28250d848ddd --- /dev/null +++ b/src/ui/public/notify/banners.test.ts @@ -0,0 +1,62 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { __newPlatformInit__, banners } from './banners'; + +__newPlatformInit__({ + add: jest.fn((...args) => ['add', args]), + remove: jest.fn((...args) => ['remove', args]), + replace: jest.fn((...args) => ['replace', args]), +}); + +it('forwards calls to newPlatformBanners.add()', () => { + expect(banners.add({ component: 'foo', priority: 10 })).toMatchInlineSnapshot(` +Array [ + "add", + Array [ + "foo", + 10, + ], +] +`); +}); + +it('forwards calls to newPlatformBanners.remove()', () => { + expect(banners.remove('id')).toMatchInlineSnapshot(` +Array [ + "remove", + Array [ + "id", + ], +] +`); +}); + +it('forwards calls to newPlatformBanners.replace()', () => { + expect(banners.set({ id: 'id', component: 'bar', priority: 100 })).toMatchInlineSnapshot(` +Array [ + "replace", + Array [ + "id", + "bar", + 100, + ], +] +`); +}); diff --git a/src/ui/public/notify/banners/banners.js b/src/ui/public/notify/banners.ts similarity index 51% rename from src/ui/public/notify/banners/banners.js rename to src/ui/public/notify/banners.ts index 3f8dd08771d43..5d5fe73946cb0 100644 --- a/src/ui/public/notify/banners/banners.js +++ b/src/ui/public/notify/banners.ts @@ -17,47 +17,19 @@ * under the License. */ -/** - * Banners represents a prioritized list of displayed components. - */ -export class Banners { - - constructor() { - // sorted in descending order (100, 99, 98...) so that higher priorities are in front - this.list = []; - this.uniqueId = 0; - this.onChangeCallback = null; - } - - _changed = () => { - if (this.onChangeCallback) { - this.onChangeCallback(); - } - } - - _remove = id => { - const index = this.list.findIndex(details => details.id === id); - - if (index !== -1) { - this.list.splice(index, 1); +import { BannersStartContract } from '../../../core/public/notifications/banners'; - return true; - } +let newPlatformBanners: BannersStartContract; - return false; +export function __newPlatformInit__(instance: BannersStartContract) { + if (newPlatformBanners) { + throw new Error('ui/chrome/api/base_path is already initialized'); } - /** - * Set the {@code callback} to invoke whenever changes are made to the banner list. - * - * Use {@code null} or {@code undefined} to unset it. - * - * @param {Function} callback The callback to use. - */ - onChange = callback => { - this.onChangeCallback = callback; - } + newPlatformBanners = instance; +} +export const banners = { /** * Add a new banner. * @@ -65,25 +37,9 @@ export class Banners { * @param {Number} priority The optional priority order to display this banner. Higher priority values are shown first. * @return {String} A newly generated ID. This value can be used to remove/replace the banner. */ - add = ({ component, priority = 0 }) => { - const id = `${++this.uniqueId}`; - const bannerDetails = { id, component, priority }; - - // find the lowest priority item to put this banner in front of - const index = this.list.findIndex(details => priority > details.priority); - - if (index !== -1) { - // we found something with a lower priority; so stick it in front of that item - this.list.splice(index, 0, bannerDetails); - } else { - // nothing has a lower priority, so put it at the end - this.list.push(bannerDetails); - } - - this._changed(); - - return id; - } + add(banner: { component: React.ReactChild; priority?: number }) { + return newPlatformBanners.add(banner.component, banner.priority); + }, /** * Remove an existing banner. @@ -91,15 +47,9 @@ export class Banners { * @param {String} id The ID of the banner to remove. * @return {Boolean} {@code true} if the ID is recognized and the banner is removed. {@code false} otherwise. */ - remove = id => { - const removed = this._remove(id); - - if (removed) { - this._changed(); - } - - return removed; - } + remove(id: string) { + return newPlatformBanners.remove(id); + }, /** * Replace an existing banner by removing it, if it exists, and adding a new one in its place. @@ -112,15 +62,7 @@ export class Banners { * @param {Number} priority The optional priority order to display this banner. Higher priority values are shown first. * @return {String} A newly generated ID. This value can be used to remove/replace the banner. */ - set = ({ component, id, priority = 0 }) => { - this._remove(id); - - return this.add({ component, priority }); - } - -} - -/** - * A singleton instance meant to represent all Kibana banners. - */ -export const banners = new Banners(); + set(banner: { id: string; component: React.ReactChild; priority?: number }) { + return newPlatformBanners.replace(banner.id, banner.component, banner.priority); + }, +}; diff --git a/src/ui/public/notify/banners/BANNERS.md b/src/ui/public/notify/banners/BANNERS.md deleted file mode 100644 index fc6bddc3aa4ed..0000000000000 --- a/src/ui/public/notify/banners/BANNERS.md +++ /dev/null @@ -1,360 +0,0 @@ -# Banners - -Use this service to surface banners at the top of the screen. The expectation is that the banner will used an -`` to render, but that is not a requirement. See [the EUI docs](https://elastic.github.io/eui/) for -more information on banners and their role within the UI. - -Banners should be considered with respect to their lifecycle. Most banners are best served by using the `add` and -`remove` functions. - -## Importing the module - -```js -import { banners } from 'ui/notify'; -``` - -## Interface - -There are three methods defined to manipulate the list of banners: `add`, `set`, and `remove`. A fourth method, -`onChange` exists to listen to changes made via `add`, `set`, and `remove`. - -### `add()` - -This is the preferred way to add banners because it implies the best usage of the banner: added once during a page's -lifecycle. For other usages, consider *not* using a banner. - -#### Syntax - -```js -const bannerId = banners.add({ - // required: - component, - // optional: - priority, -}); -``` - -##### Parameters - -| Field | Type | Description | -|-------|------|-------------| -| `component` | Any | The value displayed as the banner. | -| `priority` | Number | Optional priority, which defaults to `0` used to place the banner. | - -To add a banner, you only need to define the `component` field. - -The `priority` sorts in descending order. Items sharing the same priority are sorted from oldest to newest. For example: - -```js -const banner1 = banners.add({ component: }); -const banner2 = banners.add({ component: , priority: 0 }); -const banner3 = banners.add({ component: , priority: 1 }); -``` - -That would be displayed as: - -``` -[ fake3 ] -[ fake1 ] -[ fake2 ] -``` - -##### Returns - -| Type | Description | -|------|-------------| -| String | A newly generated ID. | - -#### Example - -This example includes buttons that allow the user to remove the banner. In some cases, you may not want any buttons -and in other cases you will want an action to proceed the banner's removal (e.g., apply an Advanced Setting). - -This makes the most sense to use when a banner is added at the beginning of the page life cycle and not expected to -be touched, except by its own buttons triggering an action or navigating away. - -```js -const bannerId = banners.add({ - component: ( - - - - banners.remove(bannerId)} - > - Dismiss - - - - window.alert('Do Something Else')} - > - Do Something Else - - - - - ), -}); -``` - -### `remove()` - -Unlike toast notifications, banners stick around until they are explicitly removed. Using the `add` example above,you can remove it by calling `remove`. - -Note: They will stick around as long as the scope is remembered by whatever set it; navigating away won't remove it -unless the scope is forgotten (e.g., when the "app" changes)! - -#### Syntax - -```js -const removed = banners.remove(bannerId); -``` - -##### Parameters - -| Field | Type | Description | -|-------|------|-------------| -| `id` | String | ID of a banner. | - -##### Returns - -| Type | Description | -|------|-------------| -| Boolean | `true` if the ID was recognized and the banner was removed. `false` otherwise. | - -#### Example - -To remove a banner, you need to pass the `id` of the banner. - -```js -if (banners.remove(bannerId)) { - // removed; otherwise it didn't exist (maybe it was already removed) -} -``` - -#### Scheduled removal - -Like toast notifications do automatically, you can have a banner automatically removed after a set of time, by -setting a timer: - -```js -setTimeout(() => banners.remove(bannerId), 15000); -``` - -Note: It is safe to remove a banner more than once as unknown IDs will be ignored. - -### `set()` - -Banners can be replaced once added by supplying their `id`. If one is supplied, then the ID will be used to replace -any banner with the same ID and a **new** `id` will be returned. - -You should only consider using `set` when the banner is manipulated frequently in the lifecycle of the page, where -maintaining the banner's `id` can be a burden. It is easier to allow `banners` to create the ID for you in most -situations where a banner is useful (e.g., set once), which safely avoids any chance to have an ID-based collision, -which happens automatically with `add`. - -Usage of `set` can imply that your use case is abusing the banner system. - -Note: `set` will only trigger the callback once for both the implicit remove and add operation. - -#### Syntax - -```js -const id = banners.set({ - // required: - component, - // optional: - id, - priority, -}); -``` - -##### Parameters - -| Field | Type | Description | -|-------|------|-------------| -| `component` | Any | The value displayed as the banner. | -| `id` | String | Optional ID used to remove an existing banner. | -| `priority` | Number | Optional priority, which defaults to `0` used to place the banner. | - -The `id` is optional because it follows the same semantics as the `remove` method: unknown IDs are ignored. This -is useful when first creating a banner so that you do not have to call `add` instead. - -##### Returns - -| Type | Description | -|------|-------------| -| String | A newly generated ID. | - -#### Example - -This example does not include any way for the user to clear the banner directly. Instead, it is cleared based on -time. Related to it being cleared by time, it can also reappear within the same page life cycle by navigating between -different paths that need it displayed. Instead of adding a new banner for every navigation, you should replace any -existing banner. - -```js -let bannerId; -let timeoutId; - -function displayBanner() { - clearTimeout(timeoutId); - - bannerId = banners.set({ - id: bannerId, // the first time it will be undefined, but reused as long as this is in the same lifecycle - component: ( - - ) - }); - - // hide the message after the user has had a chance to acknowledge it -- so it doesn't permanently stick around - banner.timeoutId = setTimeout(() => { - banners.remove(bannerId); - timeoutId = undefined; - }, 6000); -} -``` - -### `onChange()` - -For React components that intend to display the banners, it is not enough to simply `render` the `banners.list` -values. Because they can change after being rendered, the React component that renders the list must be alerted -to changes to the list. - -#### Syntax - -```js -// inside your React component -banners.onChange(() => this.forceUpdate()); -``` - -##### Parameters - -| Field | Type | Description | -|-------|------|-------------| -| `callback` | Function | The function to invoke whenever the internal banner list is changed. | - -Every new `callback` replaces the previous callback. So calling this with `null` or `undefined` will unset the -callback. - -##### Returns - -Nothing. - -#### Example - -This can be used inside of a React component to trigger a re-`render` of the banners. - -```js -import { GlobalBannerList } from 'ui/notify'; - - -``` - -### `list` - -For React components that intend to display the banners, it is not enough to simply `render` the `banners.list` -values. Because they can change after being rendered, the React component that renders the list must be alerted -to changes to the list. - -#### Syntax - -```js - -``` - -##### Returns - -| Type | Description | -|------|-------------| -| Array | The array of banner objects. | - -Banner objects are sorted in descending order based on their `priority`, in the form: - -```js -{ - id: 'banner-123', - component: , - priority: 12, -} -``` - -| Field | Type | Description | -|-------|------|-------------| -| `component` | Any | The value displayed as the banner. | -| `id` | String | The ID of the banner, which can be used as a React "key". | -| `priority` | Number | The priority of the banner. | - -#### Example - -This can be used to supply the banners to the `GlobalBannerList` React component (which is done for you). - -```js -import { GlobalBannerList } from 'ui/notify'; - - -``` - -## Use in functional tests - -Functional tests are commonly used to verify that an action yielded a successful outcome. You can place a -`data-test-subj` attribute on the banner and use it to check if the banner exists inside of your functional test. -This acts as a proxy for verifying the successful outcome. Any unrecognized field will be added as a property of the -containing element. - -```js -banners.add({ - component: ( - - ), - data-test-subj: 'my-tested-banner', -}); -``` - -This will apply the `data-test-subj` to the element containing the `component`, so the inner HTML of that element -will exclusively be the specified `component`. - -Given that `component` is expected to be a React component, you could also add the `data-test-subj` directly to it: - -```js -banners.add({ - component: ( - - ), -}); -``` \ No newline at end of file diff --git a/src/ui/public/notify/banners/__snapshots__/global_banner_list.test.js.snap b/src/ui/public/notify/banners/__snapshots__/global_banner_list.test.js.snap deleted file mode 100644 index e66ff83886adb..0000000000000 --- a/src/ui/public/notify/banners/__snapshots__/global_banner_list.test.js.snap +++ /dev/null @@ -1,22 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`GlobalBannerList is rendered 1`] = `null`; - -exports[`GlobalBannerList props banners is rendered 1`] = ` -
-
- a component -
-
- b good -
-
-`; diff --git a/src/ui/public/notify/banners/banners.test.js b/src/ui/public/notify/banners/banners.test.js deleted file mode 100644 index 69b046bcb4d8f..0000000000000 --- a/src/ui/public/notify/banners/banners.test.js +++ /dev/null @@ -1,161 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import sinon from 'sinon'; - -import { - Banners, -} from './banners'; - -describe('Banners', () => { - - describe('interface', () => { - let banners; - - beforeEach(() => { - banners = new Banners(); - }); - - describe('onChange method', () => { - - test('callback is called when a banner is added', () => { - const onChangeSpy = sinon.spy(); - banners.onChange(onChangeSpy); - banners.add({ component: 'bruce-banner' }); - expect(onChangeSpy.callCount).toBe(1); - }); - - test('callback is called when a banner is removed', () => { - const onChangeSpy = sinon.spy(); - banners.onChange(onChangeSpy); - banners.remove(banners.add({ component: 'bruce-banner' })); - expect(onChangeSpy.callCount).toBe(2); - }); - - test('callback is not called when remove is ignored', () => { - const onChangeSpy = sinon.spy(); - banners.onChange(onChangeSpy); - banners.remove('hulk'); // should not invoke callback - expect(onChangeSpy.callCount).toBe(0); - }); - - test('callback is called once when banner is replaced', () => { - const onChangeSpy = sinon.spy(); - banners.onChange(onChangeSpy); - const addBannerId = banners.add({ component: 'bruce-banner' }); - banners.set({ id: addBannerId, component: 'hulk' }); - expect(onChangeSpy.callCount).toBe(2); - }); - - }); - - describe('add method', () => { - - test('adds a banner', () => { - const id = banners.add({}); - expect(banners.list.length).toBe(1); - expect(id).toEqual(expect.stringMatching(/^\d+$/)); - }); - - test('adds a banner and ignores an ID property', () => { - const bannerId = banners.add({ id: 'bruce-banner' }); - expect(banners.list[0].id).toBe(bannerId); - expect(bannerId).not.toBe('bruce-banner'); - }); - - test('sorts banners based on priority', () => { - const test0 = banners.add({ }); - // the fact that it was set explicitly is irrelevant; that it was added second means it should be after test0 - const test0Explicit = banners.add({ priority: 0 }); - const test1 = banners.add({ priority: 1 }); - const testMinus1 = banners.add({ priority: -1 }); - const test1000 = banners.add({ priority: 1000 }); - - expect(banners.list.length).toBe(5); - expect(banners.list[0].id).toBe(test1000); - expect(banners.list[1].id).toBe(test1); - expect(banners.list[2].id).toBe(test0); - expect(banners.list[3].id).toBe(test0Explicit); - expect(banners.list[4].id).toBe(testMinus1); - }); - - }); - - describe('remove method', () => { - - test('removes a banner', () => { - const bannerId = banners.add({ component: 'bruce-banner' }); - banners.remove(bannerId); - expect(banners.list.length).toBe(0); - }); - - test('ignores unknown id', () => { - banners.add({ component: 'bruce-banner' }); - banners.remove('hulk'); - expect(banners.list.length).toBe(1); - }); - - }); - - describe('set method', () => { - - test('replaces banners', () => { - const addBannerId = banners.add({ component: 'bruce-banner' }); - const setBannerId = banners.set({ id: addBannerId, component: 'hulk' }); - - expect(banners.list.length).toBe(1); - expect(banners.list[0].component).toBe('hulk'); - expect(banners.list[0].id).toBe(setBannerId); - expect(addBannerId).not.toBe(setBannerId); - }); - - test('ignores unknown id', () => { - const id = banners.set({ id: 'fake', component: 'hulk' }); - - expect(banners.list.length).toBe(1); - expect(banners.list[0].component).toBe('hulk'); - expect(banners.list[0].id).toBe(id); - }); - - test('replaces a banner with the same ID property', () => { - const test0 = banners.add({ }); - const test0Explicit = banners.add({ priority: 0 }); - let test1 = banners.add({ priority: 1, component: 'old' }); - const testMinus1 = banners.add({ priority: -1 }); - let test1000 = banners.add({ priority: 1000, component: 'old' }); - - // change one with the same priority - test1 = banners.set({ id: test1, priority: 1, component: 'new' }); - // change one with a different priority - test1000 = banners.set({ id: test1000, priority: 1, component: 'new' }); - - expect(banners.list.length).toBe(5); - expect(banners.list[0].id).toBe(test1); - expect(banners.list[0].component).toBe('new'); - expect(banners.list[1].id).toBe(test1000); // priority became 1, so it goes after the other "1" - expect(banners.list[1].component).toBe('new'); - expect(banners.list[2].id).toBe(test0); - expect(banners.list[3].id).toBe(test0Explicit); - expect(banners.list[4].id).toBe(testMinus1); - }); - - }); - - }); -}); diff --git a/src/ui/public/notify/banners/global_banner_list.js b/src/ui/public/notify/banners/global_banner_list.js deleted file mode 100644 index ca60ae373853f..0000000000000 --- a/src/ui/public/notify/banners/global_banner_list.js +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import React, { Component } from 'react'; -import PropTypes from 'prop-types'; - -import './global_banner_list.less'; - -/** - * GlobalBannerList is a list of "banners". A banner something that is displayed at the top of Kibana that may or may not disappear. - * - * Whether or not a banner can be closed is completely up to the author of the banner. Some banners make sense to be static, such as - * banners meant to indicate the sensitivity (e.g., classification) of the information being represented. - * - * Banners are currently expected to be instances, but that is not required. - * - * @param {Array} banners The array of banners represented by objects in the form of { id, component }. - */ -export class GlobalBannerList extends Component { - - static propTypes = { - banners: PropTypes.array, - subscribe: PropTypes.func, - }; - - static defaultProps = { - banners: [], - }; - - constructor(props) { - super(props); - - if (this.props.subscribe) { - this.props.subscribe(() => this.forceUpdate()); - } - } - - render() { - if (this.props.banners.length === 0) { - return null; - } - - const flexBanners = this.props.banners.map(banner => { - const { - id, - component, - priority, - ...rest - } = banner; - - return ( -
- { component } -
- ); - }); - - return ( -
- {flexBanners} -
- ); - } -} diff --git a/src/ui/public/notify/banners/global_banner_list.test.js b/src/ui/public/notify/banners/global_banner_list.test.js deleted file mode 100644 index 5a008f33c57d9..0000000000000 --- a/src/ui/public/notify/banners/global_banner_list.test.js +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import React from 'react'; -import { render } from 'enzyme'; -import { GlobalBannerList } from './global_banner_list'; - -describe('GlobalBannerList', () => { - - test('is rendered', () => { - const component = render( - - ); - - expect(component) - .toMatchSnapshot(); - - }); - - describe('props', () => { - - describe('banners', () => { - - test('is rendered', () => { - const banners = [{ - id: 'a', - component: 'a component', - priority: 1, - }, { - 'data-test-subj': 'b', - id: 'b', - component: 'b good', - }]; - - const component = render( - - ); - - expect(component) - .toMatchSnapshot(); - }); - - }); - - }); - -}); diff --git a/src/ui/public/notify/index.js b/src/ui/public/notify/index.js index 078b4807430dd..c3ac93f89e8d8 100644 --- a/src/ui/public/notify/index.js +++ b/src/ui/public/notify/index.js @@ -21,5 +21,5 @@ export { notify } from './notify'; export { Notifier } from './notifier'; export { fatalError, addFatalErrorCallback } from './fatal_error'; export { toastNotifications } from './toasts'; -export { GlobalBannerList, banners } from './banners'; +export { banners } from './banners'; export { addAppRedirectMessageToUrl, showAppRedirectNotification } from './app_redirect'; From fc52d56ebef1406af1571e1ad8b53d012d52994b Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 18 Sep 2018 04:52:03 -0700 Subject: [PATCH 2/5] [core/public/banners] refactor to not expose react in contract, improve tests --- .../legacy_platform_service.test.ts | 20 +++ .../legacy_platform_service.ts | 1 + .../banners/banners_service.test.tsx | 44 +++++-- .../notifications/banners/banners_service.tsx | 21 ++-- .../global_banner_item.css} | 4 - .../components/global_banner_item.test.tsx | 84 +++++++++++++ .../banners/components/global_banner_item.tsx | 65 ++++++++++ .../banners/components/global_banner_list.css | 3 + .../components/global_banner_list.test.tsx | 117 ++++++++++++++++++ .../banners/components/global_banner_list.tsx | 41 ++++++ .../global_banners_container.test.tsx} | 53 ++------ .../global_banners_container.tsx} | 15 +-- .../notifications_service.test.ts | 79 ++++++++++++ .../{banners.test.ts => banners.test.tsx} | 49 +++++++- .../public/notify/{banners.ts => banners.tsx} | 31 ++++- 15 files changed, 541 insertions(+), 86 deletions(-) rename src/core/public/notifications/banners/{global_banner_list.css => components/global_banner_item.css} (61%) create mode 100644 src/core/public/notifications/banners/components/global_banner_item.test.tsx create mode 100644 src/core/public/notifications/banners/components/global_banner_item.tsx create mode 100644 src/core/public/notifications/banners/components/global_banner_list.css create mode 100644 src/core/public/notifications/banners/components/global_banner_list.test.tsx create mode 100644 src/core/public/notifications/banners/components/global_banner_list.tsx rename src/core/public/notifications/banners/{global_banner_list.test.tsx => containers/global_banners_container.test.tsx} (55%) rename src/core/public/notifications/banners/{global_banner_list.tsx => containers/global_banners_container.tsx} (81%) create mode 100644 src/core/public/notifications/notifications_service.test.ts rename src/ui/public/notify/{banners.test.ts => banners.test.tsx} (57%) rename src/ui/public/notify/{banners.ts => banners.tsx} (71%) diff --git a/src/core/public/legacy_platform/legacy_platform_service.test.ts b/src/core/public/legacy_platform/legacy_platform_service.test.ts index 913cfc79a6ae9..84314b9a5bb42 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.test.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.test.ts @@ -62,6 +62,14 @@ jest.mock('ui/notify/toasts', () => { }; }); +const mockNotifyBannersInit = jest.fn(); +jest.mock('ui/notify/banners', () => { + mockLoadOrder.push('ui/notify/banners'); + return { + __newPlatformInit__: mockNotifyBannersInit, + }; +}); + const mockLoadingCountInit = jest.fn(); jest.mock('ui/chrome/api/loading_count', () => { mockLoadOrder.push('ui/chrome/api/loading_count'); @@ -99,6 +107,7 @@ import { LegacyPlatformService } from './legacy_platform_service'; const fatalErrorsStartContract = {} as any; const notificationsStartContract = { toasts: {}, + banners: {}, } as any; const injectedMetadataStartContract: any = { @@ -180,6 +189,17 @@ describe('#start()', () => { expect(mockNotifyToastsInit).toHaveBeenCalledWith(notificationsStartContract.toasts); }); + it('passes banners service to ui/notify/banners', () => { + const legacyPlatform = new LegacyPlatformService({ + ...defaultParams, + }); + + legacyPlatform.start(defaultStartDeps); + + expect(mockNotifyBannersInit).toHaveBeenCalledTimes(1); + expect(mockNotifyBannersInit).toHaveBeenCalledWith(notificationsStartContract.banners); + }); + it('passes loadingCount service to ui/chrome/api/loading_count', () => { const legacyPlatform = new LegacyPlatformService({ ...defaultParams, diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index 2b7ae5f2bc894..1232dd3d685eb 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -63,6 +63,7 @@ export class LegacyPlatformService { require('ui/metadata').__newPlatformInit__(injectedMetadata.getLegacyMetadata()); require('ui/notify/fatal_error').__newPlatformInit__(fatalErrors); require('ui/notify/toasts').__newPlatformInit__(notifications.toasts); + require('ui/notify/banners').__newPlatformInit__(notifications.banners); require('ui/chrome/api/loading_count').__newPlatformInit__(loadingCount); require('ui/chrome/api/base_path').__newPlatformInit__(basePath); require('ui/chrome/api/ui_settings').__newPlatformInit__(uiSettings); diff --git a/src/core/public/notifications/banners/banners_service.test.tsx b/src/core/public/notifications/banners/banners_service.test.tsx index aed772b822260..086d96d584abe 100644 --- a/src/core/public/notifications/banners/banners_service.test.tsx +++ b/src/core/public/notifications/banners/banners_service.test.tsx @@ -20,11 +20,19 @@ import { unmountComponentAtNode } from 'react-dom'; import { BannersService } from './banners_service'; +function renderFn(innerHTML: string) { + return (el: HTMLDivElement) => { + el.innerHTML = innerHTML; + return () => (el.innerHTML = ''); + }; +} + describe('start.add()', () => { it('renders the component in the targetDomElement', () => { const targetDomElement = document.createElement('div'); const start = new BannersService({ targetDomElement }).start(); - start.add('foo'); + start.add(renderFn('foo')); + expect(targetDomElement).toMatchInlineSnapshot(`
{ it('renders higher-priority banners abover lower-priority ones', () => { const targetDomElement = document.createElement('div'); const start = new BannersService({ targetDomElement }).start(); - start.add('100', 100); - start.add('1', 1); - start.add('200', 200); + start.add(renderFn('100'), 100); + start.add(renderFn('1'), 1); + start.add(renderFn('200'), 200); + expect(targetDomElement).toMatchInlineSnapshot(`
{ it('removes the component from the targetDomElement', () => { const targetDomElement = document.createElement('div'); const start = new BannersService({ targetDomElement }).start(); - const id = start.add('foo'); + const id = start.add(renderFn('foo')); start.remove(id); expect(targetDomElement).toMatchInlineSnapshot(`
`); }); @@ -84,7 +93,7 @@ describe('start.remove()', () => { it('does nothing if the id is unknown', () => { const targetDomElement = document.createElement('div'); const start = new BannersService({ targetDomElement }).start(); - start.add('foo'); + start.add(renderFn('foo')); start.remove('something random'); expect(targetDomElement).toMatchInlineSnapshot(`
@@ -106,8 +115,21 @@ describe('start.replace()', () => { it('replaces the banner with the matching id', () => { const targetDomElement = document.createElement('div'); const start = new BannersService({ targetDomElement }).start(); - const id = start.add('foo'); - start.replace(id, 'bar'); + const id = start.add(renderFn('foo')); + expect(targetDomElement).toMatchInlineSnapshot(` +
+
+
+ foo +
+
+
+`); + start.replace(id, renderFn('bar')); expect(targetDomElement).toMatchInlineSnapshot(`
{ it('adds the banner if the id is unknown', () => { const targetDomElement = document.createElement('div'); const start = new BannersService({ targetDomElement }).start(); - start.add('foo'); - start.replace('something random', 'bar'); + start.add(renderFn('foo')); + start.replace('something random', renderFn('bar')); expect(targetDomElement).toMatchInlineSnapshot(`
{ it('cleans out the content of the targetDomElement', () => { const targetDomElement = document.createElement('div'); const service = new BannersService({ targetDomElement }); - service.start().add('foo-bar'); + service.start().add(renderFn('foo-bar')); service.stop(); expect(targetDomElement).toMatchInlineSnapshot(`
`); }); diff --git a/src/core/public/notifications/banners/banners_service.tsx b/src/core/public/notifications/banners/banners_service.tsx index 249f29b964d35..22ee6a4feb0de 100644 --- a/src/core/public/notifications/banners/banners_service.tsx +++ b/src/core/public/notifications/banners/banners_service.tsx @@ -20,14 +20,13 @@ import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; import * as Rx from 'rxjs'; -import { map } from 'rxjs/operators'; -import { GlobalBannerList } from './global_banner_list'; +import { GlobalBannersContainer } from './containers/global_banners_container'; export interface Banner { readonly id: string; - readonly component: React.ReactChild; readonly priority: number; + readonly render: (targetDomElement: HTMLDivElement) => (() => void) | void; } export type Banners = Banner[]; @@ -42,19 +41,19 @@ export class BannersService { public start() { let uniqueId = 0; const banners$ = new Rx.BehaviorSubject([]); - const sortedBanners$ = banners$.pipe( - map(banners => banners.slice().sort((a, b) => b.priority - a.priority)) - ); - render(, this.params.targetDomElement); + render( + , + this.params.targetDomElement + ); return { /** * Add a component that should be rendered at the top of the page along with an optional priority. */ - add: (component: React.ReactChild, priority = 0) => { + add: (renderFn: Banner['render'], priority = 0) => { const id = `${++uniqueId}`; - banners$.next([...banners$.getValue(), { id, component, priority }]); + banners$.next([...banners$.getValue(), { id, priority, render: renderFn }]); return id; }, @@ -68,11 +67,11 @@ export class BannersService { /** * Replace a banner component with a new one, potentially with a new priority */ - replace: (id: string, component: React.ReactChild, priority = 0) => { + replace: (id: string, renderFn: Banner['render'], priority = 0) => { const newId = `${++uniqueId}`; banners$.next([ ...banners$.getValue().filter(banner => banner.id !== id), - { id: newId, component, priority }, + { id: newId, priority, render: renderFn }, ]); return newId; }, diff --git a/src/core/public/notifications/banners/global_banner_list.css b/src/core/public/notifications/banners/components/global_banner_item.css similarity index 61% rename from src/core/public/notifications/banners/global_banner_list.css rename to src/core/public/notifications/banners/components/global_banner_item.css index f1ead7231f4f4..2850c314fecf3 100644 --- a/src/core/public/notifications/banners/global_banner_list.css +++ b/src/core/public/notifications/banners/components/global_banner_item.css @@ -1,7 +1,3 @@ -.globalBanner__list { - padding: 16px; -} - .globalBanner__item + .globalBanner__item { margin-top: 16px; } diff --git a/src/core/public/notifications/banners/components/global_banner_item.test.tsx b/src/core/public/notifications/banners/components/global_banner_item.test.tsx new file mode 100644 index 0000000000000..32337cb5a1950 --- /dev/null +++ b/src/core/public/notifications/banners/components/global_banner_item.test.tsx @@ -0,0 +1,84 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { mount } from 'enzyme'; +import React from 'react'; +import { GlobalBannerItem } from './global_banner_item'; + +it('calls banner render function with div element', () => { + const render = jest.fn(); + + mount( + + ); + + expect(render.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ +
, + ], +] +`); +}); + +it('calls unrender function before new render function when replaced, calls unrender when unmounted', () => { + expect.assertions(4); + + const unrender = jest.fn(); + const render = jest.fn(() => unrender); + + const unrender2 = jest.fn(); + const render2 = jest.fn(() => { + expect(unrender).toHaveBeenCalledTimes(1); + return unrender2; + }); + + const component = mount( + + ); + + component.setProps({ + banner: { + id: '123', + priority: 123, + render: render2, + }, + }); + + expect(render2).toHaveBeenCalled(); + expect(unrender2).not.toHaveBeenCalled(); + + component.unmount(); + + expect(unrender2).toHaveBeenCalled(); +}); diff --git a/src/core/public/notifications/banners/components/global_banner_item.tsx b/src/core/public/notifications/banners/components/global_banner_item.tsx new file mode 100644 index 0000000000000..80ee711fea95e --- /dev/null +++ b/src/core/public/notifications/banners/components/global_banner_item.tsx @@ -0,0 +1,65 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { Banner } from '../banners_service'; +import './global_banner_item.css'; + +interface Props { + banner: Banner; +} + +export class GlobalBannerItem extends React.Component { + private readonly ref = React.createRef(); + private unrenderBanner?: () => void; + + public componentDidMount() { + if (!this.ref.current) { + throw new Error(' mounted without ref'); + } + + this.unrenderBanner = this.props.banner.render(this.ref.current) || undefined; + } + + public componentDidUpdate(prevProps: Props) { + if (this.props.banner.render === prevProps.banner.render) { + return; + } + + if (!this.ref.current) { + throw new Error(' updated without ref'); + } + + if (this.unrenderBanner) { + this.unrenderBanner(); + } + + this.unrenderBanner = this.props.banner.render(this.ref.current) || undefined; + } + + public componentWillUnmount() { + if (this.unrenderBanner) { + this.unrenderBanner(); + } + } + + public render() { + return
; + } +} diff --git a/src/core/public/notifications/banners/components/global_banner_list.css b/src/core/public/notifications/banners/components/global_banner_list.css new file mode 100644 index 0000000000000..f186b7be608bd --- /dev/null +++ b/src/core/public/notifications/banners/components/global_banner_list.css @@ -0,0 +1,3 @@ +.globalBanner__list { + padding: 16px; +} diff --git a/src/core/public/notifications/banners/components/global_banner_list.test.tsx b/src/core/public/notifications/banners/components/global_banner_list.test.tsx new file mode 100644 index 0000000000000..f5257717669b2 --- /dev/null +++ b/src/core/public/notifications/banners/components/global_banner_list.test.tsx @@ -0,0 +1,117 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { shallow } from 'enzyme'; +import React from 'react'; +import { GlobalBannerList } from './global_banner_list'; + +it('renders nothing without banners', () => { + const component = shallow(); + expect(component).toMatchInlineSnapshot(` +
+`); +}); + +it('renders banners in descending priority order', () => { + expect( + shallow( + + ) + ).toMatchInlineSnapshot(` +
+ + +
+`); + + expect( + shallow( + + ) + ).toMatchInlineSnapshot(` +
+ + +
+`); +}); diff --git a/src/core/public/notifications/banners/components/global_banner_list.tsx b/src/core/public/notifications/banners/components/global_banner_list.tsx new file mode 100644 index 0000000000000..169965b283c4a --- /dev/null +++ b/src/core/public/notifications/banners/components/global_banner_list.tsx @@ -0,0 +1,41 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; + +import { Banners } from '../banners_service'; +import { GlobalBannerItem } from './global_banner_item'; +import './global_banner_list.css'; + +interface Props { + banners: Banners; +} + +const sortByPriority = (banners: Banners) => + banners.slice().sort((a, b) => b.priority - a.priority); + +export function GlobalBannerList(props: Props) { + return ( +
+ {sortByPriority(props.banners).map(banner => ( + + ))} +
+ ); +} diff --git a/src/core/public/notifications/banners/global_banner_list.test.tsx b/src/core/public/notifications/banners/containers/global_banners_container.test.tsx similarity index 55% rename from src/core/public/notifications/banners/global_banner_list.test.tsx rename to src/core/public/notifications/banners/containers/global_banners_container.test.tsx index 298d88f7d8a9c..29d03665770dc 100644 --- a/src/core/public/notifications/banners/global_banner_list.test.tsx +++ b/src/core/public/notifications/banners/containers/global_banners_container.test.tsx @@ -20,50 +20,23 @@ import { shallow } from 'enzyme'; import React from 'react'; import * as Rx from 'rxjs'; -import { GlobalBannerList } from './global_banner_list'; +import { GlobalBannersContainer } from './global_banners_container'; it('renders nothing without banners', () => { - const component = shallow(); + const component = shallow(); expect(component).toMatchInlineSnapshot(`""`); }); -it('renders banners in the order received', () => { - const component = shallow( - first
, - priority: Math.random(), - }, - { - id: '456', - component:
second
, - priority: Math.random(), - }, - ])} - /> - ); - expect(component).toMatchInlineSnapshot(` -
-
-
- first -
-
-
-
- second -
-
-
+it('passes banners to the GlobalBannerList', () => { + expect(shallow()) + .toMatchInlineSnapshot(` + `); }); diff --git a/src/core/public/notifications/banners/global_banner_list.tsx b/src/core/public/notifications/banners/containers/global_banners_container.tsx similarity index 81% rename from src/core/public/notifications/banners/global_banner_list.tsx rename to src/core/public/notifications/banners/containers/global_banners_container.tsx index e1ce6afcc3496..4d116a33429d7 100644 --- a/src/core/public/notifications/banners/global_banner_list.tsx +++ b/src/core/public/notifications/banners/containers/global_banners_container.tsx @@ -20,7 +20,8 @@ import React from 'react'; import * as Rx from 'rxjs'; -import { Banners } from './banners_service'; +import { Banners } from '../banners_service'; +import { GlobalBannerList } from '../components/global_banner_list'; import './global_banner_list.css'; interface Props { @@ -32,7 +33,7 @@ interface State { banners?: Banners; } -export class GlobalBannerList extends React.Component { +export class GlobalBannersContainer extends React.Component { public state: State = {}; private subscription?: Rx.Subscription; @@ -64,14 +65,6 @@ export class GlobalBannerList extends React.Component { return null; } - return ( -
- {banners.map(({ id, component, priority, ...rest }) => ( -
- {component} -
- ))} -
- ); + return ; } } diff --git a/src/core/public/notifications/notifications_service.test.ts b/src/core/public/notifications/notifications_service.test.ts new file mode 100644 index 0000000000000..8ef33e2700042 --- /dev/null +++ b/src/core/public/notifications/notifications_service.test.ts @@ -0,0 +1,79 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +jest.mock('./toasts/toasts_service'); +jest.mock('./banners/banners_service'); + +import { NotificationsService } from './notifications_service'; + +const { ToastsService } = require.requireMock('./toasts/toasts_service'); +const { BannersService } = require.requireMock('./banners/banners_service'); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it('creates toasts and banner services, passing unique containers to each which are mounted in start and unmounted after stop()', () => { + expect.assertions(16); + const targetDomElement = document.createElement('div'); + const notifications = new NotificationsService({ + targetDomElement, + }); + + expect(ToastsService).toHaveBeenCalledTimes(1); + expect(ToastsService).toHaveBeenCalledWith({ + targetDomElement: expect.any(HTMLDivElement), + }); + + expect(BannersService).toHaveBeenCalledTimes(1); + expect(BannersService).toHaveBeenCalledWith({ + targetDomElement: expect.any(HTMLDivElement), + }); + + const [[{ targetDomElement: toastsContainer }]] = ToastsService.mock.calls; + const [[{ targetDomElement: bannersContainer }]] = BannersService.mock.calls; + expect(toastsContainer).not.toBe(bannersContainer); + expect(toastsContainer.parentElement).toBe(null); + expect(bannersContainer.parentElement).toBe(null); + + ToastsService.prototype.start.mockImplementation(() => { + expect(toastsContainer.parentElement).toBe(targetDomElement); + }); + + ToastsService.prototype.stop.mockImplementation(() => { + expect(toastsContainer.parentElement).toBe(targetDomElement); + }); + + BannersService.prototype.start.mockImplementation(() => { + expect(bannersContainer.parentElement).toBe(targetDomElement); + }); + + BannersService.prototype.stop.mockImplementation(() => { + expect(bannersContainer.parentElement).toBe(targetDomElement); + }); + + notifications.start(); + notifications.stop(); + + expect(BannersService.prototype.stop).toHaveBeenCalledTimes(1); + expect(ToastsService.prototype.stop).toHaveBeenCalledTimes(1); + expect(bannersContainer.parentElement).toBe(null); + expect(toastsContainer.parentElement).toBe(null); + expect(targetDomElement).toMatchInlineSnapshot(`
`); +}); diff --git a/src/ui/public/notify/banners.test.ts b/src/ui/public/notify/banners.test.tsx similarity index 57% rename from src/ui/public/notify/banners.test.ts rename to src/ui/public/notify/banners.test.tsx index b28250d848ddd..1bb14396cf1ac 100644 --- a/src/ui/public/notify/banners.test.ts +++ b/src/ui/public/notify/banners.test.tsx @@ -17,12 +17,19 @@ * under the License. */ +import React from 'react'; import { __newPlatformInit__, banners } from './banners'; -__newPlatformInit__({ +const newPlatformBanners = { add: jest.fn((...args) => ['add', args]), remove: jest.fn((...args) => ['remove', args]), replace: jest.fn((...args) => ['replace', args]), +}; + +__newPlatformInit__(newPlatformBanners); + +beforeEach(() => { + jest.clearAllMocks(); }); it('forwards calls to newPlatformBanners.add()', () => { @@ -30,7 +37,7 @@ it('forwards calls to newPlatformBanners.add()', () => { Array [ "add", Array [ - "foo", + [Function], 10, ], ] @@ -49,14 +56,48 @@ Array [ }); it('forwards calls to newPlatformBanners.replace()', () => { - expect(banners.set({ id: 'id', component: 'bar', priority: 100 })).toMatchInlineSnapshot(` + expect(banners.set({ id: 'id', component: 'foo', priority: 100 })).toMatchInlineSnapshot(` Array [ "replace", Array [ "id", - "bar", + [Function], 100, ], ] `); }); + +describe('component is a react element', () => { + it('renders with react, returns unrender function', () => { + banners.add({ component: foo }); + const [[renderFn]] = newPlatformBanners.add.mock.calls; + const div = document.createElement('div'); + const unrender = renderFn(div); + expect(div).toMatchInlineSnapshot(` +
+ + foo + +
+`); + unrender(); + expect(div).toMatchInlineSnapshot(`
`); + }); +}); + +describe('component is a string', () => { + it('renders string with react, returns unrender function', () => { + banners.add({ component: 'foo' }); + const [[renderFn]] = newPlatformBanners.add.mock.calls; + const div = document.createElement('div'); + const unrender = renderFn(div); + expect(div).toMatchInlineSnapshot(` +
+ foo +
+`); + unrender(); + expect(div).toMatchInlineSnapshot(`
`); + }); +}); diff --git a/src/ui/public/notify/banners.ts b/src/ui/public/notify/banners.tsx similarity index 71% rename from src/ui/public/notify/banners.ts rename to src/ui/public/notify/banners.tsx index 5d5fe73946cb0..a9220ad10a8e4 100644 --- a/src/ui/public/notify/banners.ts +++ b/src/ui/public/notify/banners.tsx @@ -17,18 +17,35 @@ * under the License. */ +import React from 'react'; +import { render, unmountComponentAtNode } from 'react-dom'; + import { BannersStartContract } from '../../../core/public/notifications/banners'; let newPlatformBanners: BannersStartContract; export function __newPlatformInit__(instance: BannersStartContract) { if (newPlatformBanners) { - throw new Error('ui/chrome/api/base_path is already initialized'); + throw new Error('ui/notify/banners is already initialized'); } newPlatformBanners = instance; } +function renderWithReact(componentOrString: React.ReactElement | string) { + const component = + typeof componentOrString === 'string' ? ( + {componentOrString} + ) : ( + componentOrString + ); + + return (el: HTMLDivElement) => { + render(component, el); + return () => unmountComponentAtNode(el); + }; +} + export const banners = { /** * Add a new banner. @@ -37,8 +54,8 @@ export const banners = { * @param {Number} priority The optional priority order to display this banner. Higher priority values are shown first. * @return {String} A newly generated ID. This value can be used to remove/replace the banner. */ - add(banner: { component: React.ReactChild; priority?: number }) { - return newPlatformBanners.add(banner.component, banner.priority); + add(banner: { component: React.ReactElement | string; priority?: number }) { + return newPlatformBanners.add(renderWithReact(banner.component), banner.priority); }, /** @@ -62,7 +79,11 @@ export const banners = { * @param {Number} priority The optional priority order to display this banner. Higher priority values are shown first. * @return {String} A newly generated ID. This value can be used to remove/replace the banner. */ - set(banner: { id: string; component: React.ReactChild; priority?: number }) { - return newPlatformBanners.replace(banner.id, banner.component, banner.priority); + set(banner: { id: string; component: React.ReactElement | string; priority?: number }) { + return newPlatformBanners.replace( + banner.id, + renderWithReact(banner.component), + banner.priority + ); }, }; From d90a6e74850f295d573cd2bac9d183d034932238 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 18 Sep 2018 05:14:15 -0700 Subject: [PATCH 3/5] [core/public/banners] remove react/component verbiage from docs --- src/core/public/notifications/banners/banners_service.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/public/notifications/banners/banners_service.tsx b/src/core/public/notifications/banners/banners_service.tsx index 22ee6a4feb0de..50935ab543cac 100644 --- a/src/core/public/notifications/banners/banners_service.tsx +++ b/src/core/public/notifications/banners/banners_service.tsx @@ -49,7 +49,7 @@ export class BannersService { return { /** - * Add a component that should be rendered at the top of the page along with an optional priority. + * Add a banner that should be rendered at the top of the page along with an optional priority. */ add: (renderFn: Banner['render'], priority = 0) => { const id = `${++uniqueId}`; @@ -58,14 +58,15 @@ export class BannersService { }, /** - * Remove a component from the top of the page. + * Remove a banner from the top of the page. */ remove: (id: string) => { banners$.next(banners$.getValue().filter(banner => banner.id !== id)); }, /** - * Replace a banner component with a new one, potentially with a new priority + * Replace a banner and its priority. If the render function is not === to the + * previous render function the previous banner will be unmounted and re-rendered. */ replace: (id: string, renderFn: Banner['render'], priority = 0) => { const newId = `${++uniqueId}`; From ba227e2922d765c06979925d7deddda23b7a5e34 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 18 Sep 2018 08:06:37 -0700 Subject: [PATCH 4/5] [core/public/banners] remove old import --- .../banners/containers/global_banners_container.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/public/notifications/banners/containers/global_banners_container.tsx b/src/core/public/notifications/banners/containers/global_banners_container.tsx index 4d116a33429d7..61e5f73738db6 100644 --- a/src/core/public/notifications/banners/containers/global_banners_container.tsx +++ b/src/core/public/notifications/banners/containers/global_banners_container.tsx @@ -22,7 +22,6 @@ import * as Rx from 'rxjs'; import { Banners } from '../banners_service'; import { GlobalBannerList } from '../components/global_banner_list'; -import './global_banner_list.css'; interface Props { banners$: Rx.Observable; From 16cdfff0484492c031e0c4d1c6a21f927915f531 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 18 Sep 2018 09:15:29 -0700 Subject: [PATCH 5/5] exclude banners from i18n until #23268 is resolved --- .i18nrc.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.i18nrc.json b/.i18nrc.json index d53285671b768..2359c6328e41b 100644 --- a/.i18nrc.json +++ b/.i18nrc.json @@ -10,6 +10,7 @@ "src/ui/ui_render/bootstrap/app_bootstrap.js", "src/ui/ui_render/ui_render_mixin.js", "x-pack/plugins/monitoring/public/components/cluster/overview/alerts_panel.js", - "x-pack/plugins/monitoring/public/directives/alerts/index.js" + "x-pack/plugins/monitoring/public/directives/alerts/index.js", + "src/ui/public/notify/banners.tsx" ] }