Skip to content

Commit

Permalink
[Modal][Portal] Deprecate onRendered
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon authored and oliviertassinari committed Dec 20, 2020
1 parent 411cc61 commit 99f1a9d
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 38 deletions.
2 changes: 1 addition & 1 deletion docs/pages/api-docs/modal.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ This component shares many concepts with [react-overlays](https://react-bootstra
| <span class="prop-name">onBackdropClick</span> | <span class="prop-type">func</span> | | Callback fired when the backdrop is clicked. |
| <span class="prop-name">onClose</span> | <span class="prop-type">func</span> | | Callback fired when the component requests to be closed. The `reason` parameter can optionally be used to control the response to `onClose`.<br><br>**Signature:**<br>`function(event: object, reason: string) => void`<br>*event:* The event source of the callback.<br>*reason:* Can be: `"escapeKeyDown"`, `"backdropClick"`. |
| <span class="prop-name">onEscapeKeyDown</span> | <span class="prop-type">func</span> | | Callback fired when the escape key is pressed, `disableEscapeKeyDown` is false and the modal is in focus. |
| <span class="prop-name">onRendered</span> | <span class="prop-type">func</span> | | Callback fired once the children has been mounted into the `container`. It signals that the `open={true}` prop took effect.<br>This prop will be deprecated and removed in v5, the ref can be used instead. |
| ~~<span class="prop-name">onRendered</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the ref instead.<br><br>Callback fired once the children has been mounted into the `container`. It signals that the `open={true}` prop took effect.<br>This prop will be removed in v5, the ref can be used instead. |
| <span class="prop-name required">open<abbr title="required">*</abbr></span> | <span class="prop-type">bool</span> | | If `true`, the modal is open. |

The `ref` is forwarded to the root element.
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/api-docs/portal.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ that exists outside the DOM hierarchy of the parent component.
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | The children to render into the `container`. |
| <span class="prop-name">container</span> | <span class="prop-type">HTML element<br>&#124;&nbsp;React.Component<br>&#124;&nbsp;func</span> | | A HTML element, component instance, or function that returns either. The `container` will have the portal children appended to it.<br>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. |
| ~~<span class="prop-name">onRendered</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the ref instead.<br><br>Callback fired once the children has been mounted into the `container`.<br>This prop will be removed in v5, the ref can be used instead. |

The component cannot hold a ref.

Expand Down
10 changes: 10 additions & 0 deletions packages/material-ui/src/Modal/Modal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,17 @@ export interface ModalProps
bivarianceHack(event: {}, reason: 'backdropClick' | 'escapeKeyDown'): void;
}['bivarianceHack'];
onEscapeKeyDown?: React.ReactEventHandler<{}>;
/**
* Callback fired once the children has been mounted into the `container`.
* It signals that the `open={true}` prop took effect.
*
* This prop will be removed in v5, the ref can be used instead.
* @deprecated Use the ref instead.
*/
onRendered?: PortalProps['onRendered'];
/**
* If `true`, the modal is open.
*/
open: boolean;
}

Expand Down
5 changes: 3 additions & 2 deletions packages/material-ui/src/Modal/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
import { getThemeProps, useTheme } from '@material-ui/styles';
import { elementAcceptingRef, HTMLElementType } from '@material-ui/utils';
import ownerDocument from '../utils/ownerDocument';
import deprecatedPropType from '../utils/deprecatedPropType';
import Portal from '../Portal';
import createChainedFunction from '../utils/createChainedFunction';
import useForkRef from '../utils/useForkRef';
Expand Down Expand Up @@ -359,9 +360,9 @@ Modal.propTypes = {
* Callback fired once the children has been mounted into the `container`.
* It signals that the `open={true}` prop took effect.
*
* This prop will be deprecated and removed in v5, the ref can be used instead.
* This prop will be removed in v5, the ref can be used instead.
*/
onRendered: PropTypes.func,
onRendered: deprecatedPropType(PropTypes.func, 'Use the ref instead.'),
/**
* If `true`, the modal is open.
*/
Expand Down
14 changes: 0 additions & 14 deletions packages/material-ui/src/Modal/Modal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,20 +527,6 @@ describe('<Modal />', () => {
});
});

describe('prop: onRendered', () => {
it('should fire', () => {
const handleRendered = spy();

render(
<Modal open onRendered={handleRendered}>
<div />
</Modal>,
);

expect(handleRendered).to.have.property('callCount', 1);
});
});

describe('two modal at the same time', () => {
it('should open and close', () => {
const TestCase = (props) => (
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/Portal/Portal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ export interface PortalProps {
/**
* Callback fired once the children has been mounted into the `container`.
*
* This prop will be deprecated and removed in v5, the ref can be used instead.
* This prop will be removed in v5, the ref can be used instead.
* @deprecated Use the ref instead.
*/
onRendered?: () => void;
}
Expand Down
6 changes: 4 additions & 2 deletions packages/material-ui/src/Portal/Portal.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import * as ReactDOM from 'react-dom';
import PropTypes from 'prop-types';
import { exactProp, HTMLElementType } from '@material-ui/utils';
import deprecatedPropType from '../utils/deprecatedPropType';
import setRef from '../utils/setRef';
import useForkRef from '../utils/useForkRef';

Expand Down Expand Up @@ -86,9 +87,10 @@ Portal.propTypes = {
/**
* Callback fired once the children has been mounted into the `container`.
*
* This prop will be deprecated and removed in v5, the ref can be used instead.
* This prop will be removed in v5, the ref can be used instead.
* @deprecated Use the ref instead.
*/
onRendered: PropTypes.func,
onRendered: deprecatedPropType(PropTypes.func, 'Use the ref instead.'),
};

if (process.env.NODE_ENV !== 'production') {
Expand Down
17 changes: 0 additions & 17 deletions packages/material-ui/src/Portal/Portal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,23 +177,6 @@ describe('<Portal />', () => {
expect(document.querySelector('#test3').parentElement.nodeName).to.equal('BODY');
});

it('should call onRendered', () => {
const ref = React.createRef();
const handleRendered = spy();
render(
<Portal
ref={ref}
onRendered={() => {
handleRendered();
expect(ref.current !== null).to.equal(true);
}}
>
<div />
</Portal>,
);
expect(handleRendered.callCount).to.equal(1);
});

it('should call ref after child effect', () => {
const callOrder = [];
const handleRef = (node) => {
Expand Down

0 comments on commit 99f1a9d

Please sign in to comment.