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

Dialog does not mark other elements inert #2289

Closed
kabalage opened this issue Feb 16, 2023 · 4 comments · Fixed by #2290
Closed

Dialog does not mark other elements inert #2289

kabalage opened this issue Feb 16, 2023 · 4 comments · Fixed by #2290
Assignees

Comments

@kabalage
Copy link

What package within Headless UI are you using?

@headlessui/vue

What version of that package are you using?

For example: v1.7.10

What browser are you using?

Chrome

Reproduction URL

https://headlessui.com/examples/vue-dialog-hero
https://headlessui.com/examples/react-dialog-hero

Describe your issue

The documentation says:

All other application elements outside of the dialog will be marked as inert and thus not focusable.

I checked the code and if I understand it well, useInertOthers should mark the other elements in they body with inert and aria-hidden, but it does not happen.
Maybe I'm misunderstanding something, but I expected everything besides headlessui-portal-root to be marked. I experience the same in my project, but both the official vue and react examples behave the same.

@RobinMalfait
Copy link
Member

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

You are correct, we had a logical error that made it stop working. We also have some other rules that apply so I updated the logic to support.

Some history:
Initially we marked all other elements but the Dialog root as inert. This used to make sense because we should only be able to interact with the Dialog itself.

Some months later, it turned out that some users had issue with 3rd party libraries not working properly with the Dialog component. This is because a lot of libraries will create a "portal" to the document.body and inject a sibling element to render the contents in. From the 3rd party library's perspective this makes sense because this will guarantee that their styles and elements are applied on top of the existing code.

Rendering items as a sibling meant that an "outside click" would now be triggered the moment you interact with the Dialog. To solve this we introduced the concept of "allowed containers". This is an assumption, but currently we allow all elements rendered directly in the document.body as a valid container that you can interact with.

Now we have to be careful here, because solving the initial "mark others as inert" would still mean that we mark those "allowed containers" as inert which is not what we want because we do want to be able to interact with them.

The solution that is implemented now will only mark the main application root as inert. For example if you are using Next.js this will be the div#__next element.

Additionally, if you are using nested Dialog components, all the parent dialogs will be marked as inert as well.

Here you can see the main app root node (#__next) being marked as inert as well as all the parent dialogs:

image

That all being said. You probably won't see this being fixed on the Headless UI documentation itself. This is because we are actually cheating on those docs. We have a demo mode for the docs. The main reason for this is that we want to showcase each component being styled at the top of the page. This means that the Dialog will be open, Menu components will be open as well and so on. But doing this will cause focus traps and other accessibility features to be enabled and therefore you wouldn't be able to interact with the actual documentation. We had 2 solutions:

  1. Keep the components at the top in a "closed" state
  2. Add a demo mode that ignores some of the accessibility features so that you can still interact with the rest of the page as expected.

Hope this makes sense!


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

You can already try it using:

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

@kabalage
Copy link
Author

Thank you for your work and detailed description of the problem. 🙏
On the note about the demo mode: don't the iframes already solve the issue?

@RobinMalfait
Copy link
Member

Not 100%; For example when the Dialog opens the focus is moved into the Dialog. This means that if you click a link that goes to a specific heading in the documentation, then the Dialog would "steal" focus and the page would scroll up.

@kabalage
Copy link
Author

kabalage commented Feb 18, 2023

Oh, interesting! Thank you for clarifying it!

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