Skip to content

Commit

Permalink
chore: Experiment with simpler portal
Browse files Browse the repository at this point in the history
  • Loading branch information
pan-kot committed Jan 9, 2025
1 parent 1c32dd6 commit 5258bcd
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 43 deletions.
10 changes: 4 additions & 6 deletions src/internal/components/portal/__tests__/portal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: <p>Hello!</p> });
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: <p>Hello!</p> });
// The extra <div> 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);
});
});
});
17 changes: 5 additions & 12 deletions src/internal/components/portal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,6 @@ export interface PortalProps {
children: React.ReactNode;
}

function manageDefaultContainer(setState: React.Dispatch<React.SetStateAction<Element | null>>) {
const newContainer = document.createElement('div');
document.body.appendChild(newContainer);
setState(newContainer);
return () => {
document.body.removeChild(newContainer);
};
}

function manageAsyncContainer(
getContainer: () => Promise<HTMLElement>,
removeContainer: (container: HTMLElement) => void,
Expand Down Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<>
<Button onClick={() => setIsVisible(true)}>Show modal</Button>
<Portal>
<Portal container={container}>
<div
id="portalContentWrapper"
style={{
Expand Down Expand Up @@ -66,30 +66,31 @@ test('should call the useTelemetry hook passing down the given component name an
expect(useTelemetry).toHaveBeenCalledWith('DemoComponent', { props: { variant: 'default' } });
});

test('metadata get attached on the Portal component root DOM node when elementRef is changing', () => {
/**
* 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(<PortalDemoComponent />);
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(
<PortalDemoComponent container={customContainer ? document.body : undefined} />
);
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(<PortalDemoComponent />);
// 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(<PortalDemoComponent container={customContainer ? document.body : undefined} />);

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);
}
);

0 comments on commit 5258bcd

Please sign in to comment.