Better handle removing content and asynchronicity

cloudscape-design · Sep 2, 2024 · 86ca42c · 86ca42c
1 parent 8867f89
commit 86ca42c
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 79 deletions.
diff --git a/pages/alert/runtime-content.page.tsx b/pages/alert/runtime-content.page.tsx
@@ -27,14 +27,14 @@ awsuiPlugins.alertContent.registerContent({
       );
       return true;
     }
-    return false;
+    return null;
   },
   mountHeader: (container, context) => {
     if (context.type === 'error' && context.contentRef.current?.textContent?.match('Access denied')) {
-      render(<div>Access denied title</div>, container);
-      return true;
+      container.replaceChildren();
+      return false;
     }
-    return false;
+    return null;
   },
   unmountContent: container => unmountComponentAtNode(container),
   unmountHeader: () => {},

diff --git a/src/alert/__tests__/runtime-content.test.tsx b/src/alert/__tests__/runtime-content.test.tsx
@@ -87,6 +87,22 @@ describe.each([true, false])('existing header:%p', existingHeader => {
   });
 });
 
+test('removes header styling if runtime header is explicitly empty', async () => {
+  const plugin: AlertFlashContentConfig = {
+    id: 'test-content',
+    mountHeader(container) {
+      container.replaceChildren();
+      return false;
+    },
+  };
+  awsuiPlugins.alertContent.registerContent(plugin);
+  const { container } = render(<Alert header="Initial header content" />);
+  const alertWrapper = new AlertWrapper(container);
+  expect(alertWrapper.findHeader()).toBeTruthy();
+  await delay();
+  expect(alertWrapper.findHeader()).toBeFalsy();
+});
+
 describe('mountContent arguments', () => {
   const mountContent = jest.fn();
   beforeEach(() => {
@@ -124,7 +140,6 @@ describe('unmounting', () => {
     const plugin: AlertFlashContentConfig = {
       id: 'test-content',
       mountContent: () => true,
-      mountHeader: () => true,
       unmountContent: jest.fn(),
       unmountHeader: jest.fn(),
     };
@@ -140,28 +155,20 @@ describe('unmounting', () => {
   });
 
   test('does not unmount if not rendered', async () => {
-    const contentOnly: AlertFlashContentConfig = {
+    const noRender: AlertFlashContentConfig = {
       id: 'test-content',
-      mountContent: () => true,
+      mountContent: () => null,
+      mountHeader: () => null,
       unmountContent: jest.fn(),
       unmountHeader: jest.fn(),
     };
-    const headerOnly: AlertFlashContentConfig = {
-      id: 'test-header',
-      mountHeader: () => true,
-      unmountContent: jest.fn(),
-      unmountHeader: jest.fn(),
-    };
-    awsuiPlugins.alertContent.registerContent(contentOnly);
-    awsuiPlugins.alertContent.registerContent(headerOnly);
+    awsuiPlugins.alertContent.registerContent(noRender);
     const { unmount } = render(<Alert>Alert content</Alert>);
     await delay();
     unmount();
     await delay();
-    expect(contentOnly.unmountContent).toBeCalledTimes(1);
-    expect(contentOnly.unmountHeader).toBeCalledTimes(0);
-    expect(headerOnly.unmountContent).toBeCalledTimes(0);
-    expect(headerOnly.unmountHeader).toBeCalledTimes(1);
+    expect(noRender.unmountContent).toBeCalledTimes(0);
+    expect(noRender.unmountHeader).toBeCalledTimes(0);
   });
 });
 
@@ -209,4 +216,60 @@ describe('asynchronous rendering', () => {
     await delay(1000);
     expect(rendered).toBeFalsy();
   });
+
+  test('waits for first async plugin before trying next', async () => {
+    const asyncContent: AlertFlashContentConfig = {
+      id: 'test-content-async-1',
+      mountContent: jest.fn(async () => {
+        await pause(1000);
+        return null;
+      }),
+    };
+    const asyncContent2: AlertFlashContentConfig = {
+      id: 'test-content-async-2',
+      mountContent: jest.fn(async () => {
+        await pause(1000);
+        return true;
+      }),
+      mountHeader: jest.fn(),
+    };
+    const asyncContent3: AlertFlashContentConfig = {
+      id: 'test-content-async-3',
+      mountContent: jest.fn(),
+    };
+    awsuiPlugins.alertContent.registerContent(asyncContent);
+    awsuiPlugins.alertContent.registerContent(asyncContent2);
+    render(<Alert>Alert content</Alert>);
+    await delay();
+    expect(asyncContent.mountContent).toBeCalled();
+    expect(asyncContent2.mountContent).not.toBeCalled();
+    expect(asyncContent2.mountHeader).not.toBeCalled();
+    awsuiPlugins.alertContent.registerContent(asyncContent3);
+    expect(asyncContent3.mountContent).not.toBeCalled();
+    await delay(1000);
+    expect(asyncContent2.mountContent).toBeCalled();
+    expect(asyncContent2.mountHeader).toBeCalled();
+    expect(asyncContent3.mountContent).not.toBeCalled();
+    await delay(1000);
+    expect(asyncContent3.mountContent).not.toBeCalled();
+  });
+});
+
+test('only uses first plugin to return a result', async () => {
+  const first: AlertFlashContentConfig = {
+    id: 'test-content-1',
+    mountContent: jest.fn(() => false),
+  };
+  const second: AlertFlashContentConfig = {
+    id: 'test-content-2',
+    mountContent: jest.fn(),
+    mountHeader: jest.fn(),
+  };
+  awsuiPlugins.alertContent.registerContent(first);
+  awsuiPlugins.alertContent.registerContent(second);
+  render(<Alert />);
+  await delay();
+  expect(first.mountContent).toBeCalled();
+  expect(second.mountContent).not.toBeCalled();
+  expect(second.mountHeader).not.toBeCalled();
 });
diff --git a/src/alert/internal.tsx b/src/alert/internal.tsx
@@ -80,7 +80,7 @@ const InternalAlert = React.forwardRef(
     const isRefresh = useVisualRefresh();
     const size = isRefresh
       ? 'normal'
-      : (header || hasDiscoveredHeader) && (children || hasDiscoveredContent)
+      : (hasDiscoveredHeader ?? header) && (hasDiscoveredContent ?? children)
         ? 'big'
         : 'normal';
 
@@ -115,7 +115,7 @@ const InternalAlert = React.forwardRef(
                   <InternalIcon name={typeToIcon[type]} size={size} />
                 </div>
                 <div className={clsx(styles.message, styles.text)}>
-                  <div className={header || hasDiscoveredHeader ? styles.header : undefined} ref={headerRef}>
+                  <div className={hasDiscoveredHeader ?? header ? styles.header : undefined} ref={headerRef}>
                     {header}
                   </div>
                   <div className={styles.content} ref={contentRef}>

diff --git a/src/flashbar/__tests__/runtime-content.test.tsx b/src/flashbar/__tests__/runtime-content.test.tsx
@@ -200,7 +200,6 @@ describe('unmounting', () => {
     const plugin: AlertFlashContentConfig = {
       id: 'test-content',
       mountContent: () => true,
-      mountHeader: () => true,
       unmountContent: jest.fn(),
       unmountHeader: jest.fn(),
     };
@@ -218,7 +217,6 @@ describe('unmounting', () => {
     const plugin: AlertFlashContentConfig = {
       id: 'test-content',
       mountContent: () => true,
-      mountHeader: () => true,
       unmountContent: jest.fn(),
       unmountHeader: jest.fn(),
     };
@@ -234,28 +232,20 @@ describe('unmounting', () => {
   });
 
   test('does not unmount if not rendered', async () => {
-    const contentOnly: AlertFlashContentConfig = {
+    const noRender: AlertFlashContentConfig = {
       id: 'test-content',
-      mountContent: () => true,
+      mountContent: () => null,
+      mountHeader: () => null,
       unmountContent: jest.fn(),
       unmountHeader: jest.fn(),
     };
-    const headerOnly: AlertFlashContentConfig = {
-      id: 'test-header',
-      mountHeader: () => true,
-      unmountContent: jest.fn(),
-      unmountHeader: jest.fn(),
-    };
-    awsuiPlugins.flashContent.registerContent(contentOnly);
-    awsuiPlugins.flashContent.registerContent(headerOnly);
+    awsuiPlugins.flashContent.registerContent(noRender);
     const { unmount } = render(<Flashbar items={[{}]} />);
     await delay();
     unmount();
     await delay();
-    expect(contentOnly.unmountContent).toBeCalledTimes(1);
-    expect(contentOnly.unmountHeader).toBeCalledTimes(0);
-    expect(headerOnly.unmountContent).toBeCalledTimes(0);
-    expect(headerOnly.unmountHeader).toBeCalledTimes(1);
+    expect(noRender.unmountContent).toBeCalledTimes(0);
+    expect(noRender.unmountHeader).toBeCalledTimes(0);
   });
 });
 
@@ -312,3 +302,22 @@ describe('asynchronous rendering', () => {
     expect(rendered).toBeFalsy();
   });
 });
+
+test('only uses first plugin to return a result', async () => {
+  const first: AlertFlashContentConfig = {
+    id: 'test-content-1',
+    mountContent: jest.fn(() => false),
+  };
+  const second: AlertFlashContentConfig = {
+    id: 'test-content-2',
+    mountContent: jest.fn(),
+    mountHeader: jest.fn(),
+  };
+  awsuiPlugins.flashContent.registerContent(first);
+  awsuiPlugins.flashContent.registerContent(second);
+  render(<Flashbar items={[{ content: 'Flash' }]} />);
+  await delay();
+  expect(first.mountContent).toBeCalled();
+  expect(second.mountContent).not.toBeCalled();
+  expect(second.mountHeader).not.toBeCalled();
+});
diff --git a/src/internal/plugins/controllers/alert-flash-content.ts b/src/internal/plugins/controllers/alert-flash-content.ts
@@ -19,8 +19,17 @@ export interface AlertFlashContentContext {
 export interface AlertFlashContentConfig {
   id: string;
   orderPriority?: number;
-  mountHeader?: (container: HTMLElement, context: AlertFlashContentContext) => boolean | Promise<boolean>;
-  mountContent?: (container: HTMLElement, context: AlertFlashContentContext) => boolean | Promise<boolean>;
+  /**
+   * Return values for mountHeader and mountContent:
+   * - true: content was replaced
+   * - false: content was removed
+   * - null: nothing was done
+   */
+  mountHeader?: (container: HTMLElement, context: AlertFlashContentContext) => boolean | null | Promise<boolean | null>;
+  mountContent?: (
+    container: HTMLElement,
+    context: AlertFlashContentContext
+  ) => boolean | null | Promise<boolean | null>;
   unmountHeader?: (container: HTMLElement) => void;
   unmountContent?: (container: HTMLElement) => void;
 }

diff --git a/src/internal/plugins/helpers/use-discovered-content.tsx b/src/internal/plugins/helpers/use-discovered-content.tsx
@@ -17,72 +17,79 @@ export function createUseDiscoveredContent(onContentRegistered: AlertFlashConten
     const headerRef = useRef<HTMLDivElement>(null);
     const contentRef = useRef<HTMLDivElement>(null);
     const actionsRef = useRef<HTMLDivElement>(null);
-    const foundHeaderProviderRef = useRef<AlertFlashContentConfig>();
-    const foundContentProviderRef = useRef<AlertFlashContentConfig>();
-    const [foundHeaderProvider, setFoundHeaderProvider] = useState<AlertFlashContentConfig | null>(null);
-    const [foundContentProvider, setFoundContentProvider] = useState<AlertFlashContentConfig | null>(null);
+    const foundProvider = useRef<AlertFlashContentConfig | null>(null);
+    const [foundHeaderReplacement, setFoundHeaderReplacement] = useState<boolean | null>(null);
+    const [foundContentReplacement, setFoundContentReplacement] = useState<boolean | null>(null);
 
     useEffect(() => {
       return onContentRegistered(providers => {
         const controller = new AbortController();
-        const runHeader = async () => {
-          for (const provider of providers) {
-            if (
-              headerRef.current &&
-              !foundHeaderProvider &&
-              (await provider.mountHeader?.(headerRef.current, {
-                type,
-                headerRef,
-                contentRef,
-                actionsRef,
-                signal: controller.signal,
-              }))
-            ) {
+        const runHeader = async (provider: AlertFlashContentConfig) => {
+          if (
+            headerRef.current &&
+            (!foundProvider.current || foundProvider.current === provider) &&
+            provider.mountHeader
+          ) {
+            const result = await provider.mountHeader(headerRef.current, {
+              type,
+              headerRef,
+              contentRef,
+              actionsRef,
+              signal: controller.signal,
+            });
+            if (result !== null) {
               if (controller.signal.aborted) {
                 console.warn('[AwsUi] [Runtime alert/flash content] Async header returned after component unmounted');
                 return;
               }
-              foundHeaderProviderRef.current = provider;
-              setFoundHeaderProvider(provider);
+              foundProvider.current = provider;
+              setFoundHeaderReplacement(result);
             }
           }
         };
-        const runContent = async () => {
-          for (const provider of providers) {
-            if (
-              contentRef.current &&
-              !foundContentProvider &&
-              (await provider.mountContent?.(contentRef.current, {
-                type,
-                headerRef,
-                contentRef,
-                actionsRef,
-                signal: controller.signal,
-              }))
-            ) {
+        const runContent = async (provider: AlertFlashContentConfig) => {
+          if (
+            contentRef.current &&
+            (!foundProvider.current || foundProvider.current === provider) &&
+            provider.mountContent
+          ) {
+            const result = await provider.mountContent(contentRef.current, {
+              type,
+              headerRef,
+              contentRef,
+              actionsRef,
+              signal: controller.signal,
+            });
+            if (result !== null) {
               if (controller.signal.aborted) {
                 console.warn('[AwsUi] [Runtime alert/flash content] Async content returned after component unmounted');
                 return;
               }
-              foundContentProviderRef.current = provider;
-              setFoundContentProvider(provider);
+              foundProvider.current = provider;
+              setFoundContentReplacement(result);
             }
           }
         };
-        runHeader();
-        runContent();
+        (async () => {
+          for (const provider of providers) {
+            await Promise.all([runHeader(provider), runContent(provider)]);
+            if (controller.signal.aborted) {
+              break;
+            }
+          }
+        })();
         return () => {
           controller.abort();
-          headerRef.current && foundHeaderProviderRef.current?.unmountHeader?.(headerRef.current);
-          contentRef.current && foundContentProviderRef.current?.unmountContent?.(contentRef.current);
+          headerRef.current && foundProvider.current?.unmountHeader?.(headerRef.current);
+          contentRef.current && foundProvider.current?.unmountContent?.(contentRef.current);
         };
       });
       // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [type, header, children]);
 
     return {
-      hasDiscoveredHeader: !!foundHeaderProvider,
-      hasDiscoveredContent: !!foundContentProvider,
+      hasDiscoveredHeader: foundHeaderReplacement,
+      hasDiscoveredContent: foundContentReplacement,
       headerRef,
       contentRef,
       actionsRef,