From 8fc052a74fbb6a80fcba59a285e9613f006dcccb Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Fri, 20 Nov 2020 14:07:34 +0100 Subject: [PATCH] [Dialog][Modal] Remove disableBackdropClick (#23607) --- docs/pages/api-docs/dialog.md | 1 - docs/pages/api-docs/modal.md | 1 - .../components/dialogs/ConfirmationDialog.js | 2 - .../components/dialogs/ConfirmationDialog.tsx | 2 - .../pages/components/selects/DialogSelect.js | 13 +++--- .../pages/components/selects/DialogSelect.tsx | 16 +++---- .../pages/guides/migration-v4/migration-v4.md | 44 +++++++++++++++++++ packages/material-ui/src/Dialog/Dialog.d.ts | 5 --- packages/material-ui/src/Dialog/Dialog.js | 9 +--- .../material-ui/src/Dialog/Dialog.test.js | 15 +++++-- packages/material-ui/src/Modal/Modal.d.ts | 5 --- packages/material-ui/src/Modal/Modal.js | 8 +--- packages/material-ui/src/Modal/Modal.test.js | 19 ++++++-- 13 files changed, 87 insertions(+), 53 deletions(-) diff --git a/docs/pages/api-docs/dialog.md b/docs/pages/api-docs/dialog.md index 140be57ab8c629..e83f43a083a3f2 100644 --- a/docs/pages/api-docs/dialog.md +++ b/docs/pages/api-docs/dialog.md @@ -32,7 +32,6 @@ The `MuiDialog` name can be used for providing [default props](/customization/gl | aria-labelledby | string | | The id(s) of the element(s) that label the dialog. | | children | node | | Dialog children, usually the included sub-components. | | classes | object | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. | -| disableBackdropClick | bool | false | If `true`, clicking the backdrop will not fire the `onClose` callback. | | disableEscapeKeyDown | bool | false | If `true`, hitting escape will not fire the `onClose` callback. | | fullScreen | bool | false | If `true`, the dialog is full-screen. | | fullWidth | bool | false | If `true`, the dialog stretches to `maxWidth`.
Notice that the dialog width grow is limited by the default margin. | diff --git a/docs/pages/api-docs/modal.md b/docs/pages/api-docs/modal.md index 995cfe88b441ac..9bac6ff998b2c8 100644 --- a/docs/pages/api-docs/modal.md +++ b/docs/pages/api-docs/modal.md @@ -42,7 +42,6 @@ This component shares many concepts with [react-overlays](https://react-bootstra | closeAfterTransition | bool | false | When set to true the Modal waits until a nested Transition is completed before closing. | | container | HTML element
| func | | A HTML element or function that returns one. 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. | | disableAutoFocus | bool | false | 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.
Generally this should never be set to `true` as it makes the modal less accessible to assistive technologies, like screen readers. | -| disableBackdropClick | bool | false | If `true`, clicking the backdrop will not fire `onClose`. | | disableEnforceFocus | bool | false | If `true`, the modal will not prevent focus from leaving the modal while open.
Generally this should never be set to `true` as it makes the modal less accessible to assistive technologies, like screen readers. | | disableEscapeKeyDown | bool | false | If `true`, hitting escape will not fire `onClose`. | | disablePortal | bool | false | The `children` will be inside the DOM hierarchy of the parent component. | diff --git a/docs/src/pages/components/dialogs/ConfirmationDialog.js b/docs/src/pages/components/dialogs/ConfirmationDialog.js index 2617fd83515045..eefeb30d114652 100644 --- a/docs/src/pages/components/dialogs/ConfirmationDialog.js +++ b/docs/src/pages/components/dialogs/ConfirmationDialog.js @@ -61,8 +61,6 @@ function ConfirmationDialogRaw(props) { return ( { - setOpen(false); + const handleClose = (event, reason) => { + if (reason !== 'backdropClick') { + setOpen(false); + } }; return (
- + Fill the form
diff --git a/docs/src/pages/components/selects/DialogSelect.tsx b/docs/src/pages/components/selects/DialogSelect.tsx index ff6354595d350d..34e93ff1f9d186 100644 --- a/docs/src/pages/components/selects/DialogSelect.tsx +++ b/docs/src/pages/components/selects/DialogSelect.tsx @@ -37,19 +37,19 @@ export default function DialogSelect() { setOpen(true); }; - const handleClose = () => { - setOpen(false); + const handleClose = ( + event: React.SyntheticEvent, + reason?: string, + ) => { + if (reason !== 'backdropClick') { + setOpen(false); + } }; return (
- + Fill the form diff --git a/docs/src/pages/guides/migration-v4/migration-v4.md b/docs/src/pages/guides/migration-v4/migration-v4.md index cc96b14d8e2f7a..f5614e2ba076ad 100644 --- a/docs/src/pages/guides/migration-v4/migration-v4.md +++ b/docs/src/pages/guides/migration-v4/migration-v4.md @@ -412,6 +412,21 @@ const classes = makeStyles(theme => ({ /> ``` +- Remove the `disableBackdropClick` prop because redundant. + Ignore close events from `onClose` when `reason === 'backdropClick'` instead. + + ```diff + { + + if (reason !== 'backdropClick') { + + onClose(event, reason); + + } + + }} + /> + ``` + - [withMobileDialog] Remove this higher-order component. The hook API allows a simpler and more flexible solution: ```diff @@ -589,6 +604,35 @@ const classes = makeStyles(theme => ({ ### Modal +- Remove the `disableBackdropClick` prop because redundant. + Ignore close events from `onClose` when `reason === 'backdropClick'` instead. + + ```diff + { + + if (reason !== 'backdropClick') { + + onClose(event, reason); + + } + + }} + /> + ``` + +- Remove the `onEscapeKeyDown` prop because redundant. + Use `onClose` with `reason === "escapeKeyDown"` instead. + + ```diff + { + + if (reason === 'escapeKeyDown') { + + handleEscapeKeyDown(event); + + } + + }} + /> + ``` + - Remove `onRendered` prop. Depending on your use case either use a [callback ref](https://reactjs.org/docs/refs-and-the-dom.html#callback-refs) on the child element or an effect hook in the child component. diff --git a/packages/material-ui/src/Dialog/Dialog.d.ts b/packages/material-ui/src/Dialog/Dialog.d.ts index 0dcc86133c4d71..a62e33b8f346d5 100644 --- a/packages/material-ui/src/Dialog/Dialog.d.ts +++ b/packages/material-ui/src/Dialog/Dialog.d.ts @@ -53,11 +53,6 @@ export interface DialogProps /** Styles applied to the `Paper` component if `fullScreen={true}`. */ paperFullScreen?: string; }; - /** - * If `true`, clicking the backdrop will not fire the `onClose` callback. - * @default false - */ - disableBackdropClick?: boolean; /** * If `true`, hitting escape will not fire the `onClose` callback. * @default false diff --git a/packages/material-ui/src/Dialog/Dialog.js b/packages/material-ui/src/Dialog/Dialog.js index b66998486b5be5..d8bc318746f95c 100644 --- a/packages/material-ui/src/Dialog/Dialog.js +++ b/packages/material-ui/src/Dialog/Dialog.js @@ -145,7 +145,6 @@ const Dialog = React.forwardRef(function Dialog(props, ref) { children, classes, className, - disableBackdropClick = false, disableEscapeKeyDown = false, fullScreen = false, fullWidth = false, @@ -182,7 +181,7 @@ const Dialog = React.forwardRef(function Dialog(props, ref) { onBackdropClick(event); } - if (!disableBackdropClick && onClose) { + if (onClose) { onClose(event, 'backdropClick'); } }; @@ -196,7 +195,6 @@ const Dialog = React.forwardRef(function Dialog(props, ref) { ...BackdropProps, }} closeAfterTransition - disableBackdropClick={disableBackdropClick} disableEscapeKeyDown={disableEscapeKeyDown} onClose={onClose} open={open} @@ -272,11 +270,6 @@ Dialog.propTypes = { * @ignore */ className: PropTypes.string, - /** - * If `true`, clicking the backdrop will not fire the `onClose` callback. - * @default false - */ - disableBackdropClick: PropTypes.bool, /** * If `true`, hitting escape will not fire the `onClose` callback. * @default false diff --git a/packages/material-ui/src/Dialog/Dialog.test.js b/packages/material-ui/src/Dialog/Dialog.test.js index 83586c8b72592c..ece169bddafb16 100644 --- a/packages/material-ui/src/Dialog/Dialog.test.js +++ b/packages/material-ui/src/Dialog/Dialog.test.js @@ -115,17 +115,26 @@ describe('', () => { }); it('can ignore backdrop click and Esc keydown', () => { + function DialogWithBackdropClickDisabled(props) { + const { onClose, ...other } = props; + function handleClose(event, reason) { + if (reason !== 'backdropClick') { + onClose(event, reason); + } + } + + return ; + } const onClose = spy(); const { getByRole } = render( - foo - , + , ); const dialog = getByRole('dialog'); expect(dialog).not.to.equal(null); diff --git a/packages/material-ui/src/Modal/Modal.d.ts b/packages/material-ui/src/Modal/Modal.d.ts index ac949c230851ba..487e682d073e1d 100644 --- a/packages/material-ui/src/Modal/Modal.d.ts +++ b/packages/material-ui/src/Modal/Modal.d.ts @@ -41,11 +41,6 @@ export interface ModalProps * @default false */ disableAutoFocus?: boolean; - /** - * If `true`, clicking the backdrop will not fire `onClose`. - * @default false - */ - disableBackdropClick?: boolean; /** * If `true`, the modal will not prevent focus from leaving the modal while open. * diff --git a/packages/material-ui/src/Modal/Modal.js b/packages/material-ui/src/Modal/Modal.js index 6f2b0c0290365e..62ddc9baee5a57 100644 --- a/packages/material-ui/src/Modal/Modal.js +++ b/packages/material-ui/src/Modal/Modal.js @@ -63,7 +63,6 @@ const Modal = React.forwardRef(function Modal(inProps, ref) { closeAfterTransition = false, container, disableAutoFocus = false, - disableBackdropClick = false, disableEnforceFocus = false, disableEscapeKeyDown = false, disablePortal = false, @@ -172,7 +171,7 @@ const Modal = React.forwardRef(function Modal(inProps, ref) { onBackdropClick(event); } - if (!disableBackdropClick && onClose) { + if (onClose) { onClose(event, 'backdropClick'); } }; @@ -296,11 +295,6 @@ Modal.propTypes = { * @default false */ disableAutoFocus: PropTypes.bool, - /** - * If `true`, clicking the backdrop will not fire `onClose`. - * @default false - */ - disableBackdropClick: PropTypes.bool, /** * If `true`, the modal will not prevent focus from leaving the modal while open. * diff --git a/packages/material-ui/src/Modal/Modal.test.js b/packages/material-ui/src/Modal/Modal.test.js index ed02a9f0a0eacd..d50cc5116a197e 100644 --- a/packages/material-ui/src/Modal/Modal.test.js +++ b/packages/material-ui/src/Modal/Modal.test.js @@ -161,16 +161,29 @@ describe('', () => { }); it('should let the user disable backdrop click triggering onClose', () => { + function ModalWithDisabledBackdropClick(props) { + const { onClose, ...other } = props; + function handleClose(event, reason) { + if (reason !== 'backdropClick') { + onClose(event, reason); + } + } + + return ( + +
+ + ); + } const onClose = spy(); const { getByTestId } = render( -
- , + , ); getByTestId('backdrop').click();