refactor(Popovers): Remove custom logic (#408)

Remove custom open / openBy logic from Dialog, Popover and Responsive Popover and use the plain UI5 Web Component.
BREAKING CHANGE: **Dialog**: Remove property `open`. You can now open a dialog by attaching a ref to the dialog and call the method `open()`.
BREAKING CHANGE: **Popover**: Removed props `openBy`, `openByStyle`, `open` and `propagateOpenByClickEvent`. For opening a popover after e.g. a button click attach a ref to the popover and use the button click hander for calling `.openBy(event.target)` on the popover ref.
BREAKING CHANGE: **ResponsivePopover**: Removed props `openBy`, `openByStyle`, `open` and `propagateOpenByClickEvent`. For opening a responsive popover after e.g. a button click attach a ref to the responsive popover and use the Button click hander for calling `.open(event.target)` on the responsive popover ref.

Author: MarcusNotheis

netlify.toml

@@ -13,6 +13,8 @@
   # Default build command.
   command = "npm run build:storybook"
 
+  environment = { UI5_WEBCOMPONENTS_FOR_REACT_RELEASE_BUILD = "true" }
+
 # Deploy Preview context: all deploys generated from a pull/merge request will
 # inherit these settings.
 [context.deploy-preview]

packages/main/src/components/ActionSheet/ActionSheet.jss.ts

@@ -3,18 +3,12 @@ const styles = {
    *  is being applied to the encapsulating ul element
    */
   actionSheet: {
-    listStyleType: 'none',
     margin: 0,
     padding: '0.1875rem 0.375rem',
-    '&$tablet,&$phone': {
-      padding: '0.25rem 0.5rem'
-    },
     '& ui5-button': {
       display: 'block'
     }
-  },
-  phone: {},
-  tablet: {}
+  }
 };
 
 export default styles;

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

@@ -41,7 +41,7 @@ describe('ActionSheet', () => {
     wrapper.update();
 
     wrapper
-      .find('ui5-popover ui5-button')
+      .find('ui5-responsive-popover ui5-button')
       .first()
       .instance()
       // @ts-ignore
@@ -63,14 +63,14 @@ describe('ActionSheet', () => {
         <Button>This is my super long text!</Button>
       </ActionSheet>
     );
-    expect(legacyRef.tagName).toEqual('UI5-POPOVER');
+    expect(legacyRef.tagName).toEqual('UI5-RESPONSIVE-POPOVER');
   });
 
   test('Ref object', () => {
     const ref: RefObject<Ui5PopoverDomRef> = createRef();
     const button = <Button />;
     mount(<ActionSheet ref={ref} openBy={button} />);
-    expect((ref.current as any).tagName).toEqual('UI5-POPOVER');
+    expect((ref.current as any).tagName).toEqual('UI5-RESPONSIVE-POPOVER');
   });
 
   test('does not crash with other component', () => {

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

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

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

@@ -2,22 +2,31 @@ import { select } from '@storybook/addon-knobs';
 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 React from 'react';
+import React, { useCallback, useRef } from 'react';
 
 export default {
   title: 'Components / ActionSheet',
   component: ActionSheet
 };
 
-export const defaultStory = () => (
-  <ActionSheet
-    openBy={<Button>Open ActionSheet</Button>}
-    placement={select('placement', [PlacementType.Top, PlacementType.Bottom], PlacementType.Top)}
-  >
-    <Button icon="add">Accept</Button>
-    <Button>Reject</Button>
-    <Button>This is my super long text!</Button>
-  </ActionSheet>
-);
+export const defaultStory = () => {
+  const actionSheetRef = useRef();
+  const onButtonClick = useCallback(
+    (e) => {
+      actionSheetRef.current.open(e.target);
+    },
+    [actionSheetRef]
+  );
+  return (
+    <>
+      <Button onClick={onButtonClick}>Open ActionSheet</Button>
+      <ActionSheet ref={actionSheetRef} placementType={select('placementType', PlacementType, PlacementType.Bottom)}>
+        <Button icon="add">Accept</Button>
+        <Button>Reject</Button>
+        <Button>This is my super long text!</Button>
+      </ActionSheet>
+    </>
+  );
+};
 
 defaultStory.story = { name: 'Default' };

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

@@ -1,20 +1,18 @@
 import { addCustomCSS } from '@ui5/webcomponents-base/dist/Theming';
-import { Device } from '@ui5/webcomponents-react-base/lib/Device';
+import { createComponentStyles } from '@ui5/webcomponents-react-base/lib/createComponentStyles';
 import { StyleClassHelper } from '@ui5/webcomponents-react-base/lib/StyleClassHelper';
 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 { Popover } from '@ui5/webcomponents-react/lib/Popover';
-import React, { Children, cloneElement, FC, forwardRef, ReactElement, ReactNode, RefObject } from 'react';
-import { createComponentStyles } from '@ui5/webcomponents-react-base/lib/createComponentStyles';
-import { CommonProps } from '../../interfaces/CommonProps';
-import { Ui5PopoverDomRef } from '../../interfaces/Ui5PopoverDomRef';
+import { ResponsivePopover } from '@ui5/webcomponents-react/lib/ResponsivePopover';
+import React, { Children, cloneElement, FC, forwardRef, ReactElement, RefObject } from 'react';
+import { Ui5ResponsivePopoverDomRef } from '../../interfaces/Ui5ResponsivePopoverDomRef';
 import { ButtonPropTypes } from '../../webComponents/Button';
+import { ResponsivePopoverPropTypes } from '../../webComponents/ResponsivePopover';
 import styles from './ActionSheet.jss';
 
-export interface ActionSheetPropTypes extends CommonProps {
-  openBy: ReactNode;
+export interface ActionSheetPropTypes extends ResponsivePopoverPropTypes {
   placement?: PlacementType;
   children?: ReactElement<ButtonPropTypes> | ReactElement<ButtonPropTypes>[];
 }
@@ -34,22 +32,36 @@ addCustomCSS(
  * <code>import { ActionSheet } from '@ui5/webcomponents-react/lib/ActionSheet';</code>
  */
 const ActionSheet: FC<ActionSheetPropTypes> = forwardRef(
-  (props: ActionSheetPropTypes, ref: RefObject<Ui5PopoverDomRef>) => {
-    const { children, placement, openBy, style, slot, className } = props;
+  (props: ActionSheetPropTypes, ref: RefObject<Ui5ResponsivePopoverDomRef>) => {
+    const {
+      children,
+      style,
+      slot,
+      className,
+      allowTargetOverlap,
+      headerText,
+      horizontalAlign,
+      initialFocus,
+      modal,
+      noArrow,
+      placementType,
+      verticalAlign,
+      footer,
+      header,
+      onAfterClose,
+      onAfterOpen,
+      onBeforeClose,
+      onBeforeOpen
+    } = props;
 
     const classes = useStyles();
 
     const actionSheetClasses = StyleClassHelper.of(classes.actionSheet);
     if (className) {
       actionSheetClasses.put(className);
     }
-    if (Device.system.tablet) {
-      actionSheetClasses.put(classes.tablet);
-    } else if (Device.system.phone) {
-      actionSheetClasses.put(classes.phone);
-    }
 
-    const popoverRef: RefObject<Ui5PopoverDomRef> = useConsolidatedRef(ref);
+    const popoverRef: RefObject<Ui5ResponsivePopoverDomRef> = useConsolidatedRef(ref);
 
     const onActionButtonClicked = (handler) => (e) => {
       popoverRef.current.close();
@@ -67,20 +79,37 @@ const ActionSheet: FC<ActionSheetPropTypes> = forwardRef(
       });
     };
 
-    const passThroughProps = usePassThroughHtmlProps(props);
+    const passThroughProps = usePassThroughHtmlProps(props, [
+      'onAfterClose',
+      'onAfterOpen',
+      'onBeforeClose',
+      'onBeforeOpen'
+    ]);
 
     return (
-      <Popover
+      <ResponsivePopover
         ref={popoverRef}
-        openBy={openBy}
-        placementType={placement}
         style={style}
         slot={slot}
         className={actionSheetClasses.valueOf()}
+        allowTargetOverlap={allowTargetOverlap}
+        headerText={headerText}
+        horizontalAlign={horizontalAlign}
+        initialFocus={initialFocus}
+        modal={modal}
+        noArrow={noArrow}
+        placementType={placementType}
+        verticalAlign={verticalAlign}
+        footer={footer}
+        header={header}
+        onAfterClose={onAfterClose}
+        onAfterOpen={onAfterOpen}
+        onBeforeClose={onBeforeClose}
+        onBeforeOpen={onBeforeOpen}
         {...passThroughProps}
       >
         {Children.map(children, renderActionSheetButton)}
-      </Popover>
+      </ResponsivePopover>
     );
   }
 );

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

@@ -230,7 +230,7 @@ describe('AnalyticalTable', () => {
     const wrapper = mount(<AnalyticalTable data={data} title={'Test'} columns={columns} />);
 
     // get first column of the table and simulate dragging of it
-    let componentDrag = wrapper.find('div[role="columnheader"] div[draggable]').at(0);
+    let componentDrag = wrapper.find('div[role="columnheader"][draggable]').at(0);
     let inst = componentDrag.instance();
     // @ts-ignore
     let dragColumnId = inst.dataset.columnId;
@@ -248,7 +248,7 @@ describe('AnalyticalTable', () => {
     dataTransfer.getData = () => {
       return dragColumnId;
     };
-    let componentDrop = wrapper.find('div[role="columnheader"] div[draggable]').at(1);
+    let componentDrop = wrapper.find('div[role="columnheader"][draggable]').at(1);
     // @ts-ignore
     componentDrop.simulate('drop', { dataTransfer: dataTransfer });