Skip to content

Commit

Permalink
Fix implementation of #78
Browse files Browse the repository at this point in the history
Brain fart when writing and testing the first attempt.
I can only actually reproduce the underlying issue in the REPL so I haven't confirmed that this fixes it.
  • Loading branch information
rgossiaux committed Mar 7, 2022
1 parent b2a288c commit 39a8b57
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 34 deletions.
10 changes: 10 additions & 0 deletions src/lib/components/portal/Portal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { usePortalGroupContext } from "./PortalGroup.svelte";
import { usePortalRoot } from "$lib/internal/ForcePortalRootContext.svelte";
import { portal } from "$lib/hooks/use-portal";
import { tick } from "svelte";
let forceInRoot = usePortalRoot();
let groupTarget = usePortalGroupContext();
$: target = (() => {
Expand All @@ -20,6 +21,15 @@
let root = document.createElement("div");
root.setAttribute("id", "headlessui-portal-root");
// During initial render, the "portal" might be constructed before
// the root component has even attached, causing the portal to not work.
// This is a workaround for this issue--it can't guarantee the portal is
// **always** last, but it should catch the normal cases.
tick().then(() => {
if (root !== document.body.lastChild) {
document.body.appendChild(root);
}
});
return document.body.appendChild(root);
})();
</script>
Expand Down
23 changes: 0 additions & 23 deletions src/lib/components/portal/portal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,26 +334,3 @@ it('should cleanup the Portal properly when Svelte would not detach it', async (
expect(getPortalRoot()).not.toBe(null)
expect(getPortalRoot().childNodes).toHaveLength(1)
})

it('should move the Portal last during initial render', async () => {
expect(getPortalRoot()).toBe(null)

// We need to use a custom target because of the implementation of
// render() in the testing library
render(svelte`
<script>
let target = document.body.firstChild;
</script>
<PortalGroup {target}>
<Portal>Portal</Portal>
</PortalGroup>
<main>Main</main>
`)

await tick();

expect(document.body.innerHTML).toMatchInlineSnapshot(
`"<div> <main>Main</main><div>Portal</div></div>"`
)
})
11 changes: 0 additions & 11 deletions src/lib/hooks/use-portal.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,9 @@
import { tick } from "svelte";

export function portal(
element: HTMLElement,
target: HTMLElement | null | undefined
) {
if (target) {
target.append(element);
// During initial render, the "portal" might be constructed before
// the root component has even attached, causing the portal to not work.
// This is a workaround for this issue--it can't guarantee the portal is
// **always** last, but it should catch the normal cases.
tick().then(() => {
if (target && element !== target.lastChild) {
target.appendChild(element);
}
});
}
return {
update(newTarget: HTMLElement) {
Expand Down

0 comments on commit 39a8b57

Please sign in to comment.