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

Nested Popovers not closing on click outside #3332

Closed
raduflp opened this issue Jun 26, 2024 · 4 comments · Fixed by #3362
Closed

Nested Popovers not closing on click outside #3332

raduflp opened this issue Jun 26, 2024 · 4 comments · Fixed by #3362
Assignees

Comments

@raduflp
Copy link

raduflp commented Jun 26, 2024

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v2.1.0

What browser are you using?

Chrome

Reproduction URL

https://codesandbox.io/p/devbox/confident-pasteur-ly35gk?file=%2Fsrc%2FApp.jsx%3A25%2C1

Describe your issue

In v.2.0.4 and before nested popups would close when clicking outside of the PopoverPanel
Starting with v2.1.0 a nested Popovers close only when clicking within the parent Popover or when focusing inputs outside of the popovers

@raduflp raduflp changed the title Nexted Popovers not closing on click outside Nested Popovers not closing on click outside Jun 26, 2024
@RobinMalfait RobinMalfait self-assigned this Jul 4, 2024
@RobinMalfait
Copy link
Member

This should be fixed by #3362, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.

@raduflp
Copy link
Author

raduflp commented Jul 5, 2024

Thanks @RobinMalfait for looking into the issue.
I have updated the codesandbox to use the insiders build and part of the issue was resolved, but the behavior is still different from the pre v2.1.0 one, so I think this issue should be re-opened.

Using the insider build, after opening a parent popover and then a nested popover, on click outside will close the nested popover ✅, BUT will leave the parent popover open ⚠️.
In v.2.0.4 clicking outside closes both the parent and nested popovers.

In the updated sandbox you can see that clicking a button or focusing an input outside of popovers closes both the parent and nested popovers, and I think it's reasonable to expect a click outside to have the same effect, like in v.2.0.4

Let me know if anything is unclear and thanks again

https://codesandbox.io/p/devbox/confident-pasteur-ly35gk

@RobinMalfait
Copy link
Member

Hey @raduflp! This was actually incorrect behaviour in v2.0.4 which we improved in v2.1. The idea is that it closes one level of the stack at a time. However, the Popover also has some logic where if focus is moved outside of the Popover or PopoverGroup that everything closes.

Similarly, if you are using the keyboard, pressing esc also closes 1 level at a time instead of everything at once.

For example you can image where you have a form in the first Popover and some information in a nested Popover. Pressing esc or clicking outside would then also close the first Popover where you could loose data now if you didn't save the form.

This is also the case for other components, for example when you have nested Dialog components, and you use a Menu in one of the components then it will close the Menu first, then the nested Dialog, then the main Dialog and so on.

What's your use case for closing everything when clicking outside?

@raduflp
Copy link
Author

raduflp commented Jul 5, 2024

Our use case is a complex dashboard that lets the user drill down in a deeply nested tree structure.
The popover for a node shows some info and the subnodes. Clicking on a subnodes opens another popover and so on.
We don't have any form inputs in the popovers, so data loss is not a concern for us.

So if a user drills down down 3 levels deep, to return to level 2 he would click on the parent popover.
But if he would want to go back to the initial view, and maybe explore a part of the tree that is covered by any of the opened popovers, he would click anywhere on the dashboard and return to the initial state.
With the new behavior he now needs to click 3 times to achieve the same result, which is less than ideal.

The esc behavior closing one at a time I think makes total sense.
Closing the Dialogs one at a time also makes sense.
For Popovers the behavior also makes sense if you are using a Backdrop. I would expect that clicking a Backdrop of a Popover would close only that popover.
But if you are not using Backdrops, then having different behavior between clicking outside on a div vs a button I think has the potential to cause confusion.

I can totally see the use case when you would want to close on popover at a time, probably that should be the default behavior. But maybe there should be an escape hatch for when that behavior is not desirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants