Skip to content

Commit

Permalink
Sebastian review
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Dec 5, 2019
1 parent 559d19c commit 58838ae
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 19 deletions.
2 changes: 1 addition & 1 deletion docs/pages/api/portal.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ that exists outside the DOM hierarchy of the parent component.

| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | The children to render into the `container`. |
| <span class="prop-name required">children&nbsp;*</span> | <span class="prop-type">node</span> | | The children to render into the `container`. |
| <span class="prop-name">container</span> | <span class="prop-type">func<br>&#124;&nbsp;React.Component<br>&#124;&nbsp;Element</span> | | A node, component instance, or function that returns either. The `container` will have the portal children appended to it. By default, it uses the body of the top-level document object, so it's simply `document.body` most of the time. |
| <span class="prop-name">disablePortal</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the portal behavior. The children stay within it's parent DOM hierarchy. |
| <span class="prop-name">onRendered</span> | <span class="prop-type">func</span> | | Callback fired once the children has been mounted into the `container`.<br>This prop will be deprecated and removed in v5, the ref can be used instead. |
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Portal/Portal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export interface PortalProps {
/**
* The children to render into the `container`.
*/
children?: React.ReactNode;
children: React.ReactNode;
/**
* A node, component instance, or function that returns either.
* The `container` will have the portal children appended to it.
Expand Down
14 changes: 6 additions & 8 deletions packages/material-ui/src/Portal/Portal.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const useEnhancedEffect = typeof window !== 'undefined' ? React.useLayoutEffect
const Portal = React.forwardRef(function Portal(props, ref) {
const { children, container, disablePortal = false, onRendered } = props;
const [mountNode, setMountNode] = React.useState(null);
const handleRef = useForkRef(React.isValidElement(children) ? children.ref : null, ref);
const handleRef = useForkRef(children.ref, ref);

useEnhancedEffect(() => {
if (!disablePortal) {
Expand All @@ -46,12 +46,10 @@ const Portal = React.forwardRef(function Portal(props, ref) {
}, [onRendered, mountNode, disablePortal]);

if (disablePortal) {
if (React.isValidElement(children)) {
return React.cloneElement(children, {
ref: handleRef,
});
}
return children;
React.Children.only(children);
return React.cloneElement(children, {
ref: handleRef,
});
}

return mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode;
Expand All @@ -65,7 +63,7 @@ Portal.propTypes = {
/**
* The children to render into the `container`.
*/
children: PropTypes.node,
children: PropTypes.node.isRequired,
/**
* A node, component instance, or function that returns either.
* The `container` will have the portal children appended to it.
Expand Down
9 changes: 0 additions & 9 deletions packages/material-ui/src/Portal/Portal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,4 @@ describe('<Portal />', () => {
'ref',
]);
});

it('should render if children is undefined', () => {
render(
<div>
<Portal />
<Portal disablePortal />
</div>,
);
});
});

0 comments on commit 58838ae

Please sign in to comment.