From 3aacaf9171ac84be0335a1b0f1602ebe1383eacc Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 27 Jul 2022 10:10:11 -0600 Subject: [PATCH] [EuiPortal] convert to a function component, fix ssr bug (#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 c841faec6cf16f2fc908b08debc5468fb6076beb. * 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 * Added cypress tests for EuiPortal insertion logic * Update upcoming_changelogs/6055.md Co-authored-by: Greg Thompson * Convert EuiWrappingPopover to a function component so changes to anchor are accounted for Co-authored-by: Constance Co-authored-by: Greg Thompson --- .../__snapshots__/bottom_bar.test.tsx.snap | 26 ++-- src/components/bottom_bar/bottom_bar.test.tsx | 34 ++--- src/components/popover/wrapping_popover.tsx | 76 +++++------ .../portal/__snapshots__/portal.test.tsx.snap | 4 +- src/components/portal/portal.spec.tsx | 125 ++++++++++++++++++ src/components/portal/portal.test.tsx | 20 ++- src/components/portal/portal.tsx | 62 ++++----- upcoming_changelogs/6055.md | 3 + 8 files changed, 242 insertions(+), 108 deletions(-) create mode 100644 src/components/portal/portal.spec.tsx create mode 100644 upcoming_changelogs/6055.md diff --git a/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap b/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap index bb6baeba7da..ace065e0baa 100644 --- 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;" >

{ describe('EuiBottomBar', () => { test('is rendered', () => { - const component = render( + const component = mount( Content ); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); describe('props', () => { describe('paddingSize', () => { keysOf(paddingSizeToClassNameMap).forEach((paddingSize) => { test(`${paddingSize} is rendered`, () => { - const component = render(); + const component = mount(); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); }); }); @@ -49,31 +49,31 @@ describe('EuiBottomBar', () => { describe('position', () => { POSITIONS.forEach((position) => { test(`${position} is rendered`, () => { - const component = render(); + const component = mount(); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); }); }); test('landmarkHeading', () => { - const component = render( + const component = mount( ); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); test('affordForDisplacement can be false', () => { - const component = render(); + const component = mount(); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); test('usePortal can be false', () => { - const component = render(); + const component = mount(); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); test('bodyClassName is rendered', () => { @@ -84,17 +84,17 @@ describe('EuiBottomBar', () => { }); test('style is customized', () => { - const component = render(); + const component = mount(); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); test('position props are altered', () => { - const component = render( + const component = mount( ); - expect(component).toMatchSnapshot(); + expect(component.render()).toMatchSnapshot(); }); }); }); diff --git a/src/components/popover/wrapping_popover.tsx b/src/components/popover/wrapping_popover.tsx index 8ea55d39ead..8022d7818c2 100644 --- 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 { - 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 = ({ + button, + ...rest +}) => { + const [anchor, setAnchor] = useState(null); + const [portal, setPortal] = useState(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 ( - - - } - /> - - ); - } -} + }; + }, [anchor, button, portal]); + + return ( + + } + /> + + ); +}; diff --git a/src/components/portal/__snapshots__/portal.test.tsx.snap b/src/components/portal/__snapshots__/portal.test.tsx.snap index 9412b16620d..8a2dd595fbf 100644 --- 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`] = ` +
Content
} diff --git a/src/components/portal/portal.spec.tsx b/src/components/portal/portal.spec.tsx new file mode 100644 index 00000000000..ba240cdd647 --- /dev/null +++ b/src/components/portal/portal.spec.tsx @@ -0,0 +1,125 @@ +/* + * 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, { useState } from 'react'; +import { EuiPortal, EuiPortalProps } from './index'; + +describe('EuiPortal', () => { + describe('insertion', () => { + it('inserts at the bottom of body by default', () => { + cy.realMount(Hello); + + // verify the portal element was appended to the body + cy.get('div[data-euiportal]').then((portals) => { + expect(portals).to.have.lengthOf(1); + expect(portals.get(0)).to.equal(document.body.lastElementChild); + }); + }); + + it('inserts before a specified element', () => { + const Wrapper = () => { + const [siblingRef, setSiblingRef] = useState(null); + return ( + <> +
+ {siblingRef && ( + + Hello + + )} + + ); + }; + cy.realMount(); + + // verify the portal element was appended before the sibling + cy.get('div[data-euiportal]').then((portals) => { + cy.get('div#sibling').then((siblings) => { + expect(portals).to.have.lengthOf(1); + expect(siblings).to.have.lengthOf(1); + expect(siblings.get(0).previousElementSibling).to.equal( + portals.get(0) + ); + }); + }); + }); + + it('inserts after a specified element', () => { + const Wrapper = () => { + const [siblingRef, setSiblingRef] = useState(null); + return ( + <> +
+ {siblingRef && ( + + Hello + + )} + + ); + }; + cy.realMount(); + + // verify the portal element was appended after the sibling + cy.get('div[data-euiportal]').then((portals) => { + cy.get('div#sibling').then((siblings) => { + expect(portals).to.have.lengthOf(1); + expect(siblings).to.have.lengthOf(1); + expect(siblings.get(0).nextElementSibling).to.equal(portals.get(0)); + }); + }); + }); + + it('insert value can be changed', () => { + const Wrapper = () => { + const [siblingRef, setSiblingRef] = useState(null); + const [insert, setInsert] = useState( + undefined + ); + + return ( + <> +
+ {siblingRef && ( + <> + Hello + + + )} + + ); + }; + cy.realMount(); + + // verify the portal element was appended to the body + cy.get('div[data-euiportal]').then((portals) => { + expect(portals).to.have.lengthOf(1); + expect(portals.get(0)).to.equal(document.body.lastElementChild); + }); + + cy.contains('change insertion').click(); + + // verify the portal element is now appended after the sibling + cy.get('div[data-euiportal]').then((portals) => { + cy.get('div#sibling').then((siblings) => { + expect(portals).to.have.lengthOf(1); + expect(siblings).to.have.lengthOf(1); + expect(siblings.get(0).nextElementSibling).to.equal(portals.get(0)); + }); + }); + }); + }); +}); diff --git a/src/components/portal/portal.test.tsx b/src/components/portal/portal.test.tsx index 2bbc004cf5d..c6a86628c5b 100644 --- a/src/components/portal/portal.test.tsx +++ b/src/components/portal/portal.test.tsx @@ -11,7 +11,7 @@ import { mount } from 'enzyme'; import { EuiPortal } from './portal'; describe('EuiPortal', () => { - test('is rendered', () => { + it('is rendered', () => { const component = mount(
Content @@ -20,4 +20,22 @@ describe('EuiPortal', () => { expect(component).toMatchSnapshot(); }); + + describe('behavior', () => { + it('portalRef', () => { + const portalRef = jest.fn(); + + const component = mount( + Content + ); + + expect(portalRef).toHaveBeenCalledTimes(1); + expect(portalRef.mock.calls[0][0]).toBeInstanceOf(HTMLDivElement); + + component.unmount(); + + expect(portalRef).toHaveBeenCalledTimes(2); + expect(portalRef.mock.calls[1][0]).toBeNull(); + }); + }); }); diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 162a18a7c2c..0288d5a5bec 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -11,9 +11,10 @@ * into portals. */ -import { Component, ReactNode } from 'react'; +import { useState, useEffect, ReactNode } from 'react'; import { createPortal } from 'react-dom'; import { keysOf } from '../common'; +import { useUpdateEffect } from '../../services'; interface InsertPositionsMap { after: InsertPosition; @@ -40,43 +41,42 @@ export interface EuiPortalProps { portalRef?: (ref: HTMLDivElement | null) => void; } -export class EuiPortal extends Component { - portalNode: HTMLDivElement; - constructor(props: EuiPortalProps) { - super(props); +export const EuiPortal: React.FC = ({ + insert, + portalRef, + children, +}) => { + const [portalNode, setPortalNode] = useState(null); - const { insert } = this.props; + // pull `sibling` and `position` out of insert in case their wrapping object is recreated every render + const { sibling, position } = insert || {}; + useEffect(() => { + const portalNode = document.createElement('div'); + portalNode.dataset.euiportal = 'true'; + setPortalNode(portalNode); - this.portalNode = document.createElement('div'); - - if (insert == null) { + if (sibling == null || position == null) { // no insertion defined, append to body - document.body.appendChild(this.portalNode); + document.body.appendChild(portalNode); } else { // inserting before or after an element - const { sibling, position } = insert; - sibling.insertAdjacentElement(insertPositions[position], this.portalNode); + sibling.insertAdjacentElement(insertPositions[position], portalNode); } - } - componentDidMount() { - this.updatePortalRef(this.portalNode); - } + return () => { + if (portalNode && portalNode.parentNode) { + portalNode.parentNode.removeChild(portalNode); + } + }; + }, [sibling, position]); - componentWillUnmount() { - if (this.portalNode.parentNode) { - this.portalNode.parentNode.removeChild(this.portalNode); - } - this.updatePortalRef(null); - } + useUpdateEffect(() => { + portalRef?.(portalNode); - updatePortalRef(ref: HTMLDivElement | null) { - if (this.props.portalRef) { - this.props.portalRef(ref); - } - } + return () => { + portalRef?.(null); + }; + }, [portalNode, portalRef]); - render() { - return createPortal(this.props.children, this.portalNode); - } -} + return portalNode == null ? null : createPortal(children, portalNode); +}; diff --git a/upcoming_changelogs/6055.md b/upcoming_changelogs/6055.md new file mode 100644 index 00000000000..c869dc33f9e --- /dev/null +++ b/upcoming_changelogs/6055.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed a bug preventing `EuiPortal` from working in server-side rendering