diff --git a/packages/@headlessui-react/src/components/menu/menu.test.tsx b/packages/@headlessui-react/src/components/menu/menu.test.tsx index 92418c278b..2a11abf292 100644 --- a/packages/@headlessui-react/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.test.tsx @@ -35,12 +35,24 @@ function getMenuButton(): HTMLElement | null { return document.querySelector('[role="button"],button') } +function getMenuButtons(): HTMLElement[] { + // This is just an assumption for our tests. We assume that we only have 1 button. And if we have + // more, than we assume that it is the first one. + return Array.from(document.querySelectorAll('[role="button"],button')) +} + function getMenu(): HTMLElement | null { // This is just an assumption for our tests. We assume that our menu has this role and that it is // the first item in the DOM. return document.querySelector('[role="menu"]') } +function getMenus(): HTMLElement[] { + // This is just an assumption for our tests. We assume that our menu has this role and that it is + // the first item in the DOM. + return Array.from(document.querySelectorAll('[role="menu"]')) +} + function getMenuItems(): HTMLElement[] { // This is just an assumption for our tests. We assume that all menu items have this role. return Array.from(document.querySelectorAll('[role="menuitem"]')) @@ -2248,6 +2260,50 @@ describe('Mouse interactions', () => { }) ) + it( + 'should be possible to click outside of the menu on another menu button which should close the current menu and open the new menu', + suppressConsoleLogs(async () => { + render( +
+ + Trigger + + alice + bob + charlie + + + + + Trigger + + alice + bob + charlie + + +
+ ) + + const [button1, button2] = getMenuButtons() + + // Click the first menu button + await click(button1) + expect(getMenus()).toHaveLength(1) // Only 1 menu should be visible + + // Ensure the open menu is linked to the first button + assertMenuButtonLinkedWithMenu(button1, getMenu()) + + // Click the second menu button + await click(button2) + + expect(getMenus()).toHaveLength(1) // Only 1 menu should be visible + + // Ensure the open menu is linked to the second button + assertMenuButtonLinkedWithMenu(button2, getMenu()) + }) + ) + it( 'should be possible to hover an item and make it active', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 7530c50a0c..02c0bedb8c 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -47,7 +47,6 @@ type StateDefinition = { } enum ActionTypes { - ToggleMenu, OpenMenu, CloseMenu, @@ -114,7 +113,6 @@ function calculateActiveItemIndex( } type Actions = - | { type: ActionTypes.ToggleMenu } | { type: ActionTypes.CloseMenu } | { type: ActionTypes.OpenMenu } | { type: ActionTypes.GoToItem; focus: Focus; id?: string } @@ -129,13 +127,6 @@ const reducers: { action: Extract ) => StateDefinition } = { - [ActionTypes.ToggleMenu]: state => ({ - ...state, - menuState: match(state.menuState, { - [MenuStates.Open]: MenuStates.Closed, - [MenuStates.Closed]: MenuStates.Open, - }), - }), [ActionTypes.CloseMenu]: state => ({ ...state, menuState: MenuStates.Closed }), [ActionTypes.OpenMenu]: state => ({ ...state, menuState: MenuStates.Open }), [ActionTypes.GoToItem]: (state, action) => { @@ -237,17 +228,17 @@ export function Menu( React.useEffect(() => { function handler(event: PointerEvent) { - if (event.defaultPrevented) return if (menuState !== MenuStates.Open) return + if (buttonRef.current?.contains(event.target as HTMLElement)) return if (!itemsRef.current?.contains(event.target as HTMLElement)) { dispatch({ type: ActionTypes.CloseMenu }) - d.nextFrame(() => buttonRef.current?.focus()) + if (!event.defaultPrevented) buttonRef.current?.focus() } } - window.addEventListener('pointerdown', handler) - return () => window.removeEventListener('pointerdown', handler) + window.addEventListener('pointerup', handler) + return () => window.removeEventListener('pointerup', handler) }, [menuState, itemsRef, buttonRef, d, dispatch]) const propsBag = React.useMemo(() => ({ open: menuState === MenuStates.Open }), [menuState]) @@ -272,7 +263,6 @@ type ButtonPropsWeControl = | 'onFocus' | 'onBlur' | 'onPointerUp' - | 'onPointerDown' const DEFAULT_BUTTON_TAG = 'button' @@ -320,16 +310,18 @@ const Button = forwardRefWithAs(function Button< [dispatch, state, d] ) - const handlePointerDown = React.useCallback((event: React.PointerEvent) => { - // We have a `pointerdown` event listener in the menu for the 'outside click', so we just want - // to prevent going there if we happen to click this button. - event.preventDefault() - }, []) - - const handlePointerUp = React.useCallback(() => { - dispatch({ type: ActionTypes.ToggleMenu }) - d.nextFrame(() => state.itemsRef.current?.focus()) - }, [dispatch, d, state]) + const handlePointerUp = React.useCallback( + (event: MouseEvent) => { + if (state.menuState === MenuStates.Open) { + dispatch({ type: ActionTypes.CloseMenu }) + } else { + event.preventDefault() + dispatch({ type: ActionTypes.OpenMenu }) + d.nextFrame(() => state.itemsRef.current?.focus()) + } + }, + [dispatch, d, state] + ) const handleFocus = React.useCallback(() => { if (state.menuState === MenuStates.Open) state.itemsRef.current?.focus() @@ -354,7 +346,6 @@ const Button = forwardRefWithAs(function Button< onFocus: handleFocus, onBlur: handleBlur, onPointerUp: handlePointerUp, - onPointerDown: handlePointerDown, } return render({ ...passthroughProps, ...propsWeControl }, propsBag, DEFAULT_BUTTON_TAG) diff --git a/packages/@headlessui-vue/src/components/menu/menu.test.tsx b/packages/@headlessui-vue/src/components/menu/menu.test.tsx index a3cd55d97e..1aa7783dfa 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-vue/src/components/menu/menu.test.tsx @@ -50,12 +50,24 @@ function getMenuButton(): HTMLElement | null { return document.querySelector('button') } +function getMenuButtons(): HTMLElement[] { + // This is just an assumption for our tests. We assume that we only have 1 button. And if we have + // more, than we assume that it is the first one. + return Array.from(document.querySelectorAll('[role="button"],button')) +} + function getMenu(): HTMLElement | null { // This is just an assumption for our tests. We assume that our menu has this role and that it is // the first item in the DOM. return document.querySelector('[role="menu"]') } +function getMenus(): HTMLElement[] { + // This is just an assumption for our tests. We assume that our menu has this role and that it is + // the first item in the DOM. + return Array.from(document.querySelectorAll('[role="menu"]')) +} + function getMenuItems(): HTMLElement[] { // This is just an assumption for our tests. We assume that all menu items have this role. return Array.from(document.querySelectorAll('[role="menuitem"]')) @@ -2207,6 +2219,50 @@ describe('Mouse interactions', () => { assertMenu(getMenu(), { state: MenuState.Closed }) }) + it( + 'should be possible to click outside of the menu on another menu button which should close the current menu and open the new menu', + suppressConsoleLogs(async () => { + renderTemplate(` +
+ + Trigger + + alice + bob + charlie + + + + + Trigger + + alice + bob + charlie + + +
+ `) + + const [button1, button2] = getMenuButtons() + + // Click the first menu button + await click(button1) + expect(getMenus()).toHaveLength(1) // Only 1 menu should be visible + + // Ensure the open menu is linked to the first button + assertMenuButtonLinkedWithMenu(button1, getMenu()) + + // Click the second menu button + await click(button2) + + expect(getMenus()).toHaveLength(1) // Only 1 menu should be visible + + // Ensure the open menu is linked to the second button + assertMenuButtonLinkedWithMenu(button2, getMenu()) + }) + ) + it('should be possible to hover an item and make it active', async () => { renderTemplate(` diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts index beb94ef8ab..5d5aa8587c 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.ts +++ b/packages/@headlessui-vue/src/components/menu/menu.ts @@ -59,7 +59,6 @@ type StateDefinition = { activeItemIndex: Ref // State mutators - toggleMenu(): void closeMenu(): void openMenu(): void goToItem(focus: Focus, id?: string): void @@ -141,12 +140,6 @@ export const Menu = defineComponent({ items, searchQuery, activeItemIndex, - toggleMenu() { - menuState.value = match(menuState.value, { - [MenuStates.Closed]: MenuStates.Open, - [MenuStates.Open]: MenuStates.Closed, - }) - }, closeMenu: () => (menuState.value = MenuStates.Closed), openMenu: () => (menuState.value = MenuStates.Open), goToItem(focus: Focus, id?: string) { @@ -195,17 +188,17 @@ export const Menu = defineComponent({ onMounted(() => { function handler(event: PointerEvent) { - if (event.defaultPrevented) return if (menuState.value !== MenuStates.Open) return + if (buttonRef.value?.contains(event.target as HTMLElement)) return if (!itemsRef.value?.contains(event.target as HTMLElement)) { api.closeMenu() - nextTick(() => buttonRef.value?.focus()) + if (!event.defaultPrevented) buttonRef.value?.focus() } } - window.addEventListener('pointerdown', handler) - onUnmounted(() => window.removeEventListener('pointerdown', handler)) + window.addEventListener('pointerup', handler) + onUnmounted(() => window.removeEventListener('pointerup', handler)) }) // @ts-expect-error Types of property 'dataRef' are incompatible. @@ -234,7 +227,6 @@ export const MenuButton = defineComponent({ onKeyDown: this.handleKeyDown, onFocus: this.handleFocus, onPointerUp: this.handlePointerUp, - onPointerDown: this.handlePointerDown, } return render({ @@ -274,15 +266,14 @@ export const MenuButton = defineComponent({ } } - function handlePointerDown(event: PointerEvent) { - // We have a `pointerdown` event listener in the menu for the 'outside click', so we just want - // to prevent going there if we happen to click this button. - event.preventDefault() - } - - function handlePointerUp() { - api.toggleMenu() - nextTick(() => api.itemsRef.value?.focus()) + function handlePointerUp(event: MouseEvent) { + if (api.menuState.value === MenuStates.Open) { + api.closeMenu() + } else { + event.preventDefault() + api.openMenu() + nextTick(() => api.itemsRef.value?.focus()) + } } function handleFocus() { @@ -293,7 +284,6 @@ export const MenuButton = defineComponent({ id, el: api.buttonRef, handleKeyDown, - handlePointerDown, handlePointerUp, handleFocus, }