Skip to content

Commit d6471aa

Browse files
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
1 parent 1adea12 commit d6471aa

File tree

3 files changed

+35
-4
lines changed

3 files changed

+35
-4
lines changed

.changeset/eighty-houses-beg.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/react": patch
3+
---
4+
5+
fix(Dialog): track mousedown event to prevent accidental closing

packages/react/src/Dialog/Dialog.test.tsx

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react'
2-
import {render, waitFor} from '@testing-library/react'
2+
import {fireEvent, render, waitFor} from '@testing-library/react'
33
import userEvent from '@testing-library/user-event'
44
import {Dialog} from './Dialog'
55
import MatchMediaMock from 'jest-matchmedia-mock'
@@ -85,6 +85,24 @@ describe('Dialog', () => {
8585
expect(onClose).toHaveBeenCalledWith('escape')
8686
})
8787

88+
it('does not call `onClose` when click was not originated from backdrop', async () => {
89+
const onClose = jest.fn()
90+
91+
const {getByRole} = render(<Dialog onClose={onClose}>Pay attention to me</Dialog>)
92+
93+
expect(onClose).not.toHaveBeenCalled()
94+
95+
const dialog = getByRole('dialog')
96+
const backdrop = dialog.parentElement!
97+
98+
fireEvent.mouseDown(dialog)
99+
fireEvent.mouseUp(backdrop)
100+
// trigger the click on the backdrop, mouseUp doesn't do it for us
101+
fireEvent.click(backdrop)
102+
103+
expect(onClose).not.toHaveBeenCalled()
104+
})
105+
88106
it('calls `onClose` when keying "Escape"', async () => {
89107
const user = userEvent.setup()
90108
const onClose = jest.fn()

packages/react/src/Dialog/Dialog.tsx

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -419,14 +419,15 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
419419
footerButton.ref = autoFocusedFooterButtonRef
420420
}
421421
}
422+
const [lastMouseDownIsBackdrop, setLastMouseDownIsBackdrop] = useState<boolean>(false)
422423
const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId}
423424
const onBackdropClick = useCallback(
424425
(e: SyntheticEvent) => {
425-
if (e.target === e.currentTarget) {
426+
if (e.target === e.currentTarget && lastMouseDownIsBackdrop) {
426427
onClose('escape')
427428
}
428429
},
429-
[onClose],
430+
[onClose, lastMouseDownIsBackdrop],
430431
)
431432

432433
const dialogRef = useRef<HTMLDivElement>(null)
@@ -479,7 +480,14 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
479480
return (
480481
<>
481482
<Portal>
482-
<Backdrop ref={backdropRef} {...positionDataAttributes} onClick={onBackdropClick}>
483+
<Backdrop
484+
ref={backdropRef}
485+
{...positionDataAttributes}
486+
onClick={onBackdropClick}
487+
onMouseDown={e => {
488+
setLastMouseDownIsBackdrop(e.target === e.currentTarget)
489+
}}
490+
>
483491
<StyledDialog
484492
width={width}
485493
height={height}

0 commit comments

Comments
 (0)