Skip to content

Commit

Permalink
[Button] Fix padding for Text Button variant to adhere to spec (#13827)
Browse files Browse the repository at this point in the history
* Fix padding for Text Button variant to adhere to spec

* let's merge
  • Loading branch information
bdeloeste authored and oliviertassinari committed Dec 10, 2018
1 parent 8028253 commit 57ae964
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 6 deletions.
2 changes: 1 addition & 1 deletion docs/src/pages/customization/themes/OverridesCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const theme = createMuiTheme({
// Name of the component ⚛️ / style sheet
MuiButton: {
// Name of the rule
root: {
text: {
// Some CSS
background: 'linear-gradient(45deg, #FE6B8B 30%, #FF8E53 90%)',
borderRadius: 3,
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/customization/themes/themes.md
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ That's a really powerful feature.
const theme = createMuiTheme({
overrides: {
MuiButton: { // Name of the component ⚛️ / style sheet
root: { // Name of the rule
text: { // Name of the rule
color: 'white', // Some CSS
},
},
Expand Down
10 changes: 6 additions & 4 deletions packages/material-ui/src/Button/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ export const styles = theme => ({
justifyContent: 'inherit',
},
/* Styles applied to the root element if `variant="text"`. */
text: {},
text: {
padding: theme.spacing.unit,
},
/* Styles applied to the root element if `variant="text"` and `color="primary"`. */
textPrimary: {
color: theme.palette.primary.main,
Expand Down Expand Up @@ -250,9 +252,9 @@ function Button(props) {
[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',
[classes.flat]: text,
[classes.flatPrimary]: text && color === 'primary',
[classes.flatSecondary]: text && color === 'secondary',
[classes.contained]: contained || fab,
[classes.containedPrimary]: (contained || fab) && color === 'primary',
[classes.containedSecondary]: (contained || fab) && color === 'secondary',
Expand Down
8 changes: 8 additions & 0 deletions packages/material-ui/src/Button/Button.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('<Button />', () => {
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.text), true);
assert.strictEqual(wrapper.hasClass(classes.textPrimary), false);
assert.strictEqual(wrapper.hasClass(classes.textSecondary), false);
assert.strictEqual(wrapper.hasClass(classes.flatPrimary), false);
Expand Down Expand Up @@ -75,6 +76,7 @@ describe('<Button />', () => {
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.text), false);
assert.strictEqual(wrapper.hasClass(classes.textPrimary), false);
assert.strictEqual(wrapper.hasClass(classes.textSecondary), false);
});
Expand All @@ -88,6 +90,7 @@ describe('<Button />', () => {
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.text), false);
assert.strictEqual(wrapper.hasClass(classes.containedPrimary), true);
assert.strictEqual(wrapper.hasClass(classes.containedSecondary), false);
});
Expand All @@ -101,20 +104,23 @@ describe('<Button />', () => {
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.text), false);
assert.strictEqual(wrapper.hasClass(classes.containedPrimary), false);
assert.strictEqual(wrapper.hasClass(classes.containedSecondary), true);
});

it('should render a small button', () => {
const wrapper = shallow(<Button size="small">Hello World</Button>);
assert.strictEqual(wrapper.hasClass(classes.root), true);
assert.strictEqual(wrapper.hasClass(classes.text), true);
assert.strictEqual(wrapper.hasClass(classes.sizeSmall), true);
assert.strictEqual(wrapper.hasClass(classes.sizeLarge), false);
});

it('should render a large button', () => {
const wrapper = shallow(<Button size="large">Hello World</Button>);
assert.strictEqual(wrapper.hasClass(classes.root), true);
assert.strictEqual(wrapper.hasClass(classes.text), true);
assert.strictEqual(wrapper.hasClass(classes.sizeSmall), false);
assert.strictEqual(wrapper.hasClass(classes.sizeLarge), true);
});
Expand All @@ -134,6 +140,7 @@ describe('<Button />', () => {
assert.strictEqual(wrapper.hasClass(classes.contained), true);
assert.strictEqual(wrapper.hasClass(classes.raised), true);
assert.strictEqual(wrapper.hasClass(classes.fab), false);
assert.strictEqual(wrapper.hasClass(classes.text), false);
assert.strictEqual(wrapper.hasClass(classes.containedPrimary), false);
assert.strictEqual(wrapper.hasClass(classes.raisedPrimary), false);
assert.strictEqual(wrapper.hasClass(classes.containedSecondary), false);
Expand All @@ -150,6 +157,7 @@ describe('<Button />', () => {
assert.strictEqual(wrapper.hasClass(classes.contained), true);
assert.strictEqual(wrapper.hasClass(classes.raised), true);
assert.strictEqual(wrapper.hasClass(classes.fab), false);
assert.strictEqual(wrapper.hasClass(classes.text), false);
assert.strictEqual(wrapper.hasClass(classes.containedPrimary), true);
assert.strictEqual(wrapper.hasClass(classes.raisedPrimary), true);
assert.strictEqual(wrapper.hasClass(classes.containedSecondary), false);
Expand Down

0 comments on commit 57ae964

Please sign in to comment.