feat(tearsheet): change icondescription prop to optional (#6662)

* feat(tearsheet): change icondescription prop to optional * fix(tearsheet): test fail fix --------- Co-authored-by: Alexander Melo <alexandermelox@gmail.com>
carbon-design-system · Jan 15, 2025 · 5ba3e04 · 5ba3e04
1 parent 2273d9b
commit 5ba3e04
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 75 deletions.
diff --git a/packages/ibm-products/src/components/Tearsheet/Tearsheet.test.js b/packages/ibm-products/src/components/Tearsheet/Tearsheet.test.js
@@ -11,7 +11,6 @@ import userEvent from '@testing-library/user-event';
 import {
   expectWarn,
   expectMultipleError,
-  required,
 } from '../../global/js/utils/test-helper';
 
 import uuidv4 from '../../global/js/utils/uuidv4';
@@ -131,7 +130,6 @@ const DummyComponent = ({ props, open }) => {
 // These are tests than apply to both Tearsheet and TearsheetNarrow
 // and also (with extra props and omitting button tests) to CreateTearsheetNarrow
 let tooManyButtonsTestedAlready = false;
-let closeIconDescriptionTestedAlready = false;
 const commonTests = (Ts, name, props, testActions) => {
   it(`renders a component ${name}`, async () => {
     render(<Ts {...{ ...props, closeIconDescription }} />);
@@ -229,24 +227,6 @@ const commonTests = (Ts, name, props, testActions) => {
     screen.getByRole('button', { name: closeIconDescription });
   });
 
-  if (testActions) {
-    it('requires closeIconDescription when there are no actions', async () =>
-      expectMultipleError(
-        // prop-types only reports the first occurrence of each distinct error,
-        // which creates an unfortunate dependency between test runs
-        closeIconDescriptionTestedAlready
-          ? [required('closeIconDescription', name)]
-          : [
-              required('closeIconDescription', name),
-              required('closeIconDescription', 'TearsheetShell'),
-            ],
-        () => {
-          render(<Ts {...props} />);
-          closeIconDescriptionTestedAlready = true;
-        }
-      ));
-  }
-
   it('renders description', async () => {
     render(<Ts {...{ ...props, closeIconDescription, description }} />);
     screen.getByText(descriptionFragment);

diff --git a/packages/ibm-products/src/components/Tearsheet/Tearsheet.tsx b/packages/ibm-products/src/components/Tearsheet/Tearsheet.tsx
@@ -9,7 +9,7 @@
 import { Button, type ButtonProps } from '@carbon/react';
 // Import portions of React that are needed.
 import React, { ForwardedRef, PropsWithChildren, ReactNode } from 'react';
-import { TearsheetShell, tearsheetHasCloseIcon } from './TearsheetShell';
+import { TearsheetShell } from './TearsheetShell';
 
 import { ActionSet } from '../ActionSet';
 // Other standard imports.
@@ -22,23 +22,6 @@ import { portalType } from './TearsheetShell';
 const componentName = 'Tearsheet';
 
 // NOTE: the component SCSS is not imported here: it is rolled up separately.
-
-/**
- * The accessibility title for the close icon (if shown).
- *
- * **Note:** This prop is only required if a close icon is shown, i.e. if
- * there are a no navigation actions and/or hasCloseIcon is true.
- */
-export type CloseIconDescriptionTypes =
-  | {
-      hasCloseIcon?: false;
-      closeIconDescription?: string;
-    }
-  | {
-      hasCloseIcon: true;
-      closeIconDescription: string;
-    };
-
 // The types and DocGen commentary for the component props,
 // in alphabetical order (for consistency).
 // See https://www.npmjs.com/package/prop-types#usage.
@@ -76,6 +59,12 @@ export interface TearsheetProps extends PropsWithChildren {
    */
   className?: string;
 
+  /**
+   * The accessibility title for the close icon (if shown).
+   *
+   */
+  closeIconDescription?: string;
+
   /**
    * A description of the flow, displayed in the header area of the tearsheet.
    */
@@ -194,7 +183,7 @@ export let Tearsheet = React.forwardRef(
       influencerPosition = 'left',
       influencerWidth = 'narrow',
       ...rest
-    }: TearsheetProps & CloseIconDescriptionTypes,
+    }: TearsheetProps,
     ref: ForwardedRef<HTMLDivElement>
   ) => (
     <TearsheetShell
@@ -287,13 +276,8 @@ Tearsheet.propTypes = {
   /**
    * The accessibility title for the close icon (if shown).
    *
-   * **Note:** This prop is only required if a close icon is shown, i.e. if
-   * there are a no navigation actions and/or hasCloseIcon is true.
    */
-  /**@ts-ignore */
-  closeIconDescription: PropTypes.string.isRequired.if(
-    ({ actions, hasCloseIcon }) => tearsheetHasCloseIcon(actions, hasCloseIcon)
-  ),
+  closeIconDescription: PropTypes.string,
 
   /**
    * A description of the flow, displayed in the header area of the tearsheet.
@@ -308,7 +292,6 @@ Tearsheet.propTypes = {
    * tearsheet"), and that behavior can be overridden if required by setting
    * this prop to either true or false.
    */
-  /**@ts-ignore */
   hasCloseIcon: PropTypes.bool,
 
   /**

diff --git a/packages/ibm-products/src/components/Tearsheet/TearsheetNarrow.tsx b/packages/ibm-products/src/components/Tearsheet/TearsheetNarrow.tsx
@@ -8,10 +8,8 @@
 // Carbon and package components we use.
 import { Button, ButtonProps } from '@carbon/react';
 import {
-  CloseIconDescriptionTypes,
   TearsheetShell,
   tearsheetShellWideProps as blocked,
-  tearsheetHasCloseIcon,
 } from './TearsheetShell';
 // Import portions of React that are needed.
 import React, { ForwardedRef, PropsWithChildren, ReactNode } from 'react';
@@ -24,7 +22,7 @@ import { getDevtoolsProps } from '../../global/js/utils/devtools';
 import { pkg } from '../../settings';
 import { portalType } from './TearsheetShell';
 
-interface TearsheetNarrowBaseProps extends PropsWithChildren {
+export interface TearsheetNarrowProps extends PropsWithChildren {
   /**
    * The navigation actions to be shown as buttons in the action area at the
    * bottom of the tearsheet. Each action is specified as an object with
@@ -51,6 +49,12 @@ interface TearsheetNarrowBaseProps extends PropsWithChildren {
    */
   className?: string;
 
+  /**
+   * The accessibility title for the close icon (if shown).
+   *
+   */
+  closeIconDescription?: string;
+
   /**
    * A description of the flow, displayed in the header area of the tearsheet.
    */
@@ -118,9 +122,6 @@ interface TearsheetNarrowBaseProps extends PropsWithChildren {
   verticalPosition?: 'normal' | 'lower';
 }
 
-export type TearsheetNarrowProps = TearsheetNarrowBaseProps &
-  CloseIconDescriptionTypes;
-
 const componentName = 'TearsheetNarrow';
 
 // NOTE: the component SCSS is not imported here: it is rolled up separately.
@@ -236,14 +237,8 @@ TearsheetNarrow.propTypes = {
   /**
    * The accessibility title for the close icon (if shown).
    *
-   * **Note:** This prop is only required if a close icon is shown, i.e. if
-   * there are a no navigation actions and/or hasCloseIcon is true.
    */
-  /**@ts-ignore */
-  closeIconDescription: PropTypes.string.isRequired.if(
-    ({ actions, hasCloseIcon }) => tearsheetHasCloseIcon(actions, hasCloseIcon)
-  ),
-
+  closeIconDescription: PropTypes.string,
   /**
    * A description of the flow, displayed in the header area of the tearsheet.
    */
@@ -255,7 +250,6 @@ TearsheetNarrow.propTypes = {
    * the tearsheet is read-only or has no navigation actions (sometimes called
    * a "passive tearsheet").
    */
-  /**@ts-ignore*/
   hasCloseIcon: PropTypes.bool,
 
   /**

diff --git a/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx b/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx
@@ -59,6 +59,14 @@ interface TearsheetShellProps extends PropsWithChildren {
    */
   className?: string;
 
+  /**
+   * The accessibility title for the close icon (if shown).
+   *
+   * **Note:** This prop is only required if a close icon is shown, i.e. if
+   * there are a no navigation actions and/or hasCloseIcon is true.
+   */
+  closeIconDescription?: string;
+
   /**
    * Used to track the current step on components which use `StepsContext` and `TearsheetShell`
    */
@@ -185,16 +193,6 @@ interface TearsheetShellProps extends PropsWithChildren {
   slug?: ReactNode;
 }
 
-export type CloseIconDescriptionTypes =
-  | {
-      hasCloseIcon?: false;
-      closeIconDescription?: string;
-    }
-  | {
-      hasCloseIcon: true;
-      closeIconDescription: string;
-    };
-
 // NOTE: the component SCSS is not imported here: it is rolled up separately.
 
 // Global data structure to communicate the state of tearsheet stacking
@@ -247,7 +245,7 @@ export const TearsheetShell = React.forwardRef(
       ariaLabel,
       children,
       className,
-      closeIconDescription,
+      closeIconDescription = 'Close',
       currentStep,
       description,
       hasCloseIcon,
@@ -270,7 +268,7 @@ export const TearsheetShell = React.forwardRef(
       launcherButtonRef,
       // Collect any other property values passed in.
       ...rest
-    }: TearsheetShellProps & CloseIconDescriptionTypes,
+    }: TearsheetShellProps,
     ref: ForwardedRef<HTMLDivElement>
   ) => {
     const carbonPrefix = usePrefix();
@@ -653,9 +651,7 @@ TearsheetShell.propTypes = {
    * there are a no navigation actions and/or hasCloseIcon is true.
    */
   /**@ts-ignore*/
-  closeIconDescription: PropTypes.string.isRequired.if(
-    ({ actions, hasCloseIcon }) => tearsheetHasCloseIcon(actions, hasCloseIcon)
-  ),
+  closeIconDescription: PropTypes.string,
 
   /**
    * Optional prop that allows you to pass any component.