Skip to content

Commit

Permalink
fix(popover): now render without error after ssr
Browse files Browse the repository at this point in the history
We have to check for mount as the call to createPortal will error with `Portals are not currently supported by the server renderer...` if rehydrating after ssr. See, facebook/react#13097 (comment),  for rationale of the given changes.
  • Loading branch information
enjoylife committed Mar 3, 2019
1 parent fea5df3 commit 36b8d59
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/popover/__tests__/__snapshots__/popover.test.js.snap
Expand Up @@ -37,7 +37,7 @@ Object {
"current": null,
},
"$showArrow": true,
"id": null,
"id": "bui-mock-id",
"onMouseEnter": [Function],
"onMouseLeave": [Function],
}
Expand Down
15 changes: 13 additions & 2 deletions src/popover/__tests__/popover.test.js
Expand Up @@ -230,8 +230,9 @@ describe('Popover', () => {
</Popover>,
);

expect(CustomComponent).toHaveBeenCalledTimes(1);
expect(CustomComponent).toHaveBeenLastCalledWith(
// first render is prior to mount, second is after cdm
expect(CustomComponent).toHaveBeenCalledTimes(2);
expect(CustomComponent).toHaveBeenCalledWith(
{
$ref: wrapper.instance().anchorRef,
onClick: wrapper.instance().onAnchorClick,
Expand All @@ -241,6 +242,16 @@ describe('Popover', () => {
},
{},
);
expect(CustomComponent).toHaveBeenLastCalledWith(
{
$ref: wrapper.instance().anchorRef,
onClick: wrapper.instance().onAnchorClick,
'aria-controls': 'bui-mock-id',
'aria-haspopup': 'true',
'aria-expanded': 'true',
},
{},
);
});

test('component overrides', () => {
Expand Down
21 changes: 14 additions & 7 deletions src/popover/popover.js
Expand Up @@ -57,15 +57,18 @@ class Popover extends React.Component<PopoverPropsT, PopoverPrivateStateT> {
state = this.getDefaultState(this.props);

componentDidMount() {
if (this.props.isOpen) {
this.initializePopper();
this.addDomEvents();
}
this.generatedId = getBuiId();
this.setState({isMounted: true});
}

componentDidUpdate(prevProps: PopoverPropsT) {
if (this.props.isOpen !== prevProps.isOpen) {
componentDidUpdate(
prevProps: PopoverPropsT,
prevState: PopoverPrivateStateT,
) {
if (
this.props.isOpen !== prevProps.isOpen ||
this.state.isMounted !== prevState.isMounted
) {
if (this.props.isOpen) {
// Clear any existing timers (like previous animateOutCompleteTimer)
this.clearTimers();
Expand Down Expand Up @@ -93,6 +96,7 @@ class Popover extends React.Component<PopoverPropsT, PopoverPrivateStateT> {
arrowOffset: {left: 0, top: 0},
popoverOffset: {left: 0, top: 0},
placement: props.placement,
isMounted: false,
};
}

Expand Down Expand Up @@ -468,7 +472,10 @@ class Popover extends React.Component<PopoverPropsT, PopoverPrivateStateT> {

// Only render popover on the browser (portals aren't supported server-side)
if (__BROWSER__) {
if (this.props.isOpen || this.state.isAnimating) {
if (
this.state.isMounted &&
(this.props.isOpen || this.state.isAnimating)
) {
rendered.push(
// $FlowFixMe
ReactDOM.createPortal(this.renderPopover(), document.body),
Expand Down
1 change: 1 addition & 0 deletions src/popover/types.js
Expand Up @@ -168,6 +168,7 @@ export type PopoverPrivateStateT = {
arrowOffset: OffsetT,
popoverOffset: OffsetT,
placement: PopoverPlacementT,
isMounted: boolean,
};

export type SharedStylePropsArgT = {
Expand Down

0 comments on commit 36b8d59

Please sign in to comment.