fix(react): iconButtons have reiterate classNames (#15626)

* fix(react): iconButtons have reiterate classNames * fix(react): wrong alignment with button icon only and danger--ghost kind * fix(react): iconButtons have reiterate className --------- Co-authored-by: TJ Egan <tw15egan@gmail.com> Co-authored-by: Andrea N. Cardona <cardona.n.andrea@gmail.com>
carbon-design-system · Mar 27, 2024 · e4626be · e4626be
1 parent 6ee35d3
commit e4626be
Show file tree

Hide file tree

Showing 3 changed files with 147 additions and 107 deletions.
diff --git a/packages/react/src/components/Button/Button.tsx b/packages/react/src/components/Button/Button.tsx
@@ -7,13 +7,11 @@
 
 import PropTypes from 'prop-types';
 import React, { useRef } from 'react';
-import classNames from 'classnames';
 import { IconButton, IconButtonKind } from '../IconButton';
 import { composeEventHandlers } from '../../tools/events';
-import { usePrefix } from '../../internal/usePrefix';
-import { useId } from '../../internal/useId';
 import { PolymorphicProps } from '../../types/common';
 import { PopoverAlignment } from '../Popover';
+import ButtonBase from './ButtonBase';
 
 export const ButtonKinds = [
   'primary',
@@ -40,7 +38,7 @@ export const ButtonTooltipPositions = ['top', 'right', 'bottom', 'left'];
 
 export type ButtonTooltipPosition = (typeof ButtonTooltipPositions)[number];
 
-interface ButtonBaseProps
+export interface ButtonBaseProps
   extends React.ButtonHTMLAttributes<HTMLButtonElement> {
   /**
    * Specify the message read by screen readers for the danger button variant
@@ -124,17 +122,15 @@ function isIconOnlyButton(
 }
 
 const Button = React.forwardRef(function Button<T extends React.ElementType>(
-  {
+  props: ButtonProps<T>,
+  ref: React.Ref<unknown>
+) {
+  const tooltipRef = useRef(null);
+  const {
     as,
     children,
-    className,
-    dangerDescription = 'danger',
-    disabled = false,
     hasIconOnly = false,
-    href,
     iconDescription,
-    isExpressive = false,
-    isSelected,
     kind = 'primary',
     onBlur,
     onClick,
@@ -143,16 +139,10 @@ const Button = React.forwardRef(function Button<T extends React.ElementType>(
     onMouseLeave,
     renderIcon: ButtonImageElement,
     size,
-    tabIndex,
     tooltipAlignment = 'center',
     tooltipPosition = 'top',
-    type = 'button',
     ...rest
-  }: ButtonProps<T>,
-  ref: React.Ref<unknown>
-) {
-  const tooltipRef = useRef(null);
-  const prefix = usePrefix();
+  } = props;
 
   const handleClick = (evt: React.MouseEvent) => {
     // Prevent clicks on the tooltip from triggering the button click event
@@ -161,92 +151,10 @@ const Button = React.forwardRef(function Button<T extends React.ElementType>(
     }
   };
 
-  const buttonClasses = classNames(className, {
-    [`${prefix}--btn`]: true,
-    [`${prefix}--btn--sm`]: size === 'sm' && !isExpressive, // TODO: V12 - Remove this class
-    [`${prefix}--btn--md`]: size === 'md' && !isExpressive, // TODO: V12 - Remove this class
-    [`${prefix}--btn--xl`]: size === 'xl', // TODO: V12 - Remove this class
-    [`${prefix}--btn--2xl`]: size === '2xl', // TODO: V12 - Remove this class
-    [`${prefix}--layout--size-${size}`]: size,
-    [`${prefix}--btn--${kind}`]: kind,
-    [`${prefix}--btn--disabled`]: disabled,
-    [`${prefix}--btn--expressive`]: isExpressive,
-    [`${prefix}--btn--icon-only`]: hasIconOnly,
-    [`${prefix}--btn--selected`]: hasIconOnly && isSelected && kind === 'ghost',
-  });
-
-  const commonProps = {
-    tabIndex,
-    className: buttonClasses,
-    ref,
-  };
-
-  const buttonImage = !ButtonImageElement ? null : (
-    <ButtonImageElement
-      aria-label={iconDescription}
-      className={`${prefix}--btn__icon`}
-      aria-hidden="true"
-    />
-  );
-
   const iconOnlyImage = !ButtonImageElement ? null : <ButtonImageElement />;
 
-  const dangerButtonVariants = ['danger', 'danger--tertiary', 'danger--ghost'];
-
-  let component: React.ElementType = 'button';
-  const assistiveId = useId('danger-description');
-  const { 'aria-pressed': ariaPressed, 'aria-describedby': ariaDescribedBy } =
-    rest;
-  let otherProps: Partial<ButtonBaseProps> = {
-    disabled,
-    type,
-    'aria-describedby': dangerButtonVariants.includes(kind)
-      ? assistiveId
-      : ariaDescribedBy || undefined,
-    'aria-pressed':
-      ariaPressed ?? (hasIconOnly && kind === 'ghost' ? isSelected : undefined),
-  };
-  const anchorProps = {
-    href,
-  };
-
-  let assistiveText: JSX.Element | null = null;
-  if (dangerButtonVariants.includes(kind)) {
-    assistiveText = (
-      <span id={assistiveId} className={`${prefix}--visually-hidden`}>
-        {dangerDescription}
-      </span>
-    );
-  }
-
-  if (as) {
-    component = as;
-    otherProps = {
-      ...otherProps,
-      ...anchorProps,
-    };
-  } else if (href && !disabled) {
-    component = 'a';
-    otherProps = anchorProps;
-  }
-
   if (!isIconOnlyButton(hasIconOnly, kind)) {
-    return React.createElement(
-      component,
-      {
-        onMouseEnter,
-        onMouseLeave,
-        onFocus,
-        onBlur,
-        onClick,
-        ...rest,
-        ...commonProps,
-        ...otherProps,
-      },
-      assistiveText,
-      children,
-      buttonImage
-    );
+    return <ButtonBase ref={ref} {...props} />;
   } else {
     let align: PopoverAlignment | undefined = undefined;
 
@@ -268,6 +176,8 @@ const Button = React.forwardRef(function Button<T extends React.ElementType>(
 
     return (
       <IconButton
+        {...rest}
+        ref={ref}
         as={as}
         align={align}
         label={iconDescription}
@@ -278,9 +188,8 @@ const Button = React.forwardRef(function Button<T extends React.ElementType>(
         onFocus={onFocus}
         onBlur={onBlur}
         onClick={composeEventHandlers([onClick, handleClick])}
-        {...rest}
-        {...commonProps}
-        {...otherProps}>
+        renderIcon={iconOnlyImage ? null : ButtonImageElement} // avoid doubling the icon.
+      >
         {iconOnlyImage ?? children}
       </IconButton>
     );

diff --git a/packages/react/src/components/Button/ButtonBase.tsx b/packages/react/src/components/Button/ButtonBase.tsx
@@ -0,0 +1,130 @@
+/**
+ * Copyright IBM Corp. 2016, 2023
+ *
+ * This source code is licensed under the Apache-2.0 license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import React from 'react';
+import classNames from 'classnames';
+import { usePrefix } from '../../internal/usePrefix';
+import { useId } from '../../internal/useId';
+import { ButtonBaseProps, ButtonProps } from './Button';
+
+const ButtonBase = React.forwardRef(function ButtonBase<
+  T extends React.ElementType
+>(
+  {
+    as,
+    children,
+    className,
+    dangerDescription = 'danger',
+    disabled = false,
+    hasIconOnly = false,
+    href,
+    iconDescription,
+    isExpressive = false,
+    isSelected,
+    kind = 'primary',
+    onBlur,
+    onClick,
+    onFocus,
+    onMouseEnter,
+    onMouseLeave,
+    renderIcon: ButtonImageElement,
+    size,
+    tabIndex,
+    type = 'button',
+    ...rest
+  }: ButtonProps<T>,
+  ref: React.Ref<unknown>
+) {
+  const prefix = usePrefix();
+
+  const buttonClasses = classNames(className, {
+    [`${prefix}--btn`]: true,
+    [`${prefix}--btn--sm`]: size === 'sm' && !isExpressive, // TODO: V12 - Remove this class
+    [`${prefix}--btn--md`]: size === 'md' && !isExpressive, // TODO: V12 - Remove this class
+    [`${prefix}--btn--xl`]: size === 'xl', // TODO: V12 - Remove this class
+    [`${prefix}--btn--2xl`]: size === '2xl', // TODO: V12 - Remove this class
+    [`${prefix}--layout--size-${size}`]: size,
+    [`${prefix}--btn--${kind}`]: kind,
+    [`${prefix}--btn--disabled`]: disabled,
+    [`${prefix}--btn--expressive`]: isExpressive,
+    [`${prefix}--btn--icon-only`]:
+      hasIconOnly && !className?.includes(`${prefix}--btn--icon-only`),
+    [`${prefix}--btn--selected`]: hasIconOnly && isSelected && kind === 'ghost',
+  });
+
+  const commonProps = {
+    tabIndex,
+    className: buttonClasses,
+    ref,
+  };
+
+  const buttonImage = !ButtonImageElement ? null : (
+    <ButtonImageElement
+      aria-label={iconDescription}
+      className={`${prefix}--btn__icon`}
+      aria-hidden="true"
+    />
+  );
+
+  const dangerButtonVariants = ['danger', 'danger--tertiary', 'danger--ghost'];
+
+  let component: React.ElementType = 'button';
+  const assistiveId = useId('danger-description');
+  const { 'aria-pressed': ariaPressed, 'aria-describedby': ariaDescribedBy } =
+    rest;
+  let otherProps: Partial<ButtonBaseProps> = {
+    disabled,
+    type,
+    'aria-describedby': dangerButtonVariants.includes(kind)
+      ? assistiveId
+      : ariaDescribedBy || undefined,
+    'aria-pressed':
+      ariaPressed ?? (hasIconOnly && kind === 'ghost' ? isSelected : undefined),
+  };
+  const anchorProps = {
+    href,
+  };
+
+  let assistiveText: JSX.Element | null = null;
+  if (dangerButtonVariants.includes(kind)) {
+    assistiveText = (
+      <span id={assistiveId} className={`${prefix}--visually-hidden`}>
+        {dangerDescription}
+      </span>
+    );
+  }
+
+  if (as) {
+    component = as;
+    otherProps = {
+      ...otherProps,
+      ...anchorProps,
+    };
+  } else if (href && !disabled) {
+    component = 'a';
+    otherProps = anchorProps;
+  }
+
+  return React.createElement(
+    component,
+    {
+      onMouseEnter,
+      onMouseLeave,
+      onFocus,
+      onBlur,
+      onClick,
+      ...rest,
+      ...commonProps,
+      ...otherProps,
+    },
+    assistiveText,
+    children,
+    buttonImage
+  );
+});
+
+export default ButtonBase;
diff --git a/packages/react/src/components/IconButton/index.tsx b/packages/react/src/components/IconButton/index.tsx
@@ -7,10 +7,11 @@
 
 import PropTypes, { ReactNodeLike } from 'prop-types';
 import React, { ForwardedRef } from 'react';
-import Button, { ButtonSize } from '../Button';
+import { ButtonSize } from '../Button';
 import classNames from 'classnames';
 import { Tooltip } from '../Tooltip';
 import { usePrefix } from '../../internal/usePrefix';
+import ButtonBase from '../Button/ButtonBase';
 
 export const IconButtonKinds = [
   'primary',
@@ -139,7 +140,7 @@ const IconButton = React.forwardRef(function IconButton(
       enterDelayMs={enterDelayMs}
       label={label}
       leaveDelayMs={leaveDelayMs}>
-      <Button
+      <ButtonBase
         {...rest}
         disabled={disabled}
         kind={kind}
@@ -153,7 +154,7 @@ const IconButton = React.forwardRef(function IconButton(
           className
         )}>
         {children}
-      </Button>
+      </ButtonBase>
     </Tooltip>
   );
 });