Replace IconButton label with children (#2326)

sumup-oss · Dec 7, 2023 · 7959570 · 7959570
1 parent 5f032a1
commit 7959570
Show file tree

Hide file tree

Showing 32 changed files with 511 additions and 338 deletions.
diff --git a/.changeset/hot-eggs-agree.md b/.changeset/hot-eggs-agree.md
@@ -2,4 +2,4 @@
 '@sumup/eslint-plugin-circuit-ui': minor
 ---
 
-Added a migration for the IconButton's `icon` prop to the `circuit-ui/no-renamed-props` rule.
+Added a migration for the IconButton's `icon` and `label` props to the `circuit-ui/no-renamed-props` rule.
diff --git a/.changeset/silly-sheep-tease.md b/.changeset/silly-sheep-tease.md
@@ -2,4 +2,4 @@
 '@sumup/eslint-plugin-circuit-ui': minor
 ---
 
-Added migrations for the Avatar, Button, Hamburger, IconButton, ProgressBar, Selector and Spinner size values to the `circuit-ui/no-renamed-props` rule.
+Added migrations for the Avatar, Button, CloseButton, Hamburger, IconButton, ProgressBar, Selector and Spinner size values to the `circuit-ui/no-renamed-props` rule.
diff --git a/.changeset/yellow-books-doubt.md b/.changeset/yellow-books-doubt.md
@@ -0,0 +1,5 @@
+---
+'@sumup/circuit-ui': minor
+---
+
+Replaced the IconButton's and CloseButton's `label` prop with `children`.
diff --git a/packages/circuit-ui/components/Card/components/Header/Header.tsx b/packages/circuit-ui/components/Card/components/Header/Header.tsx
@@ -64,11 +64,9 @@ export const CardHeader = forwardRef<HTMLElement, CardHeaderProps>(
       >
         {children}
         {onClose && closeButtonLabel && (
-          <CloseButton
-            className={classes.close}
-            onClick={onClose}
-            label={closeButtonLabel}
-          />
+          <CloseButton className={classes.close} onClick={onClose}>
+            {closeButtonLabel}
+          </CloseButton>
         )}
       </header>
     );

diff --git a/packages/circuit-ui/components/Carousel/Carousel.tsx b/packages/circuit-ui/components/Carousel/Carousel.tsx
@@ -199,11 +199,12 @@ export function Carousel({
               <ButtonList className={classes.buttons}>
                 <PlayButton
                   paused={state.paused}
-                  label={state.paused ? playButtonLabel : pauseButtonLabel}
                   {...(state.paused
                     ? getPlayControlProps()
                     : getPauseControlProps())}
-                />
+                >
+                  {state.paused ? playButtonLabel : pauseButtonLabel}
+                </PlayButton>
                 <PrevButton
                   label={prevButtonLabel}
                   {...getPreviousControlProps()}

diff --git a/packages/circuit-ui/components/Carousel/components/Buttons/Buttons.spec.tsx b/packages/circuit-ui/components/Carousel/components/Buttons/Buttons.spec.tsx
@@ -23,10 +23,10 @@ describe('Buttons', () => {
   it('should have no accessibility violations', async () => {
     const { container } = render(
       <ButtonList>
-        <PlayButton label="Pause" />
-        <PlayButton label="Play" paused />
-        <PrevButton label="Previous" />
-        <NextButton label="Next" />
+        <PlayButton>Pause</PlayButton>
+        <PlayButton paused>Play</PlayButton>
+        <PrevButton>Previous</PrevButton>
+        <NextButton>Next</NextButton>
       </ButtonList>,
     );
     const actual = await axe(container);

diff --git a/packages/circuit-ui/components/Carousel/components/Buttons/Buttons.stories.tsx b/packages/circuit-ui/components/Carousel/components/Buttons/Buttons.stories.tsx
@@ -26,9 +26,11 @@ export default {
 
 export const AllButtons = () => (
   <ButtonList>
-    <PlayButton label="Pause" onClick={action('on play click')} />
-    <PlayButton label="Play" paused onClick={action('on pause click')} />
-    <PrevButton label="Previous" onClick={action('on previous click')} />
-    <NextButton label="Next" onClick={action('on next click')} />
+    <PlayButton onClick={action('on play click')}>Pause</PlayButton>
+    <PlayButton paused onClick={action('on pause click')}>
+      Play
+    </PlayButton>
+    <PrevButton onClick={action('on previous click')}>Previous</PrevButton>
+    <NextButton onClick={action('on next click')}>Next</NextButton>
   </ButtonList>
 );
diff --git a/packages/circuit-ui/components/Carousel/components/Buttons/Buttons.tsx b/packages/circuit-ui/components/Carousel/components/Buttons/Buttons.tsx
@@ -27,7 +27,7 @@ export const ButtonList = ({ className, ...props }: ButtonListProps) => (
   <div className={clsx(classes['button-list'], className)} {...props} />
 );
 
-type ButtonProps = Omit<IconButtonProps, 'children'>;
+type ButtonProps = Omit<IconButtonProps, 'icon'>;
 
 export const NextButton = (props: ButtonProps) => (
   <IconButton

diff --git a/packages/circuit-ui/components/CloseButton/CloseButton.stories.tsx b/packages/circuit-ui/components/CloseButton/CloseButton.stories.tsx
@@ -25,5 +25,5 @@ export default {
 export const Base = (args: CloseButtonProps) => <CloseButton {...args} />;
 
 Base.args = {
-  label: 'Close',
+  children: 'Close',
 };
diff --git a/packages/circuit-ui/components/CloseButton/CloseButton.tsx b/packages/circuit-ui/components/CloseButton/CloseButton.tsx
@@ -21,21 +21,22 @@ import { IconButton, IconButtonProps } from '../IconButton/IconButton.js';
 
 import classes from './CloseButton.module.css';
 
-export type CloseButtonProps = Omit<IconButtonProps, 'children'>;
+export type CloseButtonProps = Omit<IconButtonProps, 'icon'>;
 
 /**
  * A generic close button.
  */
 export const CloseButton = forwardRef<any, CloseButtonProps>(
-  ({ label = 'Close', className, ...props }, ref) => (
+  ({ label = 'Close', children = label, className, ...props }, ref) => (
     <IconButton
       type="button"
       className={clsx(classes.base, className)}
-      label={label}
       {...props}
       icon={Close}
       ref={ref}
-    />
+    >
+      {children}
+    </IconButton>
   ),
 );
 

diff --git a/packages/circuit-ui/components/Hamburger/Hamburger.tsx b/packages/circuit-ui/components/Hamburger/Hamburger.tsx
@@ -93,16 +93,26 @@ export const Hamburger = forwardRef<any, HamburgerProps>(
     return (
       <IconButton
         {...props}
+        icon={({ size: _size, ...iconProps }) => (
+          // @ts-expect-error This doesn't have to be an SVG.
+          <Skeleton
+            {...iconProps}
+            className={clsx(
+              iconProps.className,
+              classes.skeleton,
+              classes[size],
+            )}
+          >
+            <span className={clsx(classes.base, classes[size])} />
+          </Skeleton>
+        )}
         className={clsx(classes.button, className)}
         size={size}
-        label={isActive ? activeLabel : inactiveLabel}
         type="button"
         aria-pressed={isActive}
         ref={ref}
       >
-        <Skeleton className={clsx(classes.skeleton, classes[size])}>
-          <span className={clsx(classes.base, classes[size])} />
-        </Skeleton>
+        {isActive ? activeLabel : inactiveLabel}
       </IconButton>
     );
   },

diff --git a/packages/circuit-ui/components/IconButton/IconButton.spec.tsx b/packages/circuit-ui/components/IconButton/IconButton.spec.tsx
@@ -25,7 +25,9 @@ describe('IconButton', () => {
   it('should merge a custom class name with the default ones', () => {
     const className = 'foo';
     const { container } = render(
-      <IconButton label="Close" icon={Close} className={className} />,
+      <IconButton icon={Close} className={className}>
+        Close
+      </IconButton>,
     );
     const button = container.querySelector('button');
     expect(button?.className).toContain(className);
@@ -34,14 +36,16 @@ describe('IconButton', () => {
   it('should forward a ref', () => {
     const ref = createRef<HTMLHRElement>();
     const { container } = render(
-      <IconButton label="Close" icon={Close} ref={ref} />,
+      <IconButton icon={Close} ref={ref}>
+        Close
+      </IconButton>,
     );
     const button = container.querySelector('button');
     expect(ref.current).toBe(button);
   });
 
   it('should meet accessibility guidelines', async () => {
-    const { container } = render(<IconButton label="Close" icon={Close} />);
+    const { container } = render(<IconButton icon={Close}>Close</IconButton>);
     const actual = await axe(container);
     expect(actual).toHaveNoViolations();
   });

diff --git a/packages/circuit-ui/components/IconButton/IconButton.stories.tsx b/packages/circuit-ui/components/IconButton/IconButton.stories.tsx
@@ -25,6 +25,6 @@ export default {
 export const Base = (args: IconButtonProps) => <IconButton {...args} />;
 
 Base.args = {
-  label: 'Add',
+  children: 'Add',
   icon: Plus,
 };
diff --git a/packages/circuit-ui/components/IconButton/IconButton.tsx b/packages/circuit-ui/components/IconButton/IconButton.tsx
@@ -25,24 +25,25 @@ import {
   isSufficientlyLabelled,
 } from '../../util/errors.js';
 import { deprecate } from '../../util/logger.js';
+import { isString } from '../../util/type-check.js';
 
 import classes from './IconButton.module.css';
 
 export interface IconButtonProps
   extends Omit<ButtonProps, 'navigationIcon' | 'stretch' | 'children'> {
-  /**
-   * @deprecated
-   *
-   * Use the `icon` prop instead.
-   */
-  children?: ReactElement<IconProps>;
   /**
    * Communicates the action that will be performed when the user interacts
    * with the button. Use one strong, clear imperative verb and follow with a
    * one-word object if needed to clarify.
    * Displayed on hover and accessible to screen readers.
    */
-  label: string;
+  children?: ReactElement<IconProps> | string;
+  /**
+   * @deprecated
+   *
+   * Use the `children` prop instead.
+   */
+  label?: string;
 }
 
 /**
@@ -67,35 +68,42 @@ export const IconButton = forwardRef<any, IconButtonProps>(
 
     let icon: ReactElement;
 
-    if (process.env.NODE_ENV !== 'production' && !Icon && !children) {
-      throw new CircuitError('IconButton', 'The `icon` prop is missing.');
-    }
+    const labelString = isString(children) ? children : label;
 
     if (Icon) {
       icon = <Icon size={iconSize} aria-hidden="true" />;
-    } else {
+    } else if (!isString(children)) {
       const child = Children.only(children);
       icon = cloneElement(child!, {
         'aria-hidden': 'true',
         'size': (child!.props.size as string) || iconSize,
       });
+    } else {
+      throw new CircuitError('IconButton', 'The `icon` prop is missing.');
     }
 
     if (
       process.env.NODE_ENV !== 'production' &&
       process.env.NODE_ENV !== 'test' &&
-      !isSufficientlyLabelled(label)
+      !isSufficientlyLabelled(labelString)
     ) {
       throw new AccessibilityError(
         'IconButton',
-        'The `label` prop is missing or invalid.',
+        'The `children` prop is missing or invalid.',
+      );
+    }
+
+    if (process.env.NODE_ENV !== 'production' && !isString(children)) {
+      deprecate(
+        'IconButton',
+        'The `children` prop has been deprecated to pass the icon. Use the `icon` prop instead.',
       );
     }
 
-    if (process.env.NODE_ENV !== 'production' && children) {
+    if (process.env.NODE_ENV !== 'production' && label) {
       deprecate(
         'IconButton',
-        'The `children` prop has been deprecated. Use the `icon` prop instead.',
+        'The `label` prop has been deprecated. Use the `children` prop instead.',
       );
     }
 
@@ -111,14 +119,14 @@ export const IconButton = forwardRef<any, IconButtonProps>(
 
     return (
       <Button
-        title={label}
+        title={labelString}
         className={clsx(classes[size], className)}
         size={size}
         {...props}
         ref={ref}
       >
         {icon}
-        <span className={utilityClasses.hideVisually}>{label}</span>
+        <span className={utilityClasses.hideVisually}>{labelString}</span>
       </Button>
     );
   },

diff --git a/packages/circuit-ui/components/ImageInput/ImageInput.tsx b/packages/circuit-ui/components/ImageInput/ImageInput.tsx
@@ -264,24 +264,27 @@ export const ImageInput = ({
             size="s"
             variant="primary"
             destructive
-            label={clearButtonLabel}
             onClick={handleClear}
             disabled={isLoading || disabled}
             className={classes.button}
             icon={Delete}
-          />
+          >
+            {clearButtonLabel}
+          </IconButton>
         ) : (
           <IconButton
             type="button"
             size="s"
             variant="primary"
             aria-hidden="true"
             tabIndex={-1}
-            label="-" // We need to pass a label here to prevent IconButton from throwing
             disabled={isLoading || disabled}
             className={clsx(classes.button, classes.add)}
             icon={Plus}
-          />
+          >
+            {/* We need to pass a label here to prevent IconButton from throwing */}
+            -
+          </IconButton>
         )}
         <Spinner
           className={clsx(classes.spinner, isLoading && classes.loading)}

diff --git a/packages/circuit-ui/components/Modal/Modal.tsx b/packages/circuit-ui/components/Modal/Modal.tsx
@@ -134,11 +134,9 @@ export const Modal: ModalComponent<ModalProps> = ({
       <ReactModal {...reactModalProps}>
         <div className={clsx(classes.content, className)} style={style}>
           {!preventClose && closeButtonLabel && (
-            <CloseButton
-              onClick={onClose}
-              label={closeButtonLabel}
-              className={classes.close}
-            />
+            <CloseButton onClick={onClose} className={classes.close}>
+              {closeButtonLabel}
+            </CloseButton>
           )}
 
           {isFunction(children) ? children({ onClose }) : children}

diff --git a/packages/circuit-ui/components/NotificationBanner/NotificationBanner.tsx b/packages/circuit-ui/components/NotificationBanner/NotificationBanner.tsx
@@ -197,12 +197,9 @@ export const NotificationBanner = forwardRef<
         </div>
         {image && image.src && <NotificationImage {...image} />}
         {onClose && closeButtonLabel && (
-          <CloseButton
-            className={classes.close}
-            label={closeButtonLabel}
-            size="s"
-            onClick={onClose}
-          />
+          <CloseButton className={classes.close} size="s" onClick={onClose}>
+            {closeButtonLabel}
+          </CloseButton>
         )}
       </div>
     );

diff --git a/packages/circuit-ui/components/NotificationInline/NotificationInline.tsx b/packages/circuit-ui/components/NotificationInline/NotificationInline.tsx
@@ -180,12 +180,9 @@ export const NotificationInline = forwardRef<
           </div>
 
           {onClose && closeButtonLabel && (
-            <CloseButton
-              className={classes.close}
-              label={closeButtonLabel}
-              size="s"
-              onClick={onClose}
-            />
+            <CloseButton className={classes.close} size="s" onClick={onClose}>
+              {closeButtonLabel}
+            </CloseButton>
           )}
         </div>
       </div>

diff --git a/packages/circuit-ui/components/NotificationModal/NotificationModal.tsx b/packages/circuit-ui/components/NotificationModal/NotificationModal.tsx
@@ -143,11 +143,9 @@ export const NotificationModal: ModalComponent<NotificationModalProps> = ({
   return (
     <ReactModal {...reactModalProps}>
       {!preventClose && closeButtonLabel && (
-        <CloseButton
-          onClick={onClose}
-          label={closeButtonLabel}
-          className={classes.close}
-        />
+        <CloseButton onClick={onClose} className={classes.close}>
+          {closeButtonLabel}
+        </CloseButton>
       )}
       <NotificationImage image={image} />
       <Headline as="h2" size="three" className={classes.headline}>