Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 }} />);
Expand Down Expand Up @@ -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;
}
));
}
AlexanderMelox marked this conversation as resolved.
Show resolved Hide resolved

it('renders description', async () => {
render(<Ts {...{ ...props, closeIconDescription, description }} />);
screen.getByText(descriptionFragment);
Expand Down
35 changes: 9 additions & 26 deletions packages/ibm-products/src/components/Tearsheet/Tearsheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -194,7 +183,7 @@ export let Tearsheet = React.forwardRef(
influencerPosition = 'left',
influencerWidth = 'narrow',
...rest
}: TearsheetProps & CloseIconDescriptionTypes,
}: TearsheetProps,
ref: ForwardedRef<HTMLDivElement>
) => (
<TearsheetShell
Expand Down Expand Up @@ -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.
Expand All @@ -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,

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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,

/**
Expand Down
26 changes: 11 additions & 15 deletions packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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`
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -247,7 +245,7 @@ export const TearsheetShell = React.forwardRef(
ariaLabel,
children,
className,
closeIconDescription,
closeIconDescription = 'Close',
currentStep,
description,
hasCloseIcon,
Expand All @@ -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();
Expand Down Expand Up @@ -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.
Expand Down
Loading