refactor(ActionSheet): api alignment (#5956)

BREAKING CHANGE: the `portalContainer` prop has been removed as it's not needed anymore BREAKING CHANGE: the `showCancelButton` has been renamed to `hideCancelButton` and the logic has been inverted. --------- Co-authored-by: Lukas Harbarth <[email protected]>
SAP · Jun 25, 2024 · 5b2ac63 · 5b2ac63
1 parent 841e1e9
commit 5b2ac63
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 97 deletions.
diff --git a/docs/MigrationGuide.mdx b/docs/MigrationGuide.mdx
@@ -263,4 +263,36 @@ Please use the following components instead:
 
 Also, the namings of internal `data-component-name` attributes have been adjusted accordingly. E.g. `data-component-name="DynamicPageTitleSubHeader"` has been renamed to `data-component-name="ObjectPageTitleSubHeader"`
 
+## Components with API Changes
+
+### ActionSheet
+
+The prop `portalContainer` has been removed as it is no longer needed due to the [popover API](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API) which is now used in the UI5 Web Components.
+For a better aligned API, the `showCancelButton` prop has been replaced wih the `hideCancelButton` prop and the logic has been inverted.
+You only need to apply changes to your code if `showCancelButton` has been set to `false`.
+
+```tsx
+// v1
+import { ActionSheet, Button } from '@ui5/webcomponents-react';
+
+function MyComponent() {
+  return (
+    <ActionSheet showCancelButton={false}>
+      <Button>Action 1</Button>
+    </ActionSheet>
+  );
+}
+
+// v2
+import { ActionSheet, Button } from '@ui5/webcomponents-react';
+
+function MyComponent() {
+  return (
+    <ActionSheet hideCancelButton>
+      <Button>Action 1</Button>
+    </ActionSheet>
+  );
+}
+```
+
 <Footer />
diff --git a/packages/cli/src/scripts/codemod/transforms/v2/codemodConfig.json b/packages/cli/src/scripts/codemod/transforms/v2/codemodConfig.json
@@ -11,8 +11,10 @@
       "changedProps": {
         "onAfterClose": "onClose",
         "onAfterOpen": "onOpen",
-        "placementType": "placement"
-      }
+        "placementType": "placement",
+        "showCancelButton": "hideCancelButton"
+      },
+      "removedProps": ["portalContainer"]
     },
     "Avatar": {},
     "Badge": {

diff --git a/packages/cli/src/scripts/codemod/transforms/v2/main.cts b/packages/cli/src/scripts/codemod/transforms/v2/main.cts
@@ -91,6 +91,27 @@ export default function transform(file: FileInfo, api: API, options?: Options):
       return;
     }
 
+    if (componentName === 'ActionSheet') {
+      jsxElements.forEach((el) => {
+        const showCancelButton = j(el).find(j.JSXAttribute, { name: { name: 'showCancelButton' } });
+        if (showCancelButton.size() > 0) {
+          const attr = showCancelButton.get();
+          if (
+            attr.value.value &&
+            ((attr.value.value.type === 'JSXAttribute' && attr.value.value === false) ||
+              (attr.value.value.type === 'JSXExpressionContainer' && attr.value.value.expression.value === false))
+          ) {
+            j(el)
+              .find(j.JSXOpeningElement)
+              .get()
+              .value.attributes.push(j.jsxAttribute(j.jsxIdentifier('hideCancelButton'), null));
+          }
+          showCancelButton.remove();
+          isDirty = true;
+        }
+      });
+    }
+
     // Special Handling for logic inversions, etc.
     if (componentName === 'Button') {
       jsxElements.forEach((el) => {

diff --git a/packages/main/src/components/ActionSheet/ActionSheet.mdx b/packages/main/src/components/ActionSheet/ActionSheet.mdx
@@ -21,12 +21,9 @@ import * as ComponentStories from './ActionSheet.stories';
 
 <br />
 
-#### since 0.22.0
+You can open and close the `ActionSheet` component in a declarative way using the `open` and `opener` prop.
 
-We recommend opening and closing the `ActionSheet` component in a declarative way using the `open` and `opener` prop.
-You can still use the imperative way which is outlined below.
-
-```jsx
+```tsx
 const MyComponentWithActionSheet = () => {
   const [actionSheetIsOpen, setActionSheetIsOpen] = useState(false);
   return (
@@ -53,7 +50,8 @@ const MyComponentWithActionSheet = () => {
 
 **Opening an `ActionSheet` by reference and not by `id`**
 
-This web component exposes a way to pass a reference of an element instead of an `id` to the `opener` prop. Since this is not supported when passing the reference in a declarative way to a React `prop`, you have to attach the ref directly on the web component.
+The `ActionSheet` exposes a way to pass a reference of an element instead of an `id` to the `opener` prop.
+Since this is not supported when passing the reference in a declarative way to a React `prop`, you have to attach the ref directly on the web component.
 You can do that by e.g. leveraging a React Ref, and then set the corresponding property there.
 
 <MessageStrip
@@ -83,54 +81,4 @@ const ActionSheetComponent = () => {
 };
 ```
 
-#### before 0.22.0
-
-`ActionSheets` can only be opened by attaching a `ref` to the component and then call the corresponding **`showAt`** method. The method receives the target element - _on which the `ActionSheet` is to be opened_ - as parameter.
-
-```jsx
-const ActionSheetComponent = () => {
-  const actionSheetRef = useRef(null);
-  const onButtonClick = (e) => {
-    actionSheetRef.current.showAt(e.target);
-  };
-  return (
-    <>
-      <Button onClick={onButtonClick}>Open ActionSheet</Button>
-      <ActionSheet ref={actionSheetRef}>
-        <Button icon="add">Accept</Button>
-        <Button>Reject</Button>
-        <Button>This is my super long text!</Button>
-      </ActionSheet>
-    </>
-  );
-};
-```
-
-## Using ActionSheets inside other components
-
-`ActionSheets` are often used within other components, when opened this could sometimes have unwanted side effects.
-In this case, we recommend using [createPortal](https://reactjs.org/docs/portals.html) to mount the `ActionSheet` outside of the DOM hierarchy of the parent component.
-
-```JSX
-const ActionSheetComponent = () => {
-  const actionSheetRef = useRef(null);
-  const onButtonClick = (e) => {
-    actionSheetRef.current.showAt(e.target);
-  };
-  return (
-    <>
-      <Button onClick={onButtonClick}>Open ActionSheet</Button>
-      {createPortal(
-        <ActionSheet ref={actionSheetRef}>
-          <Button icon="add">Accept</Button>
-          <Button>Reject</Button>
-          <Button>This is my super long text!</Button>
-        </ActionSheet>,
-        document.body
-      )}
-    </>
-  );
-};
-```
-
 <Footer />
diff --git a/packages/main/src/components/ActionSheet/ActionSheet.stories.tsx b/packages/main/src/components/ActionSheet/ActionSheet.stories.tsx
@@ -7,7 +7,7 @@ import declineIcon from '@ui5/webcomponents-icons/dist/decline.js';
 import deleteIcon from '@ui5/webcomponents-icons/dist/delete.js';
 import emailIcon from '@ui5/webcomponents-icons/dist/email.js';
 import forwardIcon from '@ui5/webcomponents-icons/dist/forward.js';
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 import { Button } from '../../webComponents/index.js';
 import { ActionSheet } from './index.js';
 
@@ -41,10 +41,12 @@ const meta = {
 export default meta;
 type Story = StoryObj<typeof meta>;
 
-//TODO: check docs for outdated info
 export const Default: Story = {
   render(args) {
     const [actionSheetOpen, setActionSheetOpen] = useState<boolean | undefined>(args.open);
+    useEffect(() => {
+      setActionSheetOpen(args.open);
+    }, [args.open]);
     return (
       <>
         <Button

diff --git a/packages/main/src/components/ActionSheet/index.tsx b/packages/main/src/components/ActionSheet/index.tsx
@@ -6,11 +6,9 @@ import { useI18nBundle, useStylesheet } from '@ui5/webcomponents-react-base';
 import { clsx } from 'clsx';
 import type { ReactElement } from 'react';
 import { forwardRef, useEffect, useReducer, useRef, useState } from 'react';
-import { createPortal } from 'react-dom';
 import { AVAILABLE_ACTIONS, CANCEL, X_OF_Y } from '../../i18n/i18n-defaults.js';
 import { addCustomCSSWithScoping } from '../../internal/addCustomCSSWithScoping.js';
-import { useCanRenderPortal } from '../../internal/ssr.js';
-import { flattenFragments } from '../../internal/utils.js';
+import { flattenFragments, getUi5TagWithSuffix } from '../../internal/utils.js';
 import { CustomThemingParameters } from '../../themes/CustomVariables.js';
 import type { UI5WCSlotsNode } from '../../types/index.js';
 import type {
@@ -42,9 +40,9 @@ export interface ActionSheetPropTypes extends Omit<ResponsivePopoverPropTypes, '
    */
   children?: ReactElement<ButtonPropTypes> | ReactElement<ButtonPropTypes>[];
   /**
-   * Displays a cancel button below the action buttons on mobile devices. No cancel button will be shown on desktop and tablet devices.
+   * Hides the cancel button below the action buttons on mobile devices. No cancel button will be shown on desktop and tablet devices.
    */
-  showCancelButton?: boolean;
+  hideCancelButton?: boolean;
   /**
    * Defines internally used a11y properties.
    */
@@ -53,14 +51,6 @@ export interface ActionSheetPropTypes extends Omit<ResponsivePopoverPropTypes, '
       role?: string;
     };
   };
-  /**
-   * Defines where modals are rendered into via `React.createPortal`.
-   *
-   * You can find out more about this [here](https://sap.github.io/ui5-webcomponents-react/?path=/docs/knowledge-base-working-with-portals--page).
-   *
-   * Defaults to: `document.body`
-   */
-  portalContainer?: Element;
 }
 
 if (isPhone()) {
@@ -126,18 +116,7 @@ function ActionSheetButton(props: ActionSheetButtonPropTypes) {
  *
  */
 const ActionSheet = forwardRef<ResponsivePopoverDomRef, ActionSheetPropTypes>((props, ref) => {
-  const {
-    a11yConfig,
-    children,
-    className,
-    header,
-    headerText,
-    portalContainer,
-    showCancelButton = true,
-    onOpen,
-    open,
-    ...rest
-  } = props;
+  const { a11yConfig, children, className, header, headerText, hideCancelButton, onOpen, open, ...rest } = props;
 
   useStylesheet(styleData, ActionSheet.displayName);
 
@@ -148,18 +127,16 @@ const ActionSheet = forwardRef<ResponsivePopoverDomRef, ActionSheetPropTypes>((p
   }, 0);
   const childrenToRender = flattenFragments(children);
   const childrenArrayLength = childrenToRender.length;
-  const childrenLength = isPhone() && showCancelButton ? childrenArrayLength + 1 : childrenArrayLength;
+  const childrenLength = isPhone() && !hideCancelButton ? childrenArrayLength + 1 : childrenArrayLength;
 
-  const [internalOpen, setInternalOpen] = useState(open);
+  const [internalOpen, setInternalOpen] = useState(undefined);
   useEffect(() => {
-    setInternalOpen(open);
+    const tagName = getUi5TagWithSuffix('ui5-responsive-popover');
+    void customElements.whenDefined(tagName).then(() => {
+      setInternalOpen(open);
+    });
   }, [open]);
 
-  const canRenderPortal = useCanRenderPortal();
-  if (!canRenderPortal) {
-    return null;
-  }
-
   const handleCancelBtnClick = () => {
     setInternalOpen(false);
   };
@@ -237,7 +214,7 @@ const ActionSheet = forwardRef<ResponsivePopoverDomRef, ActionSheetPropTypes>((p
   };
 
   const displayHeader = isPhone();
-  return createPortal(
+  return (
     <ResponsivePopover
       open={internalOpen}
       headerText={displayHeader ? headerText : undefined}
@@ -257,7 +234,7 @@ const ActionSheet = forwardRef<ResponsivePopoverDomRef, ActionSheetPropTypes>((p
         ref={actionBtnsRef}
       >
         {childrenToRender.map(renderActionSheetButton)}
-        {isPhone() && showCancelButton && (
+        {isPhone() && !hideCancelButton && (
           <Button
             design={ButtonDesign.Negative}
             onClick={handleCancelBtnClick}
@@ -270,8 +247,7 @@ const ActionSheet = forwardRef<ResponsivePopoverDomRef, ActionSheetPropTypes>((p
           </Button>
         )}
       </div>
-    </ResponsivePopover>,
-    portalContainer ?? document.body
+    </ResponsivePopover>
   );
 });