From 110768638ee7a7d8b1749f02edab02b81cc134fd Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Mon, 6 May 2024 22:09:43 +0000 Subject: [PATCH 1/7] Trigger onClose when Dialog backdrop is clicked --- .changeset/popular-jokes-kiss.md | 5 +++++ .../ConfirmationDialog/ConfirmationDialog.tsx | 2 +- packages/react/src/Dialog/Dialog.test.tsx | 19 +++++++++++++++++-- packages/react/src/Dialog/Dialog.tsx | 18 +++++++++++++----- 4 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 .changeset/popular-jokes-kiss.md diff --git a/.changeset/popular-jokes-kiss.md b/.changeset/popular-jokes-kiss.md new file mode 100644 index 00000000000..cb5f5102995 --- /dev/null +++ b/.changeset/popular-jokes-kiss.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +`Dialog` and `ConfirmationDialog` can now be closed by clicking on the backdrop surrounding the dialog. This will cause `onClose` to be called with a new `'backdrop'` gesture. diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx index 73311e348a5..9a5a8a19df7 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx @@ -19,7 +19,7 @@ export interface ConfirmationDialogProps { * Required. This callback is invoked when a gesture to close the dialog * is performed. The first argument indicates the gesture. */ - onClose: (gesture: 'confirm' | 'close-button' | 'cancel' | 'escape') => void + onClose: (gesture: 'confirm' | 'close-button' | 'backdrop' | 'cancel' | 'escape') => void /** * Required. The title of the ConfirmationDialog. This is usually a brief diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index 3c8088542e2..da4e38cf698 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -67,7 +67,22 @@ describe('Dialog', () => { await user.click(getByLabelText('Close')) - expect(onClose).toHaveBeenCalled() + expect(onClose).toHaveBeenCalledWith('close-button') + expect(onClose).toHaveBeenCalledTimes(1) // Ensure it's not called with a backdrop gesture as well + }) + + it('calls `onClose` when clicking the backdrop', async () => { + const user = userEvent.setup() + const onClose = jest.fn() + const {getByRole} = render(Pay attention to me) + + expect(onClose).not.toHaveBeenCalled() + + const dialog = getByRole('dialog') + const backdrop = dialog.parentElement! + await user.click(backdrop) + + expect(onClose).toHaveBeenCalledWith('backdrop') }) it('calls `onClose` when keying "Escape"', async () => { @@ -80,7 +95,7 @@ describe('Dialog', () => { await user.keyboard('{Escape}') - expect(onClose).toHaveBeenCalled() + expect(onClose).toHaveBeenCalledWith('escape') }) it('changes the style for `overflow` if it is not set to "hidden"', () => { diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 15460063739..f929194eb55 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -1,4 +1,4 @@ -import React, {useCallback, useEffect, useRef, useState} from 'react' +import React, {useCallback, useEffect, useRef, useState, type SyntheticEvent} from 'react' import styled from 'styled-components' import type {ButtonProps} from '../Button' import {Button} from '../Button' @@ -98,11 +98,11 @@ export interface DialogProps extends SxProp { /** * This method is invoked when a gesture to close the dialog is used (either - * an Escape key press or clicking the "X" in the top-right corner). The + * an Escape key press, clicking the backdrop, or clicking the "X" in the top-right corner). The * gesture argument indicates the gesture that was used to close the dialog - * (either 'close-button' or 'escape'). + * ('close-button', 'backdrop', or 'escape'). */ - onClose: (gesture: 'close-button' | 'escape') => void + onClose: (gesture: 'close-button' | 'backdrop' | 'escape') => void /** * Default: "dialog". The ARIA role to assign to this dialog. @@ -414,6 +414,14 @@ const _Dialog = React.forwardRef { + if (e.target === e.currentTarget) { + onClose('backdrop') + } + }, + [onClose], + ) const dialogRef = useRef(null) useRefObjectAsForwardedRef(forwardedRef, dialogRef) @@ -465,7 +473,7 @@ const _Dialog = React.forwardRef - + Date: Mon, 6 May 2024 23:23:24 +0000 Subject: [PATCH 2/7] Update docs with new backdrop gesture --- docs/content/drafts/Dialog.mdx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/content/drafts/Dialog.mdx b/docs/content/drafts/Dialog.mdx index 791d871f6e8..baccf89d4a8 100644 --- a/docs/content/drafts/Dialog.mdx +++ b/docs/content/drafts/Dialog.mdx @@ -157,13 +157,13 @@ render() ### ConfirmationDialogProps -| Prop name | Type | Default | Description | -| :------------------- | :-------------------------------------------------------------------- | :--------- | :---------------------------------------------------------------------------------------------------------------------------- | -| title | `React.ReactNode` | | Required. Sets the title of the dialog, which by default is also used as the `aria-labelledby` attribute. | -| onClose | `(gesture: 'confirm' │ 'cancel' │ 'close-button' │ 'escape') => void` | | Required. This callback is invoked when a gesture to close the dialog is performed. The first argument indicates the gesture. | -| cancelButtonContent | `React.ReactNode` | `"Cancel"` | The content to use for the cancel button. | -| confirmButtonContent | `React.ReactNode` | `"OK"` | The content to use for the confirm button. | -| confirmButtonType | `"normal" │ "primary" │ "danger"` | `Button` | The type of button to use for the confirm button. | +| Prop name | Type | Default | Description | +| :------------------- | :----------------------------------------------- | :----------------------------- | :-------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | +| title | `React.ReactNode` | | Required. Sets the title of the dialog, which by default is also used as the `aria-labelledby` attribute. | +| onClose | `(gesture: 'confirm' │ 'cancel' │ 'close-button' | 'backdrop │ 'escape') => void` | | Required. This callback is invoked when a gesture to close the dialog is performed. The first argument indicates the gesture. | +| cancelButtonContent | `React.ReactNode` | `"Cancel"` | The content to use for the cancel button. | +| confirmButtonContent | `React.ReactNode` | `"OK"` | The content to use for the confirm button. | +| confirmButtonType | `"normal" │ "primary" │ "danger"` | `Button` | The type of button to use for the confirm button. | ### ConfirmOptions From be47622d946da0871e5448129fc2cee789790849 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Jun 2024 10:21:45 -0500 Subject: [PATCH 3/7] refactor: re-use escape gesture for backdrop click --- docs/content/drafts/Dialog.mdx | 14 +++++++------- .../src/ConfirmationDialog/ConfirmationDialog.tsx | 2 +- packages/react/src/Dialog/Dialog.tsx | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/content/drafts/Dialog.mdx b/docs/content/drafts/Dialog.mdx index baccf89d4a8..791d871f6e8 100644 --- a/docs/content/drafts/Dialog.mdx +++ b/docs/content/drafts/Dialog.mdx @@ -157,13 +157,13 @@ render() ### ConfirmationDialogProps -| Prop name | Type | Default | Description | -| :------------------- | :----------------------------------------------- | :----------------------------- | :-------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | -| title | `React.ReactNode` | | Required. Sets the title of the dialog, which by default is also used as the `aria-labelledby` attribute. | -| onClose | `(gesture: 'confirm' │ 'cancel' │ 'close-button' | 'backdrop │ 'escape') => void` | | Required. This callback is invoked when a gesture to close the dialog is performed. The first argument indicates the gesture. | -| cancelButtonContent | `React.ReactNode` | `"Cancel"` | The content to use for the cancel button. | -| confirmButtonContent | `React.ReactNode` | `"OK"` | The content to use for the confirm button. | -| confirmButtonType | `"normal" │ "primary" │ "danger"` | `Button` | The type of button to use for the confirm button. | +| Prop name | Type | Default | Description | +| :------------------- | :-------------------------------------------------------------------- | :--------- | :---------------------------------------------------------------------------------------------------------------------------- | +| title | `React.ReactNode` | | Required. Sets the title of the dialog, which by default is also used as the `aria-labelledby` attribute. | +| onClose | `(gesture: 'confirm' │ 'cancel' │ 'close-button' │ 'escape') => void` | | Required. This callback is invoked when a gesture to close the dialog is performed. The first argument indicates the gesture. | +| cancelButtonContent | `React.ReactNode` | `"Cancel"` | The content to use for the cancel button. | +| confirmButtonContent | `React.ReactNode` | `"OK"` | The content to use for the confirm button. | +| confirmButtonType | `"normal" │ "primary" │ "danger"` | `Button` | The type of button to use for the confirm button. | ### ConfirmOptions diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx index 9a5a8a19df7..73311e348a5 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx @@ -19,7 +19,7 @@ export interface ConfirmationDialogProps { * Required. This callback is invoked when a gesture to close the dialog * is performed. The first argument indicates the gesture. */ - onClose: (gesture: 'confirm' | 'close-button' | 'backdrop' | 'cancel' | 'escape') => void + onClose: (gesture: 'confirm' | 'close-button' | 'cancel' | 'escape') => void /** * Required. The title of the ConfirmationDialog. This is usually a brief diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 6744210c1d3..d9a018b7372 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -102,7 +102,7 @@ export interface DialogProps extends SxProp { * gesture argument indicates the gesture that was used to close the dialog * ('close-button', 'backdrop', or 'escape'). */ - onClose: (gesture: 'close-button' | 'backdrop' | 'escape') => void + onClose: (gesture: 'close-button' | 'escape') => void /** * Default: "dialog". The ARIA role to assign to this dialog. @@ -417,7 +417,7 @@ const _Dialog = React.forwardRef { if (e.target === e.currentTarget) { - onClose('backdrop') + onClose('escape') } }, [onClose], From 08ab86a687d353914edbc7d1748dabfe99833502 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Jun 2024 10:22:16 -0500 Subject: [PATCH 4/7] chore: update changeset --- .changeset/popular-jokes-kiss.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/popular-jokes-kiss.md b/.changeset/popular-jokes-kiss.md index cb5f5102995..5ceab42ea24 100644 --- a/.changeset/popular-jokes-kiss.md +++ b/.changeset/popular-jokes-kiss.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -`Dialog` and `ConfirmationDialog` can now be closed by clicking on the backdrop surrounding the dialog. This will cause `onClose` to be called with a new `'backdrop'` gesture. +`Dialog` and `ConfirmationDialog` can now be closed by clicking on the backdrop surrounding the dialog. This will cause `onClose` to be called with the `escape` gesture. From 2d3e1a6d58ef06f668d697d44a937923ea44ea16 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Jun 2024 10:22:50 -0500 Subject: [PATCH 5/7] docs: update jsdoc for handler --- packages/react/src/Dialog/Dialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index d9a018b7372..38bfc0d14eb 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -100,7 +100,7 @@ export interface DialogProps extends SxProp { * This method is invoked when a gesture to close the dialog is used (either * an Escape key press, clicking the backdrop, or clicking the "X" in the top-right corner). The * gesture argument indicates the gesture that was used to close the dialog - * ('close-button', 'backdrop', or 'escape'). + * ('close-button' or 'escape'). */ onClose: (gesture: 'close-button' | 'escape') => void From 036cffe099c68dc29c6984269e859d55aa5c3d93 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Jun 2024 10:30:14 -0500 Subject: [PATCH 6/7] chore: run format --- packages/react/src/Dialog/Dialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 38bfc0d14eb..40026326b58 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -102,7 +102,7 @@ export interface DialogProps extends SxProp { * gesture argument indicates the gesture that was used to close the dialog * ('close-button' or 'escape'). */ - onClose: (gesture: 'close-button' | 'escape') => void + onClose: (gesture: 'close-button' | 'escape') => void /** * Default: "dialog". The ARIA role to assign to this dialog. From 9c35602d24b97890419f51c5897102ed370562b4 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Jun 2024 12:25:45 -0500 Subject: [PATCH 7/7] test: update test to call for escape instead of backdrop --- packages/react/src/Dialog/Dialog.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index 4b99148fb46..f1d33d57d49 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -82,7 +82,7 @@ describe('Dialog', () => { const backdrop = dialog.parentElement! await user.click(backdrop) - expect(onClose).toHaveBeenCalledWith('backdrop') + expect(onClose).toHaveBeenCalledWith('escape') }) it('calls `onClose` when keying "Escape"', async () => {