From c5454783858818b4536426c279caab8a23b719a3 Mon Sep 17 00:00:00 2001 From: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:05:45 -0400 Subject: [PATCH] fix(Dialog): track mousedown event to prevent accidental closing (#4986) * fix(Dialog): track mousedown event to prevent accidental closing * Create eighty-houses-beg.md * test(Dialog): add test for accidental closure behavior * test(Dialog): remove unused var --- .changeset/eighty-houses-beg.md | 5 +++++ packages/react/src/Dialog/Dialog.test.tsx | 20 +++++++++++++++++++- packages/react/src/Dialog/Dialog.tsx | 14 +++++++++++--- 3 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 .changeset/eighty-houses-beg.md diff --git a/.changeset/eighty-houses-beg.md b/.changeset/eighty-houses-beg.md new file mode 100644 index 00000000000..f1e4e1ccd89 --- /dev/null +++ b/.changeset/eighty-houses-beg.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +fix(Dialog): track mousedown event to prevent accidental closing diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index c63823a28e9..85095fa0b20 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -1,5 +1,5 @@ import React from 'react' -import {render, waitFor} from '@testing-library/react' +import {fireEvent, render, waitFor} from '@testing-library/react' import userEvent from '@testing-library/user-event' import {Dialog} from './Dialog' import MatchMediaMock from 'jest-matchmedia-mock' @@ -85,6 +85,24 @@ describe('Dialog', () => { expect(onClose).toHaveBeenCalledWith('escape') }) + it('does not call `onClose` when click was not originated from backdrop', async () => { + const onClose = jest.fn() + + const {getByRole} = render(Pay attention to me) + + expect(onClose).not.toHaveBeenCalled() + + const dialog = getByRole('dialog') + const backdrop = dialog.parentElement! + + fireEvent.mouseDown(dialog) + fireEvent.mouseUp(backdrop) + // trigger the click on the backdrop, mouseUp doesn't do it for us + fireEvent.click(backdrop) + + expect(onClose).not.toHaveBeenCalled() + }) + it('calls `onClose` when keying "Escape"', async () => { const user = userEvent.setup() const onClose = jest.fn() diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index d251a38fca4..1f5b9276a2f 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -419,14 +419,15 @@ const _Dialog = React.forwardRef(false) const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId} const onBackdropClick = useCallback( (e: SyntheticEvent) => { - if (e.target === e.currentTarget) { + if (e.target === e.currentTarget && lastMouseDownIsBackdrop) { onClose('escape') } }, - [onClose], + [onClose, lastMouseDownIsBackdrop], ) const dialogRef = useRef(null) @@ -479,7 +480,14 @@ const _Dialog = React.forwardRef - + { + setLastMouseDownIsBackdrop(e.target === e.currentTarget) + }} + >