remove nested toasters (#2153)

.changeset/flat-apes-crash.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': minor
+---
+
+Nested `ThemeProvider`s will now reuse the same toaster instead of creating new ones.

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

@@ -18,7 +18,6 @@ import {
   ScopeProvider,
   portalContainerAtom,
   useScopedAtom,
-  useScopedSetAtom,
   useId,
 } from '../../utils/index.js';
 import type { PolymorphicForwardRefComponent } from '../../utils/index.js';
@@ -180,7 +179,9 @@ export const ThemeProvider = React.forwardRef((props, forwardedRef) => {
     <ScopeProvider>
       <HydrationProvider>
         <ThemeContext.Provider value={contextValue}>
-          <ToastProvider>
+          <ToastProvider
+            inherit={themeProp === 'inherit' && !portalContainerProp}
+          >
             {includeCss && rootElement ? (
               <FallbackStyles root={rootElement} />
             ) : null}
@@ -328,59 +329,58 @@ const PortalContainer = React.memo(
     isInheritingTheme: boolean;
   }) => {
     const [ownerDocument] = useScopedAtom(ownerDocumentAtom);
-    const setPortalContainer = useScopedSetAtom(portalContainerAtom);
+    const [portalContainer, setPortalContainer] =
+      useScopedAtom(portalContainerAtom);
+
+    // Create a new portal container only if necessary:
+    // - no explicit portalContainer prop
+    // - not inheriting theme
+    // - no parent portal container to portal into
+    // - parent portal container is in a different window (#2006)
+    const shouldSetupPortalContainer =
+      !portalContainerProp &&
+      (!isInheritingTheme ||
+        !portalContainerFromParent ||
+        (!!ownerDocument &&
+          portalContainerFromParent.ownerDocument !== ownerDocument));
 
     const id = useId();
 
+    // Synchronize atom with the correct portal target if necessary.
+    React.useEffect(() => {
+      if (shouldSetupPortalContainer) {
+        return;
+      }
+
+      const portalTarget = portalContainerProp || portalContainerFromParent;
+
+      if (portalTarget && portalTarget !== portalContainer) {
+        setPortalContainer(portalTarget);
+      }
+    });
+
     // bail if not hydrated, because portals don't work on server
     const isHydrated = useHydration() === 'hydrated';
     if (!isHydrated) {
       return null;
     }
 
-    if (portalContainerProp) {
-      return <PortaledToaster target={portalContainerProp} />;
-    }
-
-    // Create a new portal container only if necessary:
-    // - not inheriting theme
-    // - no parent portal container to portal into
-    // - parent portal container is in a different window (#2006)
-    if (
-      !isInheritingTheme ||
-      !portalContainerFromParent ||
-      (!!ownerDocument &&
-        portalContainerFromParent.ownerDocument !== ownerDocument)
-    ) {
+    if (shouldSetupPortalContainer) {
       return (
         <div style={{ display: 'contents' }} ref={setPortalContainer} id={id}>
           <Toaster />
         </div>
       );
+    } else if (portalContainerProp) {
+      return ReactDOM.createPortal(<Toaster />, portalContainerProp);
     }
 
-    return <PortaledToaster target={portalContainerFromParent} />;
+    return null;
   },
 );
 
 // ----------------------------------------------------------------------------
 
-const PortaledToaster = ({ target }: { target?: HTMLElement }) => {
-  const [portalContainer, setPortalContainer] =
-    useScopedAtom(portalContainerAtom);
-
-  // Synchronize atom with the correct portal target if necessary.
-  React.useEffect(() => {
-    if (target && target !== portalContainer) {
-      setPortalContainer(target);
-    }
-  });
-
-  return target ? ReactDOM.createPortal(<Toaster />, target) : null;
-};
-
-// ----------------------------------------------------------------------------
-
 /**
  * When `@itwin/itwinui-react/styles.css` is not imported, we will attempt to
  * dynamically import it (if possible) and fallback to loading it from a CDN.

packages/itwinui-react/src/core/Toast/Toaster.tsx

@@ -89,7 +89,13 @@ export const Toaster = () => {
 
 // ----------------------------------------------------------------------------
 
-export const ToastProvider = ({ children }: { children: React.ReactNode }) => {
+export const ToastProvider = ({
+  children,
+  inherit = false,
+}: {
+  children: React.ReactNode;
+  inherit?: boolean;
+}) => {
   const [toasterState, dispatch] = React.useReducer(toastReducer, {
     toasts: [],
     settings: {
@@ -98,6 +104,11 @@ export const ToastProvider = ({ children }: { children: React.ReactNode }) => {
     },
   });
 
+  // Re-use existing ToastProvider if found
+  if (React.useContext(ToasterStateContext) && inherit) {
+    return children;
+  }
+
   return (
     <ToasterDispatchContext.Provider value={dispatch}>
       <ToasterStateContext.Provider value={toasterState}>

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

@@ -0,0 +1,45 @@
+import * as React from 'react';
+import { useSearchParams } from '@remix-run/react';
+import { ThemeProvider, useToaster } from '@itwin/itwinui-react';
+
+export default function Page() {
+  const [searchParams] = useSearchParams();
+  const shouldPortal = searchParams.get('portal') === 'true';
+  const isClient = useIsClient();
+
+  return (
+    <>
+      <Toast text='Toast (root)' />
+
+      <ThemeProvider data-container='nested'>
+        <Toast text='Toast (nested)' />
+      </ThemeProvider>
+
+      {isClient && shouldPortal && (
+        <ThemeProvider portalContainer={document.body}>
+          <Toast text='Toast (portal)' />
+        </ThemeProvider>
+      )}
+    </>
+  );
+}
+
+function Toast({ text = 'Toast' }) {
+  const toaster = useToaster();
+
+  React.useEffect(() => {
+    toaster.informational(text, { type: 'persisting' });
+  }, [toaster]);
+
+  return null;
+}
+
+function useIsClient() {
+  const [isClient, setIsClient] = React.useState(false);
+
+  React.useEffect(() => {
+    setIsClient(true);
+  }, []);
+
+  return isClient;
+}

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

@@ -0,0 +1,24 @@
+import { test, expect } from '@playwright/test';
+
+test.describe('Toaster', () => {
+  test('should not create nested toast containers', async ({ page }) => {
+    await page.goto('/Toaster');
+
+    const rootToast = page.getByText('Toast (root)');
+    await expect(rootToast).toBeVisible();
+
+    const nestedToast = page.getByText('Toast (nested)');
+    await expect(nestedToast).toBeVisible();
+
+    await expect(page.locator('[data-container="nested"]')).not.toContainText(
+      'Toast',
+    );
+  });
+
+  test('should work when using portalContainer prop', async ({ page }) => {
+    await page.goto('/Toaster?portal=true');
+
+    const portalToast = page.getByText('Toast (portal)');
+    await expect(portalToast).toBeVisible();
+  });
+});