Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jul 27, 2022
76 changes: 31 additions & 45 deletions src/components/popover/wrapping_popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);
Comment on lines +26 to +27
Copy link
Contributor

@cee-chen cee-chen Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be useRefs instead of state, based on the usage? edit to clarify: Not a change request, just curious what the reasoning is for state

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They need to be state in order to kick-off a re-render after mount->setX calls, to trigger the useEffect. Assigning to a ref doesn't cause another pass.


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>
);
};
7 changes: 4 additions & 3 deletions src/components/portal/portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,18 @@ export const EuiPortal: React.FC<EuiPortalProps> = ({
}) => {
const [portalNode, setPortalNode] = useState<HTMLDivElement | null>(null);

// 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);

if (insert == null) {
if (sibling == null || position == null) {
// no insertion defined, append to body
document.body.appendChild(portalNode);
} else {
// inserting before or after an element
const { sibling, position } = insert;
sibling.insertAdjacentElement(insertPositions[position], portalNode);
}

Expand All @@ -67,7 +68,7 @@ export const EuiPortal: React.FC<EuiPortalProps> = ({
portalNode.parentNode.removeChild(portalNode);
}
};
}, [insert]);
}, [sibling, position]);

useUpdateEffect(() => {
portalRef?.(portalNode);
Expand Down