From 035290d3a0a2a0d69a773bc9a8f748a8e36c5d11 Mon Sep 17 00:00:00 2001 From: Mike Carmody Date: Mon, 22 Oct 2018 16:58:58 -0500 Subject: [PATCH 1/3] [Button] fix styles so text classes aren't applied to outlined buttons --- packages/material-ui/src/Button/Button.js | 7 +++---- packages/material-ui/src/Button/Button.test.js | 16 +++++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js index e4f39d1ecf3f7b..5da5bc19652518 100644 --- a/packages/material-ui/src/Button/Button.js +++ b/packages/material-ui/src/Button/Button.js @@ -228,16 +228,15 @@ function Button(props) { const fab = variant === 'fab' || variant === 'extendedFab'; const contained = variant === 'contained' || variant === 'raised'; - const text = variant === 'text' || variant === 'flat' || variant === 'outlined'; const className = classNames( classes.root, { [classes.fab]: fab, [classes.mini]: fab && mini, [classes.extendedFab]: variant === 'extendedFab', - [classes.text]: text, - [classes.textPrimary]: text && color === 'primary', - [classes.textSecondary]: text && color === 'secondary', + [classes.text]: variant === 'text', + [classes.textPrimary]: variant === 'text' && color === 'primary', + [classes.textSecondary]: variant === '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', diff --git a/packages/material-ui/src/Button/Button.test.js b/packages/material-ui/src/Button/Button.test.js index 4732955587daf0..a068e05b576445 100644 --- a/packages/material-ui/src/Button/Button.test.js +++ b/packages/material-ui/src/Button/Button.test.js @@ -149,23 +149,23 @@ describe(', ); assert.strictEqual(wrapper.hasClass(classes.root), true); - assert.strictEqual(wrapper.hasClass(classes.text), true); assert.strictEqual(wrapper.hasClass(classes.flat), true); - assert.strictEqual(wrapper.hasClass(classes.textSecondary), true); assert.strictEqual(wrapper.hasClass(classes.flatSecondary), true); + assert.strictEqual(wrapper.hasClass(classes.text), false); + assert.strictEqual(wrapper.hasClass(classes.textSecondary), false); }); }); it('should render an outlined button', () => { const wrapper = shallow(); 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.text), false); assert.strictEqual(wrapper.hasClass(classes.raised), false); assert.strictEqual(wrapper.hasClass(classes.fab), false); }); @@ -177,9 +177,10 @@ describe(', ); 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); + assert.strictEqual(wrapper.hasClass(classes.outlinedPrimary), true); + assert.strictEqual(wrapper.hasClass(classes.text), false); + assert.strictEqual(wrapper.hasClass(classes.textPrimary), false); assert.strictEqual(wrapper.hasClass(classes.contained), false); assert.strictEqual(wrapper.hasClass(classes.raised), false); assert.strictEqual(wrapper.hasClass(classes.fab), false); @@ -192,9 +193,10 @@ describe(', ); 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); + assert.strictEqual(wrapper.hasClass(classes.outlinedSecondary), true); + assert.strictEqual(wrapper.hasClass(classes.text), false); + assert.strictEqual(wrapper.hasClass(classes.textSecondary), false); assert.strictEqual(wrapper.hasClass(classes.contained), false); assert.strictEqual(wrapper.hasClass(classes.raised), false); assert.strictEqual(wrapper.hasClass(classes.fab), false); From e940ced4ab4daf65a0eccf3ddb79770214426f83 Mon Sep 17 00:00:00 2001 From: Mike Carmody Date: Mon, 22 Oct 2018 17:51:04 -0500 Subject: [PATCH 2/3] retain basic styles from 'text' in 'outlined' --- packages/material-ui/src/Button/Button.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js index 5da5bc19652518..dd35eccd6796b5 100644 --- a/packages/material-ui/src/Button/Button.js +++ b/packages/material-ui/src/Button/Button.js @@ -82,9 +82,15 @@ export const styles = theme => ({ }, /* Styles applied to the root element if `variant="outlined"` and `color="primary"`. */ outlinedPrimary: { + color: theme.palette.primary.main, border: `1px solid ${fade(theme.palette.primary.main, 0.5)}`, '&:hover': { border: `1px solid ${theme.palette.primary.main}`, + backgroundColor: fade(theme.palette.primary.main, theme.palette.action.hoverOpacity), + // Reset on touch devices, it doesn't add specificity + '@media (hover: none)': { + backgroundColor: 'transparent', + }, }, '&$disabled': { border: `1px solid ${theme.palette.action.disabled}`, @@ -92,9 +98,15 @@ export const styles = theme => ({ }, /* Styles applied to the root element if `variant="outlined"` and `color="secondary"`. */ outlinedSecondary: { + color: theme.palette.secondary.main, border: `1px solid ${fade(theme.palette.secondary.main, 0.5)}`, '&:hover': { border: `1px solid ${theme.palette.secondary.main}`, + backgroundColor: fade(theme.palette.secondary.main, theme.palette.action.hoverOpacity), + // Reset on touch devices, it doesn't add specificity + '@media (hover: none)': { + backgroundColor: 'transparent', + }, }, '&$disabled': { border: `1px solid ${theme.palette.action.disabled}`, From e3317784baede39ef47ad265f43d96cca0c2b05b Mon Sep 17 00:00:00 2001 From: Mike Carmody Date: Tue, 23 Oct 2018 07:40:53 -0500 Subject: [PATCH 3/3] replace styles on 'flat' variant --- packages/material-ui/src/Button/Button.js | 7 ++++--- packages/material-ui/src/Button/Button.test.js | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js index dd35eccd6796b5..dcda19b87767b0 100644 --- a/packages/material-ui/src/Button/Button.js +++ b/packages/material-ui/src/Button/Button.js @@ -240,15 +240,16 @@ function Button(props) { const fab = variant === 'fab' || variant === 'extendedFab'; const contained = variant === 'contained' || variant === 'raised'; + const text = variant === 'text' || variant === 'flat'; const className = classNames( classes.root, { [classes.fab]: fab, [classes.mini]: fab && mini, [classes.extendedFab]: variant === 'extendedFab', - [classes.text]: variant === 'text', - [classes.textPrimary]: variant === 'text' && color === 'primary', - [classes.textSecondary]: variant === 'text' && color === 'secondary', + [classes.text]: text, + [classes.textPrimary]: text && color === 'primary', + [classes.textSecondary]: 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', diff --git a/packages/material-ui/src/Button/Button.test.js b/packages/material-ui/src/Button/Button.test.js index a068e05b576445..599bf51fc6adac 100644 --- a/packages/material-ui/src/Button/Button.test.js +++ b/packages/material-ui/src/Button/Button.test.js @@ -149,10 +149,10 @@ describe(', ); assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass(classes.text), true); assert.strictEqual(wrapper.hasClass(classes.flat), true); + assert.strictEqual(wrapper.hasClass(classes.textSecondary), true); assert.strictEqual(wrapper.hasClass(classes.flatSecondary), true); - assert.strictEqual(wrapper.hasClass(classes.text), false); - assert.strictEqual(wrapper.hasClass(classes.textSecondary), false); }); });