From 86ca42ca13551485ed7e3ab898f4ae128b3ed441 Mon Sep 17 00:00:00 2001 From: Gethin Webster Date: Mon, 2 Sep 2024 15:04:33 +0200 Subject: [PATCH] Better handle removing content and asynchronicity --- pages/alert/runtime-content.page.tsx | 8 +- src/alert/__tests__/runtime-content.test.tsx | 93 ++++++++++++++++--- src/alert/internal.tsx | 4 +- .../__tests__/runtime-content.test.tsx | 41 ++++---- .../controllers/alert-flash-content.ts | 13 ++- .../helpers/use-discovered-content.tsx | 87 +++++++++-------- 6 files changed, 167 insertions(+), 79 deletions(-) diff --git a/pages/alert/runtime-content.page.tsx b/pages/alert/runtime-content.page.tsx index 26981079e03..ea20854654e 100644 --- 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(
Access denied title
, 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 index 383a333d3ab..9420f21489e 100644 --- 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(); + 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 content); 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 content); + 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(); + 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 index b739d3fde02..d6629dd748a 100644 --- 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(
-
+
{header}
diff --git a/src/flashbar/__tests__/runtime-content.test.tsx b/src/flashbar/__tests__/runtime-content.test.tsx index 848d0eddc70..dbe2a9664be 100644 --- 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(); 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(); + 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 index 2f44a39868e..e126dbc5e0a 100644 --- 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; - mountContent?: (container: HTMLElement, context: AlertFlashContentContext) => boolean | Promise; + /** + * 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; + mountContent?: ( + container: HTMLElement, + context: AlertFlashContentContext + ) => boolean | null | Promise; 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 index c4fe6b1034e..b78feccfb7f 100644 --- 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(null); const contentRef = useRef(null); const actionsRef = useRef(null); - const foundHeaderProviderRef = useRef(); - const foundContentProviderRef = useRef(); - const [foundHeaderProvider, setFoundHeaderProvider] = useState(null); - const [foundContentProvider, setFoundContentProvider] = useState(null); + const foundProvider = useRef(null); + const [foundHeaderReplacement, setFoundHeaderReplacement] = useState(null); + const [foundContentReplacement, setFoundContentReplacement] = useState(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,