refactor(icon): Add joinClassNames utility

And tweaked deprecation warnings

src/components/Icon/Icon.jsx

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types'
 
 import { deprecate } from '../../warn'
 import capitalize from '../../capitalize'
+import joinClassNames from '../../utils/joinClassNames'
 
 import styles from './Icon.modules.scss'
 
@@ -16,24 +17,24 @@ const Icon = ({ glyph, variant, fixedWidth, size, className, children, ...rest }
   }
 
   if (variant === 'disabled') {
-    deprecate('Icon', '\'disabled\' variant is deprecated.')
+    deprecate('Icon', 'The `disabled` variant is no longer supported. Use one of the supported variants instead.')
   }
 
   if (fixedWidth) {
-    deprecate('Icon', '\'fixedWidth\' prop is deprecated.')
+    deprecate('Icon', 'Creating `fixedWidth` icons is deprecated. Use `size` instead.')
   }
 
   if (children) {
-    deprecate('Icon', '\'children\' prop is deprecated.')
+    deprecate('Icon', 'Passing children components to Icon is deprecated. To add an accessible label use `aria-label`.')
   }
 
-  const classes = [
-    `${styles[`iconCore${capitalize(glyph)}`]}`,
-    `${variant ? styles[variant] : ''}`,
-    `${fixedWidth ? `${styles.fw}` : ''}`,
-    `${size ? `${styles[`size${size}`]}` : ''}`,
-    `${className ? `${className}` : ''}`
-  ].join(' ')
+  const classes = joinClassNames(
+    styles[`iconCore${capitalize(glyph)}`],
+    variant && styles[variant],
+    fixedWidth && styles.fw,
+    size && styles[`size${size}`],
+    className
+  )
 
   return (
     <i
@@ -91,7 +92,7 @@ Icon.propTypes = {
   /**
    * The icon size in pixels.
    */
-  size: PropTypes.oneOf(['16', '24', '48']),
+  size: PropTypes.oneOf([16, 24, 48]),
   /**
    * One or more CSS class names separated by spaces to append onto the icon.
    * Don't advertise as we plan on removing this feature soon.
@@ -107,7 +108,7 @@ Icon.propTypes = {
 Icon.defaultProps = {
   variant: 'inherit',
   fixedWidth: false,
-  size: '24',
+  size: 24,
   className: '',
   children: null
 }

src/components/Icon/__tests__/Icon.spec.jsx

@@ -41,7 +41,7 @@ describe('<Icon />', () => {
   })
 
   it('can be sized', () => {
-    const icon = doShallow({ size: '16' })
+    const icon = doShallow({ size: 16 })
 
     expect(icon).toHaveClassName('size16')
   })
@@ -60,7 +60,7 @@ describe('<Icon />', () => {
   })
 
   describe('deprecated props', () => {
-    afterEach(() => {
+    beforeEach(() => {
       jest.clearAllMocks()
     })
 
@@ -92,12 +92,12 @@ describe('<Icon />', () => {
       expect(deprecate).toHaveBeenCalled()
     })
 
-    // it('deprecates children prop', () => {
-    //   const defaultChildren = '<span>Some content</span>'
-    //   const icon = doShallow({ children: defaultChildren })
-    //
-    //   expect(deprecate).toHaveBeenCalled()
-    // })
+    it('deprecates children prop', () => {
+      const icon = doShallow({ children: 'Some content' })
+
+      expect(icon).toHaveText('Some content')
+      expect(deprecate).toHaveBeenCalled()
+    })
   })
 
   it('provides a label to specific glyphs', () => {

src/components/Icon/__tests__/__snapshots__/Icon.spec.jsx.snap

@@ -3,6 +3,6 @@
 exports[`<Icon /> renders 1`] = `
 <i
   aria-hidden="true"
-  className="iconCoreCheckmark inherit  size24 "
+  className="iconCoreCheckmark inherit size24"
 />
 `;

src/utils/__tests__/joinClassNames.spec.js

@@ -0,0 +1,15 @@
+import joinClassNames from '../joinClassNames'
+
+describe('joinClassNames', () => {
+  it('joins class names with a space in between', () => {
+    const classes = joinClassNames('class-1', 'class-2')
+
+    expect(classes).toEqual('class-1 class-2')
+  })
+
+  it('removes empties', () => {
+    const classes = joinClassNames('class-1', '', undefined, false, null, 'class-2')
+
+    expect(classes).toEqual('class-1 class-2')
+  })
+})

src/utils/joinClassNames.js

@@ -0,0 +1,3 @@
+const joinClassNames = (...classes) => classes.filter(className => className).join(' ')
+
+export default joinClassNames