remove nested portals (#2209)

.changeset/blue-shirts-draw.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+Fixed an issue where nested `Popover`s were automatically closing due to faulty "outside click" detection.

packages/itwinui-react/src/core/Popover/Popover.tsx

@@ -48,7 +48,6 @@ import {
 import type { PolymorphicForwardRefComponent } from '../../utils/index.js';
 import { usePortalTo } from '../../utils/components/Portal.js';
 import type { PortalProps } from '../../utils/components/Portal.js';
-import { ThemeProvider } from '../ThemeProvider/ThemeProvider.js';
 
 type PopoverOptions = {
   /**
@@ -399,13 +398,7 @@ export const Popover = React.forwardRef((props, forwardedRef) => {
     middleware,
   });
 
-  const [popoverElement, setPopoverElement] = React.useState<HTMLElement>();
-
-  const popoverRef = useMergedRefs(
-    popover.refs.setFloating,
-    forwardedRef,
-    setPopoverElement,
-  );
+  const popoverRef = useMergedRefs(popover.refs.setFloating, forwardedRef);
 
   const triggerId = `${useId()}-trigger`;
   const hasAriaLabel = !!props['aria-labelledby'] || !!props['aria-label'];
@@ -430,28 +423,23 @@ export const Popover = React.forwardRef((props, forwardedRef) => {
 
       {popover.open ? (
         <PopoverPortal portal={portal}>
-          <ThemeProvider
-            portalContainer={popoverElement} // portal nested popovers into this one
-          >
-            <DisplayContents />
-            <FloatingFocusManager context={popover.context} modal={false}>
-              <Box
-                className={cx(
-                  { 'iui-popover-surface': applyBackground },
-                  className,
-                )}
-                aria-labelledby={
-                  !hasAriaLabel
-                    ? popover.refs.domReference.current?.id
-                    : undefined
-                }
-                {...popover.getFloatingProps(rest)}
-                ref={popoverRef}
-              >
-                {content}
-              </Box>
-            </FloatingFocusManager>
-          </ThemeProvider>
+          <FloatingFocusManager context={popover.context} modal={false}>
+            <Box
+              className={cx(
+                { 'iui-popover-surface': applyBackground },
+                className,
+              )}
+              aria-labelledby={
+                !hasAriaLabel
+                  ? popover.refs.domReference.current?.id
+                  : undefined
+              }
+              {...popover.getFloatingProps(rest)}
+              ref={popoverRef}
+            >
+              {content}
+            </Box>
+          </FloatingFocusManager>
         </PopoverPortal>
       ) : null}
     </>

packages/itwinui-react/src/core/Table/Table.tsx

@@ -960,7 +960,7 @@ export const Table = <
                     return (
                       <ColumnHeader<T>
                         {...dragAndDropProps}
-                        key={dragAndDropProps.key}
+                        key={dragAndDropProps.key || column.id || index}
                         columnRefs={columnRefs}
                         column={column}
                         index={index}

testing/e2e/app/routes/Popover/route.tsx

@@ -1,31 +1,69 @@
-import { Popover } from '@itwin/itwinui-react';
+import * as React from 'react';
+import { ComboBox, Popover } from '@itwin/itwinui-react';
 import { useSearchParams } from '@remix-run/react';
 
 export default function Page() {
   const [searchParams] = useSearchParams();
 
+  if (searchParams.get('nestedComboBox')) {
+    return <NestedComboBoxTest />;
+  }
+
+  return <FocusTest />;
+}
+
+// ----------------------------------------------------------------------------
+
+function NestedComboBoxTest() {
+  return (
+    <Popover
+      applyBackground
+      content={
+        <div>
+          <h2>Popover title</h2>
+          <ComboBox
+            options={React.useMemo(
+              () => [
+                { label: 'Item 1', value: 1 },
+                { label: 'Item 2', value: 2 },
+                { label: 'Item 3', value: 3 },
+              ],
+              [],
+            )}
+          />
+        </div>
+      }
+    >
+      <button>Open</button>
+    </Popover>
+  );
+}
+
+// ----------------------------------------------------------------------------
+
+function FocusTest() {
+  const [searchParams] = useSearchParams();
+
   const shouldImperativeFocus = searchParams.get('imperativeFocus') === 'true';
   const shouldFocusInput = searchParams.get('focusInput') === 'true';
 
   return (
-    <>
-      <Popover
-        applyBackground
-        content={
-          <div>
-            <h2
-              tabIndex={-1}
-              ref={shouldImperativeFocus ? (el) => el?.focus() : undefined}
-            >
-              Popover title
-            </h2>
-            <p>Popover content</p>
-            {shouldFocusInput && <input />}
-          </div>
-        }
-      >
-        <button>Open</button>
-      </Popover>
-    </>
+    <Popover
+      applyBackground
+      content={
+        <div>
+          <h2
+            tabIndex={-1}
+            ref={shouldImperativeFocus ? (el) => el?.focus() : undefined}
+          >
+            Popover title
+          </h2>
+          <p>Popover content</p>
+          {shouldFocusInput && <input />}
+        </div>
+      }
+    >
+      <button>Open</button>
+    </Popover>
   );
 }

testing/e2e/app/routes/Popover/spec.ts

@@ -29,3 +29,23 @@ test.describe('Popover (focus)', () => {
     await expect(page.getByRole('heading')).toBeFocused();
   });
 });
+
+test.describe('Nested popovers', () => {
+  test('should work with ComboBox', async ({ page }) => {
+    await page.goto('/Popover?nestedComboBox=true');
+    await page.click('button');
+
+    const popover = page.getByRole('dialog');
+    const comboboxInput = page.getByRole('combobox');
+    const comboboxListbox = page.getByRole('listbox');
+
+    await expect(popover).toBeVisible();
+    await expect(comboboxInput).toBeVisible();
+    await expect(comboboxListbox).toBeVisible();
+
+    // close the combobox
+    await comboboxInput.click();
+    await expect(comboboxListbox).not.toBeVisible();
+    await expect(popover).toBeVisible();
+  });
+});