Fix chromeless NP apps not using full page width (#54550) (#54683)

* add missing conditional classes on app-wrapper and application containers

* update snapshot

* refactor and add unit tests for service

* typo

* use consistent classNames naming

src/core/public/chrome/chrome_service.tsx

@@ -127,7 +127,7 @@ export class ChromeService {
         )
       )
     );
-    this.isVisible$ = combineLatest(this.appHidden$, this.toggleHidden$).pipe(
+    this.isVisible$ = combineLatest([this.appHidden$, this.toggleHidden$]).pipe(
       map(([appHidden, toggleHidden]) => !(appHidden || toggleHidden)),
       takeUntil(this.stop$)
     );

src/core/public/rendering/app_containers.test.tsx

@@ -0,0 +1,105 @@
+/*
+ * 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 { BehaviorSubject } from 'rxjs';
+import { act } from 'react-dom/test-utils';
+import { mount } from 'enzyme';
+import React from 'react';
+
+import { AppWrapper, AppContainer } from './app_containers';
+
+describe('AppWrapper', () => {
+  it('toggles the `hidden-chrome` class depending on the chrome visibility state', () => {
+    const chromeVisible$ = new BehaviorSubject<boolean>(true);
+
+    const component = mount(<AppWrapper chromeVisible$={chromeVisible$}>app-content</AppWrapper>);
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="app-wrapper"
+      >
+        app-content
+      </div>
+    `);
+
+    act(() => chromeVisible$.next(false));
+    component.update();
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="app-wrapper hidden-chrome"
+      >
+        app-content
+      </div>
+    `);
+
+    act(() => chromeVisible$.next(true));
+    component.update();
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="app-wrapper"
+      >
+        app-content
+      </div>
+    `);
+  });
+});
+
+describe('AppContainer', () => {
+  it('adds classes supplied by chrome', () => {
+    const appClasses$ = new BehaviorSubject<string[]>([]);
+
+    const component = mount(<AppContainer classes$={appClasses$}>app-content</AppContainer>);
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="application"
+      >
+        app-content
+      </div>
+    `);
+
+    act(() => appClasses$.next(['classA', 'classB']));
+    component.update();
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="application classA classB"
+      >
+        app-content
+      </div>
+    `);
+
+    act(() => appClasses$.next(['classC']));
+    component.update();
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="application classC"
+      >
+        app-content
+      </div>
+    `);
+
+    act(() => appClasses$.next([]));
+    component.update();
+    expect(component.getDOMNode()).toMatchInlineSnapshot(`
+      <div
+        class="application"
+      >
+        app-content
+      </div>
+    `);
+  });
+});

src/core/public/rendering/app_containers.tsx

@@ -0,0 +1,37 @@
+/*
+ * 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 { Observable } from 'rxjs';
+import useObservable from 'react-use/lib/useObservable';
+import classNames from 'classnames';
+
+export const AppWrapper: React.FunctionComponent<{
+  chromeVisible$: Observable<boolean>;
+}> = ({ chromeVisible$, children }) => {
+  const visible = useObservable(chromeVisible$);
+  return <div className={classNames('app-wrapper', { 'hidden-chrome': !visible })}>{children}</div>;
+};
+
+export const AppContainer: React.FunctionComponent<{
+  classes$: Observable<string[]>;
+}> = ({ classes$, children }) => {
+  const classes = useObservable(classes$);
+  return <div className={classNames('application', classes)}>{children}</div>;
+};

src/core/public/rendering/rendering_service.test.tsx

@@ -18,72 +18,129 @@
  */
 
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 
-import { chromeServiceMock } from '../chrome/chrome_service.mock';
 import { RenderingService } from './rendering_service';
-import { InternalApplicationStart } from '../application';
+import { applicationServiceMock } from '../application/application_service.mock';
+import { chromeServiceMock } from '../chrome/chrome_service.mock';
 import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock';
 import { overlayServiceMock } from '../overlays/overlay_service.mock';
+import { BehaviorSubject } from 'rxjs';
 
 describe('RenderingService#start', () => {
-  const getService = ({ legacyMode = false }: { legacyMode?: boolean } = {}) => {
-    const rendering = new RenderingService();
-    const application = {
-      getComponent: () => <div>Hello application!</div>,
-    } as InternalApplicationStart;
-    const chrome = chromeServiceMock.createStartContract();
+  let application: ReturnType<typeof applicationServiceMock.createInternalStartContract>;
+  let chrome: ReturnType<typeof chromeServiceMock.createStartContract>;
+  let overlays: ReturnType<typeof overlayServiceMock.createStartContract>;
+  let injectedMetadata: ReturnType<typeof injectedMetadataServiceMock.createStartContract>;
+  let targetDomElement: HTMLDivElement;
+  let rendering: RenderingService;
+
+  beforeEach(() => {
+    application = applicationServiceMock.createInternalStartContract();
+    application.getComponent.mockReturnValue(<div>Hello application!</div>);
+
+    chrome = chromeServiceMock.createStartContract();
     chrome.getHeaderComponent.mockReturnValue(<div>Hello chrome!</div>);
-    const overlays = overlayServiceMock.createStartContract();
+
+    overlays = overlayServiceMock.createStartContract();
     overlays.banners.getComponent.mockReturnValue(<div>I&apos;m a banner!</div>);
 
-    const injectedMetadata = injectedMetadataServiceMock.createStartContract();
-    injectedMetadata.getLegacyMode.mockReturnValue(legacyMode);
-    const targetDomElement = document.createElement('div');
-    const start = rendering.start({
+    injectedMetadata = injectedMetadataServiceMock.createStartContract();
+
+    targetDomElement = document.createElement('div');
+
+    rendering = new RenderingService();
+  });
+
+  const startService = () => {
+    return rendering.start({
       application,
       chrome,
       injectedMetadata,
       overlays,
       targetDomElement,
     });
-    return { start, targetDomElement };
   };
 
-  it('renders application service into provided DOM element', () => {
-    const { targetDomElement } = getService();
-    expect(targetDomElement.querySelector('div.application')).toMatchInlineSnapshot(`
-      <div
-        class="application"
-      >
-        <div>
-          Hello application!
-        </div>
-      </div>
-    `);
+  describe('standard mode', () => {
+    beforeEach(() => {
+      injectedMetadata.getLegacyMode.mockReturnValue(false);
+    });
+
+    it('renders application service into provided DOM element', () => {
+      startService();
+      expect(targetDomElement.querySelector('div.application')).toMatchInlineSnapshot(`
+              <div
+                class="application class-name"
+              >
+                <div>
+                  Hello application!
+                </div>
+              </div>
+          `);
+    });
+
+    it('adds the `chrome-hidden` class to the AppWrapper when chrome is hidden', () => {
+      const isVisible$ = new BehaviorSubject(true);
+      chrome.getIsVisible$.mockReturnValue(isVisible$);
+      startService();
+
+      const appWrapper = targetDomElement.querySelector('div.app-wrapper')!;
+      expect(appWrapper.className).toEqual('app-wrapper');
+
+      act(() => isVisible$.next(false));
+      expect(appWrapper.className).toEqual('app-wrapper hidden-chrome');
+
+      act(() => isVisible$.next(true));
+      expect(appWrapper.className).toEqual('app-wrapper');
+    });
+
+    it('adds the application classes to the AppContainer', () => {
+      const applicationClasses$ = new BehaviorSubject<string[]>([]);
+      chrome.getApplicationClasses$.mockReturnValue(applicationClasses$);
+      startService();
+
+      const appContainer = targetDomElement.querySelector('div.application')!;
+      expect(appContainer.className).toEqual('application');
+
+      act(() => applicationClasses$.next(['classA', 'classB']));
+      expect(appContainer.className).toEqual('application classA classB');
+
+      act(() => applicationClasses$.next(['classC']));
+      expect(appContainer.className).toEqual('application classC');
+
+      act(() => applicationClasses$.next([]));
+      expect(appContainer.className).toEqual('application');
+    });
+
+    it('contains wrapper divs', () => {
+      startService();
+      expect(targetDomElement.querySelector('div.app-wrapper')).toBeDefined();
+      expect(targetDomElement.querySelector('div.app-wrapper-pannel')).toBeDefined();
+    });
+
+    it('renders the banner UI', () => {
+      startService();
+      expect(targetDomElement.querySelector('#globalBannerList')).toMatchInlineSnapshot(`
+              <div
+                id="globalBannerList"
+              >
+                <div>
+                  I'm a banner!
+                </div>
+              </div>
+          `);
+    });
   });
 
-  it('contains wrapper divs', () => {
-    const { targetDomElement } = getService();
-    expect(targetDomElement.querySelector('div.app-wrapper')).toBeDefined();
-    expect(targetDomElement.querySelector('div.app-wrapper-pannel')).toBeDefined();
-  });
+  describe('legacy mode', () => {
+    beforeEach(() => {
+      injectedMetadata.getLegacyMode.mockReturnValue(true);
+    });
 
-  it('renders the banner UI', () => {
-    const { targetDomElement } = getService();
-    expect(targetDomElement.querySelector('#globalBannerList')).toMatchInlineSnapshot(`
-      <div
-        id="globalBannerList"
-      >
-        <div>
-          I'm a banner!
-        </div>
-      </div>
-    `);
-  });
-
-  describe('legacyMode', () => {
     it('renders into provided DOM element', () => {
-      const { targetDomElement } = getService({ legacyMode: true });
+      startService();
+
       expect(targetDomElement).toMatchInlineSnapshot(`
           <div>
             <div
@@ -100,10 +157,8 @@ describe('RenderingService#start', () => {
     });
 
     it('returns a div for the legacy service to render into', () => {
-      const {
-        start: { legacyTargetDomElement },
-        targetDomElement,
-      } = getService({ legacyMode: true });
+      const { legacyTargetDomElement } = startService();
+
       expect(targetDomElement.contains(legacyTargetDomElement!)).toBe(true);
     });
   });

src/core/public/rendering/rendering_service.tsx

@@ -25,6 +25,7 @@ import { InternalChromeStart } from '../chrome';
 import { InternalApplicationStart } from '../application';
 import { InjectedMetadataStart } from '../injected_metadata';
 import { OverlayStart } from '../overlays';
+import { AppWrapper, AppContainer } from './app_containers';
 
 interface StartDeps {
   application: InternalApplicationStart;
@@ -65,12 +66,12 @@ export class RenderingService {
           {chromeUi}
 
           {!legacyMode && (
-            <div className="app-wrapper">
+            <AppWrapper chromeVisible$={chrome.getIsVisible$()}>
               <div className="app-wrapper-panel">
                 <div id="globalBannerList">{bannerUi}</div>
-                <div className="application">{appUi}</div>
+                <AppContainer classes$={chrome.getApplicationClasses$()}>{appUi}</AppContainer>
               </div>
-            </div>
+            </AppWrapper>
           )}
 
           {legacyMode && <div ref={legacyRef} />}

test/plugin_functional/test_suites/core_plugins/applications.ts

@@ -27,12 +27,18 @@ export default function({ getService, getPageObjects }: PluginFunctionalProvider
   const browser = getService('browser');
   const appsMenu = getService('appsMenu');
   const testSubjects = getService('testSubjects');
+  const find = getService('find');
 
   const loadingScreenNotShown = async () =>
     expect(await testSubjects.exists('kbnLoadingMessage')).to.be(false);
 
   const loadingScreenShown = () => testSubjects.existOrFail('kbnLoadingMessage');
 
+  const getAppWrapperWidth = async () => {
+    const wrapper = await find.byClassName('app-wrapper');
+    return (await wrapper.getSize()).width;
+  };
+
   const getKibanaUrl = (pathname?: string, search?: string) =>
     url.format({
       protocol: 'http:',
@@ -99,12 +105,20 @@ export default function({ getService, getPageObjects }: PluginFunctionalProvider
       await PageObjects.common.navigateToApp('chromeless');
       await loadingScreenNotShown();
       expect(await testSubjects.exists('headerGlobalNav')).to.be(false);
+
+      const wrapperWidth = await getAppWrapperWidth();
+      const windowWidth = (await browser.getWindowSize()).width;
+      expect(wrapperWidth).to.eql(windowWidth);
     });
 
     it('navigating away from chromeless application shows chrome', async () => {
       await PageObjects.common.navigateToApp('foo');
       await loadingScreenNotShown();
       expect(await testSubjects.exists('headerGlobalNav')).to.be(true);
+
+      const wrapperWidth = await getAppWrapperWidth();
+      const windowWidth = (await browser.getWindowSize()).width;
+      expect(wrapperWidth).to.be.below(windowWidth);
     });
 
     it.skip('can navigate from NP apps to legacy apps', async () => {