[EuiPortal] Revert function component change while maintaining SSR fix (

elastic#6105) * Revert "[EuiPortal] convert to a function component, fix ssr bug (elastic#6055)" This reverts commit 3aacaf9. * Re-revert / keep portal test changes * Restore `data-euiportal` attr for snapshot purposes * Restore SSR bugfix + add SSR tests * Changelog * Skip failing Cypress test that doesn't apply to class component
thompsongl · Aug 4, 2022 · 6091d38 · 6091d38
1 parent ac21e25
commit 6091d38
Show file tree

Hide file tree

Showing 8 changed files with 145 additions and 95 deletions.
diff --git a/scripts/jest/setup/polyfills.js b/scripts/jest/setup/polyfills.js
@@ -12,6 +12,8 @@ afterAll(() => {
 });
 
 beforeAll(() => {
+  if (typeof window === 'undefined') return; // SSR tests
+
   Object.defineProperty(window, 'MutationObserver', { value: MutationObserver });
   patchNotifyChange(window);
 

diff --git a/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap b/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap
@@ -6,7 +6,7 @@ Array [
     aria-label="aria-label"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium testClass1 testClass2 emotion-euiBottomBar-fixed-m"
     data-test-subj="test subject string"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -29,7 +29,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -65,7 +65,7 @@ Array [
   <section
     aria-label="This should have been label"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -87,7 +87,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingLarge emotion-euiBottomBar-fixed-l"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -109,7 +109,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -131,7 +131,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed emotion-euiBottomBar-fixed"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -153,7 +153,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingSmall emotion-euiBottomBar-fixed-s"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -175,7 +175,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -197,7 +197,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 30px; right: 30px; bottom: 30px; top: 30px;"
+    style="left:30px;right:30px;bottom:30px;top:30px"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -219,7 +219,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--static euiBottomBar--paddingMedium emotion-euiBottomBar-static-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -241,7 +241,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--sticky euiBottomBar--paddingMedium emotion-euiBottomBar-sticky-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -263,7 +263,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 12px; right: 0px; bottom: 0px;"
+    style="left:12px;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -285,7 +285,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed euiBottomBar--paddingMedium emotion-euiBottomBar-fixed-m"
-    style="left: 0px; right: 0px; bottom: 0px;"
+    style="left:0;right:0;bottom:0"
   >
     <h2
       class="emotion-euiScreenReaderOnly"

diff --git a/src/components/bottom_bar/bottom_bar.test.tsx b/src/components/bottom_bar/bottom_bar.test.tsx
@@ -8,7 +8,7 @@
 
 import React from 'react';
 import ReactDOM from 'react-dom';
-import { mount } from 'enzyme';
+import { render, mount } from 'enzyme';
 import { keysOf } from '../common';
 import { requiredProps, takeMountedSnapshot } from '../../test';
 
@@ -28,52 +28,52 @@ ReactDOM.createPortal = (children) => {
 
 describe('EuiBottomBar', () => {
   test('is rendered', () => {
-    const component = mount(
+    const component = render(
       <EuiBottomBar {...requiredProps}>Content</EuiBottomBar>
     );
 
-    expect(component.render()).toMatchSnapshot();
+    expect(component).toMatchSnapshot();
   });
 
   describe('props', () => {
     describe('paddingSize', () => {
       keysOf(paddingSizeToClassNameMap).forEach((paddingSize) => {
         test(`${paddingSize} is rendered`, () => {
-          const component = mount(<EuiBottomBar paddingSize={paddingSize} />);
+          const component = render(<EuiBottomBar paddingSize={paddingSize} />);
 
-          expect(component.render()).toMatchSnapshot();
+          expect(component).toMatchSnapshot();
         });
       });
     });
 
     describe('position', () => {
       POSITIONS.forEach((position) => {
         test(`${position} is rendered`, () => {
-          const component = mount(<EuiBottomBar position={position} />);
+          const component = render(<EuiBottomBar position={position} />);
 
-          expect(component.render()).toMatchSnapshot();
+          expect(component).toMatchSnapshot();
         });
       });
     });
 
     test('landmarkHeading', () => {
-      const component = mount(
+      const component = render(
         <EuiBottomBar landmarkHeading="This should have been label" />
       );
 
-      expect(component.render()).toMatchSnapshot();
+      expect(component).toMatchSnapshot();
     });
 
     test('affordForDisplacement can be false', () => {
-      const component = mount(<EuiBottomBar affordForDisplacement={false} />);
+      const component = render(<EuiBottomBar affordForDisplacement={false} />);
 
-      expect(component.render()).toMatchSnapshot();
+      expect(component).toMatchSnapshot();
     });
 
     test('usePortal can be false', () => {
-      const component = mount(<EuiBottomBar usePortal={false} />);
+      const component = render(<EuiBottomBar usePortal={false} />);
 
-      expect(component.render()).toMatchSnapshot();
+      expect(component).toMatchSnapshot();
     });
 
     test('bodyClassName is rendered', () => {
@@ -84,17 +84,17 @@ describe('EuiBottomBar', () => {
     });
 
     test('style is customized', () => {
-      const component = mount(<EuiBottomBar style={{ left: 12 }} />);
+      const component = render(<EuiBottomBar style={{ left: 12 }} />);
 
-      expect(component.render()).toMatchSnapshot();
+      expect(component).toMatchSnapshot();
     });
 
     test('position props are altered', () => {
-      const component = mount(
+      const component = render(
         <EuiBottomBar top={30} right={30} bottom={30} left={30} />
       );
 
-      expect(component.render()).toMatchSnapshot();
+      expect(component).toMatchSnapshot();
     });
   });
 });
diff --git a/src/components/popover/wrapping_popover.tsx b/src/components/popover/wrapping_popover.tsx
@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import React, { FunctionComponent, useState, useEffect } from 'react';
+import React, { Component } from 'react';
 import { EuiPopover, Props as EuiPopoverProps } from './popover';
 import { EuiPortal } from '../portal';
 
@@ -19,36 +19,50 @@ export interface EuiWrappingPopoverProps extends EuiPopoverProps {
  * then the button element is moved into the popover dom.
  * On unmount, the button is moved back to its original location.
  */
-export const EuiWrappingPopover: FunctionComponent<EuiWrappingPopoverProps> = ({
-  button,
-  ...rest
-}) => {
-  const [anchor, setAnchor] = useState<HTMLElement | null>(null);
-  const [portal, setPortal] = useState<HTMLElement | null>(null);
-
-  useEffect(() => {
-    if (anchor) {
-      // move the button into the popover DOM
-      anchor.insertAdjacentElement('beforebegin', button);
+export class EuiWrappingPopover extends Component<EuiWrappingPopoverProps> {
+  private portal: HTMLElement | null = null;
+  private anchor: HTMLElement | null = null;
+
+  componentDidMount() {
+    if (this.anchor) {
+      this.anchor.insertAdjacentElement('beforebegin', this.props.button);
     }
+  }
 
-    return () => {
-      if (portal) {
-        // move the button back out of the popover DOM
-        portal.insertAdjacentElement('beforebegin', button);
+  componentWillUnmount() {
+    if (this.props.button.parentNode) {
+      if (this.portal) {
+        this.portal.insertAdjacentElement('beforebegin', this.props.button);
       }
-    };
-  }, [anchor, button, portal]);
-
-  return (
-    <EuiPortal
-      portalRef={setPortal}
-      insert={{ sibling: button, position: 'after' }}
-    >
-      <EuiPopover
-        {...rest}
-        button={<div ref={setAnchor} className="euiWrappingPopover__anchor" />}
-      />
-    </EuiPortal>
-  );
-};
+    }
+  }
+
+  setPortalRef = (node: HTMLElement | null) => {
+    this.portal = node;
+  };
+
+  setAnchorRef = (node: HTMLElement | null) => {
+    this.anchor = node;
+  };
+
+  render() {
+    const { button, ...rest } = this.props;
+
+    return (
+      <EuiPortal
+        portalRef={this.setPortalRef}
+        insert={{ sibling: this.props.button, position: 'after' }}
+      >
+        <EuiPopover
+          {...rest}
+          button={
+            <div
+              ref={this.setAnchorRef}
+              className="euiWrappingPopover__anchor"
+            />
+          }
+        />
+      </EuiPortal>
+    );
+  }
+}
diff --git a/src/components/portal/portal.server.test.tsx b/src/components/portal/portal.server.test.tsx
@@ -0,0 +1,24 @@
+/**
+ * @jest-environment node
+ */
+/* eslint-disable local/require-license-header */
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import React from 'react';
+import { renderToString } from 'react-dom/server';
+
+import { EuiPortal } from './portal';
+
+describe('EuiPortal', () => {
+  it('renders server-side without crashing and as empty', () => {
+    const renderOnServer = () => renderToString(<EuiPortal>test</EuiPortal>);
+    expect(renderOnServer).not.toThrow();
+    expect(renderOnServer()).toEqual('');
+  });
+});
diff --git a/src/components/portal/portal.spec.tsx b/src/components/portal/portal.spec.tsx
@@ -77,7 +77,9 @@ describe('EuiPortal', () => {
       });
     });
 
-    it('insert value can be changed', () => {
+    // This is not currently true of the EuiPortal class component, but may be true
+    // if we convert to a function component with useEffect dependencies in the future
+    it.skip('insert value can be changed', () => {
       const Wrapper = () => {
         const [siblingRef, setSiblingRef] = useState<HTMLElement | null>(null);
         const [insert, setInsert] = useState<EuiPortalProps['insert']>(