Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust outside click handling #1667

Merged
merged 5 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Fixed SSR support on Deno ([#1671](https://github.com/tailwindlabs/headlessui/pull/1671))
- Don’t close dialog when opened during mouse up event ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Don’t close dialog when drag ends outside dialog ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Fix outside clicks to close dialog when nested, unopened dialogs are present ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))

## [1.6.6] - 2022-07-07

Expand Down
124 changes: 111 additions & 13 deletions packages/@headlessui-react/src/components/dialog/dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
getDialogs,
getDialogOverlays,
} from '../../test-utils/accessibility-assertions'
import { click, press, Keys } from '../../test-utils/interactions'
import { click, mouseDrag, press, Keys } from '../../test-utils/interactions'
import { PropsOf } from '../../types'
import { Transition } from '../transitions/transition'
import { createPortal } from 'react-dom'
Expand Down Expand Up @@ -1066,14 +1066,101 @@ describe('Mouse interactions', () => {
assertDialog({ state: DialogState.Visible })
})
)

it(
'should not close the dialog if opened during mouse up',
suppressConsoleLogs(async () => {
function Example() {
let [isOpen, setIsOpen] = useState(false)
return (
<>
<button id="trigger" onMouseUpCapture={() => setIsOpen((v) => !v)}>
Trigger
</button>
<Dialog open={isOpen} onClose={setIsOpen}>
<Dialog.Backdrop />
<Dialog.Panel>
<button id="inside">Inside</button>
<TabSentinel />
</Dialog.Panel>
</Dialog>
</>
)
}

render(<Example />)

await click(document.getElementById('trigger'))

assertDialog({ state: DialogState.Visible })

await click(document.getElementById('inside'))

assertDialog({ state: DialogState.Visible })
})
)

it(
'should not close the dialog if click starts inside the dialog but ends outside',
suppressConsoleLogs(async () => {
function Example() {
let [isOpen, setIsOpen] = useState(false)
return (
<>
<button id="trigger" onClick={() => setIsOpen((v) => !v)}>
Trigger
</button>
<div id="imoutside">this thing</div>
<Dialog open={isOpen} onClose={setIsOpen}>
<Dialog.Backdrop />
<Dialog.Panel>
<button id="inside">Inside</button>
<TabSentinel />
</Dialog.Panel>
</Dialog>
</>
)
}

render(<Example />)

// Open the dialog
await click(document.getElementById('trigger'))

assertDialog({ state: DialogState.Visible })

// Start a click inside the dialog and end it outside
await mouseDrag(document.getElementById('inside'), document.getElementById('imoutside'))

// It should not have hidden
assertDialog({ state: DialogState.Visible })

await click(document.getElementById('imoutside'))

// It's gone
assertDialog({ state: DialogState.InvisibleUnmounted })
})
)
})

describe('Nesting', () => {
function Nested({ onClose, level = 1 }: { onClose: (value: boolean) => void; level?: number }) {
type RenderStrategy = 'mounted' | 'always'

function Nested({
onClose,
open = true,
level = 1,
renderWhen = 'mounted',
}: {
onClose: (value: boolean) => void
open?: boolean
level?: number
renderWhen?: RenderStrategy
}) {
let [showChild, setShowChild] = useState(false)

return (
<Dialog open={true} onClose={onClose}>
<Dialog open={open} onClose={onClose}>
<Dialog.Overlay />

<div>
Expand All @@ -1082,31 +1169,42 @@ describe('Nesting', () => {
<button onClick={() => setShowChild(true)}>Open {level + 1} b</button>
<button onClick={() => setShowChild(true)}>Open {level + 1} c</button>
</div>
{showChild && <Nested onClose={setShowChild} level={level + 1} />}
{renderWhen === 'always' ? (
<Nested
open={showChild}
onClose={setShowChild}
level={level + 1}
renderWhen={renderWhen}
/>
) : (
showChild && <Nested open={true} onClose={setShowChild} level={level + 1} />
)}
</Dialog>
)
}

function Example() {
function Example({ renderWhen = 'mounted' }: { renderWhen: RenderStrategy }) {
let [open, setOpen] = useState(false)

return (
<>
<button onClick={() => setOpen(true)}>Open 1</button>
{open && <Nested onClose={setOpen} />}
{open && <Nested open={true} onClose={setOpen} renderWhen={renderWhen} />}
</>
)
}

it.each`
strategy | action
${'with `Escape`'} | ${() => press(Keys.Escape)}
${'with `Outside Click`'} | ${() => click(document.body)}
${'with `Click on Dialog.Overlay`'} | ${() => click(getDialogOverlays().pop()!)}
strategy | when | action
${'with `Escape`'} | ${'mounted'} | ${() => press(Keys.Escape)}
${'with `Outside Click`'} | ${'mounted'} | ${() => click(document.body)}
${'with `Click on Dialog.Overlay`'} | ${'mounted'} | ${() => click(getDialogOverlays().pop()!)}
${'with `Escape`'} | ${'always'} | ${() => press(Keys.Escape)}
${'with `Outside Click`'} | ${'always'} | ${() => click(document.body)}
`(
'should be possible to open nested Dialog components and close them $strategy',
async ({ action }) => {
render(<Example />)
'should be possible to open nested Dialog components (visible when $when) and close them $strategy',
async ({ when, action }) => {
render(<Example renderWhen={when} />)

// Verify we have no open dialogs
expect(getDialogs()).toHaveLength(0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ let DialogRoot = forwardRefWithAs(function Dialog<
return (
<StackProvider
type="Dialog"
enabled={dialogState === DialogStates.Open}
element={internalDialogRef}
onUpdate={useEvent((message, type, element) => {
if (type !== 'Dialog') return
Expand Down
24 changes: 23 additions & 1 deletion packages/@headlessui-react/src/hooks/use-outside-click.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,31 @@ export function useOutsideClick(
return cb(event, target)
}

let initialClickTarget = useRef<EventTarget | null>(null)

useWindowEvent(
'mousedown',
(event) => {
if (enabledRef.current) {
initialClickTarget.current = event.target
}
},
true
)

useWindowEvent(
'click',
(event) => handleOutsideClick(event, (event) => event.target as HTMLElement),
(event) => {
if (!initialClickTarget.current) {
return
}

handleOutsideClick(event, () => {
return initialClickTarget.current as HTMLElement
})

initialClickTarget.current = null
},

// We will use the `capture` phase so that layers in between with `event.stopPropagation()`
// don't "cancel" this outside click check. E.g.: A `Menu` inside a `DialogPanel` if the `Menu`
Expand Down
13 changes: 10 additions & 3 deletions packages/@headlessui-react/src/internal/stack-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ export function StackProvider({
onUpdate,
type,
element,
enabled,
}: {
children: ReactNode
onUpdate?: OnUpdate
type: string
element: MutableRefObject<HTMLElement | null>
enabled?: boolean
}) {
let parentUpdate = useStackContext()

Expand All @@ -49,9 +51,14 @@ export function StackProvider({
})

useIsoMorphicEffect(() => {
notify(StackMessage.Add, type, element)
return () => notify(StackMessage.Remove, type, element)
}, [notify, type, element])
let shouldNotify = enabled === undefined || enabled === true

shouldNotify && notify(StackMessage.Add, type, element)

return () => {
shouldNotify && notify(StackMessage.Remove, type, element)
}
}, [notify, type, element, enabled])

return <StackContext.Provider value={notify}>{children}</StackContext.Provider>
}
68 changes: 68 additions & 0 deletions packages/@headlessui-react/src/test-utils/interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,74 @@ export async function mouseLeave(element: Document | Element | Window | null) {
}
}

export async function mouseDrag(
startingElement: Document | Element | Window | Node | null,
endingElement: Document | Element | Window | Node | null
) {
let button = MouseButton.Left

try {
if (startingElement === null) return expect(startingElement).not.toBe(null)
if (endingElement === null) return expect(endingElement).not.toBe(null)
if (startingElement instanceof HTMLButtonElement && startingElement.disabled) return

let options = { button }

// Cancel in pointerDown cancels mouseDown, mouseUp
let cancelled = !fireEvent.pointerDown(startingElement, options)

if (!cancelled) {
cancelled = !fireEvent.mouseDown(startingElement, options)
}

// Ensure to trigger a `focus` event if the element is focusable, or within a focusable element
if (!cancelled) {
let next: HTMLElement | null = startingElement as HTMLElement | null
while (next !== null) {
if (next.matches(focusableSelector)) {
next.focus()
break
}
next = next.parentElement
}
}

fireEvent.pointerMove(startingElement, options)
if (!cancelled) {
fireEvent.mouseMove(startingElement, options)
}

fireEvent.pointerOut(startingElement, options)
if (!cancelled) {
fireEvent.mouseOut(startingElement, options)
}

// crosses over to the ending element

fireEvent.pointerOver(endingElement, options)
if (!cancelled) {
fireEvent.mouseOver(endingElement, options)
}

fireEvent.pointerMove(endingElement, options)
if (!cancelled) {
fireEvent.mouseMove(endingElement, options)
}

fireEvent.pointerUp(endingElement, options)
if (!cancelled) {
fireEvent.mouseUp(endingElement, options)
}

fireEvent.click(endingElement, options)

await new Promise(nextFrame)
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, click)
throw err
}
}

// ---

function focusNext(event: Partial<KeyboardEvent>) {
Expand Down
3 changes: 3 additions & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Fixed SSR support on Deno ([#1671](https://github.com/tailwindlabs/headlessui/pull/1671))
- Don’t close dialog when opened during mouse up event ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Don’t close dialog when drag ends outside dialog ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Fix outside clicks to close dialog when nested, unopened dialogs are present ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))

## [1.6.7] - 2022-07-12

Expand Down
Loading