From 5258bcd60e07a4cf59fe35fefe4c57881049028b Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Thu, 9 Jan 2025 11:47:43 +0100 Subject: [PATCH] chore: Experiment with simpler portal --- .../portal/__tests__/portal.test.tsx | 10 ++-- src/internal/components/portal/index.tsx | 17 ++----- .../__tests__/use-base-component.test.tsx | 51 ++++++++++--------- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/src/internal/components/portal/__tests__/portal.test.tsx b/src/internal/components/portal/__tests__/portal.test.tsx index 5fc80546e3..2c15c958fa 100644 --- a/src/internal/components/portal/__tests__/portal.test.tsx +++ b/src/internal/components/portal/__tests__/portal.test.tsx @@ -193,18 +193,16 @@ describe('Portal', () => { }); describe('when a container is not provided', () => { - test('renders to a div under body', () => { + test('renders content under body', () => { renderPortal({ children:

Hello!

}); - expect(document.querySelector('body > div > p')).toHaveTextContent('Hello!'); + expect(document.querySelector('body > p')).toHaveTextContent('Hello!'); }); test('removes container element when unmounted', () => { const { unmount } = renderPortal({ children:

Hello!

}); - // The extra
is a wrapper element that react-testing-library creates. - expect(document.querySelectorAll('body > div').length).toBe(2); + expect(document.querySelectorAll('body > p').length).toBe(1); unmount(); - expect(document.querySelectorAll('body > div').length).toBe(1); - expect(document.querySelector('body > div')).toBeEmptyDOMElement(); + expect(document.querySelectorAll('body > p').length).toBe(0); }); }); }); diff --git a/src/internal/components/portal/index.tsx b/src/internal/components/portal/index.tsx index fe0cd1574d..d070c8dcce 100644 --- a/src/internal/components/portal/index.tsx +++ b/src/internal/components/portal/index.tsx @@ -14,15 +14,6 @@ export interface PortalProps { children: React.ReactNode; } -function manageDefaultContainer(setState: React.Dispatch>) { - const newContainer = document.createElement('div'); - document.body.appendChild(newContainer); - setState(newContainer); - return () => { - document.body.removeChild(newContainer); - }; -} - function manageAsyncContainer( getContainer: () => Promise, removeContainer: (container: HTMLElement) => void, @@ -63,12 +54,14 @@ export default function Portal({ container, getContainer, removeContainer, child warnOnce('portal', '`getContainer` is required when `removeContainer` is provided'); } } - if (getContainer && removeContainer) { return manageAsyncContainer(getContainer, removeContainer, setActiveContainer); } - return manageDefaultContainer(setActiveContainer); }, [container, getContainer, removeContainer]); - return activeContainer && createPortal(children, activeContainer); + if (container || (getContainer && removeContainer)) { + return activeContainer && createPortal(children, activeContainer); + } + + return typeof document !== 'undefined' ? createPortal(children, document.body) : null; } diff --git a/src/internal/hooks/use-base-component/__tests__/use-base-component.test.tsx b/src/internal/hooks/use-base-component/__tests__/use-base-component.test.tsx index c4dae62f64..2c276bac53 100644 --- a/src/internal/hooks/use-base-component/__tests__/use-base-component.test.tsx +++ b/src/internal/hooks/use-base-component/__tests__/use-base-component.test.tsx @@ -31,14 +31,14 @@ function Demo({ variant }: { variant: string }) { /** * This demo component mimics the Modal component. */ -function PortalDemoComponent() { +function PortalDemoComponent({ container }: { container?: HTMLElement }) { const { __internalRootRef } = useBaseComponent('PortalDemoComponent'); const [isVisible, setIsVisible] = useState(false); return ( <> - +
{ - /** - * This test component uses the internal Portal component where a conditional - * rendering is happening: - * - initially the Portal content does not get rendered - * - Portal's useLayoutEffect gets fired synchronously after all DOM mutations - * - the Portal's child elements got rendered - * - * The test covers the case that the metadata got attached after the modal got opened. - */ - const { container, rerender } = render(); - const wrapper = createWrapper(container); +test.each([{ customContainer: false }, { customContainer: true }])( + 'metadata get attached on the Portal component root DOM node when elementRef is changing, custom container = $customContainer', + ({ customContainer }) => { + /** + * This test component uses the internal Portal component which delays the rendering of the content + * if the container is provided explicitly. + * + * The test covers the case that the metadata got attached after the modal got opened. + */ + const { container, rerender } = render( + + ); + const wrapper = createWrapper(container); - const getPortalRootDomNode = () => { - return document.querySelector('#portalContentWrapper')! as any; - }; + const getPortalRootDomNode = () => { + return document.querySelector('#portalContentWrapper')! as any; + }; - expect(getPortalRootDomNode()[COMPONENT_METADATA_KEY]).toBeUndefined(); - wrapper.findButton()!.click(); + wrapper.findButton()!.click(); - // By re-rendering the component we have the mechanism in place to ensure all updates - // to the DOM (attaching the metadata to the changed ref from the modal) have been done. - rerender(); + // By re-rendering the component we have the mechanism in place to ensure all updates + // to the DOM (attaching the metadata to the changed ref from the modal) have been done. + rerender(); - expect(getPortalRootDomNode()[COMPONENT_METADATA_KEY]?.name).toBe('PortalDemoComponent'); - expect(getPortalRootDomNode()[COMPONENT_METADATA_KEY]?.version).toBe(PACKAGE_VERSION); -}); + expect(getPortalRootDomNode()[COMPONENT_METADATA_KEY]?.name).toBe('PortalDemoComponent'); + expect(getPortalRootDomNode()[COMPONENT_METADATA_KEY]?.version).toBe(PACKAGE_VERSION); + } +);