From 37b033431e7eb059e4af70bc9f6c70cd9993ced8 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 20 Dec 2020 19:18:04 +0100 Subject: [PATCH] [Modal][Dialog] Deprecate duplicate props with onChange --- docs/pages/api-docs/dialog.md | 6 +-- docs/pages/api-docs/modal.md | 6 +-- .../components/dialogs/ConfirmationDialog.js | 2 - .../components/dialogs/ConfirmationDialog.tsx | 2 - .../pages/components/selects/DialogSelect.js | 2 +- .../pages/components/selects/DialogSelect.tsx | 2 +- packages/material-ui/src/Dialog/Dialog.d.ts | 3 ++ packages/material-ui/src/Dialog/Dialog.js | 24 ++++++++-- .../material-ui/src/Dialog/Dialog.test.js | 31 +++++-------- packages/material-ui/src/Modal/Modal.d.ts | 13 ++++++ packages/material-ui/src/Modal/Modal.js | 16 +++++-- packages/material-ui/src/Modal/Modal.test.js | 45 +++++-------------- 12 files changed, 79 insertions(+), 73 deletions(-) diff --git a/docs/pages/api-docs/dialog.md b/docs/pages/api-docs/dialog.md index 452f8c7bea5a03..cca63d59fc685e 100644 --- a/docs/pages/api-docs/dialog.md +++ b/docs/pages/api-docs/dialog.md @@ -32,17 +32,17 @@ 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. | +| ~~disableBackdropClick~~ | bool | false | *Deprecated*. Use the onClose prop with the `reason` argument to filter the `backdropClick` events.

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 will be full-screen | | fullWidth | bool | false | If `true`, the dialog stretches to `maxWidth`.
Notice that the dialog width grow is limited by the default margin. | | maxWidth | 'lg'
| 'md'
| 'sm'
| 'xl'
| 'xs'
| false | 'sm' | Determine the max-width of the dialog. The dialog width grows with the size of the screen. Set to `false` to disable `maxWidth`. | -| onBackdropClick | func | | Callback fired when the backdrop is clicked. | +| ~~onBackdropClick~~ | func | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `backdropClick` events.

Callback fired when the backdrop is clicked. | | onClose | func | | Callback fired when the component requests to be closed.

**Signature:**
`function(event: object, reason: string) => void`
*event:* The event source of the callback.
*reason:* Can be: `"escapeKeyDown"`, `"backdropClick"`. | | ~~onEnter~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired before the dialog enters. | | ~~onEntered~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the dialog has entered. | | ~~onEntering~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the dialog is entering. | -| onEscapeKeyDown | func | | Callback fired when the escape key is pressed, `disableKeyboard` is false and the modal is in focus. | +| ~~onEscapeKeyDown~~ | func | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.

Callback fired when the escape key is pressed, `disableKeyboard` is false and the modal is in focus. | | ~~onExit~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired before the dialog exits. | | ~~onExited~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the dialog has exited. | | ~~onExiting~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the dialog is exiting. | diff --git a/docs/pages/api-docs/modal.md b/docs/pages/api-docs/modal.md index aef58d703ac875..81e4ac317b7827 100644 --- a/docs/pages/api-docs/modal.md +++ b/docs/pages/api-docs/modal.md @@ -42,7 +42,7 @@ 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
| React.Component
| func | | A HTML element, 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. | | 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`. | +| ~~disableBackdropClick~~ | bool | false | *Deprecated*. Use the onClose prop with the `reason` argument to filter the `backdropClick` events.

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 | Disable the portal behavior. The children stay within it's parent DOM hierarchy. | @@ -50,9 +50,9 @@ This component shares many concepts with [react-overlays](https://react-bootstra | disableScrollLock | bool | false | Disable the scroll lock behavior. | | hideBackdrop | bool | false | If `true`, the backdrop is not rendered. | | keepMounted | bool | false | 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. | -| onBackdropClick | func | | Callback fired when the backdrop is clicked. | +| ~~onBackdropClick~~ | func | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `backdropClick` events.

Callback fired when the backdrop is clicked. | | onClose | func | | Callback fired when the component requests to be closed. The `reason` parameter can optionally be used to control the response to `onClose`.

**Signature:**
`function(event: object, reason: string) => void`
*event:* The event source of the callback.
*reason:* Can be: `"escapeKeyDown"`, `"backdropClick"`. | -| onEscapeKeyDown | func | | Callback fired when the escape key is pressed, `disableEscapeKeyDown` is false and the modal is in focus. | +| ~~onEscapeKeyDown~~ | func | | *Deprecated*. Use the onClose prop with the `reason` argument to handle the `escapeKeyDown` events.

Callback fired when the escape key is pressed, `disableEscapeKeyDown` is false and the modal is in focus. | | onRendered | func | | 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. | | open* | bool | | If `true`, the modal is open. | diff --git a/docs/src/pages/components/dialogs/ConfirmationDialog.js b/docs/src/pages/components/dialogs/ConfirmationDialog.js index 5c4bbe175faaa4..708bad05f5e4be 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 ( - + Fill the form
diff --git a/docs/src/pages/components/selects/DialogSelect.tsx b/docs/src/pages/components/selects/DialogSelect.tsx index 91bba98002c2a0..37d332dd2c0128 100644 --- a/docs/src/pages/components/selects/DialogSelect.tsx +++ b/docs/src/pages/components/selects/DialogSelect.tsx @@ -44,7 +44,7 @@ export default function DialogSelect() { return (
- + Fill the form diff --git a/packages/material-ui/src/Dialog/Dialog.d.ts b/packages/material-ui/src/Dialog/Dialog.d.ts index 20d1fc50ac19ee..f16f2b966b5c9f 100644 --- a/packages/material-ui/src/Dialog/Dialog.d.ts +++ b/packages/material-ui/src/Dialog/Dialog.d.ts @@ -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; /** @@ -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']; /** @@ -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']; /** diff --git a/packages/material-ui/src/Dialog/Dialog.js b/packages/material-ui/src/Dialog/Dialog.js index 24c89dcfaa3078..18a93a673d3515 100644 --- a/packages/material-ui/src/Dialog/Dialog.js +++ b/packages/material-ui/src/Dialog/Dialog.js @@ -208,7 +208,11 @@ const Dialog = React.forwardRef(function Dialog(props, ref) { ...BackdropProps, }} closeAfterTransition - disableBackdropClick={disableBackdropClick} + {...(disableBackdropClick + ? { + disableBackdropClick, + } + : {})} disableEscapeKeyDown={disableEscapeKeyDown} onEscapeKeyDown={onEscapeKeyDown} onClose={onClose} @@ -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. */ @@ -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. * @@ -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. diff --git a/packages/material-ui/src/Dialog/Dialog.test.js b/packages/material-ui/src/Dialog/Dialog.test.js index 6cad38e233fe54..6ad6249ad35afa 100644 --- a/packages/material-ui/src/Dialog/Dialog.test.js +++ b/packages/material-ui/src/Dialog/Dialog.test.js @@ -75,7 +75,6 @@ describe('', () => { }); 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); @@ -85,12 +84,7 @@ describe('', () => { }; return ( - + foo ); @@ -101,7 +95,6 @@ describe('', () => { 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); @@ -109,13 +102,13 @@ describe('', () => { }); it('can ignore backdrop click and Esc keydown', () => { - const onClose = spy(); + const onClose = []; const { getByRole } = render( { + onClose.push(reason); + }} transitionDuration={0} > foo @@ -126,10 +119,10 @@ describe('', () => { 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', () => { @@ -150,23 +143,21 @@ describe('', () => { }); it('calls onBackdropClick and onClose when clicked', () => { - const onBackdropClick = spy(); const onClose = spy(); render( - + foo , ); 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( - +

my dialog

@@ -174,7 +165,7 @@ describe('', () => { ); 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', () => { diff --git a/packages/material-ui/src/Modal/Modal.d.ts b/packages/material-ui/src/Modal/Modal.d.ts index 8d22a51e86e4be..ec708853197338 100644 --- a/packages/material-ui/src/Modal/Modal.d.ts +++ b/packages/material-ui/src/Modal/Modal.d.ts @@ -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; @@ -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. @@ -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; diff --git a/packages/material-ui/src/Modal/Modal.js b/packages/material-ui/src/Modal/Modal.js index a5e4a69be4b80c..260f11a3472199 100644 --- a/packages/material-ui/src/Modal/Modal.js +++ b/packages/material-ui/src/Modal/Modal.js @@ -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'; @@ -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. * @@ -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`. @@ -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. diff --git a/packages/material-ui/src/Modal/Modal.test.js b/packages/material-ui/src/Modal/Modal.test.js index 0a9efe8afd9558..946a9a87c8c887 100644 --- a/packages/material-ui/src/Modal/Modal.test.js +++ b/packages/material-ui/src/Modal/Modal.test.js @@ -158,12 +158,13 @@ describe('', () => { }); it('should let the user disable backdrop click triggering onClose', () => { - const onClose = spy(); + const onClose = []; const { getByTestId } = render( { + onClose.push(reason); + }} open - disableBackdropClick BackdropProps={{ 'data-testid': 'backdrop' }} >
@@ -172,20 +173,7 @@ describe('', () => { getByTestId('backdrop').click(); - expect(onClose).to.have.property('callCount', 0); - }); - - it('should call through to the user specified onBackdropClick callback', () => { - const onBackdropClick = spy(); - const { getByTestId } = render( - -
- , - ); - - getByTestId('backdrop').click(); - - expect(onBackdropClick).to.have.property('callCount', 1); + expect(onClose).to.deep.equal(['backdropClick']); }); it('should ignore the backdrop click if the event did not come from the backdrop', () => { @@ -196,16 +184,16 @@ describe('', () => {
); } - const onBackdropClick = spy(); + const onClose = spy(); const { getByTestId } = render( - +
, ); getByTestId('inner').click(); - expect(onBackdropClick).to.have.property('callCount', 0); + expect(onClose).to.have.property('callCount', 0); }); // Test case for https://github.com/mui-org/material-ui/issues/12831 @@ -247,10 +235,9 @@ describe('', () => { describe('handleKeyDown()', () => { it('when mounted, TopModal and event not esc should not call given functions', () => { - const onEscapeKeyDownSpy = spy(); const onCloseSpy = spy(); const { getByTestId } = render( - +
, ); @@ -260,17 +247,15 @@ describe('', () => { key: 'j', // Not escape }); - expect(onEscapeKeyDownSpy).to.have.property('callCount', 0); expect(onCloseSpy).to.have.property('callCount', 0); }); it('should call onEscapeKeyDown and onClose', () => { const handleKeyDown = spy(); - const onEscapeKeyDownSpy = spy(); const onCloseSpy = spy(); const { getByTestId } = render(
- +
, @@ -281,23 +266,16 @@ describe('', () => { key: 'Escape', }); - expect(onEscapeKeyDownSpy).to.have.property('callCount', 1); expect(onCloseSpy).to.have.property('callCount', 1); expect(handleKeyDown).to.have.property('callCount', 0); }); it('should not call onChange when `disableEscapeKeyDown=true`', () => { const handleKeyDown = spy(); - const onEscapeKeyDownSpy = spy(); const onCloseSpy = spy(); const { getByTestId } = render(
- +
, @@ -308,7 +286,6 @@ describe('', () => { key: 'Escape', }); - expect(onEscapeKeyDownSpy).to.have.property('callCount', 1); expect(onCloseSpy).to.have.property('callCount', 0); expect(handleKeyDown).to.have.property('callCount', 1); });