From cf51be472e94dc07d738d0d47feb84fbff9266b7 Mon Sep 17 00:00:00 2001 From: Ryan Burr Date: Sat, 13 Apr 2024 16:50:45 -0400 Subject: [PATCH 1/4] fix(dialog): prevent onClick from being overwritten --- docs/pages/material-ui/api/dialog.json | 1 + docs/translations/api-docs/dialog/dialog.json | 1 + packages/mui-material/src/Dialog/Dialog.d.ts | 4 ++++ packages/mui-material/src/Dialog/Dialog.js | 9 ++++++++ .../mui-material/src/Dialog/Dialog.test.js | 22 +++++++++++++++++++ 5 files changed, 37 insertions(+) diff --git a/docs/pages/material-ui/api/dialog.json b/docs/pages/material-ui/api/dialog.json index 6fa7e61ced70c5..472d7fd2f9815d 100644 --- a/docs/pages/material-ui/api/dialog.json +++ b/docs/pages/material-ui/api/dialog.json @@ -26,6 +26,7 @@ "deprecated": true, "deprecationInfo": "Use the onClose prop with the reason argument to handle the backdropClick events." }, + "onClick": { "type": { "name": "func" } }, "onClose": { "type": { "name": "func" }, "signature": { diff --git a/docs/translations/api-docs/dialog/dialog.json b/docs/translations/api-docs/dialog/dialog.json index 6b38d43a40179c..9213ab3e1c508a 100644 --- a/docs/translations/api-docs/dialog/dialog.json +++ b/docs/translations/api-docs/dialog/dialog.json @@ -19,6 +19,7 @@ "description": "Determine the max-width of the dialog. The dialog width grows with the size of the screen. Set to false to disable maxWidth." }, "onBackdropClick": { "description": "Callback fired when the backdrop is clicked." }, + "onClick": { "description": "Callback fired when the dialog is clicked." }, "onClose": { "description": "Callback fired when the component requests to be closed.", "typeDescriptions": { diff --git a/packages/mui-material/src/Dialog/Dialog.d.ts b/packages/mui-material/src/Dialog/Dialog.d.ts index 14fdf0f2bc5c9d..c381fac09dc8ee 100644 --- a/packages/mui-material/src/Dialog/Dialog.d.ts +++ b/packages/mui-material/src/Dialog/Dialog.d.ts @@ -52,6 +52,10 @@ export interface DialogProps extends StandardProps { * @deprecated Use the `onClose` prop with the `reason` argument to handle the `backdropClick` events. */ onBackdropClick?: ModalProps['onBackdropClick']; + /** + * Callback fired when the dialog is clicked. + */ + onClick?: ModalProps['onClick']; /** * Callback fired when the component requests to be closed. * diff --git a/packages/mui-material/src/Dialog/Dialog.js b/packages/mui-material/src/Dialog/Dialog.js index 927c9b0f44e244..a12db9aa0995b1 100644 --- a/packages/mui-material/src/Dialog/Dialog.js +++ b/packages/mui-material/src/Dialog/Dialog.js @@ -182,6 +182,7 @@ const Dialog = React.forwardRef(function Dialog(inProps, ref) { fullWidth = false, maxWidth = 'sm', onBackdropClick, + onClick, onClose, open, PaperComponent = Paper, @@ -211,6 +212,10 @@ const Dialog = React.forwardRef(function Dialog(inProps, ref) { backdropClick.current = event.target === event.currentTarget; }; const handleBackdropClick = (event) => { + if (onClick) { + onClick(event); + } + // Ignore the events not coming from the "backdrop". if (!backdropClick.current) { return; @@ -360,6 +365,10 @@ Dialog.propTypes /* remove-proptypes */ = { * @deprecated Use the `onClose` prop with the `reason` argument to handle the `backdropClick` events. */ onBackdropClick: PropTypes.func, + /** + * Callback fired when the dialog is clicked. + */ + onClick: PropTypes.func, /** * Callback fired when the component requests to be closed. * diff --git a/packages/mui-material/src/Dialog/Dialog.test.js b/packages/mui-material/src/Dialog/Dialog.test.js index 7690dd907bf183..e17f7ac397d7de 100644 --- a/packages/mui-material/src/Dialog/Dialog.test.js +++ b/packages/mui-material/src/Dialog/Dialog.test.js @@ -187,6 +187,28 @@ describe('', () => { expect(onClose.callCount).to.equal(1); }); + it('calls onBackdropClick when onClick callback also exists', () => { + const onBackdropClick = spy(); + const onClick = spy(); + render( + { + if (reason === 'backdropClick') { + onBackdropClick(); + } + }} + open + > + foo + , + ); + + clickBackdrop(screen); + expect(onBackdropClick.callCount).to.equal(1); + expect(onClick.callCount).to.equal(1); + }); + it('should ignore the backdrop click if the event did not come from the backdrop', () => { const onBackdropClick = spy(); const { getByRole } = render( From c02e2c9316f263e170e4e3c723ca6dd2b1cc2a69 Mon Sep 17 00:00:00 2001 From: Ryan Burr Date: Sat, 13 Apr 2024 18:52:59 -0400 Subject: [PATCH 2/4] chore(dialog): ignore onClick comments Co-authored-by: Olivier Tassinari Signed-off-by: Ryan Burr --- packages/mui-material/src/Dialog/Dialog.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/Dialog/Dialog.d.ts b/packages/mui-material/src/Dialog/Dialog.d.ts index c381fac09dc8ee..d3a010514e9847 100644 --- a/packages/mui-material/src/Dialog/Dialog.d.ts +++ b/packages/mui-material/src/Dialog/Dialog.d.ts @@ -53,7 +53,7 @@ export interface DialogProps extends StandardProps { */ onBackdropClick?: ModalProps['onBackdropClick']; /** - * Callback fired when the dialog is clicked. + * @ignore */ onClick?: ModalProps['onClick']; /** From b47aa61925531493959de0ae4aaefe95c3a0693e Mon Sep 17 00:00:00 2001 From: Ryan Burr Date: Sat, 13 Apr 2024 19:57:43 -0400 Subject: [PATCH 3/4] docs(dialog): generate proper docs --- docs/pages/material-ui/api/dialog.json | 1 - docs/translations/api-docs/dialog/dialog.json | 1 - packages/mui-material/src/Dialog/Dialog.js | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/pages/material-ui/api/dialog.json b/docs/pages/material-ui/api/dialog.json index 472d7fd2f9815d..6fa7e61ced70c5 100644 --- a/docs/pages/material-ui/api/dialog.json +++ b/docs/pages/material-ui/api/dialog.json @@ -26,7 +26,6 @@ "deprecated": true, "deprecationInfo": "Use the onClose prop with the reason argument to handle the backdropClick events." }, - "onClick": { "type": { "name": "func" } }, "onClose": { "type": { "name": "func" }, "signature": { diff --git a/docs/translations/api-docs/dialog/dialog.json b/docs/translations/api-docs/dialog/dialog.json index 9213ab3e1c508a..6b38d43a40179c 100644 --- a/docs/translations/api-docs/dialog/dialog.json +++ b/docs/translations/api-docs/dialog/dialog.json @@ -19,7 +19,6 @@ "description": "Determine the max-width of the dialog. The dialog width grows with the size of the screen. Set to false to disable maxWidth." }, "onBackdropClick": { "description": "Callback fired when the backdrop is clicked." }, - "onClick": { "description": "Callback fired when the dialog is clicked." }, "onClose": { "description": "Callback fired when the component requests to be closed.", "typeDescriptions": { diff --git a/packages/mui-material/src/Dialog/Dialog.js b/packages/mui-material/src/Dialog/Dialog.js index a12db9aa0995b1..b88a9103d01389 100644 --- a/packages/mui-material/src/Dialog/Dialog.js +++ b/packages/mui-material/src/Dialog/Dialog.js @@ -366,7 +366,7 @@ Dialog.propTypes /* remove-proptypes */ = { */ onBackdropClick: PropTypes.func, /** - * Callback fired when the dialog is clicked. + * @ignore */ onClick: PropTypes.func, /** From d664b9fd1af69b0f5421ce55a5aae8cfe3083813 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Mon, 15 Apr 2024 20:24:53 +0530 Subject: [PATCH 4/4] remove onClick from Modal definition --- packages/mui-material/src/Dialog/Dialog.d.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/mui-material/src/Dialog/Dialog.d.ts b/packages/mui-material/src/Dialog/Dialog.d.ts index d3a010514e9847..14fdf0f2bc5c9d 100644 --- a/packages/mui-material/src/Dialog/Dialog.d.ts +++ b/packages/mui-material/src/Dialog/Dialog.d.ts @@ -52,10 +52,6 @@ export interface DialogProps extends StandardProps { * @deprecated Use the `onClose` prop with the `reason` argument to handle the `backdropClick` events. */ onBackdropClick?: ModalProps['onBackdropClick']; - /** - * @ignore - */ - onClick?: ModalProps['onClick']; /** * Callback fired when the component requests to be closed. *