Skip to content

Commit ad7300b

Browse files
authored
Fix closing Menu when other Menu is opened (#3726)
Fixes: #3701 This PR fixes an issue where an open `Menu` is not closed when opening a new `Menu`. This is also fixed for `Listbox` and `Combobox` that used the same techniques. This happened because we recently shipped an improvement where the `Menu` opens on `pointerdown` instead of on `click`. This means that the `useOutsideClick` hook was not correct anymore because it relies on `click`. We could try and figure out that we should already close on `pointerdown` but this might not be expected for other components. Instead we want to simplify things a bit and ideally not even worry about what event caused a specific state change. Instead of trying to fight timing issues when certain events happen, this PR takes a slightly different approach. We already had the concept of a "top-layer" similar to the browser's `#top-layer` (when using native `dialog`). This essentially lets us know which component sits on top of the hierarchy. This top-layer is important because when you have the following structure: ``` <Dialog> <Menu /> </Dialog> ``` Assuming that both the `Dialog` and `Menu` are open, clicking outside or pressing escape should _only_ close the `Menu`. Once the `Menu` is closed, we should close the `Dialog`. In this case, we can enable/disable the `useOutsideClick` hook based on whether the current component is the top-layer or not. Some components like the `Menu`, `Listbox` and `Combobox` should immediately close when they are not the top-layer anymore. A `Dialog` can stay open, because you can have interactable elements like the example above in the `Dialog`. Luckily, these components that should immediately close already use their own state machine. This allows us to listen to the `OpenMenu` (or `OpenListbox`, `OpenCombobox`) event, and if that happens, we can push the current component on the shared stack machine. This now means that it doesn't matter _how_ the `Menu` is opened, but the moment a user event (click, enter, ...) opens the `Menu`, we now that we are on top of the stack. All other components could listen to push events on the stack. Once those happen, we can close the current component immediately. This has the nice side effect that we don't have to use a `useEffect` to check for state changes. We can just act immediately when an event happens. The `useOutsideClick` hooks is still used and useful in situations where you literally just clicked somewhere else. But in case you are opening another `Menu` or another `Listbox`, we can immediately close the one that was open before. ## Test plan Before: https://github.com/user-attachments/assets/f2efd94b-9aa2-404c-ad54-c8747b4d46ac After: https://github.com/user-attachments/assets/25c78fc4-c1da-4e51-89b6-4270f2804ab0
1 parent afc04bc commit ad7300b

File tree

19 files changed

+272
-102
lines changed

19 files changed

+272
-102
lines changed

packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
- Fix focus not returned to SVG Element ([#3704](https://github.com/tailwindlabs/headlessui/pull/3704))
1919
- Fix `Listbox` not focusing first or last option on ArrowUp / ArrowDown ([#3721](https://github.com/tailwindlabs/headlessui/pull/3721))
2020
- Performance improvement: only re-render top-level component when nesting components e.g.: `Menu` inside a `Dialog` ([#3722](https://github.com/tailwindlabs/headlessui/pull/3722))
21+
- Fix closing `Menu` when other `Menu` is opened ([#3726](https://github.com/tailwindlabs/headlessui/pull/3726))
2122

2223
## [2.2.2] - 2025-04-17
2324

packages/@headlessui-react/src/components/combobox/combobox-machine-glue.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createContext, useContext, useMemo } from 'react'
2+
import { useOnUnmount } from '../../hooks/use-on-unmount'
23
import { ComboboxMachine } from './combobox-machine'
34

45
export const ComboboxContext = createContext<ComboboxMachine<unknown> | null>(null)
@@ -13,8 +14,11 @@ export function useComboboxMachineContext<T>(component: string) {
1314
}
1415

1516
export function useComboboxMachine({
17+
id,
1618
virtual = null,
1719
__demoMode = false,
18-
}: Parameters<typeof ComboboxMachine.new>[0] = {}) {
19-
return useMemo(() => ComboboxMachine.new({ virtual, __demoMode }), [])
20+
}: Parameters<typeof ComboboxMachine.new>[0]) {
21+
let machine = useMemo(() => ComboboxMachine.new({ id, virtual, __demoMode }), [])
22+
useOnUnmount(() => machine.dispose())
23+
return machine
2024
}

packages/@headlessui-react/src/components/combobox/combobox-machine.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Machine } from '../../machine'
2+
import { ActionTypes as StackActionTypes, stackMachines } from '../../machines/stack-machine'
23
import type { EnsureArray } from '../../types'
34
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
45
import { sortByDomNode } from '../../utils/focus-management'
@@ -32,6 +33,8 @@ export type ComboboxOptionDataRef<T> = MutableRefObject<{
3233
}>
3334

3435
export interface State<T> {
36+
id: string
37+
3538
dataRef: MutableRefObject<{
3639
value: unknown
3740
defaultValue: unknown
@@ -405,18 +408,21 @@ let reducers: {
405408

406409
export class ComboboxMachine<T> extends Machine<State<T>, Actions<T>> {
407410
static new<T, TMultiple extends boolean | undefined>({
411+
id,
408412
virtual = null,
409413
__demoMode = false,
410414
}: {
415+
id: string
411416
virtual?: {
412417
options: TMultiple extends true ? EnsureArray<NoInfer<T>> : NoInfer<T>[]
413418
disabled?: (
414419
value: TMultiple extends true ? EnsureArray<NoInfer<T>>[number] : NoInfer<T>
415420
) => boolean
416421
} | null
417422
__demoMode?: boolean
418-
} = {}) {
423+
}) {
419424
return new ComboboxMachine({
425+
id,
420426
// @ts-expect-error TODO: Re-structure such that we don't need to ignore this
421427
dataRef: { current: {} },
422428
comboboxState: __demoMode ? ComboboxState.Open : ComboboxState.Closed,
@@ -435,6 +441,31 @@ export class ComboboxMachine<T> extends Machine<State<T>, Actions<T>> {
435441
})
436442
}
437443

444+
constructor(initialState: State<T>) {
445+
super(initialState)
446+
447+
// When the combobox is open, and it's not on the top of the hierarchy, we
448+
// should close it again.
449+
{
450+
let id = this.state.id
451+
let stackMachine = stackMachines.get(null)
452+
453+
this.disposables.add(
454+
stackMachine.on(StackActionTypes.Push, (state) => {
455+
if (
456+
!stackMachine.selectors.isTop(state, id) &&
457+
this.state.comboboxState === ComboboxState.Open
458+
) {
459+
this.actions.closeCombobox()
460+
}
461+
})
462+
)
463+
464+
this.on(ActionTypes.OpenCombobox, () => stackMachine.actions.push(id))
465+
this.on(ActionTypes.CloseCombobox, () => stackMachine.actions.pop(id))
466+
}
467+
}
468+
438469
actions = {
439470
onChange: (newValue: T) => {
440471
let { onChange, compare, mode, value } = this.state.dataRef.current

packages/@headlessui-react/src/components/combobox/combobox.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import { FormFields } from '../../internal/form-fields'
5757
import { Frozen, useFrozenData } from '../../internal/frozen'
5858
import { useProvidedId } from '../../internal/id'
5959
import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed'
60+
import { stackMachines } from '../../machines/stack-machine'
6061
import { useSlice } from '../../react-glue'
6162
import type { EnsureArray, Props } from '../../types'
6263
import { history } from '../../utils/active-element-history'
@@ -288,6 +289,8 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
288289
props: ComboboxProps<TValue, boolean | undefined, TTag>,
289290
ref: Ref<HTMLElement>
290291
) {
292+
let id = useId()
293+
291294
let providedDisabled = useDisabled()
292295
let {
293296
value: controlledValue,
@@ -315,7 +318,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
315318
defaultValue
316319
)
317320

318-
let machine = useComboboxMachine({ virtual, __demoMode })
321+
let machine = useComboboxMachine({ id, virtual, __demoMode })
319322

320323
let optionsPropsRef = useRef<_Data['optionsPropsRef']['current']>({ static: false, hold: false })
321324

@@ -401,9 +404,14 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
401404
state.optionsElement,
402405
])
403406

407+
let stackMachine = stackMachines.get(null)
408+
let isTopLayer = useSlice(
409+
stackMachine,
410+
useCallback((state) => stackMachine.selectors.isTop(state, id), [stackMachine, id])
411+
)
412+
404413
// Handle outside click
405-
let outsideClickEnabled = comboboxState === ComboboxState.Open
406-
useOutsideClick(outsideClickEnabled, [buttonElement, inputElement, optionsElement], () =>
414+
useOutsideClick(isTopLayer, [buttonElement, inputElement, optionsElement], () =>
407415
machine.actions.closeCombobox()
408416
)
409417

@@ -1082,7 +1090,7 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
10821090
})
10831091

10841092
let handlePointerDown = useEvent((event: ReactPointerEvent<HTMLButtonElement>) => {
1085-
// We use the `poitnerdown` event here since it fires before the focus
1093+
// We use the `pointerdown` event here since it fires before the focus
10861094
// event, allowing us to cancel the event before focus is moved from the
10871095
// `ComboboxInput` to the `ComboboxButton`. This keeps the input focused,
10881096
// preserving the cursor position and any text selection.

packages/@headlessui-react/src/components/dialog/dialog.tsx

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import React, {
55
Fragment,
66
createContext,
77
createRef,
8+
useCallback,
89
useContext,
910
useEffect,
1011
useMemo,
@@ -22,6 +23,7 @@ import { useEvent } from '../../hooks/use-event'
2223
import { useId } from '../../hooks/use-id'
2324
import { useInertOthers } from '../../hooks/use-inert-others'
2425
import { useIsTouchDevice } from '../../hooks/use-is-touch-device'
26+
import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect'
2527
import { useOnDisappear } from '../../hooks/use-on-disappear'
2628
import { useOutsideClick } from '../../hooks/use-outside-click'
2729
import { useOwnerDocument } from '../../hooks/use-owner'
@@ -36,6 +38,8 @@ import { useSyncRefs } from '../../hooks/use-sync-refs'
3638
import { CloseProvider } from '../../internal/close-provider'
3739
import { ResetOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed'
3840
import { ForcePortalRoot } from '../../internal/portal-force-root'
41+
import { stackMachines } from '../../machines/stack-machine'
42+
import { useSlice } from '../../react-glue'
3943
import type { Props } from '../../types'
4044
import { match } from '../../utils/match'
4145
import {
@@ -212,14 +216,33 @@ let InternalDialog = forwardRefWithAs(function InternalDialog<
212216
]),
213217
})
214218

219+
// Ensure that the Dialog is the top layer when it is opened.
220+
//
221+
// In a perfect world this is pushed / popped when we open / close the Dialog
222+
// for within an event listener. But since the state is controlled by the
223+
// user, this is the next best thing to do.
224+
let stackMachine = stackMachines.get(null)
225+
useIsoMorphicEffect(() => {
226+
if (!enabled) return
227+
228+
stackMachine.actions.push(id)
229+
return () => stackMachine.actions.pop(id)
230+
}, [stackMachine, id, enabled])
231+
232+
// Check if the dialog is the current top layer
233+
let isTopLayer = useSlice(
234+
stackMachine,
235+
useCallback((state) => stackMachine.selectors.isTop(state, id), [stackMachine, id])
236+
)
237+
215238
// Close Dialog on outside click
216-
useOutsideClick(enabled, resolveRootContainers, (event) => {
239+
useOutsideClick(isTopLayer, resolveRootContainers, (event) => {
217240
event.preventDefault()
218241
close()
219242
})
220243

221244
// Handle `Escape` to close
222-
useEscape(enabled, ownerDocument?.defaultView, (event) => {
245+
useEscape(isTopLayer, ownerDocument?.defaultView, (event) => {
223246
event.preventDefault()
224247
event.stopPropagation()
225248

packages/@headlessui-react/src/components/listbox/listbox-machine-glue.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createContext, useContext, useMemo } from 'react'
2+
import { useOnUnmount } from '../../hooks/use-on-unmount'
23
import { ListboxMachine } from './listbox-machine'
34

45
export const ListboxContext = createContext<ListboxMachine<unknown> | null>(null)
@@ -12,6 +13,14 @@ export function useListboxMachineContext<T>(component: string) {
1213
return context as ListboxMachine<T>
1314
}
1415

15-
export function useListboxMachine({ __demoMode = false } = {}) {
16-
return useMemo(() => ListboxMachine.new({ __demoMode }), [])
16+
export function useListboxMachine({
17+
id,
18+
__demoMode = false,
19+
}: {
20+
id: string
21+
__demoMode?: boolean
22+
}) {
23+
let machine = useMemo(() => ListboxMachine.new({ id, __demoMode }), [])
24+
useOnUnmount(() => machine.dispose())
25+
return machine
1726
}

packages/@headlessui-react/src/components/listbox/listbox-machine.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Machine, batch } from '../../machine'
2+
import { ActionTypes as StackActionTypes, stackMachines } from '../../machines/stack-machine'
23
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
34
import { sortByDomNode } from '../../utils/focus-management'
45
import { match } from '../../utils/match'
@@ -30,6 +31,8 @@ type ListboxOptionDataRef<T> = MutableRefObject<{
3031
}>
3132

3233
interface State<T> {
34+
id: string
35+
3336
__demoMode: boolean
3437

3538
dataRef: MutableRefObject<{
@@ -394,8 +397,9 @@ let reducers: {
394397
}
395398

396399
export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
397-
static new({ __demoMode = false } = {}) {
400+
static new({ id, __demoMode = false }: { id: string; __demoMode?: boolean }) {
398401
return new ListboxMachine({
402+
id,
399403
// @ts-expect-error TODO: Re-structure such that we don't need to ignore this
400404
dataRef: { current: {} },
401405
listboxState: __demoMode ? ListboxStates.Open : ListboxStates.Closed,
@@ -422,6 +426,27 @@ export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
422426
this.send({ type: ActionTypes.SortOptions })
423427
})
424428
})
429+
430+
// When the listbox is open, and it's not on the top of the hierarchy, we
431+
// should close it again.
432+
{
433+
let id = this.state.id
434+
let stackMachine = stackMachines.get(null)
435+
436+
this.disposables.add(
437+
stackMachine.on(StackActionTypes.Push, (state) => {
438+
if (
439+
!stackMachine.selectors.isTop(state, id) &&
440+
this.state.listboxState === ListboxStates.Open
441+
) {
442+
this.actions.closeListbox()
443+
}
444+
})
445+
)
446+
447+
this.on(ActionTypes.OpenListbox, () => stackMachine.actions.push(id))
448+
this.on(ActionTypes.CloseListbox, () => stackMachine.actions.pop(id))
449+
}
425450
}
426451

427452
actions = {

packages/@headlessui-react/src/components/listbox/listbox.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import { FormFields } from '../../internal/form-fields'
5555
import { useFrozenData } from '../../internal/frozen'
5656
import { useProvidedId } from '../../internal/id'
5757
import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed'
58+
import { stackMachines } from '../../machines/stack-machine'
5859
import { useSlice } from '../../react-glue'
5960
import type { EnsureArray, Props } from '../../types'
6061
import { isDisabledReactIssue7711 } from '../../utils/bugs'
@@ -162,6 +163,8 @@ function ListboxFn<
162163
TType = string,
163164
TActualType = TType extends (infer U)[] ? U : TType,
164165
>(props: ListboxProps<TTag, TType, TActualType>, ref: Ref<HTMLElement>) {
166+
let id = useId()
167+
165168
let providedDisabled = useDisabled()
166169
let {
167170
value: controlledValue,
@@ -188,7 +191,7 @@ function ListboxFn<
188191
defaultValue
189192
)
190193

191-
let machine = useListboxMachine({ __demoMode })
194+
let machine = useListboxMachine({ id, __demoMode })
192195
let optionsPropsRef = useRef<_Data['optionsPropsRef']['current']>({ static: false, hold: false })
193196

194197
let listRef = useRef<_Data['listRef']['current']>(new Map())
@@ -241,13 +244,19 @@ function ListboxFn<
241244

242245
let listboxState = useSlice(machine, (state) => state.listboxState)
243246

244-
// Handle outside click
245-
let outsideClickEnabled = listboxState === ListboxStates.Open
247+
let stackMachine = stackMachines.get(null)
248+
let isTopLayer = useSlice(
249+
stackMachine,
250+
useCallback((state) => stackMachine.selectors.isTop(state, id), [stackMachine, id])
251+
)
252+
246253
let [buttonElement, optionsElement] = useSlice(machine, (state) => [
247254
state.buttonElement,
248255
state.optionsElement,
249256
])
250-
useOutsideClick(outsideClickEnabled, [buttonElement, optionsElement], (event, target) => {
257+
258+
// Handle outside click
259+
useOutsideClick(isTopLayer, [buttonElement, optionsElement], (event, target) => {
251260
machine.send({ type: ActionTypes.CloseListbox })
252261

253262
if (!isFocusableElement(target, FocusableMode.Loose)) {

packages/@headlessui-react/src/components/menu/menu-machine-glue.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createContext, useContext, useMemo } from 'react'
2+
import { useOnUnmount } from '../../hooks/use-on-unmount'
23
import { MenuMachine } from './menu-machine'
34

45
export const MenuContext = createContext<MenuMachine | null>(null)
@@ -12,6 +13,8 @@ export function useMenuMachineContext(component: string) {
1213
return context
1314
}
1415

15-
export function useMenuMachine({ __demoMode = false } = {}) {
16-
return useMemo(() => MenuMachine.new({ __demoMode }), [])
16+
export function useMenuMachine({ id, __demoMode = false }: { id: string; __demoMode?: boolean }) {
17+
let machine = useMemo(() => MenuMachine.new({ id, __demoMode }), [])
18+
useOnUnmount(() => machine.dispose())
19+
return machine
1720
}

0 commit comments

Comments
 (0)