From 17241065fc97efd93f0da51a2fa4f2f189377e2f Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 26 Apr 2024 14:38:32 -0400 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20unmount=20portal=20targets=20us?= =?UTF-8?q?ed=20by=20other=20portals=20(#3144)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refactor * Don’t unmount portal targets used by other portals * Add test * Update changelog --- packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/portal/portal.test.ts | 45 +++++++++++++++++++ .../src/components/portal/portal.ts | 40 +++++++++++++++-- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index f854eac2a..02320fa8b 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Prevent closing the `Combobox` component when clicking inside the scrollbar area ([#3104](https://github.com/tailwindlabs/headlessui/pull/3104)) +- Don’t unmount portal targets used by other portals ([#3144](https://github.com/tailwindlabs/headlessui/pull/3144)) ## [1.7.20] - 2024-04-15 diff --git a/packages/@headlessui-vue/src/components/portal/portal.test.ts b/packages/@headlessui-vue/src/components/portal/portal.test.ts index 7615613eb..e333c72dd 100644 --- a/packages/@headlessui-vue/src/components/portal/portal.test.ts +++ b/packages/@headlessui-vue/src/components/portal/portal.test.ts @@ -391,3 +391,48 @@ it('should be possible to force the Portal into a specific element using PortalG `"
B
"` ) }) + +it('the root shared by multiple portals should not unmount when they change in the same tick', async () => { + let a = ref(false) + let b = ref(false) + + renderTemplate({ + template: html` +
+ Portal A + Portal B +
+ `, + setup: () => ({ a, b }), + }) + + await new Promise(nextTick) + + let root = () => document.querySelector('#headlessui-portal-root') + + // There is no portal root initially because there are no visible portals + expect(root()).toBe(null) + + // Show portal A + a.value = true + await new Promise(nextTick) + + // There is a portal root now because there is a visible portal + expect(root()).not.toBe(null) + + // Swap portal A for portal B + a.value = false + b.value = true + + await new Promise(nextTick) + + // The portal root is still there because there are still visible portals + expect(root()).not.toBe(null) + + // Hide portal B + b.value = false + await new Promise(nextTick) + + // The portal root is gone because there are no visible portals + expect(root()).toBe(null) +}) diff --git a/packages/@headlessui-vue/src/components/portal/portal.ts b/packages/@headlessui-vue/src/components/portal/portal.ts index e378f42be..dc09821fa 100644 --- a/packages/@headlessui-vue/src/components/portal/portal.ts +++ b/packages/@headlessui-vue/src/components/portal/portal.ts @@ -44,6 +44,27 @@ function getPortalRoot(contextElement?: Element | null) { return ownerDocument.body.appendChild(root) } +// A counter that keeps track of how many portals are currently using +// a specific portal root. This is used to know when we can safely +// remove the portal root from the DOM. +const counter = new WeakMap() + +function getCount(el: HTMLElement): number { + return counter.get(el) ?? 0 +} + +function setCount(el: HTMLElement, cb: (val: number) => number): number { + let newCount = cb(getCount(el)) + + if (newCount <= 0) { + counter.delete(el) + } else { + counter.set(el, newCount) + } + + return newCount +} + export let Portal = defineComponent({ name: 'Portal', props: { @@ -63,6 +84,11 @@ export let Portal = defineComponent({ : groupContext.resolveTarget() ) + // Make a note that we are using this element + if (myTarget.value) { + setCount(myTarget.value, (val) => val + 1) + } + let ready = ref(false) onMounted(() => { ready.value = true @@ -95,9 +121,17 @@ export let Portal = defineComponent({ if (!root) return if (myTarget.value !== root) return - if (myTarget.value.children.length <= 0) { - myTarget.value.parentElement?.removeChild(myTarget.value) - } + // We no longer need the portal target + let remaining = setCount(myTarget.value, (val) => val - 1) + + // However, if another portal is still using the same target + // we should not remove it. + if (remaining) return + + // There are still children in the portal, we should not remove it. + if (myTarget.value.children.length > 0) return + + myTarget.value.parentElement?.removeChild(myTarget.value) }) return () => {