diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dd30c49e..07a1d9065 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Allow to override the `type` on the `ComboboxInput` ([#1476](https://github.com/tailwindlabs/headlessui/pull/1476)) +- Ensure the the `` closes correctly ([#1477](https://github.com/tailwindlabs/headlessui/pull/1477)) ## [Unreleased - @headlessui/react] ### Fixed - Allow to override the `type` on the `Combobox.Input` ([#1476](https://github.com/tailwindlabs/headlessui/pull/1476)) +- Ensure the the `` closes correctly ([#1477](https://github.com/tailwindlabs/headlessui/pull/1477)) ## [@headlessui/vue@v1.6.2] - 2022-05-19 diff --git a/packages/@headlessui-react/src/components/popover/popover.test.tsx b/packages/@headlessui-react/src/components/popover/popover.test.tsx index 822033e73..e344a51e9 100644 --- a/packages/@headlessui-react/src/components/popover/popover.test.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.test.tsx @@ -1500,6 +1500,7 @@ describe('Keyboard interactions', () => { // Open the second popover await click(getByText('Trigger 2')) + getByText('Trigger 2')?.focus() // Ensure the second popover is open assertPopoverButton({ state: PopoverState.Visible }, getByText('Trigger 2')) @@ -2146,6 +2147,7 @@ describe('Mouse interactions', () => { // Open popover await click(getPopoverButton()) + getPopoverButton()?.focus() // Verify it is open assertPopoverButton({ state: PopoverState.Visible }) @@ -2269,4 +2271,62 @@ describe('Mouse interactions', () => { assertPopoverButton({ state: PopoverState.InvisibleHidden }) }) ) + + it( + 'should be possible to close the Popover by clicking on the Popover.Button outside the Popover.Panel', + suppressConsoleLogs(async () => { + render( + + Toggle + + + + + ) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Close the popover + await click(getPopoverButton()) + + // Verify it is closed + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + + // Verify the button is focused + assertActiveElement(getPopoverButton()) + }) + ) + + it( + 'should be possible to close the Popover by clicking on the Popover.Button outside the Popover.Panel (when using the `focus` prop)', + suppressConsoleLogs(async () => { + render( + + Toggle + + + + + ) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Close the popover + await click(getPopoverButton()) + + // Verify it is closed + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + + // Verify the button is focused + assertActiveElement(getPopoverButton()) + }) + ) }) diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 30bce4e06..e94e8b86e 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -1,7 +1,6 @@ import React, { createContext, createRef, - useCallback, useContext, useEffect, useMemo, @@ -15,6 +14,7 @@ import React, { ElementType, KeyboardEvent as ReactKeyboardEvent, MouseEvent as ReactMouseEvent, + FocusEvent as ReactFocusEvent, MutableRefObject, Ref, } from 'react' @@ -229,14 +229,14 @@ let PopoverRoot = forwardRefWithAs(function Popover< let groupContext = usePopoverGroupContext() let registerPopover = groupContext?.registerPopover - let isFocusWithinPopoverGroup = useCallback(() => { + let isFocusWithinPopoverGroup = useEvent(() => { return ( groupContext?.isFocusWithinPopoverGroup() ?? (ownerDocument?.activeElement && (button?.contains(ownerDocument.activeElement) || panel?.contains(ownerDocument.activeElement))) ) - }, [groupContext, button, panel]) + }) useEffect(() => registerPopover?.(registerBag), [registerPopover, registerBag]) @@ -269,22 +269,19 @@ let PopoverRoot = forwardRefWithAs(function Popover< } }) - let close = useCallback( - (focusableElement?: HTMLElement | MutableRefObject) => { - dispatch({ type: ActionTypes.ClosePopover }) + let close = useEvent((focusableElement?: HTMLElement | MutableRefObject) => { + dispatch({ type: ActionTypes.ClosePopover }) - let restoreElement = (() => { - if (!focusableElement) return button - if (focusableElement instanceof HTMLElement) return focusableElement - if (focusableElement.current instanceof HTMLElement) return focusableElement.current + let restoreElement = (() => { + if (!focusableElement) return button + if (focusableElement instanceof HTMLElement) return focusableElement + if (focusableElement.current instanceof HTMLElement) return focusableElement.current - return button - })() + return button + })() - restoreElement?.focus() - }, - [dispatch, button] - ) + restoreElement?.focus() + }) let api = useMemo>( () => ({ close, isPortalled }), @@ -359,96 +356,75 @@ let Button = forwardRefWithAs(function Button) => { - if (isWithinPanel) { - if (state.popoverState === PopoverStates.Closed) return - switch (event.key) { - case Keys.Space: - case Keys.Enter: - event.preventDefault() // Prevent triggering a *click* event - // @ts-expect-error - event.target.click?.() - dispatch({ type: ActionTypes.ClosePopover }) - state.button?.focus() // Re-focus the original opening Button - break - } - } else { - switch (event.key) { - case Keys.Space: - case Keys.Enter: - event.preventDefault() // Prevent triggering a *click* event - event.stopPropagation() - if (state.popoverState === PopoverStates.Closed) closeOthers?.(state.buttonId) - dispatch({ type: ActionTypes.TogglePopover }) - break - - case Keys.Escape: - if (state.popoverState !== PopoverStates.Open) return closeOthers?.(state.buttonId) - if (!internalButtonRef.current) return - if ( - ownerDocument?.activeElement && - !internalButtonRef.current.contains(ownerDocument.activeElement) - ) { - return - } - event.preventDefault() - event.stopPropagation() - dispatch({ type: ActionTypes.ClosePopover }) - break - } + let handleKeyDown = useEvent((event: ReactKeyboardEvent) => { + if (isWithinPanel) { + if (state.popoverState === PopoverStates.Closed) return + switch (event.key) { + case Keys.Space: + case Keys.Enter: + event.preventDefault() // Prevent triggering a *click* event + // @ts-expect-error + event.target.click?.() + dispatch({ type: ActionTypes.ClosePopover }) + state.button?.focus() // Re-focus the original opening Button + break } - }, - [ - dispatch, - state.popoverState, - state.buttonId, - state.button, - state.panel, - internalButtonRef, - closeOthers, - isWithinPanel, - ] - ) + } else { + switch (event.key) { + case Keys.Space: + case Keys.Enter: + event.preventDefault() // Prevent triggering a *click* event + event.stopPropagation() + if (state.popoverState === PopoverStates.Closed) closeOthers?.(state.buttonId) + dispatch({ type: ActionTypes.TogglePopover }) + break - let handleKeyUp = useCallback( - (event: ReactKeyboardEvent) => { - if (isWithinPanel) return - if (event.key === Keys.Space) { - // Required for firefox, event.preventDefault() in handleKeyDown for - // the Space key doesn't cancel the handleKeyUp, which in turn - // triggers a *click*. - event.preventDefault() + case Keys.Escape: + if (state.popoverState !== PopoverStates.Open) return closeOthers?.(state.buttonId) + if (!internalButtonRef.current) return + if ( + ownerDocument?.activeElement && + !internalButtonRef.current.contains(ownerDocument.activeElement) + ) { + return + } + event.preventDefault() + event.stopPropagation() + dispatch({ type: ActionTypes.ClosePopover }) + break } - }, - [isWithinPanel] - ) + } + }) - let handleClick = useCallback( - (event: ReactMouseEvent) => { - if (isDisabledReactIssue7711(event.currentTarget)) return - if (props.disabled) return - if (isWithinPanel) { - dispatch({ type: ActionTypes.ClosePopover }) - state.button?.focus() // Re-focus the original opening Button - } else { - event.preventDefault() - event.stopPropagation() - if (state.popoverState === PopoverStates.Closed) closeOthers?.(state.buttonId) - state.button?.focus() - dispatch({ type: ActionTypes.TogglePopover }) - } - }, - [ - dispatch, - state.button, - state.popoverState, - state.buttonId, - props.disabled, - closeOthers, - isWithinPanel, - ] - ) + let handleKeyUp = useEvent((event: ReactKeyboardEvent) => { + if (isWithinPanel) return + if (event.key === Keys.Space) { + // Required for firefox, event.preventDefault() in handleKeyDown for + // the Space key doesn't cancel the handleKeyUp, which in turn + // triggers a *click*. + event.preventDefault() + } + }) + + let handleClick = useEvent((event: ReactMouseEvent) => { + if (isDisabledReactIssue7711(event.currentTarget)) return + if (props.disabled) return + if (isWithinPanel) { + dispatch({ type: ActionTypes.ClosePopover }) + state.button?.focus() // Re-focus the original opening Button + } else { + event.preventDefault() + event.stopPropagation() + if (state.popoverState === PopoverStates.Closed) closeOthers?.(state.buttonId) + dispatch({ type: ActionTypes.TogglePopover }) + state.button?.focus() + } + }) + + let handleMouseDown = useEvent((event: ReactMouseEvent) => { + event.preventDefault() + event.stopPropagation() + }) let visible = state.popoverState === PopoverStates.Open let slot = useMemo(() => ({ open: visible }), [visible]) @@ -471,6 +447,7 @@ let Button = forwardRefWithAs(function Button { - if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault() - dispatch({ type: ActionTypes.ClosePopover }) - }, - [dispatch] - ) + let handleClick = useEvent((event: ReactMouseEvent) => { + if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault() + dispatch({ type: ActionTypes.ClosePopover }) + }) let slot = useMemo( () => ({ open: popoverState === PopoverStates.Open }), @@ -619,27 +593,24 @@ let Panel = forwardRefWithAs(function Panel { - switch (event.key) { - case Keys.Escape: - if (state.popoverState !== PopoverStates.Open) return - if (!internalPanelRef.current) return - if ( - ownerDocument?.activeElement && - !internalPanelRef.current.contains(ownerDocument.activeElement) - ) { - return - } - event.preventDefault() - event.stopPropagation() - dispatch({ type: ActionTypes.ClosePopover }) - state.button?.focus() - break - } - }, - [state, internalPanelRef, dispatch] - ) + let handleKeyDown = useEvent((event: KeyboardEvent) => { + switch (event.key) { + case Keys.Escape: + if (state.popoverState !== PopoverStates.Open) return + if (!internalPanelRef.current) return + if ( + ownerDocument?.activeElement && + !internalPanelRef.current.contains(ownerDocument.activeElement) + ) { + return + } + event.preventDefault() + event.stopPropagation() + dispatch({ type: ActionTypes.ClosePopover }) + state.button?.focus() + break + } + }) // Unlink on "unmount" children useEffect(() => { @@ -662,39 +633,33 @@ let Panel = forwardRefWithAs(function Panel { - if (!focus) return - if (state.popoverState !== PopoverStates.Open) return - if (!internalPanelRef.current) return - - let activeElement = ownerDocument?.activeElement as HTMLElement - - if ( - activeElement && - (internalPanelRef.current?.contains(activeElement) || - state.beforePanelSentinel.current?.contains?.(activeElement) || - state.afterPanelSentinel.current?.contains?.(activeElement)) - ) { - return - } - - dispatch({ type: ActionTypes.ClosePopover }) - }, - true - ) - let slot = useMemo( () => ({ open: state.popoverState === PopoverStates.Open, close }), [state, close] ) + let ourProps = { ref: panelRef, id: state.panelId, onKeyDown: handleKeyDown, + onBlur: + focus && state.popoverState === PopoverStates.Open + ? (event: ReactFocusEvent) => { + let el = event.relatedTarget as HTMLElement + if (!el) return + if (!internalPanelRef.current) return + if (internalPanelRef.current?.contains(el)) return + + dispatch({ type: ActionTypes.ClosePopover }) + + if ( + state.beforePanelSentinel.current?.contains?.(el) || + state.afterPanelSentinel.current?.contains?.(el) + ) { + el.focus({ preventScroll: true }) + } + } + : undefined, tabIndex: -1, } @@ -815,30 +780,24 @@ let Group = forwardRefWithAs(function Group([]) - let unregisterPopover = useCallback( - (registerbag: PopoverRegisterBag) => { - setPopovers((existing) => { - let idx = existing.indexOf(registerbag) - if (idx !== -1) { - let clone = existing.slice() - clone.splice(idx, 1) - return clone - } - return existing - }) - }, - [setPopovers] - ) + let unregisterPopover = useEvent((registerbag: PopoverRegisterBag) => { + setPopovers((existing) => { + let idx = existing.indexOf(registerbag) + if (idx !== -1) { + let clone = existing.slice() + clone.splice(idx, 1) + return clone + } + return existing + }) + }) - let registerPopover = useCallback( - (registerbag: PopoverRegisterBag) => { - setPopovers((existing) => [...existing, registerbag]) - return () => unregisterPopover(registerbag) - }, - [setPopovers, unregisterPopover] - ) + let registerPopover = useEvent((registerbag: PopoverRegisterBag) => { + setPopovers((existing) => [...existing, registerbag]) + return () => unregisterPopover(registerbag) + }) - let isFocusWithinPopoverGroup = useCallback(() => { + let isFocusWithinPopoverGroup = useEvent(() => { let ownerDocument = getOwnerDocument(internalGroupRef) if (!ownerDocument) return false let element = ownerDocument.activeElement @@ -852,16 +811,13 @@ let Group = forwardRefWithAs(function Group { - for (let popover of popovers) { - if (popover.buttonId !== buttonId) popover.close() - } - }, - [popovers] - ) + let closeOthers = useEvent((buttonId: string) => { + for (let popover of popovers) { + if (popover.buttonId !== buttonId) popover.close() + } + }) let contextBag = useMemo>( () => ({ diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index f243fac49..784d021ca 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -241,17 +241,19 @@ export async function click( // Cancel in pointerDown cancels mouseDown, mouseUp let cancelled = !fireEvent.pointerDown(element, options) if (!cancelled) { - fireEvent.mouseDown(element, options) + cancelled = !fireEvent.mouseDown(element, options) } // Ensure to trigger a `focus` event if the element is focusable, or within a focusable element - let next: HTMLElement | null = element as HTMLElement | null - while (next !== null) { - if (next.matches(focusableSelector)) { - next.focus() - break + if (!cancelled) { + let next: HTMLElement | null = element as HTMLElement | null + while (next !== null) { + if (next.matches(focusableSelector)) { + next.focus() + break + } + next = next.parentElement } - next = next.parentElement } fireEvent.pointerUp(element, options) diff --git a/packages/@headlessui-vue/src/components/popover/popover.test.ts b/packages/@headlessui-vue/src/components/popover/popover.test.ts index 568639aee..a5fcfaa35 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.test.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.test.ts @@ -1610,6 +1610,7 @@ describe('Keyboard interactions', () => { // Open the second popover await click(getByText('Trigger 2')) + getByText('Trigger 2')?.focus() // Ensure the second popover is open assertPopoverButton({ state: PopoverState.Visible }, getByText('Trigger 2')) @@ -2297,6 +2298,7 @@ describe('Mouse interactions', () => { // Open popover await click(getPopoverButton()) + getPopoverButton()?.focus() // Verify it is open assertPopoverButton({ state: PopoverState.Visible }) @@ -2425,4 +2427,62 @@ describe('Mouse interactions', () => { assertPopoverButton({ state: PopoverState.InvisibleHidden }) }) ) + + it( + 'should be possible to close the Popover by clicking on the Popover.Button outside the Popover.Panel', + suppressConsoleLogs(async () => { + renderTemplate(html` + + Toggle + + + + + `) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Close the popover + await click(getPopoverButton()) + + // Verify it is closed + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + + // Verify the button is focused + assertActiveElement(getPopoverButton()) + }) + ) + + it( + 'should be possible to close the Popover by clicking on the Popover.Button outside the Popover.Panel (when using the `focus` prop)', + suppressConsoleLogs(async () => { + renderTemplate(html` + + Toggle + + + + + `) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Close the popover + await click(getPopoverButton()) + + // Verify it is closed + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + + // Verify the button is focused + assertActiveElement(getPopoverButton()) + }) + ) }) diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index 7a15f2ff5..fc9a26f21 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -331,11 +331,16 @@ export let PopoverButton = defineComponent({ event.preventDefault() event.stopPropagation() if (api.popoverState.value === PopoverStates.Closed) closeOthers?.(api.buttonId) - dom(api.button)?.focus() api.togglePopover() + dom(api.button)?.focus() } } + function handleMouseDown(event: MouseEvent) { + event.preventDefault() + event.stopPropagation() + } + return () => { let visible = api.popoverState.value === PopoverStates.Open let slot = { open: visible } @@ -358,6 +363,7 @@ export let PopoverButton = defineComponent({ onKeydown: handleKeyDown, onKeyup: handleKeyUp, onClick: handleClick, + onMousedown: handleMouseDown, } let direction = useTabDirection() @@ -485,31 +491,6 @@ export let PopoverPanel = defineComponent({ focusIn(dom(api.panel)!, Focus.First) }) - // Handle focus out when we are in special "focus" mode - useEventListener( - ownerDocument.value?.defaultView, - 'focus', - () => { - if (!focus) return - if (api.popoverState.value !== PopoverStates.Open) return - if (!dom(api.panel)) return - - let activeElement = ownerDocument?.value?.activeElement as HTMLElement - - if ( - activeElement && - (dom(api.panel)?.contains(activeElement) || - dom(api.beforePanelSentinel)?.contains?.(activeElement) || - dom(api.afterPanelSentinel)?.contains?.(activeElement)) - ) { - return - } - - api.closePopover() - }, - true - ) - let usesOpenClosedState = useOpenClosed() let visible = computed(() => { if (usesOpenClosedState !== null) { @@ -524,8 +505,9 @@ export let PopoverPanel = defineComponent({ case Keys.Escape: if (api.popoverState.value !== PopoverStates.Open) return if (!dom(api.panel)) return - if (ownerDocument.value && !dom(api.panel)?.contains(ownerDocument.value.activeElement)) + if (ownerDocument.value && !dom(api.panel)?.contains(ownerDocument.value.activeElement)) { return + } event.preventDefault() event.stopPropagation() api.closePopover() @@ -534,6 +516,22 @@ export let PopoverPanel = defineComponent({ } } + function handleBlur(event: MouseEvent) { + let el = event.relatedTarget as HTMLElement + if (!el) return + if (!dom(api.panel)) return + if (dom(api.panel)?.contains(el)) return + + api.closePopover() + + if ( + dom(api.beforePanelSentinel)?.contains?.(el) || + dom(api.afterPanelSentinel)?.contains?.(el) + ) { + el.focus({ preventScroll: true }) + } + } + let direction = useTabDirection() function handleBeforeFocus() { let el = dom(api.panel) as HTMLElement @@ -614,6 +612,7 @@ export let PopoverPanel = defineComponent({ ref: api.panel, id: api.panelId, onKeydown: handleKeyDown, + onFocusout: focus && api.popoverState.value === PopoverStates.Open ? handleBlur : undefined, tabIndex: -1, } diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index b003c3872..681208fa4 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -239,17 +239,19 @@ export async function click( // Cancel in pointerDown cancels mouseDown, mouseUp let cancelled = !fireEvent.pointerDown(element, options) if (!cancelled) { - fireEvent.mouseDown(element, options) + cancelled = !fireEvent.mouseDown(element, options) } // Ensure to trigger a `focus` event if the element is focusable, or within a focusable element - let next: HTMLElement | null = element as HTMLElement | null - while (next !== null) { - if (next.matches(focusableSelector)) { - next.focus() - break + if (!cancelled) { + let next: HTMLElement | null = element as HTMLElement | null + while (next !== null) { + if (next.matches(focusableSelector)) { + next.focus() + break + } + next = next.parentElement } - next = next.parentElement } fireEvent.pointerUp(element, options)