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 portal root exists in the DOM #950

Merged
merged 2 commits into from
Nov 23, 2021
Merged

Conversation

RobinMalfait
Copy link
Collaborator

@RobinMalfait RobinMalfait commented Nov 23, 2021

When using NextJS, it happens that between page transitions the portal
root gets removed form the DOM. We will check the DOM when the target
updates, and if it doesn't exist anymore, then we will re-insert it in
the DOM.

When using NextJS, it happens that between page transitions the portal
root gets removed form the DOM. We will check the DOM when the `target`
updates, and if it doesn't exist anymore, then we will re-insert it in
the DOM.
@vercel
Copy link

vercel bot commented Nov 23, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

headlessui-react – ./packages/@headlessui-react

🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-react/7G73joPszhQjxkTS8J8PmrxgYRG3
✅ Preview: https://headlessui-react-git-ensure-portal-root-in-dom-tailwindlabs.vercel.app

headlessui-vue – ./packages/@headlessui-vue

🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-vue/BjMm5GUftFrEriCnrTKC51efWZw7
✅ Preview: https://headlessui-vue-git-ensure-portal-root-in-dom-tailwindlabs.vercel.app

@RobinMalfait RobinMalfait merged commit 8c57814 into develop Nov 23, 2021
@RobinMalfait RobinMalfait deleted the ensure-portal-root-in-dom branch November 23, 2021 12:45
RobinMalfait added a commit that referenced this pull request Jan 14, 2022
* placeholder for next release

* Ensure portal root exists in the DOM (#950)

* ensure that the portal root is always in the DOM

When using NextJS, it happens that between page transitions the portal
root gets removed form the DOM. We will check the DOM when the `target`
updates, and if it doesn't exist anymore, then we will re-insert it in
the DOM.

* update changelog

* Allow `Tabs` to be controllable (#970)

* feat(react): Allow Tab Component to be controlled

* fix falsy bug

`selectedIndex || defaultIndex` would result in the `defaultIndex` if
`selectedIndex` is set to 0. This means that if you have this code:

```js
<Tab.Group selectedIndex={0} defaultIndex={2} />
```

That you will never be able to see the very first tab, unless you
provided a negative value like `-1`.

`selectedIndex ?? defaultIndex` fixes this, since it purely checkes for
`undefined` and `null`.

* implemented controllable Tabs for Vue

* add dedicated test to ensure changing the defaultIndex has no effect

* update changelog

Co-authored-by: ChiefORZ <seb.schaffernak@gmail.com>

* Fix missing key binding in examples (#1036)

Co-authored-by: superDragon <xkloveme@gmail.com>

* Fix slice => splice typo in Vue Tabs component (#1037)

Co-authored-by: Ryan Gossiaux <ryan.gossiaux@gmail.com>

* update changelog

* Ensure correct DOM node order when performing focus actions (#1038)

* ensure that the order of DOM nodes is correct

When we are performing actions like `focusIn(list, Focus.First)` then we
have to ensrue that we are working with the correct list that is
properly sorted.

It can happen that the list of DOM nodes is out of sync. This can happen
if you have 3 Tabs, hide the second (which triggers an unmount and an
`unregister` of the Tab), then re-add the second item in the middle.
This will re-add the item to the end of the list instead of in the middle.

We can solve this by always sorting items when we are adding / removing
items, but this is a bit more error prone because it is easy to forget.
Instead we will sort it when performing the actual keyboard action.

If we didn't provide a list but an element, then we use a
getFocusableElements(element) function, but this already gives you a
correctly sorted list so we don't need to do that for this list.

* add tests to prove the correct order when performing actions

* cleanup code just for tests

It could still happen that this internal list is not ordered correctly
but that's not really a problem we just have the list to keep track of
things.

For our tests we now use the position from the DOM directly.

* update changelog

Co-authored-by: ChiefORZ <seb.schaffernak@gmail.com>
Co-authored-by: superDragon <xkloveme@gmail.com>
Co-authored-by: Ryan Gossiaux <ryan.gossiaux@gmail.com>
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.

None yet

1 participant