Skip to content

Commit

Permalink
fix(Dialog): track mousedown event to prevent accidental closing (#4986)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
francinelucca authored and TylerJDev committed Sep 23, 2024
1 parent 97286b8 commit c545478
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/eighty-houses-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

fix(Dialog): track mousedown event to prevent accidental closing
20 changes: 19 additions & 1 deletion packages/react/src/Dialog/Dialog.test.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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(<Dialog onClose={onClose}>Pay attention to me</Dialog>)

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()
Expand Down
14 changes: 11 additions & 3 deletions packages/react/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -419,14 +419,15 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
footerButton.ref = autoFocusedFooterButtonRef
}
}
const [lastMouseDownIsBackdrop, setLastMouseDownIsBackdrop] = useState<boolean>(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<HTMLDivElement>(null)
Expand Down Expand Up @@ -479,7 +480,14 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
return (
<>
<Portal>
<Backdrop ref={backdropRef} {...positionDataAttributes} onClick={onBackdropClick}>
<Backdrop
ref={backdropRef}
{...positionDataAttributes}
onClick={onBackdropClick}
onMouseDown={e => {
setLastMouseDownIsBackdrop(e.target === e.currentTarget)
}}
>
<StyledDialog
width={width}
height={height}
Expand Down

0 comments on commit c545478

Please sign in to comment.