portal PortalContainer into <body> (#2185)

iTwin · Aug 7, 2024 · e0e967e · e0e967e
1 parent bc49fe7
commit e0e967e
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 25 deletions.
diff --git a/.changeset/afraid-fans-push.md b/.changeset/afraid-fans-push.md
@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+Portal containers will now default to a `<div>` rendered at the end of `<body>` instead of a `<div>` rendered inside the `<ThemeProvider>`.
diff --git a/apps/website/src/content/docs/popover.mdx b/apps/website/src/content/docs/popover.mdx
@@ -40,7 +40,7 @@ There are some advanced positioning options available.
 
 ### Portals
 
-It is important to know that before calculating the position, the popover gets [portaled](https://react.dev/reference/react-dom/createPortal) into the nearest `ThemeProvider` to avoid [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context) issues. This behavior can be controlled using the Popover's `portal` prop or the ThemeProvider's `portalContainer` prop. Using portals can often lead to issues with keyboard accessibility, so Popover adds some additional logic (described below).
+It is important to know that before calculating the position, the popover gets [portaled](https://react.dev/reference/react-dom/createPortal) to the end of `<body>` to avoid [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context) issues. This behavior can be controlled using the Popover's `portal` prop or the ThemeProvider's `portalContainer` prop. Using portals can often lead to issues with keyboard accessibility, so Popover adds some additional logic (described below).
 
 ## Accessibility
 

diff --git a/apps/website/src/content/docs/themeprovider.mdx b/apps/website/src/content/docs/themeprovider.mdx
@@ -108,7 +108,7 @@ const Application = () => {
 
 ## Portals
 
-By default, `ThemeProvider` will also be re-used as a [portal](https://react.dev/reference/react-dom/createPortal) target for floating elements (e.g. [`Tooltip`](tooltip), [`Toast`](toast), [`DropdownMenu`](dropdownmenu), [`Dialog`](dialog), etc).
+By default, `ThemeProvider` will use an element at the end of `<body>` as the [portal](https://react.dev/reference/react-dom/createPortal) target for floating elements (e.g. [`Tooltip`](tooltip), [`Toast`](toast), [`DropdownMenu`](dropdownmenu), [`Dialog`](dialog), etc).
 
 If you want to specify a different element as the portal target for all components within a tree, the `portalContainer` prop can be used.
 

diff --git a/apps/website/src/content/docs/tooltip.mdx b/apps/website/src/content/docs/tooltip.mdx
@@ -37,7 +37,7 @@ There are some advanced props available for more granular control over positioni
 
 ### Portals
 
-It is important to know that before calculating the position, the tooltip gets [portaled](https://react.dev/reference/react-dom/createPortal) into the nearest [`ThemeProvider`](themeprovider). This is done to avoid [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context) issues in browsers where the [`popover` API](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API) is not supported. This portaling behavior can be controlled using the Tooltip's `portal` prop or the ThemeProvider's [`portalContainer`](themeprovider#portals) prop.
+It is important to know that before calculating the position, the tooltip gets [portaled](https://react.dev/reference/react-dom/createPortal) to the end of `<body>`. This is done to avoid [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context) issues in browsers where the [`popover` API](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API) is not supported. This portaling behavior can be controlled using the Tooltip's `portal` prop or the ThemeProvider's [`portalContainer`](themeprovider#portals) prop.
 
 ## Accessibility
 

diff --git a/packages/itwinui-react/src/core/Popover/Popover.test.tsx b/packages/itwinui-react/src/core/Popover/Popover.test.tsx
@@ -67,6 +67,8 @@ it('should portal to within the ThemeProvider', async () => {
   expect(screen.getByText('Popped over')).toBeVisible();
 
   const root = document.querySelector('.the-root') as HTMLElement;
-  const popover = root.querySelector('.the-popover');
-  expect(popover).toBeVisible();
+  expect(root.querySelector('.the-popover')).toBeNull();
+  expect(
+    document.body.querySelector('body > [data-iui-portal] .the-popover'),
+  ).toBeVisible();
 });
diff --git a/packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx b/packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx
@@ -376,8 +376,8 @@ const PortalContainer = React.memo(
       return null;
     }
 
-    if (shouldSetupPortalContainer) {
-      return (
+    if (shouldSetupPortalContainer && ownerDocument) {
+      return ReactDOM.createPortal(
         <Root
           theme={theme}
           themeOptions={{ ...themeOptions, applyBackground: false }}
@@ -387,7 +387,8 @@ const PortalContainer = React.memo(
           id={id}
         >
           <Toaster />
-        </Root>
+        </Root>,
+        ownerDocument.body,
       );
     } else if (portalContainerProp) {
       return ReactDOM.createPortal(<Toaster />, portalContainerProp);

diff --git a/packages/itwinui-react/src/utils/components/Portal.test.tsx b/packages/itwinui-react/src/utils/components/Portal.test.tsx
@@ -17,9 +17,10 @@ it('should work', () => {
   );
 
   expect(document.querySelector('main')).toBeEmptyDOMElement();
-  expect(
-    screen.getByTestId('root').querySelector(':scope > div'),
-  ).toHaveTextContent('thing');
+  expect(screen.getByTestId('root')).not.toHaveTextContent('thing');
+  expect(document.querySelector('body > [data-iui-portal]')).toHaveTextContent(
+    'thing',
+  );
 });
 
 it('should allow turning off', () => {
@@ -32,9 +33,9 @@ it('should allow turning off', () => {
   );
 
   expect(document.querySelector('main')).toHaveTextContent('thing');
-  expect(
-    screen.getByTestId('root').querySelector(':scope > div'),
-  ).toHaveTextContent('');
+  expect(document.querySelector('[data-iui-portal]')).not.toHaveTextContent(
+    'thing',
+  );
 });
 
 it('should accept an element', () => {
@@ -48,6 +49,9 @@ it('should accept an element', () => {
 
   expect(document.querySelector('main')).toBeEmptyDOMElement();
   expect(screen.getByTestId('root')).toHaveTextContent('');
+  expect(
+    document.querySelector('body > [data-iui-portal]'),
+  ).not.toHaveTextContent('thing');
   expect(document.body).toHaveTextContent('thing');
 });
 
@@ -79,8 +83,9 @@ it.each([null, undefined, () => null, () => undefined])(
     );
 
     expect(document.querySelector('main')).toBeEmptyDOMElement();
+    expect(screen.getByTestId('root')).not.toHaveTextContent('thing');
     expect(
-      screen.getByTestId('root').querySelector(':scope > div'),
+      document.querySelector('body > [data-iui-portal]'),
     ).toHaveTextContent('thing');
   },
 );

diff --git a/testing/e2e/app/routes/ThemeProvider/spec.ts b/testing/e2e/app/routes/ThemeProvider/spec.ts
@@ -13,15 +13,15 @@ test('should inherit the portalContainer if inheriting theme', async ({
 }) => {
   await page.goto('/ThemeProvider?nested=true');
 
-  await expect(page.locator('[data-container="main"]')).toContainText(
-    'main tooltip',
-  );
-  await expect(page.locator('[data-container="main"]')).toContainText(
-    'nested tooltip',
-  );
-  await expect(page.locator('[data-container="nested"]')).not.toContainText(
-    'nested tooltip',
-  );
+  const firstPortal = page.locator('[data-iui-portal]').first();
+
+  // both tooltips should be in the same container
+  await expect(firstPortal).toContainText('main tooltip');
+  await expect(firstPortal).toContainText('nested tooltip');
+
+  // main container should not have any tooltips because we portal to <body>
+  const mainContainer = page.locator('[data-container="main"]');
+  await expect(mainContainer).not.toContainText('tooltip');
 });
 
 test('should not inherit portalContainer across different windows', async ({
@@ -32,9 +32,12 @@ test('should not inherit portalContainer across different windows', async ({
   await page.click('button');
   const popout = await popoutPromise;
 
-  await expect(popout.locator('[data-container="popout"]')).toContainText(
+  await expect(popout.locator('[data-iui-portal]')).toContainText(
     'popout tooltip',
   );
+  await expect(popout.locator('[data-container="popout"]')).not.toContainText(
+    'tooltip',
+  );
 });
 
 test('should not cause an infinite loop when portaled', async ({ page }) => {