Robin Malfait
ad7300b076
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
97 lines
3.2 KiB
TypeScript
97 lines
3.2 KiB
TypeScript
import { Popover, Transition } from '@headlessui/react'
|
|
import React, { forwardRef } from 'react'
|
|
import { ExampleMenu } from '../menu/menu'
|
|
|
|
let Button = forwardRef(
|
|
(props: React.ComponentProps<'button'>, ref: React.MutableRefObject<HTMLButtonElement>) => {
|
|
return (
|
|
<Popover.Button
|
|
className="focus:outline-hidden border-2 border-transparent bg-gray-300 px-3 py-2 text-left focus:border-blue-900"
|
|
{...props}
|
|
ref={ref}
|
|
/>
|
|
)
|
|
}
|
|
)
|
|
|
|
export default function Home() {
|
|
let items = ['First', 'Second', 'Third', 'Fourth']
|
|
|
|
return (
|
|
<div className="flex items-center justify-center space-x-12 p-12">
|
|
<button>Previous</button>
|
|
|
|
<Popover.Group as="nav" aria-label="Mythical University" className="flex space-x-3">
|
|
<Popover as="div" className="relative">
|
|
<Transition
|
|
enter="transition ease-out duration-300 transform"
|
|
enterFrom="opacity-0"
|
|
enterTo="opacity-100"
|
|
leave="transition ease-in duration-300 transform"
|
|
leaveFrom="opacity-100"
|
|
leaveTo="opacity-0"
|
|
>
|
|
<Popover.Overlay className="fixed inset-0 z-20 bg-gray-500/75"></Popover.Overlay>
|
|
</Transition>
|
|
|
|
<Popover.Button className="focus:outline-hidden relative z-30 border-2 border-transparent bg-gray-300 px-3 py-2 focus:border-blue-900">
|
|
Normal
|
|
</Popover.Button>
|
|
<Popover.Panel className="absolute z-30 flex w-64 flex-col border-2 border-blue-900 bg-gray-100">
|
|
{items.map((item, i) => (
|
|
<Button key={item} hidden={i === 2}>
|
|
Normal - {item}
|
|
</Button>
|
|
))}
|
|
<div className="p-2">
|
|
<ExampleMenu />
|
|
</div>
|
|
</Popover.Panel>
|
|
</Popover>
|
|
|
|
<Popover as="div" className="relative">
|
|
<Button>Focus</Button>
|
|
<Popover.Panel
|
|
focus
|
|
className="absolute flex w-64 flex-col border-2 border-blue-900 bg-gray-100"
|
|
>
|
|
{items.map((item) => (
|
|
<Button key={item}>Focus - {item}</Button>
|
|
))}
|
|
</Popover.Panel>
|
|
</Popover>
|
|
|
|
<Popover as="div" className="relative">
|
|
<Button>Portal</Button>
|
|
<Popover.Panel
|
|
anchor="bottom start"
|
|
className="flex w-64 flex-col border-2 border-blue-900 bg-gray-100 [--anchor-gap:--spacing(1)]"
|
|
>
|
|
{items.map((item) => (
|
|
<Button key={item}>Portal - {item}</Button>
|
|
))}
|
|
<div className="p-2">
|
|
<ExampleMenu />
|
|
</div>
|
|
</Popover.Panel>
|
|
</Popover>
|
|
|
|
<Popover as="div" className="relative">
|
|
<Button>Focus in Portal</Button>
|
|
<Popover.Panel
|
|
focus
|
|
anchor="bottom start"
|
|
className="flex w-64 flex-col border-2 border-blue-900 bg-gray-100 [--anchor-gap:--spacing(1)]"
|
|
>
|
|
{items.map((item) => (
|
|
<Button key={item}>Focus in Portal - {item}</Button>
|
|
))}
|
|
</Popover.Panel>
|
|
</Popover>
|
|
</Popover.Group>
|
|
|
|
<button>Next</button>
|
|
</div>
|
|
)
|
|
}
|