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
342 changes: 119 additions & 223 deletions src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap

Large diffs are not rendered by default.

34 changes: 17 additions & 17 deletions src/components/bottom_bar/bottom_bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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(takeMountedSnapshot(component)).toMatchSnapshot();
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
});

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(takeMountedSnapshot(component)).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(takeMountedSnapshot(component)).toMatchSnapshot();
});
});
});

test('landmarkHeading', () => {
const component = render(
const component = mount(
<EuiBottomBar landmarkHeading="This should have been label" />
);

expect(component).toMatchSnapshot();
expect(takeMountedSnapshot(component)).toMatchSnapshot();
});

test('affordForDisplacement can be false', () => {
const component = render(<EuiBottomBar affordForDisplacement={false} />);
const component = mount(<EuiBottomBar affordForDisplacement={false} />);

expect(component).toMatchSnapshot();
expect(takeMountedSnapshot(component)).toMatchSnapshot();
});

test('usePortal can be false', () => {
const component = render(<EuiBottomBar usePortal={false} />);
const component = mount(<EuiBottomBar usePortal={false} />);

expect(component).toMatchSnapshot();
expect(takeMountedSnapshot(component)).toMatchSnapshot();
});

test('bodyClassName is rendered', () => {
Expand All @@ -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(takeMountedSnapshot(component)).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(takeMountedSnapshot(component)).toMatchSnapshot();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ export class EuiComboBoxOptionsList<T> extends Component<
componentDidMount() {
// Wait a frame, otherwise moving focus from one combo box to another will result in the class
// being removed from the body.
requestAnimationFrame(() => {
document.body.classList.add('euiBody-hasPortalContent');
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved
});
this.updatePosition();
window.addEventListener('resize', this.updatePosition);

Expand Down Expand Up @@ -163,7 +160,6 @@ export class EuiComboBoxOptionsList<T> extends Component<
}

componentWillUnmount() {
document.body.classList.remove('euiBody-hasPortalContent');
window.removeEventListener('resize', this.updatePosition);
window.removeEventListener('scroll', this.closeListOnScroll, {
capture: true,
Expand Down
1 change: 0 additions & 1 deletion src/components/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
@import 'panel/index';
@import 'page/index'; // Page needs to come after Panel for cascade specificity
@import 'popover/index';
@import 'portal/index';
@import 'tree_view/index';
@import 'resizable_container/index';
@import 'side_nav/index';
Expand Down
1 change: 0 additions & 1 deletion src/components/portal/_index.scss

This file was deleted.

6 changes: 0 additions & 6 deletions src/components/portal/_portal.scss

This file was deleted.

54 changes: 24 additions & 30 deletions src/components/portal/portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* into portals.
*/

import { Component, ReactNode } from 'react';
import { useState, useEffect, ReactNode } from 'react';
import { createPortal } from 'react-dom';
import { keysOf } from '../common';

Expand Down Expand Up @@ -40,43 +40,37 @@ export interface EuiPortalProps {
portalRef?: (ref: HTMLDivElement | null) => void;
}

export class EuiPortal extends Component<EuiPortalProps> {
portalNode: HTMLDivElement;
constructor(props: EuiPortalProps) {
super(props);
export const EuiPortal: React.FC<EuiPortalProps> = ({
insert,
portalRef,
children,
}) => {
const [portalNode, setPortalNode] = useState<HTMLDivElement>();

const { insert } = this.props;

this.portalNode = document.createElement('div');
// mount
useEffect(() => {
const portalNode = document.createElement('div');
setPortalNode(portalNode);

if (insert == 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);
}

componentWillUnmount() {
if (this.portalNode.parentNode) {
this.portalNode.parentNode.removeChild(this.portalNode);
}
this.updatePortalRef(null);
}
portalRef?.(portalNode);

updatePortalRef(ref: HTMLDivElement | null) {
if (this.props.portalRef) {
this.props.portalRef(ref);
}
}
// unmount
return () => {
if (portalNode && portalNode.parentNode) {
portalNode.parentNode.removeChild(portalNode);
}
portalRef?.(null);
};
}, [insert, portalRef]);
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved

render() {
return createPortal(this.props.children, this.portalNode);
}
}
return portalNode == null ? null : createPortal(children, portalNode);
};
2 changes: 0 additions & 2 deletions src/components/tool_tip/tool_tip_popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@ export class EuiToolTipPopover extends Component<Props> {
};

componentDidMount() {
document.body.classList.add('euiBody-hasPortalContent');
window.addEventListener('resize', this.updateDimensions);
}

componentWillUnmount() {
document.body.classList.remove('euiBody-hasPortalContent');
window.removeEventListener('resize', this.updateDimensions);
}

Expand Down
7 changes: 7 additions & 0 deletions upcoming_changelogs/6055.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
**Bug fixes**

- Fixed a bug preventing `EuiPortal` from working in server-side rendering

**Breaking changes**

- Removed styles for the `euiBody-hasPortalContent` class name
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved