From d412a851c928f349a4d8090dd828af167b1ae109 Mon Sep 17 00:00:00 2001 From: Matt Brookes Date: Fri, 22 Jun 2018 20:12:27 +0100 Subject: [PATCH] [Button] Add extended FAB variant (#11941) * [POC] Remove theme.spacing.unit --- .../demos/buttons/FloatingActionButtons.js | 8 + packages/material-ui/src/Button/Button.d.ts | 3 +- packages/material-ui/src/Button/Button.js | 75 +++++--- .../material-ui/src/Button/Button.test.js | 171 ++++++++++-------- .../src/styles/MuiThemeProvider.test.js | 8 +- pages/api/button.md | 8 +- 6 files changed, 160 insertions(+), 113 deletions(-) diff --git a/docs/src/pages/demos/buttons/FloatingActionButtons.js b/docs/src/pages/demos/buttons/FloatingActionButtons.js index ae6fd2c0d028eb..ae3fe4236de4e7 100644 --- a/docs/src/pages/demos/buttons/FloatingActionButtons.js +++ b/docs/src/pages/demos/buttons/FloatingActionButtons.js @@ -5,11 +5,15 @@ import Button from '@material-ui/core/Button'; import AddIcon from '@material-ui/icons/Add'; import Icon from '@material-ui/core/Icon'; import DeleteIcon from '@material-ui/icons/Delete'; +import NavigationIcon from '@material-ui/icons/Navigation'; const styles = theme => ({ button: { margin: theme.spacing.unit, }, + extendedIcon: { + marginRight: theme.spacing.unit, + }, }); function FloatingActionButtons(props) { @@ -22,6 +26,10 @@ function FloatingActionButtons(props) { + diff --git a/packages/material-ui/src/Button/Button.d.ts b/packages/material-ui/src/Button/Button.d.ts index a065f8348ea030..a01e336643eaa3 100644 --- a/packages/material-ui/src/Button/Button.d.ts +++ b/packages/material-ui/src/Button/Button.d.ts @@ -13,12 +13,13 @@ export interface ButtonProps extends StandardProps ({ ...theme.typography.button, lineHeight: '1.4em', // Improve readability for multiline button. boxSizing: 'border-box', - minWidth: theme.spacing.unit * 11, + minWidth: 88, minHeight: 36, - padding: `${theme.spacing.unit}px ${theme.spacing.unit * 2}px`, + padding: '8px 16px', borderRadius: 4, color: theme.palette.text.primary, transition: theme.transitions.create(['background-color', 'box-shadow'], { @@ -37,11 +37,11 @@ export const styles = theme => ({ }, }, label: { - width: '100%', display: 'inherit', alignItems: 'inherit', justifyContent: 'inherit', }, + text: {}, textPrimary: { color: theme.palette.primary.main, '&:hover': { @@ -62,17 +62,14 @@ export const styles = theme => ({ }, }, }, - flat: {}, - flatPrimary: {}, - flatSecondary: {}, + flat: {}, // legacy + flatPrimary: {}, // legacy + flatSecondary: {}, // legacy outlined: { border: `1px solid ${ theme.palette.type === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' }`, }, - colorInherit: { - color: 'inherit', - }, contained: { color: theme.palette.getContrastText(theme.palette.grey[300]), backgroundColor: theme.palette.grey[300], @@ -121,36 +118,45 @@ export const styles = theme => ({ }, }, }, - raised: {}, - raisedPrimary: {}, - raisedSecondary: {}, - focusVisible: {}, - disabled: {}, + raised: {}, // legacy + raisedPrimary: {}, // legacy + raisedSecondary: {}, // legacy fab: { borderRadius: '50%', padding: 0, minWidth: 0, width: 56, - fontSize: 24, height: 56, boxShadow: theme.shadows[6], '&:active': { boxShadow: theme.shadows[12], }, }, + extendedFab: { + borderRadius: 24, + padding: '0 16px', + width: 'initial', + minWidth: 48, + height: 48, + }, + focusVisible: {}, + disabled: {}, + colorInherit: { + color: 'inherit', + }, mini: { width: 40, height: 40, }, sizeSmall: { - padding: `${theme.spacing.unit - 1}px ${theme.spacing.unit}px`, - minWidth: theme.spacing.unit * 8, + padding: '7px 8px', + minWidth: 64, minHeight: 32, fontSize: theme.typography.pxToRem(13), }, sizeLarge: { - padding: `${theme.spacing.unit}px ${theme.spacing.unit * 3}px`, - minWidth: theme.spacing.unit * 14, + padding: '8px 24px', + minWidth: 112, minHeight: 40, fontSize: theme.typography.pxToRem(15), }, @@ -175,31 +181,32 @@ function Button(props) { ...other } = props; - const fab = variant === 'fab'; + const fab = variant === 'fab' || variant === 'extendedFab'; const contained = variant === 'contained' || variant === 'raised'; - const text = !contained && !fab; + const text = variant === 'text' || variant === 'flat' || variant === 'outlined'; const className = classNames( classes.root, { - [classes.contained]: contained || fab, [classes.fab]: fab, [classes.mini]: fab && mini, - [classes.colorInherit]: color === 'inherit', + [classes.extendedFab]: variant === 'extendedFab', + [classes.text]: text, [classes.textPrimary]: text && color === 'primary', [classes.textSecondary]: text && color === 'secondary', - [classes.flat]: text, - [classes.flatPrimary]: text && color === 'primary', - [classes.flatSecondary]: text && color === 'secondary', - [classes.containedPrimary]: !text && color === 'primary', - [classes.containedSecondary]: !text && color === 'secondary', + [classes.flat]: variant === 'text' || variant === 'flat', + [classes.flatPrimary]: (variant === 'text' || variant === 'flat') && color === 'primary', + [classes.flatSecondary]: (variant === 'text' || variant === 'flat') && color === 'secondary', + [classes.contained]: contained || fab, + [classes.containedPrimary]: (contained || fab) && color === 'primary', + [classes.containedSecondary]: (contained || fab) && color === 'secondary', [classes.raised]: contained || fab, [classes.raisedPrimary]: (contained || fab) && color === 'primary', [classes.raisedSecondary]: (contained || fab) && color === 'secondary', - [classes.text]: variant === 'text', [classes.outlined]: variant === 'outlined', [classes[`size${capitalize(size)}`]]: size !== 'medium', [classes.disabled]: disabled, [classes.fullWidth]: fullWidth, + [classes.colorInherit]: color === 'inherit', }, classNameProp, ); @@ -282,7 +289,15 @@ Button.propTypes = { /** * The type of button. */ - variant: PropTypes.oneOf(['text', 'flat', 'outlined', 'contained', 'raised', 'fab']), + variant: PropTypes.oneOf([ + 'text', + 'flat', + 'outlined', + 'contained', + 'raised', + 'fab', + 'extendedFab', + ]), }; Button.defaultProps = { diff --git a/packages/material-ui/src/Button/Button.test.js b/packages/material-ui/src/Button/Button.test.js index dbdc334cf45db3..2121e053a4ec5d 100644 --- a/packages/material-ui/src/Button/Button.test.js +++ b/packages/material-ui/src/Button/Button.test.js @@ -28,9 +28,9 @@ describe('); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.flat), true, 'should have the flat class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.flat), true); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.textPrimary), false, @@ -66,7 +66,7 @@ describe('); - assert.strictEqual(wrapper.is('.test-class-name'), true, 'should pass the test className'); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.is('.test-class-name'), true); + assert.strictEqual(wrapper.hasClass(classes.root), true); }); it('should render a primary button', () => { const wrapper = shallow(); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.flat), true, 'should have the flat class'); - assert.strictEqual(wrapper.hasClass(classes.contained), false, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.flat), true); + assert.strictEqual(wrapper.hasClass(classes.contained), false); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.textPrimary), true, @@ -115,10 +115,10 @@ describe('); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.flat), true, 'should have the flat class'); - assert.strictEqual(wrapper.hasClass(classes.contained), false, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.flat), true); + assert.strictEqual(wrapper.hasClass(classes.contained), false); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.textPrimary), false, @@ -143,13 +143,13 @@ describe('); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); assert.strictEqual( wrapper.hasClass(classes.contained), true, 'should have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.textPrimary), false, @@ -168,13 +168,13 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); assert.strictEqual( wrapper.hasClass(classes.contained), true, 'should have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.containedPrimary), true, @@ -193,9 +193,9 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.contained), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.contained), true); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.containedPrimary), false, @@ -210,14 +210,14 @@ describe('); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); assert.strictEqual( wrapper.hasClass(classes.contained), true, 'should have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.raised), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.raised), true); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.containedPrimary), false, @@ -246,14 +246,14 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); assert.strictEqual( wrapper.hasClass(classes.contained), true, 'should have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.raised), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.raised), true); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.containedPrimary), true, @@ -282,14 +282,14 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); assert.strictEqual( wrapper.hasClass(classes.contained), true, 'should have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.raised), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.raised), true); + assert.strictEqual(wrapper.hasClass(classes.fab), false); assert.strictEqual( wrapper.hasClass(classes.containedPrimary), false, @@ -314,16 +314,16 @@ describe('); - assert.strictEqual(wrapper.hasClass(classes.root, 'should have the root class'), true); - assert.strictEqual(wrapper.hasClass(classes.flat), true, 'should have the flat class'); - assert.strictEqual(wrapper.hasClass(classes.outlined), true, 'should have the outlined class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.text), true); + assert.strictEqual(wrapper.hasClass(classes.outlined), true); assert.strictEqual( wrapper.hasClass(classes.contained), false, 'should not have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.raised), false, 'should not have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), false, 'should not have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.raised), false); + assert.strictEqual(wrapper.hasClass(classes.fab), false); }); it('should render a primary outlined button', () => { @@ -332,9 +332,9 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.flat), true, 'should have the flat class'); - assert.strictEqual(wrapper.hasClass(classes.outlined), true, 'should have the outlined class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.text), true); + assert.strictEqual(wrapper.hasClass(classes.outlined), true); assert.strictEqual( wrapper.hasClass(classes.textPrimary), true, @@ -345,8 +345,8 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.flat), true, 'should have the flat class'); - assert.strictEqual(wrapper.hasClass(classes.outlined), true, 'should have the outlined class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.text), true); + assert.strictEqual(wrapper.hasClass(classes.outlined), true); assert.strictEqual( wrapper.hasClass(classes.textSecondary), true, @@ -368,20 +368,52 @@ describe('); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); assert.strictEqual( wrapper.hasClass(classes.contained), true, 'should have the contained class', ); - assert.strictEqual(wrapper.hasClass(classes.fab), true, 'should have the fab class'); - assert.strictEqual(wrapper.hasClass(classes.flat), false, 'should not have the flat class'); + assert.strictEqual(wrapper.hasClass(classes.fab), true); + assert.strictEqual( + wrapper.hasClass(classes.extendedFab), + false, + 'should not have the extendedFab class', + ); + assert.strictEqual(wrapper.hasClass(classes.flat), false); + assert.strictEqual( + wrapper.hasClass(classes.textPrimary), + false, + 'should not have the textPrimary class', + ); + assert.strictEqual( + wrapper.hasClass(classes.textSecondary), + false, + 'should not have the textSecondary class', + ); + }); + + it('should render an extended floating action button', () => { + const wrapper = shallow(); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual( + wrapper.hasClass(classes.contained), + true, + 'should have the contained class', + ); + assert.strictEqual(wrapper.hasClass(classes.fab), true); + assert.strictEqual( + wrapper.hasClass(classes.extendedFab), + true, + 'should have the extendedFab class', + ); + assert.strictEqual(wrapper.hasClass(classes.flat), false); assert.strictEqual( wrapper.hasClass(classes.textPrimary), false, @@ -400,10 +432,10 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.contained), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), true, 'should have the fab class'); - assert.strictEqual(wrapper.hasClass(classes.mini), true, 'should have the mini class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.contained), true); + assert.strictEqual(wrapper.hasClass(classes.fab), true); + assert.strictEqual(wrapper.hasClass(classes.mini), true); assert.strictEqual( wrapper.hasClass(classes.textPrimary), false, @@ -422,18 +454,15 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.contained), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), true, 'should have the fab class'); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.contained), true); + assert.strictEqual(wrapper.hasClass(classes.fab), true); assert.strictEqual( wrapper.hasClass(classes.containedPrimary), true, 'should have the containedPrimary class', ); - assert.strictEqual( - wrapper.hasClass(classes.containedSecondary, 'should have the containedPrimary class'), - false, - ); + assert.strictEqual(wrapper.hasClass(classes.containedSecondary), false); }); it('should render an secondary floating action button', () => { @@ -442,19 +471,11 @@ describe(', ); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.hasClass(classes.contained), true, 'should have the raised class'); - assert.strictEqual(wrapper.hasClass(classes.fab), true, 'should have the fab class'); - assert.strictEqual( - wrapper.hasClass(classes.containedPrimary), - false, - 'should not have the containedPrimary class', - ); - assert.strictEqual( - wrapper.hasClass(classes.containedSecondary), - true, - 'should have the containedSecondary class', - ); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.contained), true); + assert.strictEqual(wrapper.hasClass(classes.fab), true); + assert.strictEqual(wrapper.hasClass(classes.containedPrimary), false); + assert.strictEqual(wrapper.hasClass(classes.containedSecondary), true); }); it('should have a ripple by default', () => { @@ -469,12 +490,12 @@ describe('); - assert.strictEqual(wrapper.props().focusRipple, true, 'should set focusRipple to true'); + assert.strictEqual(wrapper.props().focusRipple, true); }); it('should pass disableFocusRipple to ButtonBase', () => { const wrapper = shallow(); - assert.strictEqual(wrapper.props().focusRipple, false, 'should set focusRipple to false'); + assert.strictEqual(wrapper.props().focusRipple, false); }); it('should render Icon children with right classes', () => { @@ -484,7 +505,7 @@ describe('