feat(icon): Change size prop values to numbers only

Deprecate children prop
telus · Sep 22, 2017 · 1c64c04 · 1c64c04
1 parent d0c9248
commit 1c64c04
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 13 deletions.
diff --git a/src/components/Icon/Icon.jsx b/src/components/Icon/Icon.jsx
@@ -23,6 +23,10 @@ const Icon = ({ glyph, variant, fixedWidth, size, className, children, ...rest }
     deprecate('Icon', '\'fixedWidth\' prop is deprecated.')
   }
 
+  if (children) {
+    deprecate('Icon', '\'children\' prop is deprecated.')
+  }
+
   const classes = [
     `${styles[`iconCore${capitalize(glyph)}`]}`,
     `${variant ? styles[variant] : ''}`,
@@ -37,8 +41,7 @@ const Icon = ({ glyph, variant, fixedWidth, size, className, children, ...rest }
       className={classes}
       aria-label={rest['aria-label']}
       aria-hidden={rest['aria-label'] ? undefined : 'true'}
-    >
-      {children}
+    >{children}
     </i>
   )
 }
@@ -86,9 +89,9 @@ Icon.propTypes = {
    */
   fixedWidth: PropTypes.bool,
   /**
-   *
+   * The icon size in pixels.
    */
-  size: PropTypes.oneOf(['16px', '24px', '48px']),
+  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.
@@ -97,14 +100,14 @@ Icon.propTypes = {
    */
   className: PropTypes.string,
   /**
-   * @ignore
+   * @deprecated since v0.23.0
    */
   children: PropTypes.node
 }
 Icon.defaultProps = {
   variant: 'inherit',
   fixedWidth: false,
-  size: '24px',
+  size: '24',
   className: '',
   children: null
 }

diff --git a/src/components/Icon/__tests__/Icon.spec.jsx b/src/components/Icon/__tests__/Icon.spec.jsx
@@ -41,9 +41,9 @@ describe('<Icon />', () => {
   })
 
   it('can be sized', () => {
-    const icon = doShallow({ size: '16px' })
+    const icon = doShallow({ size: '16' })
 
-    expect(icon).toHaveClassName('size16px')
+    expect(icon).toHaveClassName('size16')
   })
 
   it('supports custom CSS classes', () => {
@@ -60,37 +60,44 @@ describe('<Icon />', () => {
   })
 
   describe('deprecated props', () => {
-    it('deprecates className', () => {
+    afterEach(() => {
       jest.clearAllMocks()
+    })
+
+    it('deprecates className', () => {
       const icon = doShallow({ className: 'my-custom-class' })
 
       expect(icon).toHaveProp('className')
       expect(deprecate).toHaveBeenCalled()
     })
 
     it('deprecates style', () => {
-      jest.clearAllMocks()
       const icon = doShallow({ style: 'color: hotpink' })
 
       expect(icon).toHaveProp('style')
       expect(deprecate).toHaveBeenCalled()
     })
 
     it('deprecates disabled variant', () => {
-      jest.clearAllMocks()
       const icon = doShallow({ variant: 'disabled' })
 
       expect(icon).toHaveClassName('disabled')
       expect(deprecate).toHaveBeenCalled()
     })
 
     it('deprecates fixedWidth prop', () => {
-      jest.clearAllMocks()
       const icon = doShallow({ fixedWidth: true })
 
       expect(icon).toHaveClassName('fw')
       expect(deprecate).toHaveBeenCalled()
     })
+
+    // it('deprecates children prop', () => {
+    //   const defaultChildren = '<span>Some content</span>'
+    //   const icon = doShallow({ children: defaultChildren })
+    //
+    //   expect(deprecate).toHaveBeenCalled()
+    // })
   })
 
   it('provides a label to specific glyphs', () => {

diff --git a/src/components/Icon/__tests__/__snapshots__/Icon.spec.jsx.snap b/src/components/Icon/__tests__/__snapshots__/Icon.spec.jsx.snap
@@ -3,6 +3,6 @@
 exports[`<Icon /> renders 1`] = `
 <i
   aria-hidden="true"
-  className="iconCoreCheckmark inherit  size24px "
+  className="iconCoreCheckmark inherit  size24 "
 />
 `;