Skip to content

Commit

Permalink
[Modal][Dialog] Deprecate duplicate props with onChange
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Dec 20, 2020
1 parent 411cc61 commit 37b0334
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 73 deletions.
6 changes: 3 additions & 3 deletions docs/pages/api-docs/dialog.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@ The `MuiDialog` name can be used for providing [default props](/customization/gl
| <span class="prop-name">aria-labelledby</span> | <span class="prop-type">string</span> | | The id(s) of the element(s) that label the dialog. |
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | Dialog children, usually the included sub-components. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">disableBackdropClick</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, clicking the backdrop will not fire the `onClose` callback. |
| ~~<span class="prop-name">disableBackdropClick</span>~~ | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | *Deprecated*. Use the onClose prop with the `reason` argument to filter the `backdropClick` events.<br><br>If `true`, clicking the backdrop will not fire the `onClose` callback. |
| <span class="prop-name">disableEscapeKeyDown</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, hitting escape will not fire the `onClose` callback. |
| <span class="prop-name">fullScreen</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the dialog will be full-screen |
| <span class="prop-name">fullWidth</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the dialog stretches to `maxWidth`.<br>Notice that the dialog width grow is limited by the default margin. |
| <span class="prop-name">maxWidth</span> | <span class="prop-type">'lg'<br>&#124;&nbsp;'md'<br>&#124;&nbsp;'sm'<br>&#124;&nbsp;'xl'<br>&#124;&nbsp;'xs'<br>&#124;&nbsp;false</span> | <span class="prop-default">'sm'</span> | Determine the max-width of the dialog. The dialog width grows with the size of the screen. Set to `false` to disable `maxWidth`. |
| <span class="prop-name">onBackdropClick</span> | <span class="prop-type">func</span> | | Callback fired when the backdrop is clicked. |
| ~~<span class="prop-name">onBackdropClick</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `backdropClick` events.<br><br>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.<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">onEnter</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired before the dialog enters. |
| ~~<span class="prop-name">onEntered</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the dialog has entered. |
| ~~<span class="prop-name">onEntering</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the dialog is entering. |
| <span class="prop-name">onEscapeKeyDown</span> | <span class="prop-type">func</span> | | Callback fired when the escape key is pressed, `disableKeyboard` is false and the modal is in focus. |
| ~~<span class="prop-name">onEscapeKeyDown</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.<br><br>Callback fired when the escape key is pressed, `disableKeyboard` is false and the modal is in focus. |
| ~~<span class="prop-name">onExit</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired before the dialog exits. |
| ~~<span class="prop-name">onExited</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the dialog has exited. |
| ~~<span class="prop-name">onExiting</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the dialog is exiting. |
Expand Down
6 changes: 3 additions & 3 deletions docs/pages/api-docs/modal.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,17 @@ This component shares many concepts with [react-overlays](https://react-bootstra
| <span class="prop-name">closeAfterTransition</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | When set to true the Modal waits until a nested Transition is completed before closing. |
| <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">disableAutoFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the modal will not automatically shift focus to itself when it opens, and replace it to the last focused element when it closes. This also works correctly with any modal children that have the `disableAutoFocus` prop.<br>Generally this should never be set to `true` as it makes the modal less accessible to assistive technologies, like screen readers. |
| <span class="prop-name">disableBackdropClick</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, clicking the backdrop will not fire `onClose`. |
| ~~<span class="prop-name">disableBackdropClick</span>~~ | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | *Deprecated*. Use the onClose prop with the `reason` argument to filter the `backdropClick` events.<br><br>If `true`, clicking the backdrop will not fire `onClose`. |
| <span class="prop-name">disableEnforceFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the modal will not prevent focus from leaving the modal while open.<br>Generally this should never be set to `true` as it makes the modal less accessible to assistive technologies, like screen readers. |
| <span class="prop-name">disableEscapeKeyDown</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, hitting escape will not fire `onClose`. |
| <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">disableRestoreFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the modal will not restore focus to previously focused element once modal is hidden. |
| <span class="prop-name">disableScrollLock</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the scroll lock behavior. |
| <span class="prop-name">hideBackdrop</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the backdrop is not rendered. |
| <span class="prop-name">keepMounted</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Always keep the children in the DOM. This prop can be useful in SEO situation or when you want to maximize the responsiveness of the Modal. |
| <span class="prop-name">onBackdropClick</span> | <span class="prop-type">func</span> | | Callback fired when the backdrop is clicked. |
| ~~<span class="prop-name">onBackdropClick</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `backdropClick` events.<br><br>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">onEscapeKeyDown</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.<br><br>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 required">open<abbr title="required">*</abbr></span> | <span class="prop-type">bool</span> | | If `true`, the modal is open. |

Expand Down
2 changes: 0 additions & 2 deletions docs/src/pages/components/dialogs/ConfirmationDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ function ConfirmationDialogRaw(props) {

return (
<Dialog
disableBackdropClick
disableEscapeKeyDown
maxWidth="xs"
onEntering={handleEntering}
aria-labelledby="confirmation-dialog-title"
Expand Down
2 changes: 0 additions & 2 deletions docs/src/pages/components/dialogs/ConfirmationDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ function ConfirmationDialogRaw(props: ConfirmationDialogRawProps) {

return (
<Dialog
disableBackdropClick
disableEscapeKeyDown
maxWidth="xs"
onEntering={handleEntering}
aria-labelledby="confirmation-dialog-title"
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/selects/DialogSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default function DialogSelect() {
return (
<div>
<Button onClick={handleClickOpen}>Open select dialog</Button>
<Dialog disableBackdropClick disableEscapeKeyDown open={open} onClose={handleClose}>
<Dialog open={open} onClose={handleClose}>
<DialogTitle>Fill the form</DialogTitle>
<DialogContent>
<form className={classes.container}>
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/selects/DialogSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export default function DialogSelect() {
return (
<div>
<Button onClick={handleClickOpen}>Open select dialog</Button>
<Dialog disableBackdropClick disableEscapeKeyDown open={open} onClose={handleClose}>
<Dialog open={open} onClose={handleClose}>
<DialogTitle>Fill the form</DialogTitle>
<DialogContent>
<form className={classes.container}>
Expand Down
3 changes: 3 additions & 0 deletions packages/material-ui/src/Dialog/Dialog.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface DialogProps
children?: React.ReactNode;
/**
* If `true`, clicking the backdrop will not fire the `onClose` callback.
* @deprecated Use the onClose prop with the `reason` argument to filter the `backdropClick` events.
*/
disableBackdropClick?: boolean;
/**
Expand All @@ -44,6 +45,7 @@ export interface DialogProps
maxWidth?: 'xs' | 'sm' | 'md' | 'lg' | 'xl' | false;
/**
* Callback fired when the backdrop is clicked.
* @deprecated Use the onClose prop with the `reason` argument to handle the `backdropClick` events.
*/
onBackdropClick?: ModalProps['onBackdropClick'];
/**
Expand Down Expand Up @@ -71,6 +73,7 @@ export interface DialogProps
/**
* Callback fired when the escape key is pressed,
* `disableKeyboard` is false and the modal is in focus.
* @deprecated Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.
*/
onEscapeKeyDown?: ModalProps['onEscapeKeyDown'];
/**
Expand Down
24 changes: 20 additions & 4 deletions packages/material-ui/src/Dialog/Dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,11 @@ const Dialog = React.forwardRef(function Dialog(props, ref) {
...BackdropProps,
}}
closeAfterTransition
disableBackdropClick={disableBackdropClick}
{...(disableBackdropClick
? {
disableBackdropClick,
}
: {})}
disableEscapeKeyDown={disableEscapeKeyDown}
onEscapeKeyDown={onEscapeKeyDown}
onClose={onClose}
Expand Down Expand Up @@ -295,8 +299,12 @@ Dialog.propTypes = {
className: PropTypes.string,
/**
* If `true`, clicking the backdrop will not fire the `onClose` callback.
* @deprecated Use the onClose prop with the `reason` argument to filter the `backdropClick` events.
*/
disableBackdropClick: PropTypes.bool,
disableBackdropClick: deprecatedPropType(
PropTypes.bool,
'Use the onClose prop with the `reason` argument to filter the `backdropClick` events.',
),
/**
* If `true`, hitting escape will not fire the `onClose` callback.
*/
Expand All @@ -319,8 +327,12 @@ Dialog.propTypes = {
maxWidth: PropTypes.oneOf(['lg', 'md', 'sm', 'xl', 'xs', false]),
/**
* Callback fired when the backdrop is clicked.
* @deprecated Use the onClose prop with the `reason` argument to handle the `backdropClick` events.
*/
onBackdropClick: PropTypes.func,
onBackdropClick: deprecatedPropType(
PropTypes.func,
'Use the onClose prop with the `reason` argument to handle the `backdropClick` events.',
),
/**
* Callback fired when the component requests to be closed.
*
Expand All @@ -346,8 +358,12 @@ Dialog.propTypes = {
/**
* Callback fired when the escape key is pressed,
* `disableKeyboard` is false and the modal is in focus.
* @deprecated Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.
*/
onEscapeKeyDown: PropTypes.func,
onEscapeKeyDown: deprecatedPropType(
PropTypes.func,
'Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.',
),
/**
* Callback fired before the dialog exits.
* @deprecated Use the `TransitionProps` prop instead.
Expand Down
31 changes: 11 additions & 20 deletions packages/material-ui/src/Dialog/Dialog.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ describe('<Dialog />', () => {
});

it('calls onEscapeKeydown when pressing Esc followed by onClose and removes the content after the specified duration', () => {
const onEscapeKeyDown = spy();
const onClose = spy();
function TestCase() {
const [open, close] = React.useReducer(() => false, true);
Expand All @@ -85,12 +84,7 @@ describe('<Dialog />', () => {
};

return (
<Dialog
open={open}
transitionDuration={100}
onEscapeKeyDown={onEscapeKeyDown}
onClose={handleClose}
>
<Dialog open={open} transitionDuration={100} onClose={handleClose}>
foo
</Dialog>
);
Expand All @@ -101,21 +95,20 @@ describe('<Dialog />', () => {

dialog.click();
fireEvent.keyDown(document.querySelector('[data-mui-test="FakeBackdrop"]'), { key: 'Esc' });
expect(onEscapeKeyDown.calledOnce).to.equal(true);
expect(onClose.calledOnce).to.equal(true);

clock.tick(100);
expect(queryByRole('dialog')).to.equal(null);
});

it('can ignore backdrop click and Esc keydown', () => {
const onClose = spy();
const onClose = [];
const { getByRole } = render(
<Dialog
open
disableBackdropClick
disableEscapeKeyDown
onClose={onClose}
onClose={(event, reason) => {
onClose.push(reason);
}}
transitionDuration={0}
>
foo
Expand All @@ -126,10 +119,10 @@ describe('<Dialog />', () => {

dialog.click();
fireEvent.keyDown(document.querySelector('[data-mui-test="FakeBackdrop"]'), { key: 'Esc' });
expect(onClose.callCount).to.equal(0);
expect(onClose).to.deep.equal(['escapeKeyDown']);

clickBackdrop(document.body);
expect(onClose.callCount).to.equal(0);
expect(onClose).to.deep.equal(['escapeKeyDown', 'backdropClick']);
});

it('should spread custom props on the modal root node', () => {
Expand All @@ -150,31 +143,29 @@ describe('<Dialog />', () => {
});

it('calls onBackdropClick and onClose when clicked', () => {
const onBackdropClick = spy();
const onClose = spy();
render(
<Dialog onBackdropClick={onBackdropClick} onClose={onClose} open>
<Dialog onClose={onClose} open>
foo
</Dialog>,
);

clickBackdrop(document);
expect(onBackdropClick.callCount).to.equal(1);
expect(onClose.callCount).to.equal(1);
});

it('should ignore the backdrop click if the event did not come from the backdrop', () => {
const onBackdropClick = spy();
const onClose = spy();
const { getByRole } = render(
<Dialog onBackdropClick={onBackdropClick} open>
<Dialog onClose={onClose} open>
<div tabIndex={-1}>
<h2>my dialog</h2>
</div>
</Dialog>,
);

userClick(getByRole('heading'));
expect(onBackdropClick.callCount).to.equal(0);
expect(onClose.callCount).to.equal(0);
});

it('should not close if the target changes between the mousedown and the click', () => {
Expand Down
13 changes: 13 additions & 0 deletions packages/material-ui/src/Modal/Modal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ export interface ModalProps
closeAfterTransition?: boolean;
container?: PortalProps['container'];
disableAutoFocus?: boolean;
/**
* If `true`, clicking the backdrop will not fire the `onClose` callback.
* @deprecated Use the onClose prop with the `reason` argument to filter the `backdropClick` events.
*/
disableBackdropClick?: boolean;
disableEnforceFocus?: boolean;
disableEscapeKeyDown?: boolean;
Expand All @@ -20,6 +24,10 @@ export interface ModalProps
hideBackdrop?: boolean;
keepMounted?: boolean;
manager?: ModalManager;
/**
* Callback fired when the backdrop is clicked.
* @deprecated Use the onClose prop with the `reason` argument to handle the `backdropClick` events.
*/
onBackdropClick?: React.ReactEventHandler<{}>;
/**
* Callback fired when the component requests to be closed.
Expand All @@ -30,6 +38,11 @@ export interface ModalProps
onClose?: {
bivarianceHack(event: {}, reason: 'backdropClick' | 'escapeKeyDown'): void;
}['bivarianceHack'];
/**
* Callback fired when the escape key is pressed,
* `disableKeyboard` is false and the modal is in focus.
* @deprecated Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.
*/
onEscapeKeyDown?: React.ReactEventHandler<{}>;
onRendered?: PortalProps['onRendered'];
open: boolean;
Expand Down
16 changes: 13 additions & 3 deletions packages/material-ui/src/Modal/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as ReactDOM from 'react-dom';
import PropTypes from 'prop-types';
import { getThemeProps, useTheme } from '@material-ui/styles';
import { elementAcceptingRef, HTMLElementType } from '@material-ui/utils';
import deprecatedPropType from '../utils/deprecatedPropType';
import ownerDocument from '../utils/ownerDocument';
import Portal from '../Portal';
import createChainedFunction from '../utils/createChainedFunction';
Expand Down Expand Up @@ -298,7 +299,10 @@ Modal.propTypes = {
/**
* If `true`, clicking the backdrop will not fire `onClose`.
*/
disableBackdropClick: PropTypes.bool,
disableBackdropClick: deprecatedPropType(
PropTypes.bool,
'Use the onClose prop with the `reason` argument to filter the `backdropClick` events.',
),
/**
* If `true`, the modal will not prevent focus from leaving the modal while open.
*
Expand Down Expand Up @@ -341,7 +345,10 @@ Modal.propTypes = {
/**
* Callback fired when the backdrop is clicked.
*/
onBackdropClick: PropTypes.func,
onBackdropClick: deprecatedPropType(
PropTypes.func,
'Use the onClose prop with the `reason` argument to handle the `backdropClick` events.',
),
/**
* Callback fired when the component requests to be closed.
* The `reason` parameter can optionally be used to control the response to `onClose`.
Expand All @@ -354,7 +361,10 @@ Modal.propTypes = {
* Callback fired when the escape key is pressed,
* `disableEscapeKeyDown` is false and the modal is in focus.
*/
onEscapeKeyDown: PropTypes.func,
onEscapeKeyDown: deprecatedPropType(
PropTypes.func,
'Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.',
),
/**
* Callback fired once the children has been mounted into the `container`.
* It signals that the `open={true}` prop took effect.
Expand Down
Loading

0 comments on commit 37b0334

Please sign in to comment.