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

Ensure the main tree and parent Dialog components are marked as inert #2290

Merged
merged 6 commits into from
Feb 17, 2023

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Feb 17, 2023

This PR fixes a bug where the logic for enabling the useInertOthers hook was incorrect. We replaced the useInertOthers with a simpler useInert hook. This is required because before we marked all other elements but the Dialog as inert. This is not correct because there will be elements that we don't have control over.

For example: 3rd party tools that inject a portal in the root. We will mark these elements as "valid" containers. Therefore all the direct children of the body are valid elements.

To solve this issue, we will make sure that we mark the main tree as inert. This is valid to do because the Dialog will be rendered in a Portal as a sibling of the main tree.

We also have to solve a similar issue for nested dialogs. We will mark all the parent dialogs as inert while keeping the top most dialog as non-inert.

image

Fixes: #2289

We want to apply `inert` when we _don't_ have nested dialogs, because if
we _do_ have nested dialogs, then the inert should be applied from the
nested dialog (or visually the top most dialog).
@vercel
Copy link

vercel bot commented Feb 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 3:46PM (UTC)
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 3:46PM (UTC)

@RobinMalfait RobinMalfait changed the title Ensure other elements are marked with inert when the Dialog is open Ensure the main tree and parent Dialog components are marked with inert Feb 17, 2023
@RobinMalfait RobinMalfait changed the title Ensure the main tree and parent Dialog components are marked with inert Ensure the main tree and parent Dialog components are marked as inert Feb 17, 2023
@RobinMalfait RobinMalfait merged commit 10efaa9 into main Feb 17, 2023
@RobinMalfait RobinMalfait deleted the fix/issue-2289 branch February 17, 2023 15:49
RobinMalfait added a commit that referenced this pull request Apr 24, 2024
We used to have a `useInertOthers` hook, but it also made everything
inside `document.body` inert. This means that third party packages or
browser extensions that inject something in the `document.body` were
also marked as `inert`. This is something we don't want.

We fixed that previously by introducing a simpler `useInert` where we
explicitly marked certain elements as inert: #2290

But I believe this new implementation is better, especially with this
commit where we stop once we hit `document.body`. This means that we
will never mark `body > *` elements as `inert`.
RobinMalfait added a commit that referenced this pull request Apr 24, 2024
…onents (#3124)

* move duplicated `useScrollLock` to dedicated hook

* accept `enabled` prop on `Portal` component

This way we can always use `<Portal>`, but enable / disable it
conditionally.

* use `useSyncRefs` in portal

This allows us to _not_ provide the ref is no ref was passed in.

* refactor inner workings of `useInert`

moved logic from the `useEffect`, to module scope. We will re-use this
logic in a future commit.

* add `useInertOthers` hook

Mark all elements on the page as inert, except for the ones that are allowed.

We move up the tree from the allowed elements, and mark all their
siblings as `inert`. If any of the children happens to be a parent of
one of the elements, then that child will not be marked as `inert`.

```
<body>                                    <!-- Stop at body -->
  <header></header>                       <!-- Inert, sibling of parent of allowed element -->
  <main>                                  <!-- Not inert, parent of allowed element -->
    <div>Sidebar</div>                    <!-- Inert, sibling of parent of allowed element -->
    <div>                                 <!-- Not inert, parent of allowed element -->
      <Listbox>                           <!-- Not inert, parent of allowed element -->
        <ListboxButton></ListboxButton>   <!-- Not inert, allowed element -->
        <ListboxOptions></ListboxOptions> <!-- Not inert, allowed element -->
      </Listbox>
    </div>
  </main>
  <footer></footer>                       <!-- Inert, sibling of parent of allowed element -->
</body>
```

* add `portal` prop, and change meaning of `modal` prop on `MenuItems`

- This adds a `portal` prop that renders the `MenuItems` in a portal.
  Defaults to `false`.
  - If you pass an `anchor` prop, the `portal` prop will always be set
    to `true`.
- The `modal` prop enables the following behavior:
  - Scroll locking is enabled when the `modal` prop is passed and the
    `Menu` is open.
  - Other elements but the `Menu` are marked as `inert`.

* add `portal` prop, and change meaning of `modal` prop on `ListboxOptions`

- This adds a `portal` prop that renders the `ListboxOptions` in a
  portal. Defaults to `false`.
  - If you pass an `anchor` prop, the `portal` prop will always be set
    to `true`.
- The `modal` prop enables the following behavior:
  - Scroll locking is enabled when the `modal` prop is passed and the
    `Listbox` is open.
  - Other elements but the `Listbox` are marked as `inert`.

* add `portal` and `modal` prop on `ComboboxOptions`

- This adds a `portal` prop that renders the `ComboboxOptions` in a
  portal. Defaults to `false`.
  - If you pass an `anchor` prop, the `portal` prop will always be set
    to `true`.
- The `modal` prop enables the following behavior:
  - Scroll locking is enabled when the `modal` prop is passed and the
    `Combobox` is open.
  - Other elements but the `Combobox` are marked as `inert`.

* add `portal` prop, and change meaning of `modal` prop on `PopoverPanel`

- This adds a `portal` prop that renders the `PopoverPanel` in a portal.
  Defaults to `false`.
  - If you pass an `anchor` prop, the `portal` prop will always be set
    to `true`.
- The `modal` prop enables the following behavior:
  - Scroll locking is enabled when the `modal` prop is passed and the
    `Panel` is open.

* simplify popover playground, use provided `anchor` prop

* remove internal `Modal` component

This is now implemented on a per component basis with some hooks.

* remove `Modal` handling from `Dialog`

The `Modal` component is removed, so there is no need to handle this in
the `Dialog`. It's also safe to remove because the components with
"portals" that are rendered inside the `Dialog` are portalled into the
`Dialog` and not as a sibling of the `Dialog`.

* ensure we use `groupTarget` if it is already available

Before this, we were waiting for a "next render" to mount the portal if
it was used inside a specific group. This happens when using `<Portal/>`
inside of a `<Dialog/>`.

* update changelog

* add tests for `useInertOthers`

* ensure we stop before the `body`

We used to have a `useInertOthers` hook, but it also made everything
inside `document.body` inert. This means that third party packages or
browser extensions that inject something in the `document.body` were
also marked as `inert`. This is something we don't want.

We fixed that previously by introducing a simpler `useInert` where we
explicitly marked certain elements as inert: #2290

But I believe this new implementation is better, especially with this
commit where we stop once we hit `document.body`. This means that we
will never mark `body > *` elements as `inert`.

* add `allowed` and `disallowed` to `useInertOthers`

This way we have a list of allowed and disallowed containers. The
`disallowed` elements will be marked as inert as-is.

The allowed elements will not be marked as `inert`, but it will mark its
children as inert. Then goes op the parent tree and repeats the process.

* simplify `useInertOthers` in `Dialog` code

* update `use-inert` tests to always use `useInertOthers`

* remove `useInert` hook in favor of `useInertOthers`

* rename `use-inert` to `use-inert-others`

* cleanup default values for `useInertOthers`
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 this pull request may close these issues.

Dialog does not mark other elements inert
1 participant