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

Unexpected keyboard focus behaviour on Popover with portalled panel #1839

Closed
lenzls opened this issue Sep 9, 2022 · 1 comment · Fixed by #1842
Closed

Unexpected keyboard focus behaviour on Popover with portalled panel #1839

lenzls opened this issue Sep 9, 2022 · 1 comment · Fixed by #1842
Assignees

Comments

@lenzls
Copy link

lenzls commented Sep 9, 2022

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.6.6

What browser are you using?

Tested in Firefox and Chromium

Reproduction URL
https://codepen.io/simondl/pen/ZEoWQBq?editors=1010

Describe your issue
I'm using the Popover component and the Popover.Panel is portalled to a different DOM node.

I expect that when the keyboard focus moves out of the Popover.Panel, it is placed back on the Popover.Panel. Instead, it is being placed on the sibling element of the Popover.Panel in the portalled location.

The linked codepen shows the same popover component in two different DOM structure. And shows a small reproduction recipe. Case (1) shows exactly my error case. Case (2) shows a different DOM structure, where the focus is handled to my expectation.

Potential cause
The focus-sentinels are only rendered when isPortalled is true (see popover.tsx#L753).

In my case, isPortalled is false. Therefore, the focus-sentinels are not rendered and there is no special focus handling.

The reason is, that the isPortalled check only returns true if button and panel are descendants of different 'root' elements.

    for (let root of document.querySelectorAll('body > *')) {
      if (Number(root?.contains(button)) ^ Number(root?.contains(panel))) {
        return true
      }
    }

taken from popover.tsx#L219

I believe this check is flawed. In my case, button and panel are part of the same 'root' element, but within that in very different locations.

I believe a better condition for isPortalled might be to check if Button and Panel are direct siblings. That way, the focus-sentinels would render whenever the native browser focus management would not suffice.

@RobinMalfait
Copy link
Member

Hey! Thank you for your bug report!
Much appreciated! 🙏

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

You can already try it using:

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

I updated your CodePen example with this insiders version and it behaves as expected: https://codepen.io/RobinMalfait/pen/WNJMoVa?editors=1010

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