[APM] Use core.chrome to set window title (#73232)

I noticed there's a `core.chrome.docTitle.change` method. It can take a string or array of strings and provides its own separator character if given an array. Replace our setting of `window.document.title` directly in the APM and Observability plugins with using the chrome method. This changes the title to be, for example, "トランザクション - opbeans-node - サービス - APM - Elastic" instead of "トランザクション | opbeans-node | サービス | APM | Elastic", using " - " as a separator instead of " | ".
elastic · Jul 27, 2020 · 2a77307 · 2a77307
1 parent bc65c5e
commit 2a77307
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 34 deletions.
diff --git a/x-pack/plugins/apm/public/components/app/Main/UpdateBreadcrumbs.test.tsx b/x-pack/plugins/apm/public/components/app/Main/UpdateBreadcrumbs.test.tsx
@@ -16,6 +16,7 @@ import {
 } from '../../../context/ApmPluginContext/MockApmPluginContext';
 
 const setBreadcrumbs = jest.fn();
+const changeTitle = jest.fn();
 
 function mountBreadcrumb(route: string, params = '') {
   mount(
@@ -27,6 +28,7 @@ function mountBreadcrumb(route: string, params = '') {
             ...mockApmPluginContextValue.core,
             chrome: {
               ...mockApmPluginContextValue.core.chrome,
+              docTitle: { change: changeTitle },
               setBreadcrumbs,
             },
           },
@@ -42,23 +44,14 @@ function mountBreadcrumb(route: string, params = '') {
 }
 
 describe('UpdateBreadcrumbs', () => {
-  let realDoc: Document;
-
   beforeEach(() => {
-    realDoc = window.document;
-    (window.document as any) = {
-      title: 'Kibana',
-    };
     setBreadcrumbs.mockReset();
+    changeTitle.mockReset();
   });
 
-  afterEach(() => {
-    (window.document as any) = realDoc;
-  });
-
-  it('Homepage', () => {
+  it('Changes the homepage title', () => {
     mountBreadcrumb('/');
-    expect(window.document.title).toMatchInlineSnapshot(`"APM"`);
+    expect(changeTitle).toHaveBeenCalledWith(['APM']);
   });
 
   it('/services/:serviceName/errors/:groupId', () => {
@@ -90,9 +83,13 @@ describe('UpdateBreadcrumbs', () => {
       },
       { text: 'myGroupId', href: undefined },
     ]);
-    expect(window.document.title).toMatchInlineSnapshot(
-      `"myGroupId | Errors | opbeans-node | Services | APM"`
-    );
+    expect(changeTitle).toHaveBeenCalledWith([
+      'myGroupId',
+      'Errors',
+      'opbeans-node',
+      'Services',
+      'APM',
+    ]);
   });
 
   it('/services/:serviceName/errors', () => {
@@ -104,9 +101,12 @@ describe('UpdateBreadcrumbs', () => {
       { text: 'opbeans-node', href: '#/services/opbeans-node?kuery=myKuery' },
       { text: 'Errors', href: undefined },
     ]);
-    expect(window.document.title).toMatchInlineSnapshot(
-      `"Errors | opbeans-node | Services | APM"`
-    );
+    expect(changeTitle).toHaveBeenCalledWith([
+      'Errors',
+      'opbeans-node',
+      'Services',
+      'APM',
+    ]);
   });
 
   it('/services/:serviceName/transactions', () => {
@@ -118,9 +118,12 @@ describe('UpdateBreadcrumbs', () => {
       { text: 'opbeans-node', href: '#/services/opbeans-node?kuery=myKuery' },
       { text: 'Transactions', href: undefined },
     ]);
-    expect(window.document.title).toMatchInlineSnapshot(
-      `"Transactions | opbeans-node | Services | APM"`
-    );
+    expect(changeTitle).toHaveBeenCalledWith([
+      'Transactions',
+      'opbeans-node',
+      'Services',
+      'APM',
+    ]);
   });
 
   it('/services/:serviceName/transactions/view?transactionName=my-transaction-name', () => {
@@ -139,8 +142,12 @@ describe('UpdateBreadcrumbs', () => {
       },
       { text: 'my-transaction-name', href: undefined },
     ]);
-    expect(window.document.title).toMatchInlineSnapshot(
-      `"my-transaction-name | Transactions | opbeans-node | Services | APM"`
-    );
+    expect(changeTitle).toHaveBeenCalledWith([
+      'my-transaction-name',
+      'Transactions',
+      'opbeans-node',
+      'Services',
+      'APM',
+    ]);
   });
 });
diff --git a/x-pack/plugins/apm/public/components/app/Main/UpdateBreadcrumbs.tsx b/x-pack/plugins/apm/public/components/app/Main/UpdateBreadcrumbs.tsx
@@ -22,10 +22,7 @@ interface Props {
 }
 
 function getTitleFromBreadCrumbs(breadcrumbs: Breadcrumb[]) {
-  return breadcrumbs
-    .map(({ value }) => value)
-    .reverse()
-    .join(' | ');
+  return breadcrumbs.map(({ value }) => value).reverse();
 }
 
 class UpdateBreadcrumbsComponent extends React.Component<Props> {
@@ -43,7 +40,9 @@ class UpdateBreadcrumbsComponent extends React.Component<Props> {
       }
     );
 
-    document.title = getTitleFromBreadCrumbs(this.props.breadcrumbs);
+    this.props.core.chrome.docTitle.change(
+      getTitleFromBreadCrumbs(this.props.breadcrumbs)
+    );
     this.props.core.chrome.setBreadcrumbs(breadcrumbs);
   }
 

diff --git a/x-pack/plugins/observability/public/application/application.test.tsx b/x-pack/plugins/observability/public/application/application.test.tsx
@@ -0,0 +1,29 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { renderApp } from './';
+import { Observable } from 'rxjs';
+import { CoreStart, AppMountParameters } from 'src/core/public';
+
+describe('renderApp', () => {
+  it('renders', () => {
+    const core = ({
+      application: { currentAppId$: new Observable(), navigateToUrl: () => {} },
+      chrome: { docTitle: { change: () => {} }, setBreadcrumbs: () => {} },
+      i18n: { Context: ({ children }: { children: React.ReactNode }) => children },
+      uiSettings: { get: () => false },
+    } as unknown) as CoreStart;
+    const params = ({
+      element: window.document.createElement('div'),
+    } as unknown) as AppMountParameters;
+
+    expect(() => {
+      const unmount = renderApp(core, params);
+      unmount();
+    }).not.toThrowError();
+  });
+});
diff --git a/x-pack/plugins/observability/public/application/index.tsx b/x-pack/plugins/observability/public/application/index.tsx
@@ -23,10 +23,7 @@ const observabilityLabelBreadcrumb = {
 };
 
 function getTitleFromBreadCrumbs(breadcrumbs: Breadcrumbs) {
-  return breadcrumbs
-    .map(({ text }) => text)
-    .reverse()
-    .join(' | ');
+  return breadcrumbs.map(({ text }) => text).reverse();
 }
 
 function App() {
@@ -42,7 +39,7 @@ function App() {
             const breadcrumb = [observabilityLabelBreadcrumb, ...route.breadcrumb];
             useEffect(() => {
               core.chrome.setBreadcrumbs(breadcrumb);
-              document.title = getTitleFromBreadCrumbs(breadcrumb);
+              core.chrome.docTitle.change(getTitleFromBreadCrumbs(breadcrumb));
             }, [core, breadcrumb]);
 
             const { query, path: pathParams } = useRouteParams(route.params);