Skip to content

Commit

Permalink
[Portal] Migrate to React hooks (#15399)
Browse files Browse the repository at this point in the history
* portal migrated to hooks

* complete the effort :)

* ref weak convention

* use null when ref is applied to an element
  • Loading branch information
gautam-pahuja authored and oliviertassinari committed Apr 30, 2019
1 parent 7239977 commit 02f135d
Show file tree
Hide file tree
Showing 28 changed files with 108 additions and 147 deletions.
2 changes: 1 addition & 1 deletion docs/src/modules/components/AppSearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ const useStyles = makeStyles(theme => ({
function AppSearch(props) {
const { userLanguage } = props;
const classes = useStyles();
const inputRef = React.useRef();
const inputRef = React.useRef(null);
const theme = useTheme();

React.useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/demos/progress/CircularIntegration.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function CircularIntegration() {
const classes = useStyles();
const [loading, setLoading] = React.useState(false);
const [success, setSuccess] = React.useState(false);
const timer = React.useRef(undefined);
const timer = React.useRef();

const buttonClassname = clsx({
[classes.buttonSuccess]: success,
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/demos/progress/CircularIntegration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function CircularIntegration() {
const classes = useStyles();
const [loading, setLoading] = React.useState(false);
const [success, setSuccess] = React.useState(false);
const timer = React.useRef<number | undefined>(undefined);
const timer = React.useRef<number>();

const buttonClassname = clsx({
[classes.buttonSuccess]: success,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function ClickAwayListener(props) {
const mountedRef = useMountedRef();
const movedRef = React.useRef(false);

const nodeRef = React.useRef();
const nodeRef = React.useRef(null);
// can be removed once we drop support for non ref forwarding class components
const handleOwnRef = React.useCallback(instance => {
// #StrictMode ready
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Collapse/Collapse.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const Collapse = React.forwardRef(function Collapse(props, ref) {
...other
} = props;
const timer = React.useRef();
const wrapperRef = React.useRef();
const wrapperRef = React.useRef(null);
const autoTransitionDuration = React.useRef();

React.useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Dialog/Dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ const Dialog = React.forwardRef(function Dialog(props, ref) {
...other
} = props;

const mouseDownTarget = React.useRef(null);
const mouseDownTarget = React.useRef();
const handleMouseDown = event => {
mouseDownTarget.current = event.target;
};
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/InputBase/Textarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ const Textarea = React.forwardRef(function Textarea(props, ref) {
const { onChange, rows, rowsMax, style, value, ...other } = props;

const { current: isControlled } = React.useRef(value != null);
const inputRef = React.useRef();
const inputRef = React.useRef(null);
const [state, setState] = React.useState({});
const shadowRef = React.useRef();
const shadowRef = React.useRef(null);
const handleRef = useForkRef(ref, inputRef);

const syncHeight = React.useCallback(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/ListItem/ListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const ListItem = React.forwardRef(function ListItem(props, ref) {
dense: dense || context.dense || false,
alignItems,
};
const listItemRef = React.useRef();
const listItemRef = React.useRef(null);
useEnhancedEffect(() => {
if (autoFocus) {
if (listItemRef.current) {
Expand Down
6 changes: 3 additions & 3 deletions packages/material-ui/src/Menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ const Menu = React.forwardRef(function Menu(props, ref) {

const autoFocus = autoFocusProp !== undefined ? autoFocusProp : !disableAutoFocusItem;

const menuListActionsRef = React.useRef();
const firstValidItemRef = React.useRef();
const firstSelectedItemRef = React.useRef();
const menuListActionsRef = React.useRef(null);
const firstValidItemRef = React.useRef(null);
const firstSelectedItemRef = React.useRef(null);

const getContentAnchorEl = () => firstSelectedItemRef.current || firstValidItemRef.current;

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/MenuList/MenuList.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const useEnhancedEffect = typeof window === 'undefined' ? React.useEffect : Reac

const MenuList = React.forwardRef(function MenuList(props, ref) {
const { actions, autoFocus, className, onKeyDown, disableListWrap, ...other } = props;
const listRef = React.useRef();
const listRef = React.useRef(null);
const textCriteriaRef = React.useRef({
keys: [],
repeating: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Modal/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class Modal extends React.Component {
};

handlePortalRef = ref => {
this.mountNode = ref ? ref.getMountNode() : ref;
this.mountNode = ref;
};

handleModalRef = ref => {
Expand Down
6 changes: 1 addition & 5 deletions packages/material-ui/src/Modal/Modal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
describeConformance,
} from '@material-ui/core/test-utils';
import Fade from '../Fade';
import Portal from '../Portal';
import Backdrop from '../Backdrop';
import Modal from './Modal';

Expand Down Expand Up @@ -176,10 +175,7 @@ describe('<Modal />', () => {

it('should render the content into the portal', () => {
wrapper.setProps({ open: true });
const portalLayer = wrapper
.find(Portal)
.instance()
.getMountNode();
const portalLayer = document.querySelector('[data-mui-test="Modal"]');
const container = document.getElementById('container');
const heading = document.getElementById('heading');

Expand Down
6 changes: 3 additions & 3 deletions packages/material-ui/src/Modal/TrapFocus.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ function TrapFocus(props) {
open,
} = props;
const ignoreNextEnforceFocus = React.useRef();
const sentinelStart = React.useRef();
const sentinelEnd = React.useRef();
const sentinelStart = React.useRef(null);
const sentinelEnd = React.useRef(null);
const lastFocus = React.useRef();
const rootRef = React.useRef();
const rootRef = React.useRef(null);
// can be removed once we drop support for non ref forwarding class components
const handleOwnRef = React.useCallback(instance => {
// #StrictMode ready
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Popper/Popper.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const Popper = React.forwardRef(function Popper(props, ref) {
transition,
...other
} = props;
const tooltipRef = React.useRef();
const tooltipRef = React.useRef(null);
const popperRef = React.useRef();
const [exited, setExited] = React.useState(!props.open);
const [placement, setPlacement] = React.useState();
Expand Down
88 changes: 28 additions & 60 deletions packages/material-ui/src/Portal/Portal.js
Original file line number Diff line number Diff line change
@@ -1,82 +1,49 @@
import React from 'react';
import ReactDOM from 'react-dom';
import PropTypes from 'prop-types';
import ownerDocument from '../utils/ownerDocument';
import { useForkRef } from '../utils/reactHelpers';
import { exactProp } from '@material-ui/utils';

function getContainer(container, defaultContainer) {
function getContainer(container) {
container = typeof container === 'function' ? container() : container;
return ReactDOM.findDOMNode(container) || defaultContainer;
return ReactDOM.findDOMNode(container);
}

function getOwnerDocument(element) {
return ownerDocument(ReactDOM.findDOMNode(element));
}
const useEnhancedEffect = typeof window !== 'undefined' ? React.useLayoutEffect : React.useEffect;

/**
* Portals provide a first-class way to render children into a DOM node
* that exists outside the DOM hierarchy of the parent component.
*/
class Portal extends React.Component {
componentDidMount() {
this.setMountNode(this.props.container);

// Only rerender if needed
if (!this.props.disablePortal) {
this.forceUpdate(this.props.onRendered);
const Portal = React.forwardRef(function Portal(props, ref) {
const { children, container, disablePortal, onRendered } = props;
const [mountNode, setMountNode] = React.useState(null);
const childRef = React.useRef(null);
const handleRef = useForkRef(children.ref, childRef);

useEnhancedEffect(() => {
if (!disablePortal) {
setMountNode(getContainer(container) || document.body);
}
}

componentDidUpdate(prevProps) {
if (
prevProps.container !== this.props.container ||
prevProps.disablePortal !== this.props.disablePortal
) {
this.setMountNode(this.props.container);
}, [container]);

// Only rerender if needed
if (!this.props.disablePortal) {
this.forceUpdate(() => {
if (this.props.onRendered) {
// This might be triggered earlier than the componentDidUpdate of a parent element.
// We need to account for it.
clearTimeout(this.renderedTimer);
this.renderedTimer = setTimeout(this.props.onRendered);
}
});
}
React.useEffect(() => {
if (onRendered && mountNode) {
onRendered();
}
}

componentWillUnmount() {
this.mountNode = null;
clearTimeout(this.renderedTimer);
}
}, [mountNode, onRendered]);

setMountNode(container) {
if (this.props.disablePortal) {
this.mountNode = ReactDOM.findDOMNode(this).parentElement;
return;
}
React.useImperativeHandle(ref, () => mountNode || childRef.current, [mountNode]);

this.mountNode = getContainer(container, getOwnerDocument(this).body);
if (disablePortal) {
React.Children.only(children);
return React.cloneElement(children, {
ref: handleRef,
});
}

/**
* @public
*/
getMountNode = () => this.mountNode;

render() {
const { children, disablePortal } = this.props;

if (disablePortal) {
return children;
}

return this.mountNode ? ReactDOM.createPortal(children, this.mountNode) : null;
}
}
return mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode;
});

Portal.propTypes = {
/**
Expand Down Expand Up @@ -106,7 +73,8 @@ Portal.defaultProps = {
};

if (process.env.NODE_ENV !== 'production') {
Portal.propTypes = exactProp(Portal.propTypes);
// eslint-disable-next-line
Portal['propTypes' + ''] = exactProp(Portal.propTypes);
}

export default Portal;
54 changes: 43 additions & 11 deletions packages/material-ui/src/Portal/Portal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { assert } from 'chai';
import { spy } from 'sinon';
import PropTypes from 'prop-types';
import { createMount, createRender } from '@material-ui/core/test-utils';
import consoleErrorMock from 'test/utils/consoleErrorMock';
import Portal from './Portal';
import Select from '../Select';
import MenuItem from '../MenuItem';
Expand Down Expand Up @@ -86,6 +87,14 @@ describe('<Portal />', () => {
return;
}

beforeEach(() => {
consoleErrorMock.spy();
});

afterEach(() => {
consoleErrorMock.reset();
});

it('render nothing on the server', () => {
const markup1 = render(<div>Bar</div>);
assert.strictEqual(markup1.text(), 'Bar');
Expand All @@ -109,24 +118,39 @@ describe('<Portal />', () => {
});

it('should have access to the mountNode', () => {
const wrapper = mount(
<Portal>
const refSpy1 = spy();
mount(
<Portal ref={refSpy1}>
<h1>Foo</h1>
</Portal>,
);
const instance = wrapper.find('Portal').instance();
assert.strictEqual(instance.getMountNode(), instance.mountNode);
assert.deepEqual(refSpy1.args, [[null], [null], [document.body]]);
const refSpy2 = spy();
mount(
<Portal disablePortal ref={refSpy2}>
<h1 className="woofPortal">Foo</h1>
</Portal>,
);
assert.deepEqual(refSpy2.args, [[document.querySelector('.woofPortal')]]);
});

it('should render in a different node', () => {
const wrapper = mount(
<Portal>
<h1 className="woofPortal">Foo</h1>
</Portal>,
<div>
<h1 className="woofPortal1">Foo</h1>
<Portal>
<h1 className="woofPortal2">Foo</h1>
</Portal>
</div>,
);
assert.strictEqual(
wrapper.getDOMNode().contains(document.querySelector('.woofPortal1')),
true,
);
assert.strictEqual(
wrapper.getDOMNode().contains(document.querySelector('.woofPortal2')),
false,
);
const instance = wrapper.find('Portal').instance();
assert.notStrictEqual(instance.mountNode, null, 'should have a mountNode');
assert.strictEqual(document.querySelectorAll('.woofPortal').length, 1);
});

it('should unmount when parent unmounts', () => {
Expand Down Expand Up @@ -270,7 +294,15 @@ describe('<Portal />', () => {
wrapper.setProps({ container: null });

setTimeout(() => {
assert.deepEqual(callOrder, ['onRendered', 'cDU', 'cDU', 'cDU', 'onRendered']);
assert.deepEqual(callOrder, [
'onRendered',
'cDU',
'onRendered',
'cDU',
'onRendered',
'cDU',
'onRendered',
]);
done();
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/RadioGroup/RadioGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import RadioGroupContext from './RadioGroupContext';

const RadioGroup = React.forwardRef(function RadioGroup(props, ref) {
const { actions, children, name, value: valueProp, onChange, ...other } = props;
const rootRef = React.useRef();
const rootRef = React.useRef(null);
const { current: isControlled } = React.useRef(props.value != null);
const [valueState, setValue] = React.useState(() => {
if (!isControlled) {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/SelectInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
variant,
...other
} = props;
const displayRef = React.useRef();
const displayRef = React.useRef(null);
const ignoreNextBlur = React.useRef(false);
const { current: isOpenControlled } = React.useRef(props.open != null);
const [menuMinWidthState, setMenuMinWidthState] = React.useState();
Expand Down
Loading

0 comments on commit 02f135d

Please sign in to comment.