[EuiPortal] convert to a function component, fix ssr bug (elastic#6055)

* convert EuiPortal to a function component, remove euiBody-hasPortalContent css class and usages * changelog * Revert "remove euiBody-hasPortalContent css class and usages" This reverts commit c841fae. * Refactor EuiPortal to a function component; added unit test around portalRef; updated EuiBottomBar tests to account for new portal lifecycle * Update upcoming_changelogs/6055.md Co-authored-by: Constance <[email protected]> * Added cypress tests for EuiPortal insertion logic * Update upcoming_changelogs/6055.md Co-authored-by: Greg Thompson <[email protected]> * Convert EuiWrappingPopover to a function component so changes to anchor are accounted for Co-authored-by: Constance <[email protected]> Co-authored-by: Greg Thompson <[email protected]>
yuliacech · Jul 27, 2022 · 3aacaf9 · 3aacaf9
1 parent 853cc04
commit 3aacaf9
Show file tree

Hide file tree

Showing 8 changed files with 242 additions and 108 deletions.
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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <h2
       class="emotion-euiScreenReaderOnly"
@@ -131,7 +131,7 @@ Array [
   <section
     aria-label="Page level controls"
     class="euiBottomBar euiBottomBar--fixed emotion-euiBottomBar-fixed"
-    style="left:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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:0;bottom:0"
+    style="left: 12px; right: 0px; bottom: 0px;"
   >
     <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:0;right:0;bottom:0"
+    style="left: 0px; right: 0px; bottom: 0px;"
   >
     <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 { render, mount } from 'enzyme';
+import { 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 = render(
+    const component = mount(
       <EuiBottomBar {...requiredProps}>Content</EuiBottomBar>
     );
 
-    expect(component).toMatchSnapshot();
+    expect(component.render()).toMatchSnapshot();
   });
 
   describe('props', () => {
     describe('paddingSize', () => {
       keysOf(paddingSizeToClassNameMap).forEach((paddingSize) => {
         test(`${paddingSize} is rendered`, () => {
-          const component = render(<EuiBottomBar paddingSize={paddingSize} />);
+          const component = mount(<EuiBottomBar paddingSize={paddingSize} />);
 
-          expect(component).toMatchSnapshot();
+          expect(component.render()).toMatchSnapshot();
         });
       });
     });
 
     describe('position', () => {
       POSITIONS.forEach((position) => {
         test(`${position} is rendered`, () => {
-          const component = render(<EuiBottomBar position={position} />);
+          const component = mount(<EuiBottomBar position={position} />);
 
-          expect(component).toMatchSnapshot();
+          expect(component.render()).toMatchSnapshot();
         });
       });
     });
 
     test('landmarkHeading', () => {
-      const component = render(
+      const component = mount(
         <EuiBottomBar landmarkHeading="This should have been label" />
       );
 
-      expect(component).toMatchSnapshot();
+      expect(component.render()).toMatchSnapshot();
     });
 
     test('affordForDisplacement can be false', () => {
-      const component = render(<EuiBottomBar affordForDisplacement={false} />);
+      const component = mount(<EuiBottomBar affordForDisplacement={false} />);
 
-      expect(component).toMatchSnapshot();
+      expect(component.render()).toMatchSnapshot();
     });
 
     test('usePortal can be false', () => {
-      const component = render(<EuiBottomBar usePortal={false} />);
+      const component = mount(<EuiBottomBar usePortal={false} />);
 
-      expect(component).toMatchSnapshot();
+      expect(component.render()).toMatchSnapshot();
     });
 
     test('bodyClassName is rendered', () => {
@@ -84,17 +84,17 @@ describe('EuiBottomBar', () => {
     });
 
     test('style is customized', () => {
-      const component = render(<EuiBottomBar style={{ left: 12 }} />);
+      const component = mount(<EuiBottomBar style={{ left: 12 }} />);
 
-      expect(component).toMatchSnapshot();
+      expect(component.render()).toMatchSnapshot();
     });
 
     test('position props are altered', () => {
-      const component = render(
+      const component = mount(
         <EuiBottomBar top={30} right={30} bottom={30} left={30} />
       );
 
-      expect(component).toMatchSnapshot();
+      expect(component.render()).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, { Component } from 'react';
+import React, { FunctionComponent, useState, useEffect } from 'react';
 import { EuiPopover, Props as EuiPopoverProps } from './popover';
 import { EuiPortal } from '../portal';
 
@@ -19,50 +19,36 @@ 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 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);
+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);
     }
-  }
 
-  componentWillUnmount() {
-    if (this.props.button.parentNode) {
-      if (this.portal) {
-        this.portal.insertAdjacentElement('beforebegin', this.props.button);
+    return () => {
+      if (portal) {
+        // move the button back out of the popover DOM
+        portal.insertAdjacentElement('beforebegin', button);
       }
-    }
-  }
-
-  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>
-    );
-  }
-}
+    };
+  }, [anchor, button, portal]);
+
+  return (
+    <EuiPortal
+      portalRef={setPortal}
+      insert={{ sibling: button, position: 'after' }}
+    >
+      <EuiPopover
+        {...rest}
+        button={<div ref={setAnchor} className="euiWrappingPopover__anchor" />}
+      />
+    </EuiPortal>
+  );
+};
diff --git a/src/components/portal/__snapshots__/portal.test.tsx.snap b/src/components/portal/__snapshots__/portal.test.tsx.snap
@@ -5,7 +5,9 @@ exports[`EuiPortal is rendered 1`] = `
   <EuiPortal>
     <Portal
       containerInfo={
-        <div>
+        <div
+          data-euiportal="true"
+        >
           Content
         </div>
       }
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,7 +5,9 @@ exports[`EuiPortal is rendered 1`] = ` @@
       <EuiPortal>
         <Portal
           containerInfo={
-            <div>
+            <div
+              data-euiportal="true"
+            >
               Content
             </div>
           }
@@ Expand Down @@