[Portal] Render single div for container element

UNRELEASED.md

@@ -8,7 +8,9 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Enhancements
 
-Updated `Textfield` with a `type` of `number` to not render a spinner if step is set to `0` ([#3477](https://github.com/Shopify/polaris-react/pull/3477))
+- Converted `Portal` to a functional component ([#3465](https://github.com/Shopify/polaris-react/pull/3465))
+- Changed `Portal` to render all `Portals` in a single div ([#3465](https://github.com/Shopify/polaris-react/pull/3465))
+- Updated `Textfield` with a `type` of `number` to not render a spinner if step is set to `0` ([#3477](https://github.com/Shopify/polaris-react/pull/3477))
 
 ### Bug fixes
 

src/components/Pagination/tests/Pagination.test.tsx

@@ -1,10 +1,6 @@
 import React from 'react';
 // eslint-disable-next-line no-restricted-imports
-import {
-  mountWithAppProvider,
-  findByTestID,
-  ReactWrapper,
-} from 'test-utilities/legacy';
+import {mountWithAppProvider, ReactWrapper} from 'test-utilities/legacy';
 import {mountWithApp} from 'test-utilities';
 import {Tooltip, TextField} from 'components';
 
@@ -18,6 +14,13 @@ interface HandlerMap {
   [eventName: string]: (event: any) => void;
 }
 
+jest.mock('../../Portal', () => ({
+  ...(jest.requireActual('../../Portal') as any),
+  Portal() {
+    return null;
+  },
+}));
+
 const listenerMap: HandlerMap = {};
 
 describe('<Pagination />', () => {
@@ -47,12 +50,10 @@ describe('<Pagination />', () => {
 
   describe('tooltip', () => {
     it('renders a tooltip if nextTooltip is provided and hasNext is true', () => {
-      const pagination = mountWithAppProvider(
-        <Pagination nextTooltip="k" hasNext />,
-      );
-      pagination.find(Tooltip).simulate('focus');
-
-      expect(findByTestID(pagination, 'TooltipOverlayLabel').text()).toBe('k');
+      const pagination = mountWithApp(<Pagination nextTooltip="k" hasNext />);
+      expect(pagination).toContainReactComponent(Tooltip, {
+        content: 'k',
+      });
     });
 
     it('does not render a tooltip if nextTooltip is provided and hasNext is false', () => {
@@ -63,12 +64,12 @@ describe('<Pagination />', () => {
     });
 
     it('renders a tooltip if previousToolTip is provided and hasPrevious is true', () => {
-      const pagination = mountWithAppProvider(
+      const pagination = mountWithApp(
         <Pagination previousTooltip="j" hasPrevious />,
       );
-      pagination.find(Tooltip).simulate('focus');
-
-      expect(findByTestID(pagination, 'TooltipOverlayLabel').text()).toBe('j');
+      expect(pagination).toContainReactComponent(Tooltip, {
+        content: 'j',
+      });
     });
 
     it('does not render  tooltip if previousToolTip is provided and hasPrevious is false', () => {

src/components/Portal/Portal.tsx

@@ -1,4 +1,4 @@
-import React, {PureComponent} from 'react';
+import React, {useState, useRef, useContext, useEffect} from 'react';
 import {createPortal} from 'react-dom';
 
 import {ThemeContext} from '../../utilities/theme';
@@ -11,70 +11,90 @@ export interface PortalProps {
   onPortalCreated?(): void;
 }
 
-interface State {
-  isMounted: boolean;
-}
+export const UNIQUE_CONTAINER_ID = 'polaris-portal-container';
 
 const getUniqueID = globalIdGeneratorFactory('portal-');
 
-export class Portal extends PureComponent<PortalProps, State> {
-  static defaultProps = {idPrefix: ''};
-  static contextType = ThemeContext;
-  context!: React.ContextType<typeof ThemeContext>;
+export function Portal({
+  children,
+  idPrefix = '',
+  onPortalCreated = noop,
+}: PortalProps) {
+  const [isMounted, setIsMounted] = useState(false);
+  const portalNode = useRef<HTMLElement | null>(null);
+  const portalsContainerNode = useRef<HTMLElement | null>(null);
+  const context = useContext(ThemeContext);
 
-  state: State = {isMounted: false};
+  const portalId =
+    idPrefix !== '' ? `${idPrefix}-${getUniqueID()}` : getUniqueID();
 
-  private portalNode: HTMLElement | null = null;
+  useEffect(() => {
+    const containerNode = document.getElementById(UNIQUE_CONTAINER_ID);
 
-  private portalId =
-    this.props.idPrefix !== ''
-      ? `${this.props.idPrefix}-${getUniqueID()}`
-      : getUniqueID();
+    portalsContainerNode.current =
+      containerNode || document.createElement('div');
 
-  componentDidMount() {
-    this.portalNode = document.createElement('div');
-    this.portalNode.setAttribute(portal.props[0], this.portalId);
+    if (!containerNode)
+      portalsContainerNode.current.setAttribute('id', UNIQUE_CONTAINER_ID);
+    portalNode.current = document.createElement('div');
+    portalNode.current.setAttribute(portal.props[0], portalId);
 
-    if (this.context != null) {
-      const {cssCustomProperties} = this.context;
+    if (context != null && !containerNode) {
+      const {cssCustomProperties} = context;
       if (cssCustomProperties != null) {
-        this.portalNode.setAttribute('style', cssCustomProperties);
+        portalsContainerNode.current.setAttribute('style', cssCustomProperties);
       } else {
-        this.portalNode.removeAttribute('style');
+        portalsContainerNode.current.removeAttribute('style');
       }
     }
-    document.body.appendChild(this.portalNode);
-    this.setState({isMounted: true});
-  }
 
-  componentDidUpdate(_: PortalProps, prevState: State) {
-    const {onPortalCreated = noop} = this.props;
+    if (!containerNode) {
+      document.body.appendChild(portalsContainerNode.current);
+    }
+
+    portalsContainerNode.current.appendChild(portalNode.current);
 
-    if (this.portalNode && this.context != null) {
-      const {cssCustomProperties, textColor} = this.context;
+    setIsMounted(true);
+  }, [isMounted, context, portalId]);
+
+  useEffect(() => {
+    if (isMounted && portalsContainerNode.current && context != null) {
+      const {cssCustomProperties, textColor} = context;
       if (cssCustomProperties != null) {
-        const style = `${cssCustomProperties};color:${textColor};`;
-        this.portalNode.setAttribute('style', style);
+        const style = textColor
+          ? `${cssCustomProperties};color:${textColor};`
+          : `${cssCustomProperties}`;
+        const currentStyle = portalsContainerNode.current.getAttribute('style');
+        if (style !== currentStyle) {
+          portalsContainerNode.current.setAttribute('style', style);
+        }
       } else {
-        this.portalNode.removeAttribute('style');
+        portalsContainerNode.current.removeAttribute('style');
       }
     }
-    if (!prevState.isMounted && this.state.isMounted) {
-      onPortalCreated();
-    }
-  }
 
-  componentWillUnmount() {
-    if (this.portalNode) {
-      document.body.removeChild(this.portalNode);
+    return function cleanup() {
+      const containerNode = portalsContainerNode.current
+        ? portalsContainerNode.current
+        : document.getElementById(UNIQUE_CONTAINER_ID);
+      if (portalNode.current && containerNode) {
+        containerNode.removeChild(portalNode.current);
+        if (containerNode && containerNode.childElementCount === 0) {
+          document.body.removeChild(containerNode);
+        }
+      }
+    };
+  }, [isMounted, context]);
+
+  useEffect(() => {
+    if (isMounted) {
+      onPortalCreated();
     }
-  }
+  }, [isMounted, onPortalCreated]);
 
-  render() {
-    return this.portalNode && this.state.isMounted
-      ? createPortal(this.props.children, this.portalNode)
-      : null;
-  }
+  return portalNode.current && isMounted
+    ? createPortal(children, portalNode.current)
+    : null;
 }
 
 function noop() {}

src/components/Portal/tests/Portal.test.tsx

@@ -1,9 +1,9 @@
 import React, {createRef} from 'react';
-import {mount} from 'test-utilities';
+import {mount, mountWithApp} from 'test-utilities';
 // eslint-disable-next-line no-restricted-imports
 import {mountWithAppProvider} from 'test-utilities/legacy';
 
-import {Portal} from '../Portal';
+import {Portal, UNIQUE_CONTAINER_ID} from '../Portal';
 import {portal} from '../../shared';
 
 jest.mock('react-dom', () => ({
@@ -24,6 +24,45 @@ describe('<Portal />', () => {
     createPortalSpy.mockImplementation(() => null);
   });
 
+  afterEach(() => {
+    createPortalSpy.mockRestore();
+  });
+
+  describe('container', () => {
+    it('creates a portal container with the UNIQUE_CONTAINER_ID', () => {
+      const appendChildSpy = jest.spyOn(document.body, 'appendChild');
+      mountWithApp(<Portal />);
+      expect(appendChildSpy).toHaveBeenLastCalledWith(
+        expect.objectContaining({
+          id: UNIQUE_CONTAINER_ID,
+        }),
+      );
+      appendChildSpy.mockReset();
+    });
+
+    it('sets CSS custom properties on the portal node', () => {
+      const setSpy = jest.spyOn(Element.prototype, 'setAttribute');
+      const portal = mountWithAppProvider(<Portal />, {
+        features: {newDesignLanguage: true},
+        theme: {
+          colors: {surface: '#000000'},
+        },
+      });
+
+      expect(setSpy).toHaveBeenLastCalledWith(
+        'style',
+        portal.context().cssCustomProperties,
+      );
+      setSpy.mockRestore();
+    });
+
+    it('removes CSS custom properties from the portal node', () => {
+      const removeSpy = jest.spyOn(Element.prototype, 'removeAttribute');
+      mountWithAppProvider(<Portal />);
+      expect(removeSpy).toHaveBeenCalledWith('style');
+    });
+  });
+
   describe('children', () => {
     it('get passed into the portal creation method', () => {
       const children = <div />;
@@ -100,24 +139,4 @@ describe('<Portal />', () => {
       mount(<Portal />);
     }).not.toThrow();
   });
-
-  it('sets CSS custom properties on the portal node', () => {
-    const setSpy = jest.spyOn(Element.prototype, 'setAttribute');
-    const portal = mountWithAppProvider(<Portal />, {
-      features: {newDesignLanguage: true},
-      theme: {
-        colors: {surface: '#000000'},
-      },
-    });
-    expect(setSpy).toHaveBeenCalledWith(
-      'style',
-      portal.context().cssCustomProperties,
-    );
-  });
-
-  it('removes CSS custom properties from the portal node', () => {
-    const removeSpy = jest.spyOn(Element.prototype, 'removeAttribute');
-    mountWithAppProvider(<Portal />);
-    expect(removeSpy).toHaveBeenCalledWith('style');
-  });
 });

src/components/Tabs/tests/Tabs.test.tsx

@@ -9,6 +9,13 @@ import {getVisibleAndHiddenTabIndices} from '../utilities';
 import {FeaturesContext} from '../../../utilities/features';
 import {Popover} from '../../Popover';
 
+jest.mock('../../Portal', () => ({
+  ...(jest.requireActual('../../Portal') as any),
+  Portal() {
+    return null;
+  },
+}));
+
 describe('<Tabs />', () => {
   const tabs: TabsProps['tabs'] = [
     {content: 'Tab 1', id: 'tab-1'},