Convert NotificationsService to use targetDomElement$

src/core/public/core_system.test.ts

@@ -17,6 +17,9 @@
  * under the License.
  */
 
+import { Observable } from 'rxjs';
+import { take } from 'rxjs/operators';
+
 import { BasePathService } from './base_path';
 import { ChromeService } from './chrome';
 import { FatalErrorsService } from './fatal_errors';
@@ -463,15 +466,21 @@ describe('Notifications targetDomElement', () => {
 
  const [notifications] = MockNotificationsService.mock.instances;
 
- let targetDomElementParentInStart: HTMLElement;
+ const [[{ targetDomElement$ }]] = MockNotificationsService.mock.calls as [
+ [{ targetDomElement$: Observable<HTMLElement> }]
+ ];
+
+ let targetDomElementParentInStart: HTMLElement | null;
  (notifications as any).start.mockImplementation(() => {
- targetDomElementParentInStart = targetDomElement.parentElement;
+ targetDomElement$.pipe(take(1)).subscribe({
+ next: targetDomElement => {
+ // The targetDomElement should already be a child once it's received by the NotificationsService
+ expect(targetDomElement.parentElement).not.toBeNull();
+ targetDomElementParentInStart = targetDomElement.parentElement;
+ },
+ });
  });
 
- // targetDomElement should not have a parent element when the LegacyPlatformService is constructed
- const [[{ targetDomElement }]] = MockNotificationsService.mock.calls;
- expect(targetDomElement).toHaveProperty('parentElement', null);
-
  // starting the core system should mount the targetDomElement as a child of the rootDomElement
  await core.start();
  expect(targetDomElementParentInStart!).toBe(rootDomElement);

src/core/public/core_system.ts

@@ -19,6 +19,8 @@
 
 import './core.css';
 
+import { Subject } from 'rxjs';
+
 import { BasePathService } from './base_path';
 import { ChromeService } from './chrome';
 import { FatalErrorsService } from './fatal_errors';
@@ -62,7 +64,7 @@ export class CoreSystem {
  private readonly plugins: PluginsService;
 
  private readonly rootDomElement: HTMLElement;
- private readonly notificationsTargetDomElement: HTMLDivElement;
+ private readonly notificationsTargetDomElement$: Subject<HTMLDivElement>;
  private readonly legacyPlatformTargetDomElement: HTMLDivElement;
 
  constructor(params: Params) {
@@ -90,9 +92,9 @@ export class CoreSystem {
  },
  });
 
- this.notificationsTargetDomElement = document.createElement('div');
+ this.notificationsTargetDomElement$ = new Subject();
  this.notifications = new NotificationsService({
- targetDomElement: this.notificationsTargetDomElement,
+ targetDomElement$: this.notificationsTargetDomElement$.asObservable(),
  });
  this.http = new HttpService();
  this.basePath = new BasePathService();
@@ -144,9 +146,14 @@ export class CoreSystem {
  // ensure the rootDomElement is empty
  this.rootDomElement.textContent = '';
  this.rootDomElement.classList.add('coreSystemRootDomElement');
- this.rootDomElement.appendChild(this.notificationsTargetDomElement);
+
+ const notificationsTargetDomElement = document.createElement('div');
+ this.rootDomElement.appendChild(notificationsTargetDomElement);
  this.rootDomElement.appendChild(this.legacyPlatformTargetDomElement);
 
+ // Only provide the DOM element to notifications once it's attached to the page.
+ this.notificationsTargetDomElement$.next(notificationsTargetDomElement);
+
  this.legacyPlatform.start({
  i18n,
  injectedMetadata,
@@ -172,6 +179,7 @@ export class CoreSystem {
  this.uiSettings.stop();
  this.chrome.stop();
  this.i18n.stop();
+ this.notificationsTargetDomElement$.complete();
  this.rootDomElement.textContent = '';
  }
 }

src/core/public/notifications/notifications_service.ts

@@ -17,11 +17,12 @@
  * under the License.
  */
 
+import { Observable, Subject, Subscription } from 'rxjs';
 import { I18nStart } from '../i18n';
 import { ToastsService } from './toasts';
 
 interface Params {
- targetDomElement: HTMLElement;
+ targetDomElement$: Observable<HTMLElement>;
 }
 
 interface Deps {
@@ -31,17 +32,28 @@ interface Deps {
 export class NotificationsService {
  private readonly toasts: ToastsService;
 
- private readonly toastsContainer: HTMLElement;
+ private readonly toastsContainer$: Subject<HTMLElement>;
+ private domElemSubscription?: Subscription;
 
  constructor(private readonly params: Params) {
- this.toastsContainer = document.createElement('div');
+ this.toastsContainer$ = new Subject<HTMLElement>(); //
  this.toasts = new ToastsService({
- targetDomElement: this.toastsContainer,
+ targetDomElement$: this.toastsContainer$.asObservable(),
  });
  }
 
  public start({ i18n }: Deps) {
- this.params.targetDomElement.appendChild(this.toastsContainer);
+ this.domElemSubscription = this.params.targetDomElement$.subscribe({
+ next: targetDomElement => {
+ this.domElemSubscription!.add(() => {
+ targetDomElement.textContent = '';
+ });
+
+ const toastsContainer = document.createElement('div');
+ targetDomElement.appendChild(toastsContainer);
+ this.toastsContainer$.next(toastsContainer);
+ },
+ });
 
  return {
  toasts: this.toasts.start({ i18n }),
@@ -51,7 +63,9 @@ export class NotificationsService {
  public stop() {
  this.toasts.stop();
 
- this.params.targetDomElement.textContent = '';
+ if (this.domElemSubscription) {
+ this.domElemSubscription.unsubscribe();
+ }
  }
 }
 

src/core/public/notifications/toasts/toasts_service.test.tsx

@@ -24,6 +24,7 @@ jest.mock('react-dom', () => ({
  unmountComponentAtNode: mockReactDomUnmount,
 }));
 
+import { of } from 'rxjs';
 import { ToastsService } from './toasts_service';
 import { ToastsStart } from './toasts_start';
 
@@ -37,7 +38,7 @@ describe('#start()', () => {
  it('renders the GlobalToastList into the targetDomElement param', async () => {
  const targetDomElement = document.createElement('div');
  targetDomElement.setAttribute('test', 'target-dom-element');
- const toasts = new ToastsService({ targetDomElement });
+ const toasts = new ToastsService({ targetDomElement$: of(targetDomElement) });
 
  expect(mockReactDomRender).not.toHaveBeenCalled();
  toasts.start({ i18n: mockI18n });
@@ -46,7 +47,7 @@ describe('#start()', () => {
 
  it('returns a ToastsStart', () => {
  const toasts = new ToastsService({
- targetDomElement: document.createElement('div'),
+ targetDomElement$: of(document.createElement('div')),
  });
 
  expect(toasts.start({ i18n: mockI18n })).toBeInstanceOf(ToastsStart);
@@ -57,7 +58,7 @@ describe('#stop()', () => {
  it('unmounts the GlobalToastList from the targetDomElement', () => {
  const targetDomElement = document.createElement('div');
  targetDomElement.setAttribute('test', 'target-dom-element');
- const toasts = new ToastsService({ targetDomElement });
+ const toasts = new ToastsService({ targetDomElement$: of(targetDomElement) });
 
  toasts.start({ i18n: mockI18n });
 
@@ -69,17 +70,19 @@ describe('#stop()', () => {
  it('does not fail if start() was never called', () => {
  const targetDomElement = document.createElement('div');
  targetDomElement.setAttribute('test', 'target-dom-element');
- const toasts = new ToastsService({ targetDomElement });
+ const toasts = new ToastsService({ targetDomElement$: of(targetDomElement) });
  expect(() => {
  toasts.stop();
  }).not.toThrowError();
  });
 
  it('empties the content of the targetDomElement', () => {
  const targetDomElement = document.createElement('div');
- const toasts = new ToastsService({ targetDomElement });
+ const toasts = new ToastsService({ targetDomElement$: of(targetDomElement) });
 
- targetDomElement.appendChild(document.createTextNode('foo bar'));
+ // This now depends on start to be called which isn't great....
+ toasts.start({ i18n: mockI18n });
+ // targetDomElement.appendChild(document.createTextNode('foo bar'));
  toasts.stop();
  expect(targetDomElement.childNodes).toHaveLength(0);
  });

src/core/public/notifications/toasts/toasts_service.tsx

@@ -19,42 +19,62 @@
 
 import React from 'react';
 import { render, unmountComponentAtNode } from 'react-dom';
+import { Observable, Subscription } from 'rxjs';
 
 import { Toast } from '@elastic/eui';
 import { I18nStart } from '../../i18n';
 import { GlobalToastList } from './global_toast_list';
 import { ToastsStart } from './toasts_start';
 
 interface Params {
- targetDomElement: HTMLElement;
+ targetDomElement$: Observable<HTMLElement>;
 }
 
 interface Deps {
  i18n: I18nStart;
 }
 
 export class ToastsService {
+ private domElemSubscription?: Subscription;
+ private targetDomElement?: HTMLElement;
+
  constructor(private readonly params: Params) {}
 
  public start({ i18n }: Deps) {
  const toasts = new ToastsStart();
 
- render(
- <i18n.Context>
- <GlobalToastList
- dismissToast={(toast: Toast) => toasts.remove(toast)}
- toasts$={toasts.get$()}
- />
- </i18n.Context>,
- this.params.targetDomElement
- );
+ this.domElemSubscription = this.params.targetDomElement$.subscribe({
+ next: targetDomElement => {
+ this.cleanupTargetDomElement();
+ this.targetDomElement = targetDomElement;
+
+ render(
+ <i18n.Context>
+ <GlobalToastList
+ dismissToast={(toast: Toast) => toasts.remove(toast)}
+ toasts$={toasts.get$()}
+ />
+ </i18n.Context>,
+ targetDomElement
+ );
+ },
+ });
 
  return toasts;
  }
 
  public stop() {
- unmountComponentAtNode(this.params.targetDomElement);
+ this.cleanupTargetDomElement();
+
+ if (this.domElemSubscription) {
+ this.domElemSubscription.unsubscribe();
+ }
+ }
 
- this.params.targetDomElement.textContent = '';
+ private cleanupTargetDomElement() {
+ if (this.targetDomElement) {
+ unmountComponentAtNode(this.targetDomElement);
+ this.targetDomElement.textContent = '';
+ }
  }
 }