feat(components): support custom icon renderer (#1908)

* feat(components): support custom icon renderer

Fixes #1750, #1749.

* chore(Button): change code style of the story

src/components/Button/Button-story.js

@@ -12,28 +12,29 @@ import { withKnobs, boolean, select } from '@storybook/addon-knobs';
 import { iconAddSolid, iconSearch } from 'carbon-icons';
 import AddFilled16 from '@carbon/icons-react/lib/add--filled/16';
 import Search16 from '@carbon/icons-react/lib/search/16';
-import { settings } from 'carbon-components';
 import Button from '../Button';
 import ButtonSkeleton from '../Button/Button.Skeleton';
-import { componentsX } from '../../internal/FeatureFlags';
-
-const { prefix } = settings;
+import { breakingChangesX } from '../../internal/FeatureFlags';
 
 const icons = {
   None: 'None',
-  'Add with filled circle (iconAddSolid from `carbon-icons`)': componentsX
-    ? 'AddFilled16'
-    : 'iconAddSolid',
-  'Search (iconSearch from `carbon-icons`)': componentsX
-    ? 'Search16'
-    : 'iconSearch',
 };
 
+if (breakingChangesX) {
+  icons['Add with filled circle (iconAddSolid from `carbon-icons`)'] =
+    'iconAddSolid';
+  icons['Search (iconSearch from `carbon-icons`)'] = 'iconSearch';
+}
+
+icons['Add with filled circle (AddFilled16 from `@carbon/icons`)'] =
+  'AddFilled16';
+icons['Search (Search16 from `@carbon/icons`)'] = 'Search16';
+
 const iconMap = {
   iconAddSolid,
   iconSearch,
-  AddFilled16: <AddFilled16 className={`${prefix}--btn__icon`} />,
-  Search16: <Search16 className={`${prefix}--btn__icon`} />,
+  AddFilled16,
+  Search16,
 };
 
 const kinds = {
@@ -45,23 +46,31 @@ const kinds = {
 };
 
 const props = {
-  regular: () => ({
-    className: 'some-class',
-    kind: select('Button kind (kind)', kinds, 'primary'),
-    disabled: boolean('Disabled (disabled)', false),
-    small: boolean('Small (small)', false),
-    icon: iconMap[select('Icon (icon)', icons, 'none')],
-    onClick: action('onClick'),
-    onFocus: action('onFocus'),
-  }),
-  set: () => ({
-    className: 'some-class',
-    disabled: boolean('Disabled (disabled)', false),
-    small: boolean('Small (small)', false),
-    icon: iconMap[select('Icon (icon)', icons, 'none')],
-    onClick: action('onClick'),
-    onFocus: action('onFocus'),
-  }),
+  regular: () => {
+    const iconToUse = iconMap[select('Icon (icon)', icons, 'none')];
+    return {
+      className: 'some-class',
+      kind: select('Button kind (kind)', kinds, 'primary'),
+      disabled: boolean('Disabled (disabled)', false),
+      small: boolean('Small (small)', false),
+      renderIcon: !iconToUse || iconToUse.svgData ? undefined : iconToUse,
+      icon: !iconToUse || !iconToUse.svgData ? undefined : iconToUse,
+      onClick: action('onClick'),
+      onFocus: action('onFocus'),
+    };
+  },
+  set: () => {
+    const iconToUse = iconMap[select('Icon (icon)', icons, 'none')];
+    return {
+      className: 'some-class',
+      disabled: boolean('Disabled (disabled)', false),
+      small: boolean('Small (small)', false),
+      renderIcon: !iconToUse || iconToUse.svgData ? undefined : iconToUse,
+      icon: !iconToUse || !iconToUse.svgData ? undefined : iconToUse,
+      onClick: action('onClick'),
+      onFocus: action('onFocus'),
+    };
+  },
 };
 
 const CustomLink = ({ children, href, ...other }) => (

src/components/Button/Button-test.js

@@ -7,6 +7,7 @@
 
 import React from 'react';
 import { iconSearch } from 'carbon-icons';
+import Search16 from '@carbon/icons-react/lib/search/16';
 import Button from '../Button';
 import Link from '../Link';
 import ButtonSkeleton from '../Button/Button.Skeleton';
@@ -121,8 +122,35 @@ describe('Button', () => {
       </Button>
     );
     const icon = iconButton.find('svg');
+
+    it('should have the appropriate icon', () => {
+      expect(icon.hasClass('bx--btn__icon')).toBe(true);
+    });
+
+    it('should return error if icon given without description', () => {
+      const props = {
+        icon: 'search',
+      };
+      // eslint-disable-next-line quotes
+      const error = new Error(
+        'icon/renderIcon property specified without also providing an iconDescription property.'
+      );
+      expect(Button.propTypes.iconDescription(props)).toEqual(error);
+    });
+  });
+
+  describe('Renders custom icon buttons', () => {
+    const iconButton = mount(
+      <Button renderIcon={Search16} iconDescription="Search">
+        Search
+      </Button>
+    );
+    const originalIcon = mount(<Search16 />).find('svg');
+    const icon = iconButton.find('svg');
+
     it('should have the appropriate icon', () => {
       expect(icon.hasClass('bx--btn__icon')).toBe(true);
+      expect(icon.children().html()).toBe(originalIcon.children().html());
     });
 
     it('should return error if icon given without description', () => {
@@ -131,7 +159,7 @@ describe('Button', () => {
       };
       // eslint-disable-next-line quotes
       const error = new Error(
-        'icon property specified without also providing an iconDescription property.'
+        'icon/renderIcon property specified without also providing an iconDescription property.'
       );
       expect(Button.propTypes.iconDescription(props)).toEqual(error);
     });

src/components/Button/Button.js

@@ -9,12 +9,15 @@ import PropTypes from 'prop-types';
 import React from 'react';
 import Icon from '../Icon';
 import classNames from 'classnames';
+import warning from 'warning';
 import { settings } from 'carbon-components';
 import { ButtonTypes } from '../../prop-types/types';
-import { componentsX } from '../../internal/FeatureFlags';
+import { breakingChangesX } from '../../internal/FeatureFlags';
 
 const { prefix } = settings;
 
+let didWarnAboutDeprecation = false;
+
 const Button = ({
   children,
   as,
@@ -26,6 +29,7 @@ const Button = ({
   tabIndex,
   type,
   inputref,
+  renderIcon,
   icon,
   iconDescription,
   ...other
@@ -46,23 +50,30 @@ const Button = ({
     className: buttonClasses,
     ref: inputref,
   };
-  const buttonImage = (() => {
-    if (componentsX && icon && React.isValidElement(icon)) {
-      return icon;
-    }
-    if (!componentsX && icon) {
-      return (
-        <Icon
-          icon={Object(icon) === icon ? icon : undefined}
-          name={Object(icon) !== icon ? icon : undefined}
-          description={iconDescription}
-          className={`${prefix}--btn__icon`}
-          aria-hidden="true"
-        />
-      );
-    }
-    return null;
-  })();
+
+  if (__DEV__ && breakingChangesX && icon) {
+    warning(
+      didWarnAboutDeprecation,
+      'The `icon` property in the `Button` component is being removed in the next release of ' +
+        '`carbon-components-react`. Please use `renderIcon` instead.'
+    );
+    didWarnAboutDeprecation = true;
+  }
+
+  const hasRenderIcon = Object(renderIcon) === renderIcon;
+  const ButtonImageElement = hasRenderIcon
+    ? renderIcon
+    : !breakingChangesX && icon && Icon;
+  const buttonImage = !ButtonImageElement ? null : (
+    <ButtonImageElement
+      icon={!hasRenderIcon && Object(icon) === icon ? icon : undefined}
+      name={!hasRenderIcon && Object(icon) !== icon ? icon : undefined}
+      aria-label={!hasRenderIcon ? undefined : iconDescription}
+      description={hasRenderIcon ? undefined : iconDescription}
+      className={`${prefix}--btn__icon`}
+      aria-hidden={true}
+    />
+  );
 
   let component = 'button';
   let otherProps = {
@@ -147,6 +158,12 @@ Button.propTypes = {
    */
   role: PropTypes.string,
 
+  /**
+   * Optional prop to allow overriding the icon rendering.
+   * Can be a React component class
+   */
+  renderIcon: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
+
   /**
    * Specify an icon to include in the Button through a string or object
    * representing the SVG data of the icon
@@ -167,9 +184,9 @@ Button.propTypes = {
    * be read by screen readers
    */
   iconDescription: props => {
-    if (props.icon && !props.iconDescription) {
+    if ((props.icon || props.renderIcon) && !props.iconDescription) {
       return new Error(
-        'icon property specified without also providing an iconDescription property.'
+        'icon/renderIcon property specified without also providing an iconDescription property.'
       );
     }
     return undefined;

src/components/DataTable/TableToolbarAction.js

@@ -6,34 +6,46 @@
  */
 
 import cx from 'classnames';
+import warning from 'warning';
 import PropTypes from 'prop-types';
 import React from 'react';
 import { settings } from 'carbon-components';
 import Icon from '../Icon';
 import isRequiredOneOf from '../../prop-types/isRequiredOneOf';
-import { componentsX } from '../../internal/FeatureFlags';
+import { breakingChangesX } from '../../internal/FeatureFlags';
 
 const { prefix } = settings;
 
+let didWarnAboutDeprecation = false;
+
 const TableToolbarAction = ({
   className,
+  renderIcon,
   icon,
   iconName,
   iconDescription,
   ...rest
 }) => {
+  if (__DEV__ && breakingChangesX && (icon || iconName)) {
+    warning(
+      didWarnAboutDeprecation,
+      'The `icon`/`iconName` properties in the `TableToolbarAction` component is being removed in the next release of ' +
+        '`carbon-components-react`. Please use `renderIcon` instead.'
+    );
+    didWarnAboutDeprecation = true;
+  }
+
   const toolbarActionClasses = cx(className, `${prefix}--toolbar-action`);
   const tableToolbarActionIcon = (() => {
-    if (componentsX && icon) {
-      const IconTag = icon;
+    if (Object(renderIcon) === renderIcon) {
+      const IconTag = renderIcon;
       return (
         <IconTag
           className={`${prefix}--toolbar-action__icon`}
           aria-label={iconDescription}
         />
       );
-    }
-    if (!componentsX && icon) {
+    } else if (!breakingChangesX && (icon || iconName)) {
       return (
         <Icon
           className={`${prefix}--toolbar-action__icon`}
@@ -57,6 +69,12 @@ TableToolbarAction.propTypes = {
   className: PropTypes.string,
 
   ...isRequiredOneOf({
+    /**
+     * Optional prop to allow overriding the toolbar icon rendering.
+     * Can be a React component class
+     */
+    renderIcon: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
+
     /**
      * Specify the icon for the toolbar action
      */

src/components/DataTable/__tests__/TableToolbarAction-test.js

@@ -8,6 +8,7 @@
 import React from 'react';
 import { mount } from 'enzyme';
 import { iconAddSolid } from 'carbon-icons';
+import Download16 from '@carbon/icons-react/lib/download/16';
 import { TableToolbarAction } from '../';
 
 describe('DataTable.TableToolbarAction', () => {
@@ -22,3 +23,14 @@ describe('DataTable.TableToolbarAction', () => {
     expect(wrapper).toMatchSnapshot();
   });
 });
+
+describe('Custom icon in DataTable.TableToolbarAction', () => {
+  it('should render', () => {
+    const iconAction = mount(
+      <TableToolbarAction renderIcon={Download16} iconDescription="Download" />
+    );
+    const originalIcon = mount(<Download16 />).find('svg');
+    const icon = iconAction.find('svg');
+    expect(icon.children().html()).toBe(originalIcon.children().html());
+  });
+});

src/components/DataTable/__tests__/__snapshots__/TableBatchAction-test.js.snap

@@ -44,7 +44,7 @@ exports[`DataTable.TableBatchAction should render 1`] = `
       type="button"
     >
       <Icon
-        aria-hidden="true"
+        aria-hidden={true}
         className="bx--btn__icon"
         description="Add"
         fillRule="evenodd"
@@ -75,7 +75,7 @@ exports[`DataTable.TableBatchAction should render 1`] = `
       >
         <svg
           alt="Add"
-          aria-hidden="true"
+          aria-hidden={true}
           aria-label="Add"
           className="bx--btn__icon"
           fillRule="evenodd"

src/components/DataTable/stories/default.js

@@ -8,6 +8,9 @@
 import React from 'react';
 import { action } from '@storybook/addon-actions';
 import { iconDownload, iconEdit, iconSettings } from 'carbon-icons';
+import Download16 from '@carbon/icons-react/lib/download/16';
+import Edit16 from '@carbon/icons-react/lib/edit/16';
+import Settings16 from '@carbon/icons-react/lib/settings/16';
 import Button from '../../Button';
 import DataTable, {
   Table,
@@ -26,9 +29,6 @@ import DataTable, {
   TableToolbarContent,
   TableToolbarSearch,
 } from '../../DataTable';
-import Download16 from '@carbon/icons-react/lib/download/16';
-import Edit16 from '@carbon/icons-react/lib/edit/16';
-import Settings16 from '@carbon/icons-react/lib/settings/16';
 import { batchActionClick, initialRows, headers } from './shared';
 import { componentsX } from '../../../internal/FeatureFlags';
 
@@ -65,17 +65,20 @@ export default ({ short, shouldShowBorder }) => (
           <TableToolbarSearch onChange={onInputChange} />
           <TableToolbarContent>
             <TableToolbarAction
-              icon={componentsX ? Download16 : iconDownload}
+              renderIcon={!componentsX ? undefined : Download16}
+              icon={componentsX ? undefined : iconDownload}
               iconDescription="Download"
               onClick={action('TableToolbarAction - Download')}
             />
             <TableToolbarAction
-              icon={componentsX ? Edit16 : iconEdit}
+              renderIcon={!componentsX ? undefined : Edit16}
+              icon={componentsX ? undefined : iconEdit}
               iconDescription="Edit"
               onClick={action('TableToolbarAction - Edit')}
             />
             <TableToolbarAction
-              icon={componentsX ? Settings16 : iconSettings}
+              renderIcon={!componentsX ? undefined : Settings16}
+              icon={componentsX ? undefined : iconSettings}
               iconDescription="Settings"
               onClick={action('TableToolbarAction - Settings')}
             />