alternative proposal

mui · Feb 28, 2019 · af5b132 · af5b132
1 parent 0f06000
commit af5b132
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 33 deletions.
diff --git a/packages/material-ui/src/Icon/Icon.js b/packages/material-ui/src/Icon/Icon.js
@@ -8,7 +8,7 @@ export const styles = theme => ({
   /* Styles applied to the root element. */
   root: {
     userSelect: 'none',
-    fontSize: 24,
+    fontSize: theme.typography.pxToRem(24),
     width: '1em',
     height: '1em',
     // Chrome fix for https://bugs.chromium.org/p/chromium/issues/detail?id=820541
@@ -41,11 +41,11 @@ export const styles = theme => ({
   },
   /* Styles applied to the root element if `fontSize="small"`. */
   fontSizeSmall: {
-    fontSize: 20,
+    fontSize: theme.typography.pxToRem(20),
   },
   /* Styles applied to the root element if `fontSize="large"`. */
   fontSizeLarge: {
-    fontSize: 36,
+    fontSize: theme.typography.pxToRem(36),
   },
 });
 

diff --git a/packages/material-ui/src/IconButton/IconButton.d.ts b/packages/material-ui/src/IconButton/IconButton.d.ts
@@ -15,6 +15,7 @@ export type IconButtonClassKey =
   | 'colorPrimary'
   | 'colorSecondary'
   | 'disabled'
+  | 'sizeSmall'
   | 'label';
 
 declare const IconButton: React.ComponentType<IconButtonProps>;

diff --git a/packages/material-ui/src/IconButton/IconButton.js b/packages/material-ui/src/IconButton/IconButton.js
@@ -64,21 +64,18 @@ export const styles = theme => ({
   },
   /* Styles applied to the root element if `disabled={true}`. */
   disabled: {},
+  /* Styles applied to the root element if `size="small"`. */
+  sizeSmall: {
+    padding: 3,
+    fontSize: theme.typography.pxToRem(20),
+  },
   /* Styles applied to the children container element. */
   label: {
     width: '100%',
     display: 'flex',
     alignItems: 'inherit',
     justifyContent: 'inherit',
   },
-  /* Styles applied to the root element if `size="small"`. */
-  sizeSmall: {
-    padding: 3,
-    minWidth: 24,
-    fontSize: 18,
-  },
-  /* Styles applied to the root element if `size="medium"`. */
-  sizeMedium: {},
 });
 
 /**
@@ -95,8 +92,8 @@ function IconButton(props) {
         {
           [classes[`color${capitalize(color)}`]]: color !== 'default',
           [classes.disabled]: disabled,
+          [classes[`size${capitalize(size)}`]]: size !== 'medium',
         },
-        classes[`size${capitalize(size)}`],
         className,
       )}
       centerRipple

diff --git a/packages/material-ui/src/IconButton/IconButton.test.js b/packages/material-ui/src/IconButton/IconButton.test.js
@@ -3,7 +3,12 @@ import ReactDOM from 'react-dom';
 import { spy } from 'sinon';
 import { assert } from 'chai';
 import PropTypes from 'prop-types';
-import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils';
+import {
+  createShallow,
+  createMount,
+  getClasses,
+  findOutermostIntrinsic,
+} from '@material-ui/core/test-utils';
 import consoleErrorMock from 'test/utils/consoleErrorMock';
 import Icon from '../Icon';
 import ButtonBase from '../ButtonBase';
@@ -85,22 +90,16 @@ describe('<IconButton />', () => {
     assert.strictEqual(wrapper.props().centerRipple, true);
   });
 
-  it('should render with sizeSmall class when prop size="small" provided', () => {
-    const wrapper = shallow(<IconButton size="small">book</IconButton>);
-    assert.strictEqual(wrapper.hasClass(classes.sizeSmall), true);
-    assert.strictEqual(wrapper.hasClass(classes.sizeMedium), false);
-  });
-
-  it('should render with sizeMedium class when prop size="medium" provided', () => {
-    const wrapper = shallow(<IconButton size="medium">book</IconButton>);
-    assert.strictEqual(wrapper.hasClass(classes.sizeSmall), false);
-    assert.strictEqual(wrapper.hasClass(classes.sizeMedium), true);
-  });
-
-  it('should render with sizeMedium class when no size is provided', () => {
-    const wrapper = shallow(<IconButton size="medium">book</IconButton>);
-    assert.strictEqual(wrapper.hasClass(classes.sizeSmall), false);
-    assert.strictEqual(wrapper.hasClass(classes.sizeMedium), true);
+  describe('prop: size', () => {
+    it('should render the right class', () => {
+      let wrapper;
+      wrapper = mount(<IconButton size="small">book</IconButton>);
+      assert.strictEqual(findOutermostIntrinsic(wrapper).hasClass(classes.sizeSmall), true);
+      wrapper = mount(<IconButton size="medium">book</IconButton>);
+      assert.strictEqual(findOutermostIntrinsic(wrapper).hasClass(classes.sizeSmall), false);
+      wrapper = mount(<IconButton>book</IconButton>);
+      assert.strictEqual(findOutermostIntrinsic(wrapper).hasClass(classes.sizeSmall), false);
+    });
   });
 
   describe('prop: disabled', () => {

diff --git a/packages/material-ui/src/SvgIcon/SvgIcon.js b/packages/material-ui/src/SvgIcon/SvgIcon.js
@@ -13,7 +13,7 @@ export const styles = theme => ({
     display: 'inline-block',
     fill: 'currentColor',
     flexShrink: 0,
-    fontSize: 24,
+    fontSize: theme.typography.pxToRem(24),
     transition: theme.transitions.create('fill', {
       duration: theme.transitions.duration.shorter,
     }),
@@ -44,11 +44,11 @@ export const styles = theme => ({
   },
   /* Styles applied to the root element if `fontSize="small"`. */
   fontSizeSmall: {
-    fontSize: 20,
+    fontSize: theme.typography.pxToRem(20),
   },
   /* Styles applied to the root element if `fontSize="large"`. */
   fontSizeLarge: {
-    fontSize: 35,
+    fontSize: theme.typography.pxToRem(35),
   },
 });
 

diff --git a/pages/api/icon-button.md b/pages/api/icon-button.md
@@ -41,8 +41,8 @@ This property accepts the following keys:
 | <span class="prop-name">colorPrimary</span> | Styles applied to the root element if `color="primary"`.
 | <span class="prop-name">colorSecondary</span> | Styles applied to the root element if `color="secondary"`.
 | <span class="prop-name">disabled</span> | Styles applied to the root element if `disabled={true}`.
-| <span class="prop-name">label</span> | Styles applied to the children container element.
 | <span class="prop-name">sizeSmall</span> | Styles applied to the root element if `size="small"`.
+| <span class="prop-name">label</span> | Styles applied to the children container element.
 
 Have a look at [overriding with classes](/customization/overrides/#overriding-with-classes) section
 and the [implementation of the component](https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/IconButton/IconButton.js)