From 77a025535d686a735b926e47e674f997caa7d41a Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Thu, 28 Feb 2019 18:04:15 +0100 Subject: [PATCH] alternative proposal --- packages/material-ui/src/Icon/Icon.js | 6 ++-- .../src/IconButton/IconButton.d.ts | 1 + .../material-ui/src/IconButton/IconButton.js | 15 ++++----- .../src/IconButton/IconButton.test.js | 33 +++++++++---------- packages/material-ui/src/SvgIcon/SvgIcon.js | 6 ++-- pages/api/icon-button.md | 2 +- 6 files changed, 30 insertions(+), 33 deletions(-) diff --git a/packages/material-ui/src/Icon/Icon.js b/packages/material-ui/src/Icon/Icon.js index ab8044f4ccfcc8..1b47c06a6feb95 100644 --- 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 index 4ab997f1627447..df86e8687f7745 100644 --- 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; diff --git a/packages/material-ui/src/IconButton/IconButton.js b/packages/material-ui/src/IconButton/IconButton.js index a21965de3d0979..1785edbe39ba96 100644 --- a/packages/material-ui/src/IconButton/IconButton.js +++ b/packages/material-ui/src/IconButton/IconButton.js @@ -64,6 +64,11 @@ 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%', @@ -71,14 +76,6 @@ export const styles = theme => ({ 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 @@ const IconButton = React.forwardRef(function IconButton(props, ref) { { [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 index f47056b566ed04..4c3fdd74b6ba47 100644 --- 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('', () => { assert.strictEqual(wrapper.props().centerRipple, true); }); - it('should render with sizeSmall class when prop size="small" provided', () => { - const wrapper = shallow(book); - 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(book); - 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(book); - 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(book); + assert.strictEqual(findOutermostIntrinsic(wrapper).hasClass(classes.sizeSmall), true); + wrapper = mount(book); + assert.strictEqual(findOutermostIntrinsic(wrapper).hasClass(classes.sizeSmall), false); + wrapper = mount(book); + 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 index 086efc9d5904b1..35f4995469d57b 100644 --- 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 index 7d4f6793cc98ca..f795e0e0d03754 100644 --- a/pages/api/icon-button.md +++ b/pages/api/icon-button.md @@ -40,8 +40,8 @@ This property accepts the following keys: | colorPrimary | Styles applied to the root element if `color="primary"`. | colorSecondary | Styles applied to the root element if `color="secondary"`. | disabled | Styles applied to the root element if `disabled={true}`. -| label | Styles applied to the children container element. | sizeSmall | Styles applied to the root element if `size="small"`. +| label | 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)