Merge pull request #4108 from nathanmarks/enhancedbutton-background-s…

…tyle

[Buttons] Address various browser compatibility issues

src/FlatButton/FlatButton.js

@@ -201,22 +201,17 @@ class FlatButton extends Component {
     const hovered = (this.state.hovered || this.state.isKeyboardFocused) && !disabled;
 
     const mergedRootStyles = Object.assign({}, {
+      height: buttonHeight,
+      minWidth: buttonMinWidth,
       color: defaultTextColor,
       transition: transitions.easeOut(),
-      fontSize: fontSize,
-      letterSpacing: 0,
-      textTransform: textTransform,
-      fontWeight: fontWeight,
       borderRadius: 2,
       userSelect: 'none',
       position: 'relative',
       overflow: 'hidden',
       backgroundColor: hovered ? buttonHoverColor : buttonBackgroundColor,
-      lineHeight: `${buttonHeight}px`,
-      minWidth: buttonMinWidth,
       padding: 0,
       margin: 0,
-      // That's the default value for a button but not a link
       textAlign: 'center',
     }, style);
 
@@ -241,8 +236,16 @@ class FlatButton extends Component {
       }
     }
 
+    const mergedLabelStyles = Object.assign({
+      letterSpacing: 0,
+      textTransform: textTransform,
+      fontWeight: fontWeight,
+      fontSize: fontSize,
+      lineHeight: `${buttonHeight}px`,
+    }, labelStyleIcon, labelStyle);
+
     const labelElement = label ? (
-      <FlatButtonLabel label={label} style={Object.assign({}, labelStyleIcon, labelStyle)} />
+      <FlatButtonLabel label={label} style={mergedLabelStyles} />
     ) : undefined;
 
     // Place label before or after children.

src/RaisedButton/RaisedButton.js

@@ -54,31 +54,28 @@ function getStyles(props, context, state) {
     }
   }
 
-  const buttonHeight = style && style.height || `${button.height}px`;
+  const buttonHeight = style && style.height || button.height;
+  const borderRadius = 2;
 
   return {
     root: {
       display: 'inline-block',
-      minWidth: fullWidth ? '100%' : button.minWidth,
-      height: buttonHeight,
       transition: transitions.easeOut(),
     },
-    container: {
-      lineHeight: buttonHeight,
+    button: {
       position: 'relative',
-      height: '100%',
+      minWidth: fullWidth ? '100%' : button.minWidth,
+      height: buttonHeight,
       width: '100%',
       padding: 0,
-      overflow: 'hidden',
-      borderRadius: 2,
+      borderRadius: borderRadius,
       transition: transitions.easeOut(),
       backgroundColor: backgroundColor,
       // That's the default value for a button but not a link
       textAlign: 'center',
     },
     label: {
       position: 'relative',
-      verticalAlign: 'middle',
       opacity: 1,
       fontSize: '14px',
       letterSpacing: 0,
@@ -97,6 +94,8 @@ function getStyles(props, context, state) {
     },
     overlay: {
       height: buttonHeight,
+      lineHeight: `${buttonHeight}px`,
+      borderRadius: borderRadius,
       backgroundColor: (state.keyboardFocused || state.hovered) && !disabled &&
         fade(labelColor, amount),
       transition: transitions.easeOut(),
@@ -386,7 +385,7 @@ class RaisedButton extends Component {
           {...buttonEventHandlers}
           ref="container"
           disabled={disabled}
-          style={styles.container}
+          style={styles.button}
           focusRippleColor={mergedRippleStyles.color}
           touchRippleColor={mergedRippleStyles.color}
           focusRippleOpacity={mergedRippleStyles.opacity}

src/internal/EnhancedButton.js

@@ -266,7 +266,6 @@ class EnhancedButton extends Component {
 
     const mergedStyles = Object.assign({
       border: 10,
-      background: 'none',
       boxSizing: 'border-box',
       display: 'inline-block',
       fontFamily: this.context.muiTheme.baseTheme.fontFamily,
@@ -285,6 +284,12 @@ class EnhancedButton extends Component {
       verticalAlign: other.hasOwnProperty('href') ? 'middle' : null,
     }, style);
 
+
+    // Passing both background:none & backgroundColor can break due to object iteration order
+    if (!mergedStyles.backgroundColor && !mergedStyles.background) {
+      mergedStyles.background = 'none';
+    }
+
     if (disabled && linkButton) {
       return (
         <span

src/internal/EnhancedButton.spec.js

@@ -86,7 +86,7 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(touched, false, 'should not trigger the touchTap');
   });
 
-  it('can be styled', () => {
+  it('should be styleable', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton style={{color: 'red'}}>Button</EnhancedButton>
     );
@@ -98,7 +98,6 @@ describe('<EnhancedButton />', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton>Button</EnhancedButton>
     );
-
     assert.strictEqual(wrapper.node.props.style.background, 'none', 'should be none');
     wrapper.setProps({
       style: {
@@ -108,7 +107,19 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(wrapper.node.props.style.background, 'blue', 'should be blue');
   });
 
-  it('can set the button type', () => {
+  it('should not have "background: none" style when passed a backgroundColor', () => {
+    const wrapper = shallowWithContext(
+      <EnhancedButton>Button</EnhancedButton>
+    );
+    assert.strictEqual(wrapper.node.props.style.background, 'none', 'should be none');
+    wrapper.setProps({style: {backgroundColor: 'blue'}});
+    assert.strictEqual(wrapper.node.props.style.background, undefined, 'background should be undefined');
+    assert.strictEqual(wrapper.node.props.style.backgroundColor, 'blue', 'backgroundColor should be blue');
+    wrapper.setProps({style: {backgroundColor: null}});
+    assert.strictEqual(wrapper.node.props.style.background, 'none', 'should be none');
+  });
+
+  it('should set the button type', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton type="submit">Button</EnhancedButton>
     );
@@ -118,14 +129,14 @@ describe('<EnhancedButton />', () => {
     assert.ok(wrapper.is('button[type="reset"]'), 'should have the type attribute');
   });
 
-  it('passes through other html attributes', () => {
+  it('should pass through other html attributes', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton name="hello">Button</EnhancedButton>
     );
     assert.ok(wrapper.is('button[name="hello"]'), 'should have the name attribute');
   });
 
-  it('handles focus propagation based on disabled props', () => {
+  it('should handle focus propagation based on disabled props', () => {
     const eventStack = [];
     eventStack.reset = () => eventStack.splice(0, eventStack.length);
 
@@ -154,7 +165,7 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(eventStack.length, 2);
   });
 
-  it('has a TouchRipple and controls it using props', () => {
+  it('have a TouchRipple and control it using props', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton
         centerRipple={true}
@@ -180,7 +191,7 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(wrapper.find('TouchRipple').length, 0, 'should not have a TouchRipple');
   });
 
-  it('has a FocusRipple when keyboard focused (tracked internally) and controls it using props', () => {
+  it('have a FocusRipple when keyboard focused (tracked internally) and control it using props', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton
         focusRippleColor="red"
@@ -222,7 +233,7 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(wrapper.find('FocusRipple').length, 1, 'should have a FocusRipple');
   });
 
-  it('removes a FocusRipple on blur', () => {
+  it('should remove a FocusRipple on blur', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton>Button</EnhancedButton>
     );
@@ -234,7 +245,7 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(wrapper.find('FocusRipple').length, 0, 'should not have a FocusRipple');
   });
 
-  it('has no ripples when both are disabled', () => {
+  it('should have no ripples when both are disabled', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton
         keyboardFocused={true}
@@ -248,15 +259,15 @@ describe('<EnhancedButton />', () => {
     assert.strictEqual(wrapper.find('FocusRipple').length, 0, 'should not have a FocusRipple');
   });
 
-  it('has no ripples when button is disabled', () => {
+  it('should have no ripples when button is disabled', () => {
     const wrapper = shallowWithContext(
       <EnhancedButton keyboardFocused={true} disabled={true}>Button</EnhancedButton>
     );
     assert.strictEqual(wrapper.find('TouchRipple').length, 0, 'should not have a TouchRipple');
     assert.strictEqual(wrapper.find('FocusRipple').length, 0, 'should not have a FocusRipple');
   });
 
-  it('fires the touchtap handler if keyboard focused and the enter or space keys are hit', () => {
+  it('should fire the touchtap handler if keyboard focused and the enter or space keys are hit', () => {
     const eventStack = [];
     eventStack.reset = () => eventStack.splice(0, eventStack.length);