fix: use createPortal for all internal popovers (#636)

Author: MarcusNotheis

packages/main/src/components/ActionSheet/ActionSheet.stories.mdx

@@ -0,0 +1,67 @@
+import { Meta, Title, Description, Story, Preview, Props } from '@storybook/addon-docs/blocks';
+import { createSelectArgTypes } from '@shared/stories/createSelectArgTypes';
+import { ActionSheet } from '@ui5/webcomponents-react/lib/ActionSheet';
+import { Button } from '@ui5/webcomponents-react/lib/Button';
+import { PlacementType } from '@ui5/webcomponents-react/lib/PlacementType';
+import { PopoverHorizontalAlign } from '@ui5/webcomponents-react/lib/PopoverHorizontalAlign';
+import { PopoverVerticalAlign } from '@ui5/webcomponents-react/lib/PopoverVerticalAlign';
+import { useCallback, useRef } from 'react';
+
+<Meta
+  title="Components / ActionSheet"
+  component={ActionSheet}
+  argTypes={{
+    ...createSelectArgTypes({
+      placementType: PlacementType,
+      horizontalAlign: PopoverHorizontalAlign,
+      verticalAlign: PopoverVerticalAlign
+    }),
+    children: {
+      type: null
+    },
+    footer: {
+      type: null
+    },
+    header: {
+      type: null
+    },
+    ref: {
+      type: null
+    }
+  }}
+  args={{
+    horizontalAlign: PopoverHorizontalAlign.Center,
+    placementType: PlacementType.Right,
+    verticalAlign: PopoverVerticalAlign.Center
+  }}
+/>
+
+<Title />
+<Subtitle />
+<Description />
+
+<Preview>
+  <Story name="Default">
+    {(args) => {
+      const actionSheetRef = useRef();
+      const onButtonClick = useCallback(
+        (e) => {
+          actionSheetRef.current.open(e.target);
+        },
+        [actionSheetRef]
+      );
+      return (
+        <>
+          <Button onClick={onButtonClick}>Open ActionSheet</Button>
+          <ActionSheet ref={actionSheetRef} {...args}>
+            <Button icon="add">Accept</Button>
+            <Button>Reject</Button>
+            <Button>This is my super long text!</Button>
+          </ActionSheet>
+        </>
+      );
+    }}
+  </Story>
+</Preview>
+
+<Props story="Default" components={{ ActionSheet }} />

packages/main/src/components/ActionSheet/ActionSheet.test.tsx

@@ -1,52 +1,40 @@
+import { cleanStaticAreaAfterEachTest, fireEvent, render, screen } from '@shared/tests';
 import { createPassThroughPropsTest } from '@shared/tests/utils';
-import { mount } from 'enzyme';
 import { ActionSheet } from '@ui5/webcomponents-react/lib/ActionSheet';
 import { Button } from '@ui5/webcomponents-react/lib/Button';
 import { Label } from '@ui5/webcomponents-react/lib/Label';
 import React, { createRef, RefObject } from 'react';
-import sinon from 'sinon';
 import { Ui5PopoverDomRef } from '../../interfaces/Ui5PopoverDomRef';
 
 describe('ActionSheet', () => {
+  cleanStaticAreaAfterEachTest();
+
   test('Render without Crashing', () => {
-    const button = <Button />;
-    const wrapper = mount(
-      <ActionSheet openBy={button} className="myCustomClass">
-        <Button icon={'add'}>Accept</Button>
+    const ref = createRef();
+    const wrapper = render(
+      <ActionSheet className="myCustomClass" ref={ref}>
+        <Button>Accept</Button>
         <Button>Reject</Button>
         <Button>This is my super long text!</Button>
       </ActionSheet>
     );
-    expect(wrapper.render()).toMatchSnapshot();
+
+    expect(wrapper.container.parentElement).toMatchSnapshot();
   });
 
   test('Button Click handler', () => {
-    const callback = sinon.spy();
-    const button = <Button />;
-    const wrapper = mount(
-      <ActionSheet openBy={button} className="myCustomClass">
-        <Button icon={'add'} onClick={callback}>
-          Accept
-        </Button>
+    const callback = jest.fn();
+    render(
+      <ActionSheet className="myCustomClass">
+        <Button onClick={callback}>Accept</Button>
         <Button>Reject</Button>
         <Button>This is my super long text!</Button>
       </ActionSheet>
     );
-    wrapper
-      .find('ui5-button')
-      .first()
-      .instance()
-      // @ts-ignore
-      .fireEvent('click');
-    wrapper.update();
 
-    wrapper
-      .find('ui5-responsive-popover ui5-button')
-      .first()
-      .instance()
-      // @ts-ignore
-      .fireEvent('click');
-    expect(callback.called).toBe(true);
+    fireEvent.click(screen.getByText('Accept'));
+
+    expect(callback).toBeCalled();
   });
 
   test('Test Legacy Ref', () => {
@@ -55,10 +43,9 @@ describe('ActionSheet', () => {
     const ref = (el) => {
       legacyRef = el;
     };
-    const button = <Button />;
-    mount(
-      <ActionSheet ref={ref} openBy={button}>
-        <Button icon={'add'}>Accept</Button>
+    render(
+      <ActionSheet ref={ref}>
+        <Button>Accept</Button>
         <Button>Reject</Button>
         <Button>This is my super long text!</Button>
       </ActionSheet>
@@ -68,19 +55,17 @@ describe('ActionSheet', () => {
 
   test('Ref object', () => {
     const ref: RefObject<Ui5PopoverDomRef> = createRef();
-    const button = <Button />;
-    mount(<ActionSheet ref={ref} openBy={button} />);
+    render(<ActionSheet ref={ref} />);
     expect((ref.current as any).tagName).toEqual('UI5-RESPONSIVE-POPOVER');
   });
 
   test('does not crash with other component', () => {
-    const button = <Button />;
-    const wrapper = mount(
-      <ActionSheet openBy={button}>
+    const { container } = render(
+      <ActionSheet>
         <Label>I should not crash</Label>
       </ActionSheet>
     );
-    expect(wrapper.render()).toMatchSnapshot();
+    expect(container.parentElement).toMatchSnapshot();
   });
 
   createPassThroughPropsTest(ActionSheet);

packages/main/src/components/ActionSheet/__snapshots__/ActionSheet.test.tsx.snap

@@ -1,46 +1,51 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`ActionSheet Render without Crashing 1`] = `
-<ui5-responsive-popover
-  class="ActionSheet-actionSheet-0 myCustomClass"
-  horizontal-align="Center"
-  placement-type="Right"
-  vertical-align="Center"
->
-  <ui5-button
-    data-is-action-sheet-button=""
-    design="Transparent"
-    icon="add"
+<body>
+  <div />
+  <ui5-responsive-popover
+    class="ActionSheet-actionSheet myCustomClass"
+    horizontal-align="Center"
+    placement-type="Right"
+    vertical-align="Center"
   >
-    Accept
-  </ui5-button>
-  <ui5-button
-    data-is-action-sheet-button=""
-    design="Transparent"
-  >
-    Reject
-  </ui5-button>
-  <ui5-button
-    data-is-action-sheet-button=""
-    design="Transparent"
-  >
-    This is my super long text!
-  </ui5-button>
-</ui5-responsive-popover>
+    <ui5-button
+      data-is-action-sheet-button=""
+      design="Transparent"
+    >
+      Accept
+    </ui5-button>
+    <ui5-button
+      data-is-action-sheet-button=""
+      design="Transparent"
+    >
+      Reject
+    </ui5-button>
+    <ui5-button
+      data-is-action-sheet-button=""
+      design="Transparent"
+    >
+      This is my super long text!
+    </ui5-button>
+  </ui5-responsive-popover>
+</body>
 `;
 
 exports[`ActionSheet does not crash with other component 1`] = `
-<ui5-responsive-popover
-  class="ActionSheet-actionSheet-0"
-  horizontal-align="Center"
-  placement-type="Right"
-  vertical-align="Center"
->
-  <ui5-label
-    data-is-action-sheet-button=""
-    design="Transparent"
+<body>
+  <div />
+  <ui5-responsive-popover
+    class="ActionSheet-actionSheet"
+    horizontal-align="Center"
+    placement-type="Right"
+    vertical-align="Center"
   >
-    I should not crash
-  </ui5-label>
-</ui5-responsive-popover>
+    <ui5-label
+      data-is-action-sheet-button=""
+      design="Transparent"
+    >
+      I should not crash
+    </ui5-label>
+  </ui5-responsive-popover>
+</body>
 `;

packages/main/src/components/ActionSheet/demo.stories.tsx

@@ -4,16 +4,15 @@ import { StyleClassHelper } from '@ui5/webcomponents-react-base/lib/StyleClassHe
 import { useConsolidatedRef } from '@ui5/webcomponents-react-base/lib/useConsolidatedRef';
 import { usePassThroughHtmlProps } from '@ui5/webcomponents-react-base/lib/usePassThroughHtmlProps';
 import { ButtonDesign } from '@ui5/webcomponents-react/lib/ButtonDesign';
-import { PlacementType } from '@ui5/webcomponents-react/lib/PlacementType';
 import { ResponsivePopover } from '@ui5/webcomponents-react/lib/ResponsivePopover';
 import React, { Children, cloneElement, FC, forwardRef, ReactElement, RefObject } from 'react';
+import { createPortal } from 'react-dom';
 import { Ui5ResponsivePopoverDomRef } from '../../interfaces/Ui5ResponsivePopoverDomRef';
 import { ButtonPropTypes } from '../../webComponents/Button';
 import { ResponsivePopoverPropTypes } from '../../webComponents/ResponsivePopover';
 import styles from './ActionSheet.jss';
 
-export interface ActionSheetPropTypes extends ResponsivePopoverPropTypes {
-  placement?: PlacementType;
+export interface ActionSheetPropTypes extends Omit<ResponsivePopoverPropTypes, 'children'> {
   children?: ReactElement<ButtonPropTypes> | ReactElement<ButtonPropTypes>[];
 }
 
@@ -56,10 +55,7 @@ const ActionSheet: FC<ActionSheetPropTypes> = forwardRef(
 
     const classes = useStyles();
 
-    const actionSheetClasses = StyleClassHelper.of(classes.actionSheet);
-    if (className) {
-      actionSheetClasses.put(className);
-    }
+    const actionSheetClasses = StyleClassHelper.of(classes.actionSheet).putIfPresent(className);
 
     const popoverRef: RefObject<Ui5ResponsivePopoverDomRef> = useConsolidatedRef(ref);
 
@@ -86,12 +82,12 @@ const ActionSheet: FC<ActionSheetPropTypes> = forwardRef(
       'onBeforeOpen'
     ]);
 
-    return (
+    return createPortal(
       <ResponsivePopover
         ref={popoverRef}
         style={style}
         slot={slot}
-        className={actionSheetClasses.valueOf()}
+        className={actionSheetClasses.className}
         allowTargetOverlap={allowTargetOverlap}
         headerText={headerText}
         horizontalAlign={horizontalAlign}
@@ -109,14 +105,12 @@ const ActionSheet: FC<ActionSheetPropTypes> = forwardRef(
         {...passThroughProps}
       >
         {Children.map(children, renderActionSheetButton)}
-      </ResponsivePopover>
+      </ResponsivePopover>,
+      document.body
     );
   }
 );
 
-ActionSheet.defaultProps = {
-  placement: PlacementType.Bottom
-};
 ActionSheet.displayName = 'ActionSheet';
 
 export { ActionSheet };