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

refactor(popper): Update to use react portal as default #545

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions modules/common/react/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ includes custom `data-*` attributes such as `data-test-id` to help facilitate au

#### `containerElement: Element`

> The element that contains the portal children when `portal` is true.
> The containing element for Popper elements. The Popper uses
> [Portals](https://reactjs.org/docs/portals.html) to place the DOM elements of the Popper as the
> last child of the specified container element.

Default: `document.body`

Expand Down Expand Up @@ -112,10 +114,3 @@ Default: `bottom`
> Addtional options passed to the `popper.js` instance.

---

#### `portal: boolean`

> Flag to determine whether to use a portal for the popper. When false, the popper stays within the
> DOM hierarchy of it's parent. When true, the popper is attached to the `containerElement`.

Default: `true`
41 changes: 12 additions & 29 deletions modules/common/react/lib/Popper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ export interface PopperProps extends React.HTMLAttributes<HTMLDivElement> {
*/
children: React.ReactNode;
/**
* The element that contains the portal children when `portal` is true.
* The containing element for Popper elements. The Popper uses
* {@link https://reactjs.org/docs/portals.html Portals} to place the DOM elements of the Popper as the
* last child of the specified container element.
* @default document.body
*/
containerElement?: Element;
/**
Expand All @@ -32,11 +35,6 @@ export interface PopperProps extends React.HTMLAttributes<HTMLDivElement> {
* The additional options passed to the Popper's `popper.js` instance.
*/
popperOptions?: PopperOptions;
/**
* If true, attach the Popper to the `containerElement`. If false, render the Popper within the DOM hierarchy of its parent.
* @default true
*/
portal: boolean;
}

export class Popper extends React.PureComponent<PopperProps> {
Expand All @@ -45,7 +43,7 @@ export class Popper extends React.PureComponent<PopperProps> {
static defaultProps = {
open: true,
placement: 'bottom',
portal: true,
containerElement: document.body,
};

public componentWillUnmount() {
Expand All @@ -56,36 +54,21 @@ export class Popper extends React.PureComponent<PopperProps> {
}

public render() {
const {anchorElement, children, open, placement, popperOptions, ...elemProps} = this.props;

if (!this.props.open) {
return null;
}

if (!this.props.portal) {
return this.renderPopper();
}

// do not use defaultProps for containerElement because document may not be statically available
// at require time in some testing environments; instead we safely default at runtime
return ReactDOM.createPortal(this.renderPopper(), this.props.containerElement || document.body);
}

public renderPopper() {
const {
anchorElement,
children,
containerElement,
open,
placement,
popperOptions,
portal,
...elemProps
} = this.props;

return (
const popperElement = (
<div {...elemProps} ref={this.openPopper}>
{this.props.children}
</div>
);

// do not use defaultProps for containerElement because document may not be statically available
// at require time in some testing environments; instead we safely default at runtime
return ReactDOM.createPortal(popperElement, this.props.containerElement || document.body);
}

private openPopper = (popperNode: HTMLDivElement) => {
Expand Down
9 changes: 0 additions & 9 deletions modules/common/react/spec/Popper.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,6 @@ describe('Popper', () => {
component.unmount();
});

test('should render with Popper.js inline when portal is false', () => {
const component = renderPopper({portal: false});

expect(PopperJS).toHaveBeenCalled();
expect(ReactDOM.createPortal).not.toHaveBeenCalled();
expect(component.find('#content').length).toBe(1);
component.unmount();
});

test('should render nothing when open is false', () => {
const component = renderPopper({open: false});

Expand Down
3 changes: 1 addition & 2 deletions modules/popup/react/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ import * as React from 'react';
import {Popper} from '@workday/canvas-kit-react-common';
import {Popup} from '@workday/canvas-kit-react-popup';

// We use Popper from Material UI for our positioning
<Popper placement={'bottom'} open={this.state.open} anchorElement={anchorEl}>
<Popper placement={'bottom'} open={this.state.open} anchorEl={anchorEl}>
<Popup
width={300}
heading={'Popup Title'}
Expand Down
3 changes: 1 addition & 2 deletions modules/toast/react/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ import * as React from 'react';
import {Popper} from '@workday/canvas-kit-react-common';
import {Toast} from '@workday/canvas-kit-react-toast';

// We use Popper from Material UI for our positioning
<Popper placement={'top'} open={this.state.open} anchorElement={anchorEl}>
<Popper placement={'top'} open={this.state.open} anchorEl={anchorEl}>
<Toast onClose={this.handleClose}>{this.props.children}</Toast>
</Popper>;
```
Expand Down