Skip to content

Commit

Permalink
fix dialog event propagation (#422)
Browse files Browse the repository at this point in the history
* re-export the `screen` utility for quick debugging purposes

* stop event propagation when clicking inside a Dialog

Fixes: #414
  • Loading branch information
RobinMalfait committed Apr 21, 2021
1 parent d4ffbee commit c8b297b
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 4 deletions.
70 changes: 70 additions & 0 deletions packages/@headlessui-react/src/components/dialog/dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -496,4 +496,74 @@ describe('Mouse interactions', () => {
assertActiveElement(getByText('Hello'))
})
)

it(
'should stop propagating click events when clicking on the Dialog.Overlay',
suppressConsoleLogs(async () => {
let wrapperFn = jest.fn()
function Example() {
let [isOpen, setIsOpen] = useState(true)
return (
<div onClick={wrapperFn}>
<Dialog open={isOpen} onClose={setIsOpen}>
Contents
<Dialog.Overlay />
<TabSentinel />
</Dialog>
</div>
)
}
render(<Example />)

// Verify it is open
assertDialog({ state: DialogState.Visible })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)

// Click the Dialog.Overlay to close the Dialog
await click(getDialogOverlay())

// Verify it is closed
assertDialog({ state: DialogState.InvisibleUnmounted })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)
})
)

it(
'should stop propagating click events when clicking on an element inside the Dialog',
suppressConsoleLogs(async () => {
let wrapperFn = jest.fn()
function Example() {
let [isOpen, setIsOpen] = useState(true)
return (
<div onClick={wrapperFn}>
<Dialog open={isOpen} onClose={setIsOpen}>
Contents
<button onClick={() => setIsOpen(false)}>Inside</button>
<TabSentinel />
</Dialog>
</div>
)
}
render(<Example />)

// Verify it is open
assertDialog({ state: DialogState.Visible })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)

// Click the button inside the the Dialog
await click(getByText('Inside'))

// Verify it is closed
assertDialog({ state: DialogState.InvisibleUnmounted })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)
})
)
})
16 changes: 15 additions & 1 deletion packages/@headlessui-react/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ let DEFAULT_DIALOG_TAG = 'div' as const
interface DialogRenderPropArg {
open: boolean
}
type DialogPropsWeControl = 'id' | 'role' | 'aria-modal' | 'aria-describedby' | 'aria-labelledby'
type DialogPropsWeControl =
| 'id'
| 'role'
| 'aria-modal'
| 'aria-describedby'
| 'aria-labelledby'
| 'onClick'

let DialogRenderFeatures = Features.RenderStrategy | Features.Static

Expand Down Expand Up @@ -176,6 +182,8 @@ let DialogRoot = forwardRefWithAs(function Dialog<
if (event.key !== Keys.Escape) return
if (dialogState !== DialogStates.Open) return
if (containers.current.size > 1) return // 1 is myself, otherwise other elements in the Stack
event.preventDefault()
event.stopPropagation()
close()
})

Expand Down Expand Up @@ -243,6 +251,10 @@ let DialogRoot = forwardRefWithAs(function Dialog<
'aria-modal': dialogState === DialogStates.Open ? true : undefined,
'aria-labelledby': state.titleId,
'aria-describedby': describedby,
onClick(event: ReactMouseEvent) {
event.preventDefault()
event.stopPropagation()
},
}
let passthroughProps = rest

Expand Down Expand Up @@ -302,6 +314,8 @@ let Overlay = forwardRefWithAs(function Overlay<
let handleClick = useCallback(
(event: ReactMouseEvent) => {
if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
event.preventDefault()
event.stopPropagation()
close()
},
[close]
Expand Down
86 changes: 86 additions & 0 deletions packages/@headlessui-vue/src/components/dialog/dialog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,4 +607,90 @@ describe('Mouse interactions', () => {
assertActiveElement(getByText('Hello'))
})
)

it(
'should stop propagating click events when clicking on the Dialog.Overlay',
suppressConsoleLogs(async () => {
let wrapperFn = jest.fn()
renderTemplate({
template: `
<div @click="wrapperFn">
<Dialog v-if="true" :open="isOpen" @close="setIsOpen">
Contents
<DialogOverlay />
<TabSentinel />
</Dialog>
</div>
`,
setup() {
let isOpen = ref(true)
return {
isOpen,
wrapperFn,
setIsOpen(value: boolean) {
isOpen.value = value
},
}
},
})

// Verify it is open
assertDialog({ state: DialogState.Visible })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)

// Click the Dialog.Overlay to close the Dialog
await click(getDialogOverlay())

// Verify it is closed
assertDialog({ state: DialogState.InvisibleUnmounted })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)
})
)

it(
'should stop propagating click events when clicking on an element inside the Dialog',
suppressConsoleLogs(async () => {
let wrapperFn = jest.fn()
renderTemplate({
template: `
<div @click="wrapperFn">
<Dialog v-if="true" :open="isOpen" @close="setIsOpen">
Contents
<button @click="setIsOpen(false)">Inside</button>
<TabSentinel />
</Dialog>
</div>
`,
setup() {
let isOpen = ref(true)
return {
isOpen,
wrapperFn,
setIsOpen(value: boolean) {
isOpen.value = value
},
}
},
})

// Verify it is open
assertDialog({ state: DialogState.Visible })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)

// Click the button inside the the Dialog
await click(getByText('Inside'))

// Verify it is closed
assertDialog({ state: DialogState.InvisibleUnmounted })

// Verify that the wrapper function has not been called yet
expect(wrapperFn).toHaveBeenCalledTimes(0)
})
)
})
11 changes: 10 additions & 1 deletion packages/@headlessui-vue/src/components/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export let Dialog = defineComponent({
'aria-modal': this.dialogState === DialogStates.Open ? true : undefined,
'aria-labelledby': this.titleId,
'aria-describedby': this.describedby,
onClick: this.handleClick,
}
let { open, initialFocus, ...passThroughProps } = this.$props
let slot = { open: this.dialogState === DialogStates.Open }
Expand Down Expand Up @@ -188,6 +189,8 @@ export let Dialog = defineComponent({
if (event.key !== Keys.Escape) return
if (dialogState.value !== DialogStates.Open) return
if (containers.value.size > 1) return // 1 is myself, otherwise other elements in the Stack
event.preventDefault()
event.stopPropagation()
api.close()
})

Expand Down Expand Up @@ -241,6 +244,10 @@ export let Dialog = defineComponent({
dialogState,
titleId,
describedby,
handleClick(event: MouseEvent) {
event.preventDefault()
event.stopPropagation()
},
}
},
})
Expand Down Expand Up @@ -276,7 +283,9 @@ export let DialogOverlay = defineComponent({

return {
id,
handleClick() {
handleClick(event: MouseEvent) {
event.preventDefault()
event.stopPropagation()
api.close()
},
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mount } from '@vue/test-utils'
import { logDOM, fireEvent } from '@testing-library/dom'
import { logDOM, fireEvent, screen } from '@testing-library/dom'

let mountedWrappers = new Set()

Expand Down Expand Up @@ -58,4 +58,4 @@ if (typeof afterEach === 'function') {
afterEach(() => cleanup())
}

export { fireEvent }
export { fireEvent, screen }

0 comments on commit c8b297b

Please sign in to comment.