[Button] Fix visual focus state according to spec

packages/material-ui/src/Button/Button.js

@@ -7,6 +7,7 @@ import { componentPropType, chainPropTypes } from '@material-ui/utils';
 import withStyles from '../styles/withStyles';
 import { fade } from '../styles/colorManipulator';
 import ButtonBase from '../ButtonBase';
+import deprecatedPropType from '../utils/deprecatedPropType';
 import { capitalize } from '../utils/helpers';
 
 export const styles = theme => ({
@@ -36,6 +37,9 @@ export const styles = theme => ({
     '&$disabled': {
       color: theme.palette.action.disabled,
     },
+    '&$focusVisible': {
+      backgroundColor: fade(theme.palette.text.primary, theme.palette.action.focusOpacity),
+    },
   },
   /* Styles applied to the span element that wraps the children. */
   label: {
@@ -58,6 +62,9 @@ export const styles = theme => ({
         backgroundColor: 'transparent',
       },
     },
+    '&$focusVisible': {
+      backgroundColor: fade(theme.palette.primary.main, theme.palette.action.focusOpacity),
+    },
   },
   /* Styles applied to the root element if `variant="text"` and `color="secondary"`. */
   textSecondary: {
@@ -69,6 +76,9 @@ export const styles = theme => ({
         backgroundColor: 'transparent',
       },
     },
+    '&$focusVisible': {
+      backgroundColor: fade(theme.palette.secondary.main, theme.palette.action.focusOpacity),
+    },
   },
   /* Styles applied to the root element for backwards compatibility with legacy variant naming. */
   flat: {},
@@ -153,6 +163,9 @@ export const styles = theme => ({
         backgroundColor: theme.palette.primary.main,
       },
     },
+    '&$focusVisible': {
+      backgroundColor: fade(theme.palette.primary.main, 1 - 2 * theme.palette.action.focusOpacity),
+    },
   },
   /* Styles applied to the root element if `variant="[contained | fab]"` and `color="secondary"`. */
   containedSecondary: {
@@ -165,6 +178,12 @@ export const styles = theme => ({
         backgroundColor: theme.palette.secondary.main,
       },
     },
+    '&$focusVisible': {
+      backgroundColor: fade(
+        theme.palette.secondary.main,
+        1 - 2 * theme.palette.action.focusOpacity,
+      ),
+    },
   },
   /* Styles applied to the root element for backwards compatibility with legacy variant naming. */
   raised: {}, // legacy
@@ -274,8 +293,9 @@ function Button(props) {
     <ButtonBase
       className={className}
       disabled={disabled}
-      focusRipple={!disableFocusRipple}
       focusVisibleClassName={classNames(classes.focusVisible, focusVisibleClassName)}
+      // using defaultProps will trigger deprecationWarnings
+      focusRipple={disableFocusRipple == null ? other.focusRipple : !disableFocusRipple}
       {...other}
     >
       <span className={classes.label}>{children}</span>
@@ -314,7 +334,11 @@ Button.propTypes = {
    * If `true`, the  keyboard focus ripple will be disabled.
    * `disableRipple` must also be true.
    */
-  disableFocusRipple: PropTypes.bool,
+  disableFocusRipple: deprecatedPropType(
+    PropTypes.bool,
+    'Focus ripple is not part of Material design. ' +
+      'If you want to enable it use `focusRipple` which is passed to `ButtonBase`',
+  ),
   /**
    * If `true`, the ripple effect will be disabled.
    */
@@ -390,7 +414,6 @@ Button.defaultProps = {
   color: 'default',
   component: 'button',
   disabled: false,
-  disableFocusRipple: false,
   fullWidth: false,
   mini: false,
   size: 'medium',

packages/material-ui/src/Button/Button.test.js

@@ -296,14 +296,32 @@ describe('<Button />', () => {
     assert.strictEqual(wrapper.props().disableRipple, true);
   });
 
-  it('should have a focusRipple by default', () => {
+  it('should have no focusRipple by default', () => {
     const wrapper = shallow(<Button>Hello World</Button>);
-    assert.strictEqual(wrapper.props().focusRipple, true);
+    assert.isNotOk(wrapper.props().focusRipple);
   });
 
-  it('should pass disableFocusRipple to ButtonBase', () => {
-    const wrapper = shallow(<Button disableFocusRipple>Hello World</Button>);
-    assert.strictEqual(wrapper.props().focusRipple, false);
+  describe('disableFocusRipple', () => {
+    beforeEach(() => {
+      consoleErrorMock.spy();
+    });
+    afterEach(() => {
+      consoleErrorMock.reset();
+    });
+
+    it('should disable focusRipple in ButtonBase', () => {
+      const wrapper = shallow(<Button disableFocusRipple>Hello World</Button>);
+      assert.strictEqual(wrapper.props().focusRipple, false);
+    });
+
+    it('is deprecated', () => {
+      shallow(<Button disableFocusRipple>Hello World</Button>);
+      assert.strictEqual(consoleErrorMock.callCount(), 1);
+      assert.include(
+        consoleErrorMock.args()[0][0],
+        'The prop `disableFocusRipple` of `Button` is deprecated.',
+      );
+    });
   });
 
   it('should render Icon children with right classes', () => {

packages/material-ui/src/styles/createPalette.d.ts

@@ -12,6 +12,7 @@ export interface TypeText {
 
 export interface TypeAction {
   active: string;
+  focusOpacity: number;
   hover: string;
   hoverOpacity: number;
   selected: string;

packages/material-ui/src/styles/createPalette.js

@@ -31,6 +31,7 @@ export const light = {
   action: {
     // The color of an active action like an icon button.
     active: 'rgba(0, 0, 0, 0.54)',
+    focusOpacity: 0.12,
     // The color of an hovered action.
     hover: 'rgba(0, 0, 0, 0.08)',
     hoverOpacity: 0.08,
@@ -58,6 +59,7 @@ export const dark = {
   },
   action: {
     active: common.white,
+    focusOpacity: 0.15,
     hover: 'rgba(255, 255, 255, 0.1)',
     hoverOpacity: 0.1,
     selected: 'rgba(255, 255, 255, 0.2)',

packages/material-ui/src/utils/deprecatedPropType.js

@@ -11,7 +11,9 @@ function deprecatedPropType(validator, reason) {
     if (typeof props[propName] !== 'undefined') {
       return new Error(
         `The ${location} \`${propFullNameSafe}\` of ` +
-          `\`${componentNameSafe}\` is deprecated. ${reason}`,
+          `\`${componentNameSafe}\` is deprecated. ${reason}${
+            process.env.NODE_ENV === 'test' ? Date.now() : ''
+          }`,
       );
     }
 

pages/api/button.md

@@ -23,7 +23,7 @@ import Button from '@material-ui/core/Button';
 | <span class="prop-name">color</span> | <span class="prop-type">enum:&nbsp;'default'&nbsp;&#124;<br>&nbsp;'inherit'&nbsp;&#124;<br>&nbsp;'primary'&nbsp;&#124;<br>&nbsp;'secondary'<br></span> | <span class="prop-default">'default'</span> | The color of the component. It supports those theme colors that make sense for this component. |
 | <span class="prop-name">component</span> | <span class="prop-type">componentPropType</span> | <span class="prop-default">'button'</span> | The component used for the root node. Either a string to use a DOM element or a component. |
 | <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the button will be disabled. |
-| <span class="prop-name">disableFocusRipple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the  keyboard focus ripple will be disabled. `disableRipple` must also be true. |
+| ~~<span class="prop-name">disableFocusRipple</span>~~ | <span class="prop-type">bool</span> |   | *Deprecated*. undefined<br><br>If `true`, the  keyboard focus ripple will be disabled. `disableRipple` must also be true. |
 | <span class="prop-name">disableRipple</span> | <span class="prop-type">bool</span> |   | If `true`, the ripple effect will be disabled. |
 | <span class="prop-name">fullWidth</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the button will take up the full width of its container. |
 | <span class="prop-name">href</span> | <span class="prop-type">string</span> |   | The URL to link to when the button is clicked. If defined, an `a` element will be used as the root node. |