[ButtonBase] Fix an accessibility issue

package.json

@@ -56,7 +56,6 @@
     "keycode": "^2.1.1",
     "lodash": "^4.17.2",
     "object-assign": "^4.1.0",
-    "react-addons-create-fragment": "^15.4.0",
     "react-addons-transition-group": "^15.4.0",
     "react-event-listener": "^0.4.0",
     "recompose": "^0.20.2",

src/internal/ButtonBase.js

@@ -2,7 +2,6 @@
 
 import React, { Component, PropTypes } from 'react';
 import ReactDOM from 'react-dom';
-import createFragment from 'react-addons-create-fragment';
 import { createStyleSheet } from 'jss-theme-reactor';
 import classNames from 'classnames';
 import keycode from 'keycode';
@@ -54,7 +53,14 @@ export default class ButtonBase extends Component {
      */
     className: PropTypes.string,
     component: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
+    /**
+     * If `true`, the base button will be disabled.
+     */
     disabled: PropTypes.bool,
+    /**
+     * If `true`, the base button will have a keyboard focus ripple.
+     * `ripple` must also be true.
+     */
     focusRipple: PropTypes.bool,
     keyboardFocusedClassName: PropTypes.string,
     onBlur: PropTypes.func,
@@ -68,6 +74,9 @@ export default class ButtonBase extends Component {
     onMouseUp: PropTypes.func,
     onTouchEnd: PropTypes.func,
     onTouchStart: PropTypes.func,
+    /**
+     * If `true`, the base button will have a ripple.
+     */
     ripple: PropTypes.oneOfType([PropTypes.bool, PropTypes.func]),
     role: PropTypes.string,
     tabIndex: PropTypes.string,
@@ -211,14 +220,7 @@ export default class ButtonBase extends Component {
       children,
       className: classNameProp,
       component,
-      /**
-       * If `true`, the base button will be disabled.
-       */
       disabled,
-      /**
-       * If `true`, the base button will have a keyboard focus ripple.
-       * Ripple must also be true.
-       */
       focusRipple, // eslint-disable-line no-unused-vars
       keyboardFocusedClassName,
       onBlur, // eslint-disable-line no-unused-vars
@@ -230,10 +232,8 @@ export default class ButtonBase extends Component {
       onMouseUp, // eslint-disable-line no-unused-vars
       onTouchEnd, // eslint-disable-line no-unused-vars
       onTouchStart, // eslint-disable-line no-unused-vars
-      /**
-       * If `true`, the base button will have a ripple.
-       */
       ripple,
+      tabIndex,
       type,
       ...other
     } = this.props;
@@ -256,31 +256,29 @@ export default class ButtonBase extends Component {
       onMouseUp: this.handleMouseUp,
       onTouchEnd: this.handleTouchEnd,
       onTouchStart: this.handleTouchStart,
-      tabIndex: this.props.tabIndex,
+      tabIndex: disabled ? '-1' : tabIndex,
       className,
       ...other,
     };
 
-    let element = component;
+    let Element = component;
 
     if (other.href) {
-      element = 'a';
+      Element = 'a';
     }
 
-    if (element === 'button') {
+    if (Element === 'button') {
       buttonProps.type = type;
       buttonProps.disabled = disabled;
-    } else if (element !== 'a') {
+    } else if (Element !== 'a') {
       buttonProps.role = this.props.hasOwnProperty('role') ? this.props.role : 'button';
     }
 
-    return React.createElement(
-      element,
-      buttonProps,
-      createFragment({
-        children,
-        ripple: this.renderRipple(ripple, centerRipple),
-      }),
+    return (
+      <Element {...buttonProps}>
+        {children}
+        {this.renderRipple(ripple, centerRipple)}
+      </Element>
     );
   }
 }

src/internal/ButtonBase.spec.js

@@ -307,4 +307,11 @@ describe('<ButtonBase />', () => {
       }, 200);
     });
   });
+
+  describe('prop: disabled', () => {
+    it('should apply the right tabIndex', () => {
+      const wrapper = shallow(<ButtonBase disabled>Hello</ButtonBase>);
+      assert.strictEqual(wrapper.props().tabIndex, '-1', 'should not receive the focus');
+    });
+  });
 });

test/regressions/site/index.html

@@ -6,8 +6,6 @@
       name="viewport"
       content="width=device-width, initial-scale=1, user-scalable=0, maximum-scale=1, minimum-scale=1"
     >
-    <link href="https://fonts.googleapis.com/css?family=Roboto:400,500,700,400italic" rel="stylesheet" type="text/css">
-    <link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
   </head>
   <body>
     <div id="app"></div>

test/regressions/site/package.json

@@ -12,6 +12,7 @@
     "webpack-dev-server": "^1.16.2"
   },
   "dependencies": {
-    "react-router": "^3.0.0"
+    "react-router": "^3.0.0",
+    "webfontloader": "^1.6.26"
   }
 }

test/regressions/site/src/index.js

@@ -4,16 +4,27 @@ import { AppContainer } from 'react-hot-loader';
 import RedBox from 'redbox-react';
 import React from 'react';
 import { render } from 'react-dom';
+import webFont from 'webfontloader';
 import App from './App';
 
 const rootEl = document.getElementById('app');
 
-render(
-  <AppContainer errorReporter={RedBox}>
-    <App />
-  </AppContainer>,
-  rootEl,
-);
+webFont.load({
+  google: {
+    families: [
+      'Roboto:400,500,700,400italic',
+      'Material+Icons',
+    ],
+  },
+  active: () => {
+    render(
+      <AppContainer errorReporter={RedBox}>
+        <App />
+      </AppContainer>,
+      rootEl,
+    );
+  },
+});
 
 if (module.hot) {
   module.hot.accept('./App', () => {