From 081c0a97ea139936c090f2767188b92d219548ad Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Sun, 11 Dec 2016 10:10:32 -0500 Subject: [PATCH 1/8] Refactor switches, simplify code, fix related bugs. - Refactors SwitchBase into a factory - Refactors Checkbox, Radio, Switch to use new Factory - Drastically simplify RadioGroup --- .eslintrc.js | 1 + .flowconfig | 1 + .../selection-controls/RadioButtonsGroup.js | 1 + .../demos/selection-controls/SwitchLabels.js | 1 - src/Checkbox/Checkbox.js | 83 +---- src/Checkbox/Checkbox.spec.js | 74 +---- src/IconButton/IconButton.js | 6 + src/Radio/Radio.js | 99 +----- src/Radio/Radio.spec.js | 64 +--- src/Radio/RadioGroup.js | 166 +--------- src/Switch/Switch.js | 81 +---- src/Switch/Switch.spec.js | 81 ++--- src/internal/ButtonBase.js | 31 +- src/internal/SelectionLabel.js | 68 ---- src/internal/SelectionLabel.spec.js | 78 ----- src/internal/SwitchBase.js | 292 +++++++++--------- src/internal/SwitchBase.spec.js | 92 +++--- src/internal/SwitchLabel.js | 87 ++++++ src/internal/SwitchLabel.spec.js | 89 ++++++ src/styles/MuiThemeProvider.js | 13 +- src/utils/keyboardFocus.js | 34 ++ test/integration/RadioGroup.test.js | 276 ----------------- .../chrome-53.0.2785.143-linux.png | Bin 1467 -> 1452 bytes .../chrome-53.0.2785.143-linux.png | Bin 2741 -> 2934 bytes .../chrome-53.0.2785.143-linux.png | Bin 3218 -> 3159 bytes 25 files changed, 521 insertions(+), 1197 deletions(-) delete mode 100644 src/internal/SelectionLabel.js delete mode 100644 src/internal/SelectionLabel.spec.js create mode 100644 src/internal/SwitchLabel.js create mode 100644 src/internal/SwitchLabel.spec.js create mode 100644 src/utils/keyboardFocus.js delete mode 100644 test/integration/RadioGroup.test.js diff --git a/.eslintrc.js b/.eslintrc.js index b355580c766d45..5bf4781d3a0baa 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -28,6 +28,7 @@ module.exports = { 'no-console': 'error', // airbnb is using warn 'no-param-reassign': 'off', 'no-prototype-builtins': 'off', + 'no-use-before-define': ['error', { 'functions': false }], // airbnb have functions: true, annoying 'object-curly-spacing': 'off', // use babel plugin rule 'operator-linebreak': ['error', 'after'], // aibnb is disabling this rule 'babel/object-curly-spacing': ['error', 'always'], diff --git a/.flowconfig b/.flowconfig index e3ab7c4e868795..e0e4f5cd0cea7b 100644 --- a/.flowconfig +++ b/.flowconfig @@ -1,6 +1,7 @@ [ignore] .*/node_modules/fbjs/* +.*/node_modules/react-native/* .*/node_modules/jss/lib/.*\.js\.flow .*/node_modules/react-swipeable-views/src/.* .*/node_modules/react-native/* diff --git a/docs/site/src/demos/selection-controls/RadioButtonsGroup.js b/docs/site/src/demos/selection-controls/RadioButtonsGroup.js index 973c6f24797606..ba889b55d47ced 100644 --- a/docs/site/src/demos/selection-controls/RadioButtonsGroup.js +++ b/docs/site/src/demos/selection-controls/RadioButtonsGroup.js @@ -32,6 +32,7 @@ export default class RadioButtonsGroup extends Component { Gender this.setState({ checkedA: checked })} label="A" - labelReverse /> { return { @@ -20,79 +18,10 @@ export const styleSheet = createStyleSheet('Checkbox', (theme) => { }; }); -export default function Checkbox(props, context) { - const { - className, - checkedClassName, - disabled, - disabledClassName, - label, - labelClassName, - labelReverse, - ...other - } = props; - const classes = context.styleManager.render(styleSheet); +const Checkbox = createSwitch({ styleSheet }); - const switchProps = { - className: classNames(classes.default, className), - checkedClassName: classNames(classes.checked, checkedClassName), - disabled, - disabledClassName: classNames(classes.disabled, disabledClassName), - ...other, - }; - - if (label) { - return ( - - - - ); - } - - return ; -} - -Checkbox.propTypes = { - checkedClassName: PropTypes.string, - /** - * The CSS class name of the root element. - */ - className: PropTypes.string, - /** - * If `true`, the control will be disabled. - */ - disabled: PropTypes.bool, - /** - * The CSS class name of the switch element when disabled. - */ - disabledClassName: PropTypes.string, - /** - * The text to be used in an enclosing label element. - */ - label: PropTypes.node, - /** - * The className to be used in an enclosing label element. - */ - labelClassName: PropTypes.string, - /** - * Will reverse the order of the element and the label. - */ - labelReverse: PropTypes.bool, -}; +Checkbox.displayName = 'Checkbox'; -Checkbox.defaultProps = { - disabled: false, - labelReverse: false, -}; +export { Checkbox }; -Checkbox.contextTypes = { - styleManager: PropTypes.object.isRequired, -}; +export default withSwitchLabel(Checkbox); diff --git a/src/Checkbox/Checkbox.spec.js b/src/Checkbox/Checkbox.spec.js index fd7e6a3795adee..928b54b291979f 100644 --- a/src/Checkbox/Checkbox.spec.js +++ b/src/Checkbox/Checkbox.spec.js @@ -1,10 +1,9 @@ // @flow weak /* eslint-env mocha */ -import React from 'react'; import { assert } from 'chai'; import { createShallowWithContext } from 'test/utils'; -import Checkbox, { styleSheet } from './Checkbox'; +import LabelCheckbox, { Checkbox, styleSheet } from './Checkbox'; describe('', () => { let shallow; @@ -15,68 +14,25 @@ describe('', () => { classes = shallow.context.styleManager.render(styleSheet); }); - it('should render a SwitchBase when label not present', () => { - const wrapper = shallow( - , - ); - assert.strictEqual(wrapper.is('SwitchBase'), true, 'should be a SwitchBase'); - }); - - it('should render a label', () => { - const wrapper = shallow( - , - ); - assert.strictEqual(wrapper.is('SelectionLabel'), true, 'should be a SelectionLabel'); - }); - - it('should render with the default and checked classes', () => { - const wrapper = shallow( - , - ); - const switchBase = wrapper.find('SwitchBase'); - assert.strictEqual(wrapper.hasClass('foo'), true, 'should have the "foo" class'); - assert.strictEqual(switchBase.hasClass('woof'), true, 'should have the "woof" class'); - assert.strictEqual(switchBase.hasClass(classes.default), true, 'should have the default class'); - assert.strictEqual( - switchBase.prop('checkedClassName').indexOf('meow') !== -1, - true, - 'should have the "meow" class', - ); - assert.strictEqual( - switchBase.prop('checkedClassName').indexOf(classes.checked) !== -1, - true, - 'should have the checked class', - ); - }); - - it('should spread custom props on the switchBase node', () => { - const wrapper = shallow(); - const switchBase = wrapper.find('SwitchBase'); - assert.strictEqual(switchBase.prop('data-my-prop'), 'woof', 'custom prop should be woof'); + describe('styleSheet', () => { + it('should have the classes required for SwitchBase', () => { + assert.strictEqual(typeof classes.default, 'string'); + assert.strictEqual(typeof classes.checked, 'string'); + assert.strictEqual(typeof classes.disabled, 'string'); + }); }); - describe('prop: disabled', () => { - it('should disable the component', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.props().disabled, true, 'should pass the property down the tree'); + describe('named Checkbox export', () => { + it('should be a SwitchBase with the displayName set for debugging', () => { + assert.strictEqual(Checkbox.name, 'SwitchBase'); + assert.strictEqual(Checkbox.displayName, 'Checkbox'); }); }); - describe('prop: disabledClassName', () => { - it('should provide the class', () => { - const className = 'foo'; - const wrapper = shallow(); - assert.strictEqual( - wrapper.find('SwitchBase').props().disabledClassName.indexOf(className) !== -1, - true, - 'should have the custom disabled class', - ); + describe('default export', () => { + it('should be Checkbox wrapped with SwitchLabel', () => { + assert.strictEqual(LabelCheckbox.name, 'SwitchLabel'); + assert.strictEqual(LabelCheckbox.displayName, 'withSwitchLabel(Checkbox)'); }); }); }); diff --git a/src/IconButton/IconButton.js b/src/IconButton/IconButton.js index 832020b7267b9f..31da2c96e53eff 100644 --- a/src/IconButton/IconButton.js +++ b/src/IconButton/IconButton.js @@ -62,6 +62,7 @@ export const styleSheet = createStyleSheet('IconButton', (theme) => { export default function IconButton(props, context) { const { accent, + buttonRef, children, className, contrast, @@ -80,6 +81,7 @@ export default function IconButton(props, context) { centerRipple keyboardFocusedClassName={classes.keyboardFocused} disabled={disabled} + ref={buttonRef} {...other} > @@ -96,6 +98,10 @@ IconButton.propTypes = { * If true, will use the theme's accent color. */ accent: PropTypes.bool, + /** + * @ignore + */ + buttonRef: PropTypes.func, /** * The icon element. If a string is passed, * it will be used as a material icon font ligature. diff --git a/src/Radio/Radio.js b/src/Radio/Radio.js index e279bbe48fa140..26ba4f77fa7eb6 100644 --- a/src/Radio/Radio.js +++ b/src/Radio/Radio.js @@ -1,10 +1,8 @@ // @flow weak -import React, { PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; -import classNames from 'classnames'; -import SwitchBase from '../internal/SwitchBase'; -import SelectionLabel from '../internal/SelectionLabel'; +import { createSwitch } from '../internal/SwitchBase'; +import { withSwitchLabel } from '../internal/SwitchLabel'; export const styleSheet = createStyleSheet('Radio', (theme) => { return { @@ -20,90 +18,15 @@ export const styleSheet = createStyleSheet('Radio', (theme) => { }; }); -export default function Radio(props, context) { - const { - className, - checkedClassName, - disabled, - disabledClassName, - label, - labelClassName, - labelReverse, - onChange, - value, - ...other - } = props; - const classes = context.styleManager.render(styleSheet); - - const switchProps = { - className: classNames(classes.default, className), - checkedClassName: classNames(classes.checked, checkedClassName), - icon: 'radio_button_unchecked', - checkedIcon: 'radio_button_checked', - type: 'radio', - value, - onChange, - disabled, - disabledClassName: classNames(classes.disabled, disabledClassName), - ...other, - }; - - if (label) { - return ( - - - - ); - } - - return ; -} - +const Radio = createSwitch({ + styleSheet, + inputType: 'radio', + defaultIcon: 'radio_button_unchecked', + defaultCheckedIcon: 'radio_button_checked', +}); -Radio.propTypes = { - checkedClassName: PropTypes.string, - /** - * The CSS class name of the root element. - */ - className: PropTypes.string, - /** - * If `true`, the control will be disabled. - */ - disabled: PropTypes.bool, - /** - * The CSS class name of the switch element when disabled. - */ - disabledClassName: PropTypes.string, - /** - * The text to be used in an enclosing label element. - */ - label: PropTypes.node, - /** - * The className to be used in an enclosing label element. - */ - labelClassName: PropTypes.string, - /** - * Will reverse the order of the element and the label. - */ - labelReverse: PropTypes.bool, - name: PropTypes.string, - onChange: PropTypes.func, - value: PropTypes.string, -}; +Radio.displayName = 'Radio'; -Radio.defaultProps = { - disabled: false, - labelReverse: false, -}; +export { Radio }; -Radio.contextTypes = { - styleManager: PropTypes.object.isRequired, -}; +export default withSwitchLabel(Radio); diff --git a/src/Radio/Radio.spec.js b/src/Radio/Radio.spec.js index ee0d5f8820336b..f249db498bf861 100644 --- a/src/Radio/Radio.spec.js +++ b/src/Radio/Radio.spec.js @@ -1,10 +1,9 @@ // @flow weak /* eslint-env mocha */ -import React from 'react'; import { assert } from 'chai'; import { createShallowWithContext } from 'test/utils'; -import Radio, { styleSheet } from './Radio'; +import LabelRadio, { Radio, styleSheet } from './Radio'; describe('', () => { let shallow; @@ -15,58 +14,25 @@ describe('', () => { classes = shallow.context.styleManager.render(styleSheet); }); - it('should render a SwitchBase when label not present', () => { - const wrapper = shallow( - , - ); - assert.strictEqual(wrapper.is('SwitchBase'), true, 'should be a SwitchBase'); - }); - - it('should render a label', () => { - const wrapper = shallow( - , - ); - assert.strictEqual(wrapper.is('SelectionLabel'), true, 'should be a SelectionLabel'); - }); - - it('should render with the default and checked classes', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.hasClass('woof'), true, 'should have the "woof" class'); - assert.strictEqual(wrapper.hasClass(classes.default), true, 'should have the default class'); - assert.strictEqual( - wrapper.prop('checkedClassName').indexOf('meow') !== -1, - true, - 'should have the "meow" class', - ); - assert.strictEqual( - wrapper.prop('checkedClassName').indexOf(classes.checked) !== -1, - true, - 'should have the checked class', - ); - }); - - it('should spread custom props on the switchBase node', () => { - const wrapper = shallow(); - const switchBase = wrapper.find('SwitchBase'); - assert.strictEqual(switchBase.prop('data-my-prop'), 'woof', 'custom prop should be woof'); + describe('styleSheet', () => { + it('should have the classes required for SwitchBase', () => { + assert.strictEqual(typeof classes.default, 'string'); + assert.strictEqual(typeof classes.checked, 'string'); + assert.strictEqual(typeof classes.disabled, 'string'); + }); }); - describe('prop: disabled', () => { - it('should disable the component', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.props().disabled, true, 'should pass the property down the tree'); + describe('named Radio export', () => { + it('should be a SwitchBase with the displayName set for debugging', () => { + assert.strictEqual(Radio.name, 'SwitchBase'); + assert.strictEqual(Radio.displayName, 'Radio'); }); }); - describe('prop: disabledClassName', () => { - it('should provide the class', () => { - const className = 'foo'; - const wrapper = shallow(); - assert.strictEqual( - wrapper.find('SwitchBase').props().disabledClassName.indexOf(className) !== -1, - true, - 'should have the custom disabled class', - ); + describe('default export', () => { + it('should be Radio wrapped with SwitchLabel', () => { + assert.strictEqual(LabelRadio.name, 'SwitchLabel'); + assert.strictEqual(LabelRadio.displayName, 'withSwitchLabel(Radio)'); }); }); }); diff --git a/src/Radio/RadioGroup.js b/src/Radio/RadioGroup.js index 903e30225ecaec..4138b2188d29e3 100644 --- a/src/Radio/RadioGroup.js +++ b/src/Radio/RadioGroup.js @@ -1,34 +1,8 @@ -// @flow weak + // @flow weak import React, { Component, Children, cloneElement, PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; -import { findDOMNode } from 'react-dom'; import classNames from 'classnames'; -import keycode from 'keycode'; -import querySelectorAll from 'dom-helpers/query/querySelectorAll'; -import contains from 'dom-helpers/query/contains'; -import activeElement from 'dom-helpers/activeElement'; -import ownerDocument from 'dom-helpers/ownerDocument'; - -function changeFocus(currentFocusIndex = 0, event, radios) { - const key = keycode(event); - - if (key === 'down' || key === 'right') { - event.preventDefault(); - if (currentFocusIndex < radios.length - 1) { - radios[currentFocusIndex + 1].focus(); - } else { - radios[0].focus(); - } - } else if (key === 'up' || key === 'left') { - event.preventDefault(); - if (currentFocusIndex > 0) { - radios[currentFocusIndex - 1].focus(); - } else { - radios[radios.length - 1].focus(); - } - } -} export const styleSheet = createStyleSheet('RadioGroup', () => { return { @@ -68,173 +42,41 @@ export default class RadioGroup extends Component { styleManager: PropTypes.object.isRequired, }; - static childContextTypes = { - radioGroup: PropTypes.object, - }; - - state = { - currentTabIndex: undefined, - focused: false, // used for optional label required/error - selectedValue: undefined, // used for uncontrolled support - }; - - componentWillMount() { - this.isControlled = this.props.selectedValue !== undefined; - - // not controlled, use internal state - if (!this.isControlled && this.props.defaultValue) { - this.setState({ selectedValue: this.props.defaultValue }); - } - } - - componentDidMount() { - this.resetTabIndex(); - } - - componentWillUnmount() { - clearTimeout(this.blurTimer); - } - - group = undefined; - blurTimer = undefined; - isControlled = undefined; - - handleBlur = (event) => { - this.blurTimer = setTimeout(() => { - if (this.group) { - const group = findDOMNode(this.group); - const currentFocus = activeElement(ownerDocument(group)); - if (!contains(group, currentFocus)) { - this.resetTabIndex(); - this.setState({ focused: false }); - } - } - }, 50); - - if (this.props.onBlur) { - this.props.onBlur(event); - } - }; - - handleKeyDown = (event) => { - const group = findDOMNode(this.group); - - if (group) { - const currentFocus = activeElement(ownerDocument(group)); - const radios = querySelectorAll(group, '[role="radio"]'); - - let currentFocusIndex = -1; - - for (let i = 0; i < radios.length; i += 1) { - if (radios[i] === currentFocus || contains(radios[i], currentFocus)) { - currentFocusIndex = i; - break; - } - } - - changeFocus(currentFocusIndex, event, radios); - } - - if (this.props.onKeyDown) { - this.props.onKeyDown(event); - } - }; - handleRadioChange = (event, checked) => { - if (checked) { - if (!this.isControlled) { - this.setState({ selectedValue: event.target.value }); - } - if (this.props.onChange) { - this.props.onChange(event, event.target.value); - } - } - }; - - handleRadioFocus = (event) => { - const group = findDOMNode(this.group); - if (group) { - this.setState({ focused: true }); - const radios = querySelectorAll(group, '[role="radio"]'); - for (let i = 0; i < radios.length; i += 1) { - if (radios[i] === event.currentTarget || contains(radios[i], event.currentTarget)) { - this.setTabIndex(i); - break; - } - } + if (checked && this.props.onChange) { + this.props.onChange(event, event.target.value); } }; - focus() { - const { currentTabIndex } = this.state; - if (currentTabIndex && currentTabIndex >= 0) { - const group = findDOMNode(this.group); - const radios = querySelectorAll(group, '[role="radio"]'); - if (radios && radios[currentTabIndex]) { - radios[currentTabIndex].focus(); - } - } - } - - resetTabIndex() { - const group = findDOMNode(this.group); - const currentFocus = activeElement(ownerDocument(group)); - const radios = querySelectorAll(group, '[role="radio"]'); - const currentFocusIndex = radios.indexOf(currentFocus); - - if (currentFocusIndex !== -1) { - return this.setTabIndex(currentFocusIndex); - } - - const selectedRadio = group.querySelector('[aria-checked="true"]'); - - if (selectedRadio) { - return this.setTabIndex(radios.indexOf(selectedRadio)); - } - - return this.setTabIndex(0); - } - - setTabIndex(n) { - this.setState({ currentTabIndex: n }); - } - render() { const { children, className: classNameProp, component: ComponentProp, name, - selectedValue: selectedValueProp, + selectedValue, onChange, // eslint-disable-line no-unused-vars ...other } = this.props; - const selectedValue = this.isControlled ? selectedValueProp : this.state.selectedValue; const classes = this.context.styleManager.render(styleSheet); return ( { this.group = c; }} role="radiogroup" {...other} - onKeyDown={this.handleKeyDown} - onBlur={this.handleBlur} > {Children.map(children, (child, index) => { return cloneElement(child, { key: index, name, checked: selectedValue === child.props.value, - tabIndex: index === this.state.currentTabIndex ? '0' : '-1', onChange: this.handleRadioChange, - onFocus: this.handleRadioFocus, }); })} ); } } - diff --git a/src/Switch/Switch.js b/src/Switch/Switch.js index 96731f3f0a6310..eb4808ebce9549 100644 --- a/src/Switch/Switch.js +++ b/src/Switch/Switch.js @@ -3,8 +3,8 @@ import React, { PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; import classNames from 'classnames'; -import SwitchBase from '../internal/SwitchBase'; -import SelectionLabel from '../internal/SelectionLabel'; +import { createSwitch } from '../internal/SwitchBase'; +import { withSwitchLabel } from '../internal/SwitchLabel'; export const styleSheet = createStyleSheet('Switch', (theme) => { const { palette } = theme; @@ -57,91 +57,34 @@ export const styleSheet = createStyleSheet('Switch', (theme) => { }; }); -export default function Switch(props, context) { +const SwitchBase = createSwitch({ styleSheet }); + +function Switch(props, context) { const { className, - checkedClassName, - disabled, - disabledClassName, - label, - labelClassName, - labelReverse, ...other } = props; - const classes = context.styleManager.render(styleSheet); + const classes = context.styleManager.render(styleSheet); const icon =
; - const switchProps = { - className: classes.default, - checkedClassName: classNames(classes.checked, checkedClassName), - disabledClassName: classNames(classes.disabled, disabledClassName), - icon, - checkedIcon: icon, - type: 'checkbox', - disabled, - ...other, - }; - - if (label) { - return ( - -
- -
-
- - ); - } return (
- +
); } Switch.propTypes = { - /** - * The CSS class name of the switch element when checked. - */ - checkedClassName: PropTypes.string, - /** - * The CSS class name of the root element. - */ className: PropTypes.string, - /** - * If `true`, the control will be disabled. - */ - disabled: PropTypes.bool, - /** - * The CSS class name of the switch element when disabled. - */ - disabledClassName: PropTypes.string, - /** - * The text to be used in an enclosing label element. - */ - label: PropTypes.node, - /** - * The className to be used in an enclosing label element. - */ - labelClassName: PropTypes.string, - /** - * Will reverse the order of the element and the label. - */ - labelReverse: PropTypes.bool, -}; - -Switch.defaultProps = { - disabled: false, - labelReverse: false, }; Switch.contextTypes = { styleManager: PropTypes.object.isRequired, }; + + +export { Switch }; + +export default withSwitchLabel(Switch); diff --git a/src/Switch/Switch.spec.js b/src/Switch/Switch.spec.js index a45268adea4750..589d6dd7ec2b82 100644 --- a/src/Switch/Switch.spec.js +++ b/src/Switch/Switch.spec.js @@ -4,7 +4,7 @@ import React from 'react'; import { assert } from 'chai'; import { createShallowWithContext } from 'test/utils'; -import Switch, { styleSheet } from './Switch'; +import LabelSwitch, { Switch, styleSheet } from './Switch'; describe('', () => { let shallow; @@ -15,60 +15,47 @@ describe('', () => { classes = shallow.context.styleManager.render(styleSheet); }); - it('should render a SwitchBase inside a div when label not present', () => { - const wrapper = shallow( - , - ); - assert.strictEqual(wrapper.is('div'), true, 'should be a div'); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); - assert.strictEqual(wrapper.childAt(0).is('SwitchBase'), true, 'should be a SwitchBase'); + describe('styleSheet', () => { + it('should have the classes required for SwitchBase', () => { + assert.strictEqual(typeof classes.default, 'string'); + assert.strictEqual(typeof classes.checked, 'string'); + assert.strictEqual(typeof classes.disabled, 'string'); + }); }); - it('should render a label', () => { - const wrapper = shallow( - , - ); - assert.strictEqual(wrapper.is('SelectionLabel'), true, 'should be a SelectionLabel'); - }); + describe('named Switch export', () => { + let wrapper; - it('should render with the default and checked classes', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.hasClass('woof'), true, 'should have the "woof" class'); - assert.strictEqual(wrapper.childAt(0).hasClass(classes.default), true, 'should have the default class'); - assert.strictEqual( - wrapper.childAt(0).prop('checkedClassName').indexOf('meow') !== -1, - true, - 'should have the "meow" class', - ); - assert.strictEqual( - wrapper.childAt(0).prop('checkedClassName').indexOf(classes.checked) !== -1, - true, - 'should have the checked class', - ); - }); + beforeEach(() => { + wrapper = shallow(); + }); - it('should spread custom props on the SwitchBase node', () => { - const wrapper = shallow(); - const switchBase = wrapper.find('SwitchBase'); - assert.strictEqual(switchBase.prop('data-my-prop'), 'woof', 'custom prop should be woof'); - }); + it('should render a div with the root and user classes', () => { + assert.strictEqual(wrapper.is('div'), true); + assert.strictEqual(wrapper.hasClass(classes.root), true); + assert.strictEqual(wrapper.hasClass('foo'), true); + }); + + it('should render SwitchBase with a custom div icon with the icon class', () => { + const switchBase = wrapper.childAt(0); + + assert.strictEqual(switchBase.is('SwitchBase'), true); + assert.strictEqual(switchBase.prop('icon').type, 'div'); + assert.strictEqual(switchBase.prop('icon').props.className, classes.icon); + }); + + it('should render the bar as the 2nd child', () => { + const bar = wrapper.childAt(1); - describe('prop: disabled', () => { - it('should disable the component', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.find('SwitchBase').props().disabled, true, 'should disable the switch node'); + assert.strictEqual(bar.is('div'), true); + assert.strictEqual(bar.hasClass(classes.bar), true); }); }); - describe('prop: disabledClassName', () => { - it('should provide the class', () => { - const className = 'foo'; - const wrapper = shallow(); - assert.strictEqual( - wrapper.find('SwitchBase').props().disabledClassName.indexOf(className) !== -1, - true, - 'should have the custom disabled class', - ); + describe('default export', () => { + it('should be Switch wrapped with SwitchLabelt', () => { + assert.strictEqual(LabelSwitch.name, 'SwitchLabel'); + assert.strictEqual(LabelSwitch.displayName, 'withSwitchLabel(Switch)'); }); }); }); diff --git a/src/internal/ButtonBase.js b/src/internal/ButtonBase.js index cd2e7bacecead5..7aaac6c5c5d083 100644 --- a/src/internal/ButtonBase.js +++ b/src/internal/ButtonBase.js @@ -1,31 +1,13 @@ /* eslint-disable flowtype/require-valid-file-annotation */ import React, { Component, PropTypes } from 'react'; -import ReactDOM from 'react-dom'; +import { findDOMNode } from 'react-dom'; import { createStyleSheet } from 'jss-theme-reactor'; import classNames from 'classnames'; import keycode from 'keycode'; -import addEventListener from '../utils/addEventListener'; +import { listenForFocusKeys, focusKeyPressed } from '../utils/keyboardFocus'; import { TouchRipple, createRippleHandler } from '../Ripple'; -let listening = false; -let focusKeyPressed = false; - -function isFocusKey(event) { - return ['tab', 'enter', 'space', 'esc', 'up', 'down', 'left', 'right'].indexOf(keycode(event)) !== -1; -} - -function listenForFocusKeys() { - if (!listening) { - addEventListener(window, 'keyup', (event) => { - if (isFocusKey(event)) { - focusKeyPressed = true; - } - }); - listening = true; - } -} - export const styleSheet = createStyleSheet('ButtonBase', () => { return { buttonBase: { @@ -121,6 +103,8 @@ export default class ButtonBase extends Component { button = null; keyboardFocusTimeout = undefined; + focus = () => this.button.focus(); + handleKeyDown = (event) => { const { component, focusRipple, onKeyDown, onClick } = this.props; const key = keycode(event); @@ -140,6 +124,7 @@ export default class ButtonBase extends Component { // Keyboard accessibility for non interactive elements if ( + event.target === this.button && onClick && component !== 'a' && component !== 'button' && @@ -163,7 +148,7 @@ export default class ButtonBase extends Component { handleMouseDown = createRippleHandler(this, 'MouseDown', 'start', () => { clearTimeout(this.keyboardFocusTimeout); - focusKeyPressed = false; + focusKeyPressed(false); if (this.state.keyboardFocused) { this.setState({ keyboardFocused: false }); } @@ -190,9 +175,9 @@ export default class ButtonBase extends Component { // first if focus was called programatically inside a keydown handler event.persist(); setTimeout(() => { - if (focusKeyPressed && document.activeElement === ReactDOM.findDOMNode(this.button)) { + if (focusKeyPressed() && document.activeElement === findDOMNode(this.button)) { this.keyDown = false; - focusKeyPressed = false; + focusKeyPressed(false); this.setState({ keyboardFocused: true }); if (this.props.onKeyboardFocus) { this.props.onKeyboardFocus(event); diff --git a/src/internal/SelectionLabel.js b/src/internal/SelectionLabel.js deleted file mode 100644 index 477b8c8c9e87e9..00000000000000 --- a/src/internal/SelectionLabel.js +++ /dev/null @@ -1,68 +0,0 @@ -// @flow weak - -import React, { PropTypes } from 'react'; -import { createStyleSheet } from 'jss-theme-reactor'; -import classNames from 'classnames'; - -export const styleSheet = createStyleSheet('SelectionLabel', (theme) => { - return { - root: { - marginLeft: -12, - marginRight: 16, // used for row presentation of radio/checkbox - display: 'flex', - alignItems: 'center', - cursor: 'pointer', - }, - reverse: { - flexDirection: 'row-reverse', - }, - disabled: { - color: theme.palette.text.disabled, - cursor: 'not-allowed', - }, - }; -}); - -export default function SelectionLabel(props, context) { - const { - disabled, - label, - labelClassName: labelClassNameProp, - labelReverse, - children, - } = props; - const classes = context.styleManager.render(styleSheet); - const labelClassName = classNames(classes.root, { - [classes.reverse]: labelReverse, - }, labelClassNameProp); - - return ( - - ); -} - -SelectionLabel.propTypes = { - children: PropTypes.node, - /** - * If `true`, the control will be disabled. - */ - disabled: PropTypes.bool.isRequired, - label: PropTypes.node, - labelClassName: PropTypes.string, - labelReverse: PropTypes.bool.isRequired, -}; - -SelectionLabel.contextTypes = { - styleManager: PropTypes.object.isRequired, -}; diff --git a/src/internal/SelectionLabel.spec.js b/src/internal/SelectionLabel.spec.js deleted file mode 100644 index 04a0d884bac0f1..00000000000000 --- a/src/internal/SelectionLabel.spec.js +++ /dev/null @@ -1,78 +0,0 @@ -// @flow weak -/* eslint-env mocha */ - -import React from 'react'; -import { assert } from 'chai'; -import { createShallowWithContext } from 'test/utils'; -import SelectionLabel, { styleSheet } from './SelectionLabel'; - -describe('', () => { - let shallow; - let classes; - - before(() => { - shallow = createShallowWithContext(); - classes = shallow.context.styleManager.render(styleSheet); - }); - - let wrapper; - - beforeEach(() => { - wrapper = shallow( - , - ); - }); - - it('should render a label', () => { - assert.strictEqual(wrapper.is('label'), true, 'should be a label'); - }); - - it('should render the label text inside an additional span', () => { - const span = wrapper.childAt(0); - assert.strictEqual(span.is('span'), true, 'should render a span'); - assert.strictEqual(span.childAt(0).node, 'Pizza', 'should be the label text'); - }); - - it('should render with accessibility attributes', () => { - assert.strictEqual( - wrapper.prop('role'), - 'presentation', - 'should set the role to presentation for screen readers', - ); - const span = wrapper.childAt(0); - assert.strictEqual( - span.prop('role'), - 'presentation', - 'should set the span role to presentation for screen readers', - ); - assert.strictEqual( - span.prop('aria-hidden'), - 'true', - 'should set to aria hidden for screen readers', - ); - }); - - it('should render with the default and custom classes', () => { - assert.strictEqual(wrapper.hasClass('foo'), true, 'should have the "foo" class'); - assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the "root" class'); - }); - - describe('prop: disabled', () => { - it('should disable the component', () => { - wrapper.setProps({ - disabled: true, - }); - - assert.strictEqual( - wrapper.childAt(0).hasClass(classes.disabled), - true, - 'should have the disabled class', - ); - }); - }); -}); diff --git a/src/internal/SwitchBase.js b/src/internal/SwitchBase.js index 8defd655fa54b3..7b67a173882771 100644 --- a/src/internal/SwitchBase.js +++ b/src/internal/SwitchBase.js @@ -10,8 +10,9 @@ export const styleSheet = createStyleSheet('SwitchBase', () => { root: { display: 'inline-flex', alignItems: 'center', + transition: 'none', }, - switch: { + input: { cursor: 'inherit', position: 'absolute', opacity: 0, @@ -25,155 +26,160 @@ export const styleSheet = createStyleSheet('SwitchBase', () => { }; }); -export default class SwitchBase extends Component { - static propTypes = { - /** - * SwitchBase is checked if true. - */ - checked: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]), - /** - * The CSS class name of the root element when checked. - */ - checkedClassName: PropTypes.string, - checkedIcon: PropTypes.node, - /** - * The CSS class name of the root element. - */ - className: PropTypes.string, - /** - * @ignore - */ - defaultChecked: PropTypes.bool, - /** - * If `true`, the switch will be disabled. - */ - disabled: PropTypes.bool, - /** - * The CSS class name of the root element when disabled. - */ - disabledClassName: PropTypes.string, - icon: PropTypes.node, - /** - * Callback function that is fired when the switch is changed. - * - * @param {object} event `change` event - * @param {boolean} checked The `checked` value of the switch - */ - onChange: PropTypes.func, - /** - * If false, the ripple effect will be disabled. - */ - ripple: PropTypes.bool, - type: PropTypes.oneOf(['checkbox', 'radio']), - value: PropTypes.string, - }; - - static defaultProps = { - icon: 'check_box_outline_blank', - checkedIcon: 'check_box', - type: 'checkbox', - }; - - static contextTypes = { - styleManager: PropTypes.object.isRequired, - }; - - state = {}; - - componentWillMount() { - const { props } = this; +export function createSwitch({ + defaultIcon = 'check_box_outline_blank', + defaultCheckedIcon = 'check_box', + inputType = 'checkbox', + styleSheet: switchStyleSheet, +} = {}) { + return class SwitchBase extends Component { + static propTypes = { + /** + * SwitchBase is checked if true. + */ + checked: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]), + /** + * The CSS class name of the root element when checked. + */ + checkedClassName: PropTypes.string, + checkedIcon: PropTypes.node, + /** + * The CSS class name of the root element. + */ + className: PropTypes.string, + /** + * @ignore + */ + defaultChecked: PropTypes.bool, + /** + * If `true`, the switch will be disabled. + */ + disabled: PropTypes.bool, + /** + * The CSS class name of the root element when disabled. + */ + disabledClassName: PropTypes.string, + icon: PropTypes.node, + /* + * @ignore + */ + name: PropTypes.string, + /** + * Callback function that is fired when the switch is changed. + * + * @param {object} event `change` event + * @param {boolean} checked The `checked` value of the switch + */ + onChange: PropTypes.func, + /** + * If false, the ripple effect will be disabled. + */ + ripple: PropTypes.bool, + value: PropTypes.string, + }; + + static defaultProps = { + icon: defaultIcon, + checkedIcon: defaultCheckedIcon, + }; + + static contextTypes = { + styleManager: PropTypes.object.isRequired, + }; + + state = {}; + + componentWillMount() { + const { props } = this; + + this.isControlled = props.checked !== undefined; + + if (!this.isControlled) { // not controlled, use internal state + this.setState({ checked: props.defaultChecked !== undefined ? props.defaultChecked : false }); + } + } - this.isControlled = props.checked !== undefined; + input = undefined; + button = undefined; + isControlled = undefined; - if (!this.isControlled) { // not controlled, use internal state - this.setState({ checked: props.defaultChecked !== undefined ? props.defaultChecked : false }); - } - } + focus = () => this.input.focus(); - input = undefined; - isControlled = undefined; + handleInputChange = (event) => { + let newChecked; - handleInputChange = (event) => { - let newChecked; + if (this.isControlled) { + newChecked = !this.props.checked; + } else { + newChecked = !this.state.checked; + if (this.input && this.input.checked !== newChecked) { + this.input.checked = newChecked; + } + this.setState({ checked: !this.state.checked }); + } - if (this.isControlled) { - newChecked = !this.props.checked; - } else { - newChecked = !this.state.checked; - if (this.input && this.input.checked !== newChecked) { - this.input.checked = newChecked; + if (this.props.onChange) { + this.props.onChange(event, newChecked); + } + }; + + render() { + const { + checked: checkedProp, + className: classNameProp, + checkedClassName, + checkedIcon, + disabled, + disabledClassName, + icon: iconProp, + name, + onChange, // eslint-disable-line no-unused-vars + ripple, + value, + ...other + } = this.props; + + + const checked = this.isControlled ? checkedProp : this.state.checked; + const classes = this.context.styleManager.render(styleSheet); + const switchClasses = switchStyleSheet ? this.context.styleManager.render(switchStyleSheet) : {}; + + const className = classNames(classes.root, switchClasses.default, classNameProp, { + [classNames(switchClasses.checked, checkedClassName)]: checked, + [classNames(switchClasses.disabled, disabledClassName)]: disabled, + }); + + let icon = checked ? checkedIcon : iconProp; + + if (typeof icon === 'string') { + icon = ; } - this.setState({ checked: !this.state.checked }); - } - if (this.props.onChange) { - this.props.onChange(event, newChecked); + return ( + { this.button = c; }} + className={className} + disabled={disabled} + ripple={ripple} + tabIndex={null} + role={undefined} + {...other} + > + {icon} + { this.input = c; }} + type={inputType} + name={name} + checked={this.isControlled ? checkedProp : undefined} + onChange={this.handleInputChange} + className={classes.input} + disabled={disabled} + value={value} + /> + + ); } }; - - // Handle button interactions when - // IconButton is interacted with using space/enter - handleClick = (event) => { - if (event.target !== this.input) { - this.input.click(); - } - } - - render() { - const { - checked: checkedProp, - className: classNameProp, - checkedClassName, - checkedIcon, - icon: iconProp, - disabled, - disabledClassName, - onChange, // eslint-disable-line no-unused-vars - ripple, - type, - value, - ...other - } = this.props; - - const classes = this.context.styleManager.render(styleSheet); - const checked = this.isControlled ? checkedProp : this.state.checked; - - const className = classNames(classes.root, classNameProp, { - [checkedClassName]: checkedClassName && checked, - [disabledClassName]: disabledClassName && disabled, - }); - - let icon = checked ? checkedIcon : iconProp; - - if (typeof icon === 'string') { - icon = ; - } - - return ( - - {icon} - { this.input = c; }} - type={type} - checked={this.isControlled ? checkedProp : undefined} - onChange={this.handleInputChange} - className={classes.switch} - disabled={disabled} - value={value} - /> - - ); - } } diff --git a/src/internal/SwitchBase.spec.js b/src/internal/SwitchBase.spec.js index 9aecd593ec41c7..0bec17bb48dbb6 100644 --- a/src/internal/SwitchBase.spec.js +++ b/src/internal/SwitchBase.spec.js @@ -4,56 +4,16 @@ import React from 'react'; import { assert } from 'chai'; import { createShallowWithContext, createMountWithContext } from 'test/utils'; -import SwitchBase, { styleSheet } from './SwitchBase'; - -function assertIsChecked(classes, wrapper) { - const iconButton = wrapper.find('span').at(0); - - assert.strictEqual( - iconButton.hasClass('test-class-checked'), - true, - 'should have the checked class on the root node', - ); - assert.strictEqual( - iconButton.prop('aria-checked'), - true, - 'should pass aria-checked=true to the root node', - ); - - const input = wrapper.find('input'); - assert.strictEqual(input.node.checked, true, 'the DOM node should be checked'); - - const icon = wrapper.find('span.material-icons'); - assert.strictEqual(icon.text(), 'check_box', 'should be the check_box icon'); -} - -function assertIsNotChecked(classes, wrapper) { - const iconButton = wrapper.find('span').at(0); - - assert.strictEqual( - iconButton.hasClass('test-class-checked'), - false, - 'should not have the checked class on the root node', - ); - assert.strictEqual( - iconButton.prop('aria-checked'), - false, - 'should pass aria-checked=false to the root node', - ); - - const input = wrapper.find('input'); - assert.strictEqual(input.node.checked, false, 'the DOM node should not be checked'); - - const icon = wrapper.find('span.material-icons'); - assert.strictEqual(icon.text(), 'check_box_outline_blank', 'should be the check_box_outline_blank icon'); -} +import { createSwitch, styleSheet } from './SwitchBase'; describe('', () => { let shallow; let classes; let mount; + let SwitchBase; before(() => { + SwitchBase = createSwitch(); shallow = createShallowWithContext(); mount = createMountWithContext(); classes = shallow.context.styleManager.render(styleSheet); @@ -121,7 +81,7 @@ describe('', () => { describe('controlled', () => { let wrapper; - before(() => { + beforeEach(() => { wrapper = mount( ', () => { }); it('should uncheck the checkbox', () => { + wrapper.setProps({ checked: true }); wrapper.setProps({ checked: false }); assertIsNotChecked(classes, wrapper); }); @@ -153,7 +114,7 @@ describe('', () => { describe('uncontrolled', () => { let wrapper; - before(() => { + beforeEach(() => { wrapper = mount( ', () => { }); it('should check the checkbox', () => { - const input = wrapper.find('input'); - input.node.checked = true; - wrapper.instance().handleInputChange({}); + wrapper.find('input').node.click(); assertIsChecked(classes, wrapper); }); it('should uncheck the checkbox', () => { - const input = wrapper.find('input'); - input.node.checked = false; - wrapper.instance().handleInputChange({}); + wrapper.find('input').node.click(); + wrapper.find('input').node.click(); assertIsNotChecked(classes, wrapper); }); }); }); + +function assertIsChecked(classes, wrapper) { + const iconButton = wrapper.find('span').at(0); + + assert.strictEqual( + iconButton.hasClass('test-class-checked'), + true, + 'should have the checked class on the root node', + ); + + const input = wrapper.find('input'); + assert.strictEqual(input.node.checked, true, 'the DOM node should be checked'); + + const icon = wrapper.find('span.material-icons'); + assert.strictEqual(icon.text(), 'check_box', 'should be the check_box icon'); +} + +function assertIsNotChecked(classes, wrapper) { + const iconButton = wrapper.find('span').at(0); + + assert.strictEqual( + iconButton.hasClass('test-class-checked'), + false, + 'should not have the checked class on the root node', + ); + + const input = wrapper.find('input'); + assert.strictEqual(input.node.checked, false, 'the DOM node should not be checked'); + + const icon = wrapper.find('span.material-icons'); + assert.strictEqual(icon.text(), 'check_box_outline_blank', 'should be the check_box_outline_blank icon'); +} diff --git a/src/internal/SwitchLabel.js b/src/internal/SwitchLabel.js new file mode 100644 index 00000000000000..26c829300ad36c --- /dev/null +++ b/src/internal/SwitchLabel.js @@ -0,0 +1,87 @@ +// @flow weak +/* eslint-disable jsx-a11y/label-has-for */ + +import React, { Component, PropTypes } from 'react'; +import { createStyleSheet } from 'jss-theme-reactor'; +import classNames from 'classnames'; + +export const styleSheet = createStyleSheet('SwitchLabel', (theme) => { + return { + root: { + display: 'inline-flex', + alignItems: 'center', + cursor: 'pointer', + }, + hasLabel: { + marginLeft: -12, + marginRight: 16, // used for row presentation of radio/checkbox + }, + disabled: { + color: theme.palette.text.disabled, + cursor: 'not-allowed', + }, + }; +}); + +export function withSwitchLabel(SwitchComponent) { + const switchComponentName = SwitchComponent.displayName || SwitchComponent.name; + + return class SwitchLabel extends Component { + static displayName = `withSwitchLabel(${switchComponentName})`; + + static propTypes = { + /** + * If `true`, the control will be disabled. + */ + disabled: PropTypes.bool, + /** + * The text to be used in an enclosing label element. + */ + label: PropTypes.node, + /** + * The className to be used in an enclosing label element. + */ + labelClassName: PropTypes.string, + }; + + static contextTypes = { + styleManager: PropTypes.object.isRequired, + }; + + switch = undefined; + + focus() { + if (this.switch && this.switch.focus) { + this.switch.focus(); + } + } + + render() { + const { + disabled, + label, + labelClassName: labelClassNameProp, + ...other + } = this.props; + + const classes = this.context.styleManager.render(styleSheet); + + const labelClassName = classNames(classes.root, { + [classes.hasLabel]: label && label.length, + }, labelClassNameProp); + + return ( + + ); + } + }; +} diff --git a/src/internal/SwitchLabel.spec.js b/src/internal/SwitchLabel.spec.js new file mode 100644 index 00000000000000..7feef0a342464c --- /dev/null +++ b/src/internal/SwitchLabel.spec.js @@ -0,0 +1,89 @@ +// @flow weak +/* eslint-env mocha */ + +import React from 'react'; +import { assert } from 'chai'; +import { spy } from 'sinon'; +import { createShallowWithContext } from 'test/utils'; +import { withSwitchLabel, styleSheet } from './SwitchLabel'; + +describe('', () => { + let shallow; + let classes; + + before(() => { + shallow = createShallowWithContext(); + classes = shallow.context.styleManager.render(styleSheet); + }); + + describe('exports.withSwitchLabel', () => { + it('should be a function', () => { + assert.strictEqual(typeof withSwitchLabel, 'function'); + }); + + it('should return a SwitchLabel component with a wrapped displayName', () => { + const SwitchLabel = withSwitchLabel({ displayName: 'Foo' }); + assert.strictEqual(SwitchLabel.displayName, 'withSwitchLabel(Foo)'); + + const SwitchLabelFn = withSwitchLabel(function Foo() {}); // eslint-disable-line prefer-arrow-callback + assert.strictEqual(SwitchLabelFn.displayName, 'withSwitchLabel(Foo)'); + }); + }); + + describe('SwitchLabelFoo', () => { + let SwitchLabelFoo; + let wrapper; + + beforeEach(() => { + class Foo {} + SwitchLabelFoo = withSwitchLabel(Foo); + wrapper = shallow( + , + ); + }); + + it('should have the correct displayName', () => { + assert.strictEqual(SwitchLabelFoo.displayName, 'withSwitchLabel(Foo)'); + }); + + it('should render a label', () => { + assert.strictEqual(wrapper.is('label'), true, 'should be a label'); + }); + + it('should render the label text inside an additional span', () => { + const span = wrapper.childAt(1); + assert.strictEqual(span.is('span'), true, 'should render a span'); + assert.strictEqual(span.childAt(0).node, 'Pizza', 'should be the label text'); + }); + + it('should render with the default and custom classes', () => { + assert.strictEqual(wrapper.hasClass('foo'), true, 'should have the "foo" class'); + assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the "root" class'); + }); + + describe('imperative methods', () => { + it('should forward the focus method to the base component stored on switch', () => { + const focusSpy = spy(); + wrapper.instance().switch = { + focus: focusSpy, + }; + wrapper.instance().focus(); + assert.strictEqual(focusSpy.callCount, 1); + }); + }); + + describe('prop: disabled', () => { + it('should disable the component', () => { + wrapper.setProps({ + disabled: true, + }); + + assert.strictEqual( + wrapper.childAt(1).hasClass(classes.disabled), + true, + 'should have the disabled class', + ); + }); + }); + }); +}); diff --git a/src/styles/MuiThemeProvider.js b/src/styles/MuiThemeProvider.js index 536c99caa99c18..72751ac015d5cd 100644 --- a/src/styles/MuiThemeProvider.js +++ b/src/styles/MuiThemeProvider.js @@ -17,7 +17,6 @@ export const MUI_SHEET_ORDER = [ 'TouchRipple', 'ButtonBase', - 'SwitchBase', 'FormLabel', 'FormGroup', @@ -33,17 +32,19 @@ export const MUI_SHEET_ORDER = [ 'SvgIcon', + 'SwitchBase', + 'Switch', + 'Checkbox', + 'Radio', + 'RadioGroup', + 'SwitchLabel', + 'Dialog', 'DialogActions', 'DialogContent', 'DialogContentText', 'DialogTitle', - 'Switch', - 'Checkbox', - 'Radio', - 'RadioGroup', - 'TabIndicator', 'Tab', 'Tabs', diff --git a/src/utils/keyboardFocus.js b/src/utils/keyboardFocus.js new file mode 100644 index 00000000000000..ff3c1b80de09a4 --- /dev/null +++ b/src/utils/keyboardFocus.js @@ -0,0 +1,34 @@ +// @flow weak + +import keycode from 'keycode'; +import addEventListener from '../utils/addEventListener'; + +const FOCUS_KEYS = ['tab', 'enter', 'space', 'esc', 'up', 'down', 'left', 'right']; + +const internal = { + listening: false, + focusKeyPressed: false, +}; + +function isFocusKey(event) { + return FOCUS_KEYS.indexOf(keycode(event)) !== -1; +} + +export function listenForFocusKeys() { + if (!internal.listening) { + addEventListener(window, 'keyup', (event) => { + if (isFocusKey(event)) { + internal.focusKeyPressed = true; + } + }); + internal.listening = true; + } +} + +export function focusKeyPressed(pressed) { + if (typeof pressed !== 'undefined') { + internal.focusKeyPressed = Boolean(pressed); + } + + return internal.focusKeyPressed; +} diff --git a/test/integration/RadioGroup.test.js b/test/integration/RadioGroup.test.js deleted file mode 100644 index 41768b47fa461f..00000000000000 --- a/test/integration/RadioGroup.test.js +++ /dev/null @@ -1,276 +0,0 @@ -// @flow weak -/* eslint-env mocha */ - -import React from 'react'; -import keycode from 'keycode'; -import { assert } from 'chai'; -import { spy } from 'sinon'; -import RadioGroup from 'src/Radio/RadioGroup'; -import Radio from 'src/Radio'; -import { createMountWithContext } from 'test/utils'; - -function assertRadioSelected(wrapper, selectedIndex) { - const radios = wrapper.find('Radio'); - - radios.forEach((radio, index) => { - if (index === selectedIndex) { - assert.strictEqual(radio.prop('checked'), true, 'selected radio should be checked'); - assert.strictEqual(radio.find('[role="radio"]').prop('aria-checked'), true, - 'radio should be aria-checked'); - assert.strictEqual(radio.find('input').prop('checked'), true, 'selected radio should be checked'); - } else { - assert.strictEqual(radio.prop('checked'), false, 'radio should not be checked'); - assert.strictEqual(radio.find('[role="radio"]').prop('aria-checked'), false, - 'radio should not be aria-checked'); - assert.strictEqual(radio.find('input').prop('checked'), false, 'radio should not be checked'); - } - }); -} - -function assertRadioTabIndexed(wrapper, tabIndexed) { - const radios = wrapper.find('Radio'); - - radios.forEach((radio, index) => { - if (index === tabIndexed) { - assert.strictEqual(radio.prop('tabIndex'), '0', 'selected radio should have the tab index'); - } else { - assert.strictEqual(radio.prop('tabIndex'), '-1', `radio at index ${index} should not be tab focusable`); - } - }); -} - -function assertRadioFocused(wrapper, tabIndexed) { - const radios = wrapper.find('Radio'); - - radios.forEach((radio, index) => { - if (index === tabIndexed) { - assert.strictEqual(radio.find('[role="radio"]').get(0), document.activeElement, 'should be focused'); - } - }); -} - -describe(' integration', () => { - let mount; - - before(() => { - mount = createMountWithContext(); - }); - after(() => { - mount.cleanUp(); - }); - - describe('controlled radio interaction', () => { - let handleChange; - let wrapper; - - before(() => { - handleChange = spy(); - wrapper = mount( - - - - - - , - ); - }); - - it('should detect controlled use', () => { - assert.strictEqual(wrapper.instance().isControlled, true); - }); - - it('should mark up the root node with role and our supplied aria-label', () => { - const rootDiv = wrapper.find('[data-mui-test="RadioGroup"]'); - assert.strictEqual(rootDiv.length, 1, 'should exist'); - assert.strictEqual(rootDiv.prop('aria-label'), 'Cheese', true); - assert.strictEqual(rootDiv.prop('role'), 'radiogroup', true); - }); - - it('should have radios inside the group', () => { - const radios = wrapper.find('[role="radio"]'); - assert.strictEqual(radios.length, 4, 'should have 4 radios'); - }); - - it('should have nothing selected but the first item tabIndexed', () => { - assertRadioSelected(wrapper, -1); - assertRadioTabIndexed(wrapper, 0); - }); - - it('should focus the second value', () => { - wrapper.find('[role="radio"]').first().get(0).focus(); - wrapper.simulate('keyDown', { which: keycode('down') }); - assertRadioTabIndexed(wrapper, 1); - assertRadioFocused(wrapper, 1); - }); - - it('should reset the tabIndex to the first item after blur', (done) => { - document.activeElement.blur(); - setTimeout(() => { - assertRadioTabIndexed(wrapper, 0); - done(); - }, 110); - }); - - it('should select/choose the first value', () => { - wrapper.find('[role="radio"]').first().simulate('click'); - wrapper.find('[role="radio"]').first().get(0).focus(); - assert.strictEqual(handleChange.callCount, 1, 'should have called handleChange'); - assert.strictEqual(handleChange.args[0][1], '0x', 'should pass 0x as the 2nd arg'); - - // controlled! - // this is our pseudo managed state in action - wrapper.setProps({ selectedValue: '0x' }); - assertRadioSelected(wrapper, 0); - assertRadioTabIndexed(wrapper, 0); - }); - - it('should focus the second value', () => { - wrapper.simulate('keyDown', { which: keycode('down') }); - assertRadioTabIndexed(wrapper, 1); - assertRadioFocused(wrapper, 1); - }); - - it('should focus the third value', () => { - wrapper.simulate('keyDown', { which: keycode('down') }); - assertRadioTabIndexed(wrapper, 2); - assertRadioFocused(wrapper, 2); - }); - - it('should select the third value', () => { - wrapper.find('[role="radio"]').at(2).simulate('click'); - assert.strictEqual(handleChange.callCount, 2, 'should have called handleChange'); - assert.strictEqual(handleChange.args[1][1], '2x', 'should pass 2x as the 2nd arg'); - }); - - it('should focus the fourth value', () => { - wrapper.simulate('keyDown', { which: keycode('down') }); - assertRadioTabIndexed(wrapper, 3); - assertRadioFocused(wrapper, 3); - }); - - it('should focus the second value', () => { - wrapper.simulate('keyDown', { which: keycode('up') }); - wrapper.simulate('keyDown', { which: keycode('up') }); - assertRadioTabIndexed(wrapper, 1); - assertRadioFocused(wrapper, 1); - }); - - it('should select the second value', () => { - wrapper.find('[role="radio"]').at(1).simulate('click'); - assert.strictEqual(handleChange.callCount, 3, 'should have called handleChange'); - assert.strictEqual(handleChange.args[2][1], '1x', 'should pass 1x as the 2nd arg'); - }); - }); - - describe('initially selected value', () => { - let wrapper; - - before(() => { - wrapper = mount( - - - - - - , - ); - }); - - it('should set the tabIndex to the third radio', () => { - assertRadioTabIndexed(wrapper, 2); - }); - - it('should set focus and tabIndex to the second radio', () => { - wrapper.find('[role="radio"]').at(2).get(0).focus(); - wrapper.simulate('keyDown', { which: keycode('up') }); - assertRadioTabIndexed(wrapper, 1); - assertRadioFocused(wrapper, 1); - }); - - it('should reset the tabIndex to the selected item after blur', (done) => { - document.activeElement.blur(); - setTimeout(() => { - assertRadioTabIndexed(wrapper, 2); - done(); - }, 110); - }); - }); - - describe('uncontrolled interaction', () => { - let handleChange; - let wrapper; - - before(() => { - handleChange = spy(); - wrapper = mount( - - - - - - , - ); - }); - - it('should detect uncontrolled use', () => { - assert.strictEqual(wrapper.instance().isControlled, false); - }); - - it('should have nothing selected but the first item tabIndexed', () => { - assertRadioSelected(wrapper, -1); - assertRadioTabIndexed(wrapper, 0); - }); - - it('should select/choose the first value', () => { - wrapper.find('[role="radio"]').first().simulate('click'); - wrapper.find('[role="radio"]').first().get(0).focus(); - assert.strictEqual(handleChange.callCount, 1, 'should have called handleChange'); - assert.strictEqual(handleChange.args[0][1], '0x', 'should pass 0x as the 2nd arg'); - assertRadioSelected(wrapper, 0); - assertRadioTabIndexed(wrapper, 0); - }); - - it('should select/choose the third value', () => { - wrapper.find('[role="radio"]').at(2).simulate('click'); - wrapper.find('[role="radio"]').at(2).get(0).focus(); - assert.strictEqual(handleChange.callCount, 2, 'should have called handleChange'); - assert.strictEqual(handleChange.args[1][1], '2x', 'should pass 0x as the 2nd arg'); - assertRadioSelected(wrapper, 2); - assertRadioTabIndexed(wrapper, 2); - }); - - it('should set focus and tabIndex to the fourth radio', () => { - wrapper.simulate('keyDown', { which: keycode('right') }); - assertRadioTabIndexed(wrapper, 3); - assertRadioFocused(wrapper, 3); - }); - - it('should set focus and tabIndex to the first radio', () => { - wrapper.simulate('keyDown', { which: keycode('down') }); - assertRadioTabIndexed(wrapper, 0); - assertRadioFocused(wrapper, 0); - }); - - it('should set focus and tabIndex to the fourth radio', () => { - wrapper.simulate('keyDown', { which: keycode('left') }); - assertRadioTabIndexed(wrapper, 3); - assertRadioFocused(wrapper, 3); - }); - - it('should set focus and tabIndex to the third radio', () => { - wrapper.simulate('keyDown', { which: keycode('up') }); - assertRadioTabIndexed(wrapper, 2); - assertRadioFocused(wrapper, 2); - }); - }); -}); diff --git a/test/regressions/screenshots/baseline/Checkbox/DisabledCheckbox/chrome-53.0.2785.143-linux.png b/test/regressions/screenshots/baseline/Checkbox/DisabledCheckbox/chrome-53.0.2785.143-linux.png index 1d0dae5c336c37687befb41acc91f4643d4a1d0d..b6b5822dd1802b550446c34e4ca90a467b9098df 100644 GIT binary patch literal 1452 zcmeAS@N?(olHy`uVBq!ia0vp^XMos`gAGXLMEz6*QcRvMjv*C{Z=~ujF)*;Ef_QHa zZuFM^FMs^w^Sx(ZW_m7gs&eFUEV#H$Wzy2-#T@UJX8%9#*QBA;cdOg^^@9X{qnK4e zyJz&T-TQlzqsG$x|LnG|-1w`?rb$13pGZfCxUMC?m%i70k9zJhV^`&FFetgk<7RLk5f^CY)$;m*KQ-!`?`TYF+%COZn>qE2@|1UG( z$uyhYlISr>1t{AJl>PF>s(*Sh+gGA3#@gCCxxRk><;%gxjvWI!cgyYELk2E+?hV%jxq#T$9tMO>$Nsn)6SIL|t(_OwXyxs2ze z2OziH3BJFt_VKG%UGJxQsj^wk@smq3@* zS0BB;zFtN~rl7R+=vHtL0QJr(R`F~q?hDi|^nd&A-KO2UPk)cwTXpow6JR_V+S~Wf zEX++yYdd+;bCb^Exb>4$B|m@uJVWw&T3XtSIdfD_8@>8!_33$&!h)cc7jn!jEi4`Y z-H2xP?gd9|w{F?uB6GYZKPjnck;aZads4Epy5wC0TZ;QoxqLs1LBW@pnc3;J)M@k0 zCwF(3+f;w+Dcadn+^6DMaIP?X%8un)Q@ILdJ{v!N{Mh38kBP;78eJSs4Ie)i&ME$L z!V(yUCkiZ{zbLWN(bWxH8B+1_5v!xXgE`8cLUW4ms2*b$Ic}IaD@cU(I?zk^-v_V0 zD$((HhKVmQ8)O61?)&d6uV$V2@a1>yB$bDsK21{KC^$D!h4Y-{JE_NvSzDuGVq!LI z-Ripi_R|+HI_A!mePoeVRyM6MZ1vHjM_Hqzqf2(j_VxAMxOr2tFF7qeed5Dc-+z}_ z$=cQb^8qEd9o=3w4wo-qUbAl9rtRC0fBR+z^o5BO@2$6HjsgY-wzg+qz0#_#uFlEN zH@36$TYPcB-8^9yMxz>Waq&I1zo+e9b0j`Mgmv+!A0LzB_Sc>L9}LWU(b12?S6$W0 z&d$D*>Kdr9efg0IE`diJW-VKidCC_U%73CnTzQscdd*sPCq%@x#|g-D7ID2}mh6$W zRjN=XIqgo%Ou^%ZTcdb?ZjD;&CUbm-qbP0l+XkK D!sfSV literal 1467 zcmeAS@N?(olHy`uVBq!ia0vp^bs)^a1|%OTFYN$QOr9=|Ar*{or0Om)FtC<@cyA9n zdP`?ZH$41ZJ4YpX>!c+ejH0;~7iO+$iHOQnb*jt$-)6$?XwEgs)urTED~F!cu8HP` zWqgO*721RCR$01zwz>Ya?QmjSpu2$}*6)F-_Sr$)92odV>Y6|ouPcZ1fj4!`y ztMtCmydabBdcC}?HEzv@FXzEs)n`EE1q`@FdI?Uizj$&dD`U(ecFaPFh}CY{BX zUq0A-+@`<5u=w1=Imto$>i?fRdX)A5v(wY{|9rh3FTpXxskpef=GT|b`l|2mWS`YD zEelu}kWgAG`nj5sD>d@i$B%}~moKk)uQ~m6PEO8-O`8tw)j4fpXZP&O7m@e2Pm5;u z_VR{>h0S1Va;mPbF4-L`)yuXvto!HB%ChqEoqP5?xp(iL+}*tCZi^jPUrnm3n|Jqa zte(F9OtwQJr#AIm%s7%^l4K%fDAl`T_ip9u@4s7GTPGJ6Pj*{;@YO3VYinyAJ-woe z3XgU7-xrsa9eegnt=Fyj>ebM)va*Pnm?s}TOvn^4Xs!GA=j8VM`xP$^xAWKh`!jR( zYVD%0Rdd^4eYN`i`!`R!^SX8GPF#ImD!1zT>o@P;8_V!%igoWvV+_s!I%s|T{!bTX zo9Cb4Wn<=QWomV5Y)?y1KMCZqH8TQvP74n>Z{EE536OU*Y2tOA1TI#`1D7rd{a+TM zRajkp`tDs>V`JkxyLawX4A+{fG1cqNy?aIF<;TyQ;rX7Imv>^Smuj4~Q`#??ER9+lq$5`OmTAh$u+?kUuP-hrI8b1bl9<@2;<;nr zzGZjwCa=aJVa&@Ua_io`XD?oKMPMwI z?LPbIlhI{`gfp9T9{U61b;p}GZ$A9*;5eH$Ic@XF_b`SDm&9ZPo{#q~zbvU(W58p0 z-dSR@MiYB`s>L-etv!*?tPB9R}B=MEM%Z~uexY$8259TdZp7w zR&)98RjVDXJ|A59ex^v`p*h7C&kKP0;IL(TG2e5UbC$>geb-;Ty}f-8OH-xU)~MdI zXML|-yEen*piMV0;$)6LnN#dDE8$-;P|Bk3q^hUWtYtv{yqvtezWXb$W*sT?(eo6# zWb#y1;LPMTYuA4K_}G2gv}r$539`3PX7iRUNA@C1O=9@0k~wQq z_5Xjig))>0+<(qU4+xQ-6X-Vw(KF@FY z;pEoD6HCCH<-o}G6jW2`QaWVtWaA+NAVcjbFdwxfo&YNH5OMYKI?xiLCAw|fwu-f@ nu4)0Jeoq+Lr5KjT|HoEo+aJ2>`nyxW(wM>1)z4*}Q$iB}D$J%4 diff --git a/test/regressions/screenshots/baseline/Radio/DisabledRadio/chrome-53.0.2785.143-linux.png b/test/regressions/screenshots/baseline/Radio/DisabledRadio/chrome-53.0.2785.143-linux.png index fcf58e4ef7af687b83512b43aee7a51b5ce106c8..ce639bdc71ea7f49e5b01c9604f65b0d69cab705 100644 GIT binary patch literal 2934 zcmcguc{H2p8rM>kRBe|q##UELEfGr-qdF>O)NvIs)Xwj zT57AUiYjSpJEEke1gRQh8(S@%*FE>noO932ow@(aAMg8}@7eD*%8Qy|Q?lxf9{)Iz#`&OLdgOTfli=jt5@AeYouH}~uU(3nKad38i z5h&L+cEsfCijjxxMe%qgN-y8A9=dE;LsRo{OG_#Tv3uoz?K&Dz6kOqXj~W^*YR8BM z)D_x%a55`DroDfE-}uh?TkGo~m|;|Ob1(7v#DqN4i!$c>BDZu{jo|ZX&v<8NM^fkr zYjjk{ni8AxhesIhL9c-w1iCoiAuBce%a?n(Yp?qIDHv5G66uDJ8zMDa)05ScJqDhp zQmKe^a-z22;eU>(n}|Nhat{f~jyCS!-8JeZen+85#CIw!hEmOoO^OE}OfJbB9^+L{Uljm2W2A_-5}ezIx&Y35{L+}xaMIwwFTRp7Y% zZXa@xur}Q-{2VsZmwnpT-=FhTtKQE8$}iRld*nxT)UqhL^lbAc5Lkc}7!A>Z@UHYf zVYP&>#K9m+}JP}xt?8Id;&OTHYl(5Lum8v2HQWH`Sz{cxzD@0pBKDp zWvZQ9&-;LPIj;-8Iz`+1YCbrBanTRlu&}VOtN5mG7M|+{g8gyi^b^)h5c0>jO=Mhh zI|W-dHZf63pa%s9qv{AWN39!!nAhmyL14aIPm!H^QI@g@O_GL7vZBQ;?SM$x+uI{5 zm`rQnY8SXQ<#U{Q@vWsDM7n9Y-FY=LIV&|Qns~O=k;zRE*viVvfI$#}wj~ZigoZmx zlgtEh+~49HH!Kk0tbA{$-hJ`h9_q1!N*?B6)kTh)})CRbcuaMpghQfWuxQ$YyX-Y@c60r2lf?WO@SQ#c(-Lu%<#+{lZpa& zbBlgi7DXBxm*UDVRgVr5bg9e%f^GF^xt+S4f`SBqGWCqVc=^b6M?K07QNBYaqON$U z?dbbUnf+We4VP+uB0VGHu^&}oetG#Wx4&?BJ{Sfe*VX9*rvhl&CL|;n#qQq&fpmjb z(&;u@xoB02V~(PrU>pyl<|TJ6IwRWnqI$Nwo@`)rbgCcQG3(d8d*kok{T*W3rV23) z4GU`uTawTGufvkvNGOCKB_%D5Oz}0iK!ib;Ppn6tl{b#IBzk)%(CL~Ro0}24TRH$e zmer%2sm=scXlUr0@p1Rq*utI+1z>NZ2D#(gm($bA-kD6VBnUsR){rYqv8J){WjVkV zKlxCsHw-$k7>dmP_x5+aNN7e6N`o=2p}|moST|P*D1g#?TRX25Ju}P5b;#-7+u5|S z66+S50Z;AmP|ooTl*;iG8zkJwe8JHXW7Ht@p^!um81ya>>X2iG z@?o06w=*I)evs+Ef0lEheRoy3-f!5~0Po^rq1Igv65N+vI_5haaX?Fer?PPO2u@Dj z^?vzG)%#qf{m(i`fpWX#L{@(7072rhMoR7Y#Kembr#2#9s&))Q*H7mxdxOM)LGqYh zSLuOr3Fj`94c~Tq=?TJ&dcwMQhn1h`iFYiqQ$J9q9HVL~mxqbBCHNfZo{4#FqDoO3 zT3APC*zfuE>@pv4PdDzDTMlsHAktQzyLV`x_$vRVQG-m06>4{ zOaefj%CGCTAR`$yTlv9;*5!87cFph1eGV9<-tR`HB)4nLNO@_p-P^EWRTD&B!8lqG zxc%HXizQ_ux;)b>AtE9YzWL*q7YmKML!7Fdn$3wmJ}HWV;j1%81q1|IqBhgZE*4Hd z)f%mG;=m*BTg(8sV9IRUx`Pd8I}?th09xSTzbq>QZeE%=TjkuE?1@(qKMHX7airpH zbJ%fqeG<549KH25P)RF)<$VX)1t|bnUQS*fE}I5TGuP1820q_9S$HniD=bWUaue_1 z;OgN)Mk#k}3U%I{jQw1&Fj`#;us-^xo(Y(xj*xc7e`0{tfWLCZ!`oY+6PEaU8;zJh zDanjlSjYwRFnG1=s-Am9L>@}{mKzihEwJ;``U0?2E4LE{QT`1j&8$*;I5J$6k2MYF zW+Z>0=pwrTptl_q_iC5+(#a(cywq%v45LQ$Gv-g6YSbY3sAmfSg6-}$BTcrYdE$4o zVUUQuPx^Z?3XGalxeSoLX1el5;mfDFU3%6*O>&@0KoU%iZ+^PXlI?`StR-lYCr_T- zYP(3c=KyVPyCrfEZ(E~44HH}Wz7!4W%!dz&0JX|h@(GcJZhSw@DGTrpCLy3cia*Re#;3=sT$rQ7F l>HjRQ`j?8H@A@mpMywDSmpa)u1ObgS584cisygo)_cQS3hQ0s* literal 2741 zcmb`JdoqzhB-OemLe zB$rs1p+aIrnxo0i+98(_YG`5-LcL$@Iq$pY-S_Ny|LC0W@Ap05%k%p^&-Zzr&-1

2k)ta6=|T8!k(~jiI<#*B|evskPF`$Ep#pZ#0}oo zxfr3|PJ3U-s=x{h!^t>$%H;R?kuqmX>pPY|$+LW(JM370~g%Fk>Q8MZrq3Y$#0|Yx}wlZ&t!?Cc)Vj zTvWC&Z&EF-6f(Y|N@r6;O^cR3EO>f%B;pM6y2Y3iCR*;BAu(b@Mf5423q?4hzYMfhjwW+>5gm6az;cuXNPC6$0}SRF))80jXC% zUbK{a)nPuzP`}o+b-G{nOOi!o2 z`2BZS-YJ8;wRKU)<fawsvY7z(a$YU(Moq4&*-F_fj$ z@Ros5ZlyyA3Km8pEzS)cTw2386k~4l7nTkx5R#t+Sf1pO4eWSiXKd!XrZbJI+}GK! zWBX%G15Jyf(=ATwm=&zzp zfU5Ps{POtu^W7RppeZhy+S=GhU|?rwM}3Nfg4OGTXV!`XTcwyGM(T8vy+NEI@85h@EN~$=I0JmP=ROdKH@| zJqs}MqfiREyMOq&Y1ZD}z5y{JNaDEfFm&rOi^h@1NT{ou+uH8#$?LAwR{dfO3McSP zSbe8?*ymySP%yH<-rd8)@9f!qJaWPRo?cT7Xw}LP)Ze?;H8iA{kdUykA?9Qc1b!+B zZ?UT$p4!;B8QOFO7+k}qYs4sON*_CRY<_`6IFvCW&gxwMp& zZI6)U1cS!5wkx&5@=R?%H2k@6IZcr7(sa7QwfZ)p1nTxwjLFT*17U;CFhQdyVxZl( zyz7X3aOjOYJfVATPhJCJ z2S*%i%vgQsaNF^FE~=LM=jBmf;ph)CT0CafZ;0%l0o$HK{pBq>>9rV zP%ftQ3JihUgujYbC@XLGGef4@3}PONxWTA+@r-;>P|(eyqQ#j(H9-837XH&y`g?&} z1&jGzzMWbS%51|Q?&Oh-3a$g9u+SMS zK!vBEGJ?XUY;nsLTQ4yZmJ3KmaLv@-l4uKT^Z+131{Lb;GsI=FzSPlYK-o?d&bhL( zlJILJT04g`%L-?PWSSm79vFW9e9=`G{k=1wPF7LPIAH57N8Rgk-PZ@+0L+H*XWI6D zftb-OdZ$n*q(1BI-JT&p@`TH_Bh7`IRo z%@5cN%CB1;k9Ah0-V!lx4M|qGz;B6&5xqQ);Sg*23%l8dEW%E9lzS?42PlxmvqQls z;A$0G*PA!`KK=j(pqf&OFEK+1NBnyG+#v*3=g0BcW!yt*#Ospt)L=Vi81sS0D>|yGGAzYhD_e; z#ls%LJ_0)Ngoh7kpiiHz)=RiRlt@CG-Wa5;|H-P^l& z<@^+SW@6%Jjv+MV+=C1fA90$d))@s4<1@!aG11YMaJIe99_NuTWiCojNlxji6#xh` z3NFKMe!a4O^X5%(*jZ0fU5pt$D4g6ysUCkt9Jf{0XjGw)>7<{dn{$#%N8!p`^ub_N g{i|aTKy_0_ZM6MjCsA!bxQ9w$t?(ymEqyQi3s0N=bpQYW diff --git a/test/regressions/screenshots/baseline/Switch/DisabledSwitch/chrome-53.0.2785.143-linux.png b/test/regressions/screenshots/baseline/Switch/DisabledSwitch/chrome-53.0.2785.143-linux.png index 48a60810b9bc9cb0de5b3f1f5dd8c3aaf30d9f31..751ac747c85cf1016772d4f0745d4f723ca8bbf5 100644 GIT binary patch literal 3159 zcmcgvYgkfg9(Q)TSXoI?l9?9aH8nA7z`TrxHl%1ao>DhWhs+qvR5UdVcg#l#5sM0o z5KCv#Io@^6yXFnYEEr?bd@2mh&{adT!Yl4e`(fvK_SxNU^Wks~hjZTd|Nd_Omuts; zj&59UxL!v`XCv`j!U=Hx3=SXNHQ;k==ESey^d}<0%|G$0Nkhgn9UVP=?YF5m>BYZ5 z3y&1iMLA8{{IGoLjoBqOm(p3W>u07-FMLAHoLb7Rc6Z;M_5%H)@|zzF4{k6QUAv8w zEGJ1$1>V01ktomVY-xDu-tfT7sk_f_Gu2J)_X8KZ{^$#AQ-qw}q{b-|Z^LLbQ@1T+ zq`8}(P2-&U*$SP?)L(K{Im_t+TGKHIfgq%{{;7jLEGVEDzMIA87AI&EvoO5I81UJu`~%*e~l-IZ_Q4$U8?N%|Z% z&dv*Zd%bwQrXGILdx=}BtP6%z)z#HUkRC-=Cfu;6O=<^H$M=OkqqePGDK!a^exli42gyo#C{52OcE*CaJ8OF;97 zy88MM=H}+FJpGVPDH?65nt5r;mToM3tb!rI!|?Ff81v@Mtx;fB#`33A;t}lHwQD|?Kwx}#mFbi|q3})5v z97sJ?dI&}NBf2!U9wVhauIj{h-%C_k_HqGKADonBKEA#P#HLM~m?J1e*iAj^Glz{z zrSgfACWnMdK;oLKUx`ujPW(}!@@bqs?cNzu%jXzqoScjug8^sH2g zC^%Qw2$#IsvJXxX^{4GodUoPzGYS5Klev@zcmxZ;5a+Ig^-fN*8_MdlM3lOQ2DFqW zf8Nux?rPg#r=~8jv60-cjRG2ARH4|Tj*p3PHR-X&*_61KYqORGFIZpGX=_gH2{WQW|%-8&>*tI5U42fP(c~BUokwK zisSe}Q4^p(pdUioBEWJ|rIMMEWOnzS7ESQu>+0%ubIAU;UpgK>OioBJ0FX7Q1-ZE~ z1|(T@Ow5mJ;UYZ2AB+^cYGyY3-zH&S-Bb%<7zdsXX64^6qiRK2>o;()ca+O$l`ek# zFzUrv7&m9EEGOh!3@bV^k~QBZZy*n_ z*nm@7^lni(C)`ltRWjhAiSpKWVuLfuZ3RcO6E#ctA;zMOoE~MpeM5_8ff|)O(`Ddv z`6E*)&6}O79U5+qkTt1qg%gF!l1v(>%6djlGe{CvQ<8)<13CSR#v0xx`2_M3RRCdU zys`*huX0AWH~bc#McV|8K73sDwuLWxP(}?2XDh_}TbJCq7$?RO3L5on9a7%A*rJ%5 zyR*D}g)u*Bz{MzLMu088URZV+N}k==-Q5j1sU~VY)=tl6R@lY|j|h%H^`%wTgMY8G zuHT?EiVv>#k<-EN_zbveUHZXjiK6)^Az-b_S%P;-Gf7#xp*>Dr`Sko$oZ~K#7CE5n;Qu(RhBJ(nwAxq{UCmM+9VGAAn6Th{O$4 z8}+Bbh&=3F??i};0fu9xM%DICJaaIW(wfwOx!RYb@f1E+i8o=Xh4C?x8QYSYNPZ2^ zNm>uagdaNs>>N4_wc2lRAYFxHPf_@>XMjt3&bFtJOTjQ9IZ8Z*k46Vjdu9a#qe(t| zQL-+~f~#yvvitBWh%br^;)?*?!9`XiStvVfm;W{>{N^qC31}E68knfGMWz+hF7IlG7$X@bj8OZ#6}7e9S^w)t(KX%vp!AMC68!sO z*s7!^fQmpwQ5_8Tpd-H;+!0c2Tbi2qxjX`(Dc()Fj6Susl z)&%uUe6kboMmJ?)(`ORBxtOMlAfe2Cg~xfOLn$lHt}9CiHk-J-1>$4se(hO+@D(~e zu=Z+M(a;&1`3Pf|SwRkhqhq_a&_6j&m_# zPR3-m;sRk6S+6-H`}q%DS4@fSpbich*b-afhV%dgeExiBe@N3KR7Ycg^>)3CBbS?v z{g5-iPw^I)g|WSwWmU@Tp~Hs{J6&103f8Q~Z&l9;7jg7tkZU!k^JrJO%w$MjiU8{k-5oyxZh1LXN^Q4viu?C^jb6sO&BT!Yo1v;SIf;D!j(Yx?Fkh}sxD0>fwtnr2*kmG0V z!yI#;O?(2SKg`T$0>i~QcG0DvyNFT4xOY;2$V2AfZMGF8B3QBo>xDxHpQ#&TbO5CX zUhydejeuMeq-gF&UB#yF)W*t)XQ3=CAt46{q;&|~X6l#ZF!j5!04PvJoFlLWSht+c z#-hk|0J-?!z38~c;Lo2w11`*Z?STsj=MN)5iJ*`AA^XPT?mRtLa&ljj8b!7fM}yq{ zM+3m;QX_z?vbmYW3)lAU3QGFH6HrMF!7r_bXWa|*a5^|R=E@aPAn#R0|FiM9u}_yH zFI>10z9ZLXEijz>G^5C>KPca#J;;j}4pO_UtgNo3MP%(26+qpwDFXoa@Zm$0-kP5% z$FzHx6a($vz=BoL&kf%Jlu}&0511S533h(56~Lyn)CLqx;cGW-?HQo~2M}$jBegG0 ziCci}l7!zOS4;3wTIR2dEVUE66Z8?OYA=GF_}8gRL~a_;o=}PO_>soh2_NHPexiJt z?>MC4Xie}sDhESEZOQ;W!Vig4);)kRM0dU@fO;)o7aBcc-((JA?0fbD+4vj52n4g3 zyV0xe6~aYUViZVc2|na+FjE)puJa#HEp0=|4<=RA{JYrND5j0YXQM5oYB7#PY zf*}ZKu(yB^1$^)jC_zC27?;#W0yIEWgaD}o5W{}B+nw!9XSO@j{Ui6x$<4jzd;A_> z&TRAZnP;@bNKa319%(aiJFJ=Ti8GiB*BAB%39!s05jOBy7(MjBuUx7@w-t z1@(WT-)q=gvPyFI7e;#w2|)z56OYnujsA8fSMUF{JRHKX)WZrTU+IB(6Bl4|`)h_U z?yCKA-m4>x{jysbEvj_-;|yHWBMm>Ux&-Ml5xNocH{I`1ZaQ+a~VG zT{ttG*+;7`8m+`cH4d;*M?)zrg~hAl@MOv@2fw%*mBvjLl`=B^0`b&f*~_2_ZT6nypcvhM*k2f@`9*w|p`6Ys#KvCP|c;t{I05Mgto$_prJj?XwTMhL*UU;zMetp)u zqtKFfTD9i&-yL+LT{W`=9<18J-Q{wL`a)Jgn61HvpS&!hvnbeilb2#y$QO*8%g za)a&$>ytfgc|IPjkwg?VKey?{gQIDytH$E1-;XKNJdXX!`P5&q4KW4Gv9*K4^@~HI zpTM-4IDd8hZHM zXyn1ps^$kb?aQM&`$h-k#3?$3i$ZCEC7~r0=PbXH_QMu*s;IoYVQ|pteH=vCrm8QV zN#eE+eDnPI^BQ(bTN_4f{uJNa`7DMkFFOz!S#taK##^$LB$M2hw6HnEL_FETc&-hp z7J7X8^l8YqLTgTQ7Do#smShSy5MO@t;LYHmsH)0Opt>HB%vo?E&e$Orr7%d`*$hXP zV9nL$SWVx&sLrYzsJ(>3u=tSDraVkS%_+b9jlYSJ7dGA{>0r4pk8XT=BS;3$m(Sz} zAR=6z^i^o#CnNyEOU>Rf15k!~^E!w{LXhl0h_f*(?pqmKU*Igq-n4IfDh=FVY+|)~{WPEW`6$N#Tbo}yWEd@JD|3{-N|)UVS(DuYIkxd|>Z~J8+J&mFs0Cx||uex&|jh0|T+J+QmcK=D?mYvZG$|5tG z_j<5LQ$)D&d>~JFNvLba&`7WEs8(xnZj%iuo)e82fBECI!c@c#$LHm(bjro~3Or9V z_Vuj-%tN2h!^_-N0N}-|zluI`k< zEXTeX7`On>cMX~@r-T^~7si~>%o}R+U-9_;xSN_v40+H{Ssb}|RCOzKcMdbxu65%qouELY+ zydgm}8QOBY0CH#)K|S_w3H_aMz{aAXp&?dn*vxJL|F{gT1P20H-YLafT3J0UbIh@u z!J00EQc2r+1LvD51kQMWMy@;V`RSeU<9 zi13*Zo>tHVrqP<5+vQrJ{SYoS|7%A9uB7C4r`DYmct>H`$Kw$C)d z6qEun`ApmoMT~+{3Il_2`r5;4B{p#t7A69xEyUvX@WLBK3|=pv=585Y=uI>r8Ez(m z6~|TkgMl+jD6Rc(rwzCw%_XV1nJmI7(LGc#08(tO@F6;j;Wh<^6^ABQ@aBk2EEdYh z`ca*R@f!hru|z)a~GTx&^vrf0Dy_8$)oxtgK}fS{R|iQdW?6 z=*Yn`;Cg$z*}8S>{wdwe%YW);_U1*Wrkw?3(S}3;0|h?RhkvWJq8W?LC3x~iDV}2h z$H9QoE|Wdh@bP2}#`7rFD^qAZcNW(sEHd(3UEM~rvvU;e6c=-vyL5VzSdKmNAo!c3 zN2wtppSCck2e-RMN&llf%f3Uyrxm!Mw1*`DqBdW3t2ptF2PCHz(%mhe_P+&(gBU;# zNL(1OOzTWdFexr@um}fO*uH2Eo%#!3nKErm&>seP{xvkn=f4a74|~UE-?T@QU6M*m zKl@E{y=J4yXm|>wJ5Ob)`raNJ&l&pO`M4KY!3N}v!XKvPz)SvFCf+k_0~Ve7tCvaZ YXQUEJ Date: Wed, 14 Dec 2016 12:07:45 -0500 Subject: [PATCH 2/8] Support keyboard focus for contained elements --- .flowconfig | 1 - src/internal/ButtonBase.js | 35 +++++++++++++++++------------------ src/utils/keyboardFocus.js | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/.flowconfig b/.flowconfig index e0e4f5cd0cea7b..e3ab7c4e868795 100644 --- a/.flowconfig +++ b/.flowconfig @@ -1,7 +1,6 @@ [ignore] .*/node_modules/fbjs/* -.*/node_modules/react-native/* .*/node_modules/jss/lib/.*\.js\.flow .*/node_modules/react-swipeable-views/src/.* .*/node_modules/react-native/* diff --git a/src/internal/ButtonBase.js b/src/internal/ButtonBase.js index 7aaac6c5c5d083..2c8c176036fdbd 100644 --- a/src/internal/ButtonBase.js +++ b/src/internal/ButtonBase.js @@ -5,7 +5,7 @@ import { findDOMNode } from 'react-dom'; import { createStyleSheet } from 'jss-theme-reactor'; import classNames from 'classnames'; import keycode from 'keycode'; -import { listenForFocusKeys, focusKeyPressed } from '../utils/keyboardFocus'; +import { listenForFocusKeys, detectKeyboardFocus, focusKeyPressed } from '../utils/keyboardFocus'; import { TouchRipple, createRippleHandler } from '../Ripple'; export const styleSheet = createStyleSheet('ButtonBase', () => { @@ -170,24 +170,23 @@ export default class ButtonBase extends Component { }); handleFocus = (event) => { - if (!this.props.disabled) { - // setTimeout is needed because the focus event fires - // first if focus was called programatically inside a keydown handler - event.persist(); - setTimeout(() => { - if (focusKeyPressed() && document.activeElement === findDOMNode(this.button)) { - this.keyDown = false; - focusKeyPressed(false); - this.setState({ keyboardFocused: true }); - if (this.props.onKeyboardFocus) { - this.props.onKeyboardFocus(event); - } - } - }, 150); - - if (this.props.onFocus) { - this.props.onFocus(event); + if (this.props.disabled) { + return; + } + + event.persist(); + + detectKeyboardFocus(this, findDOMNode(this.button), () => { + this.keyDown = false; + this.setState({ keyboardFocused: true }); + + if (this.props.onKeyboardFocus) { + this.props.onKeyboardFocus(event); } + }); + + if (this.props.onFocus) { + this.props.onFocus(event); } }; diff --git a/src/utils/keyboardFocus.js b/src/utils/keyboardFocus.js index ff3c1b80de09a4..d9b065685e5dc0 100644 --- a/src/utils/keyboardFocus.js +++ b/src/utils/keyboardFocus.js @@ -1,6 +1,7 @@ // @flow weak import keycode from 'keycode'; +import contains from 'dom-helpers/query/contains'; import addEventListener from '../utils/addEventListener'; const FOCUS_KEYS = ['tab', 'enter', 'space', 'esc', 'up', 'down', 'left', 'right']; @@ -14,6 +15,19 @@ function isFocusKey(event) { return FOCUS_KEYS.indexOf(keycode(event)) !== -1; } +export function detectKeyboardFocus(instance, element, cb, attempt = 1) { + instance.keyboardFocusTimeout = setTimeout(() => { + if ( + focusKeyPressed() && + (document.activeElement === element || contains(element, document.activeElement)) + ) { + cb(); + } else if (attempt < 5) { + detectKeyboardFocus(instance, element, cb, attempt + 1); + } + }, 40); +} + export function listenForFocusKeys() { if (!internal.listening) { addEventListener(window, 'keyup', (event) => { From 1a532a058b8edd01e270538589509877bb7875a8 Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Wed, 14 Dec 2016 21:44:39 -0500 Subject: [PATCH 3/8] Swap default/label exports for switches --- src/Checkbox/Checkbox.js | 6 ++++-- src/Checkbox/Checkbox.spec.js | 6 +++--- src/Checkbox/index.js | 2 +- src/Radio/Radio.js | 6 ++++-- src/Radio/Radio.spec.js | 6 +++--- src/Radio/index.js | 2 +- src/Switch/Switch.js | 6 ++++-- src/Switch/Switch.spec.js | 8 ++++---- src/Switch/index.js | 2 +- src/internal/SwitchLabel.js | 18 +++++++++++++----- src/internal/SwitchLabel.spec.js | 5 +++++ .../src/tests/Checkbox/DisabledCheckbox.js | 4 ++-- .../site/src/tests/Radio/DisabledRadio.js | 4 ++-- .../tests/RadioGroup/RadioGroupWithLabel.js | 2 +- .../RadioGroup/RadioGroupWithLabelError.js | 2 +- .../RadioGroup/RadioGroupWithLabelRequired.js | 2 +- .../src/tests/RadioGroup/SimpleRadioGroup.js | 2 +- .../site/src/tests/Switch/DisabledSwitch.js | 4 ++-- 18 files changed, 53 insertions(+), 34 deletions(-) diff --git a/src/Checkbox/Checkbox.js b/src/Checkbox/Checkbox.js index a559da0da53710..60c18f4127f404 100644 --- a/src/Checkbox/Checkbox.js +++ b/src/Checkbox/Checkbox.js @@ -22,6 +22,8 @@ const Checkbox = createSwitch({ styleSheet }); Checkbox.displayName = 'Checkbox'; -export { Checkbox }; +export default Checkbox; -export default withSwitchLabel(Checkbox); +const LabelCheckbox = withSwitchLabel(Checkbox); + +export { LabelCheckbox }; diff --git a/src/Checkbox/Checkbox.spec.js b/src/Checkbox/Checkbox.spec.js index 928b54b291979f..7c01873db54695 100644 --- a/src/Checkbox/Checkbox.spec.js +++ b/src/Checkbox/Checkbox.spec.js @@ -3,7 +3,7 @@ import { assert } from 'chai'; import { createShallowWithContext } from 'test/utils'; -import LabelCheckbox, { Checkbox, styleSheet } from './Checkbox'; +import Checkbox, { LabelCheckbox, styleSheet } from './Checkbox'; describe('', () => { let shallow; @@ -22,14 +22,14 @@ describe('', () => { }); }); - describe('named Checkbox export', () => { + describe('default Checkbox export', () => { it('should be a SwitchBase with the displayName set for debugging', () => { assert.strictEqual(Checkbox.name, 'SwitchBase'); assert.strictEqual(Checkbox.displayName, 'Checkbox'); }); }); - describe('default export', () => { + describe('named LabelCheckbox export', () => { it('should be Checkbox wrapped with SwitchLabel', () => { assert.strictEqual(LabelCheckbox.name, 'SwitchLabel'); assert.strictEqual(LabelCheckbox.displayName, 'withSwitchLabel(Checkbox)'); diff --git a/src/Checkbox/index.js b/src/Checkbox/index.js index e343c47d91c063..92279045609180 100644 --- a/src/Checkbox/index.js +++ b/src/Checkbox/index.js @@ -1,4 +1,4 @@ /* eslint-disable flowtype/require-valid-file-annotation */ export default from './Checkbox'; -export Checkbox from './Checkbox'; +export Checkbox, { LabelCheckbox } from './Checkbox'; diff --git a/src/Radio/Radio.js b/src/Radio/Radio.js index 26ba4f77fa7eb6..5a03b4c3dd87ce 100644 --- a/src/Radio/Radio.js +++ b/src/Radio/Radio.js @@ -27,6 +27,8 @@ const Radio = createSwitch({ Radio.displayName = 'Radio'; -export { Radio }; +export default Radio; -export default withSwitchLabel(Radio); +const LabelRadio = withSwitchLabel(Radio); + +export { LabelRadio }; diff --git a/src/Radio/Radio.spec.js b/src/Radio/Radio.spec.js index f249db498bf861..5d0d8c22abf7c0 100644 --- a/src/Radio/Radio.spec.js +++ b/src/Radio/Radio.spec.js @@ -3,7 +3,7 @@ import { assert } from 'chai'; import { createShallowWithContext } from 'test/utils'; -import LabelRadio, { Radio, styleSheet } from './Radio'; +import Radio, { LabelRadio, styleSheet } from './Radio'; describe('', () => { let shallow; @@ -22,14 +22,14 @@ describe('', () => { }); }); - describe('named Radio export', () => { + describe('default Radio export', () => { it('should be a SwitchBase with the displayName set for debugging', () => { assert.strictEqual(Radio.name, 'SwitchBase'); assert.strictEqual(Radio.displayName, 'Radio'); }); }); - describe('default export', () => { + describe('named LabelRadio export', () => { it('should be Radio wrapped with SwitchLabel', () => { assert.strictEqual(LabelRadio.name, 'SwitchLabel'); assert.strictEqual(LabelRadio.displayName, 'withSwitchLabel(Radio)'); diff --git a/src/Radio/index.js b/src/Radio/index.js index 4e66bd389cefaf..ece22d8b8205bc 100644 --- a/src/Radio/index.js +++ b/src/Radio/index.js @@ -1,5 +1,5 @@ /* eslint-disable flowtype/require-valid-file-annotation */ export default from './Radio'; -export Radio from './Radio'; +export Radio, { LabelRadio } from './Radio'; export RadioGroup from './RadioGroup'; diff --git a/src/Switch/Switch.js b/src/Switch/Switch.js index eb4808ebce9549..cd534933c03b82 100644 --- a/src/Switch/Switch.js +++ b/src/Switch/Switch.js @@ -85,6 +85,8 @@ Switch.contextTypes = { }; -export { Switch }; +export default Switch; -export default withSwitchLabel(Switch); +const LabelSwitch = withSwitchLabel(Switch); + +export { LabelSwitch }; diff --git a/src/Switch/Switch.spec.js b/src/Switch/Switch.spec.js index 589d6dd7ec2b82..51d5fb1a9bbadd 100644 --- a/src/Switch/Switch.spec.js +++ b/src/Switch/Switch.spec.js @@ -4,7 +4,7 @@ import React from 'react'; import { assert } from 'chai'; import { createShallowWithContext } from 'test/utils'; -import LabelSwitch, { Switch, styleSheet } from './Switch'; +import Switch, { LabelSwitch, styleSheet } from './Switch'; describe('', () => { let shallow; @@ -23,7 +23,7 @@ describe('', () => { }); }); - describe('named Switch export', () => { + describe('default Switch export', () => { let wrapper; beforeEach(() => { @@ -52,8 +52,8 @@ describe('', () => { }); }); - describe('default export', () => { - it('should be Switch wrapped with SwitchLabelt', () => { + describe('named LabelSwitch export', () => { + it('should be Switch wrapped with SwitchLabel', () => { assert.strictEqual(LabelSwitch.name, 'SwitchLabel'); assert.strictEqual(LabelSwitch.displayName, 'withSwitchLabel(Switch)'); }); diff --git a/src/Switch/index.js b/src/Switch/index.js index 25b128547f8937..d2a68d98ac9d39 100644 --- a/src/Switch/index.js +++ b/src/Switch/index.js @@ -1,4 +1,4 @@ /* eslint-disable flowtype/require-valid-file-annotation */ export default from './Switch'; -export Switch from './Switch'; +export Switch, { LabelSwitch } from './Switch'; diff --git a/src/internal/SwitchLabel.js b/src/internal/SwitchLabel.js index 26c829300ad36c..1656268f563ab9 100644 --- a/src/internal/SwitchLabel.js +++ b/src/internal/SwitchLabel.js @@ -70,13 +70,21 @@ export function withSwitchLabel(SwitchComponent) { [classes.hasLabel]: label && label.length, }, labelClassNameProp); + const switchElement = ( + { this.switch = c; }} + disabled={disabled} + {...other} + /> + ); + + if (!label) { + return switchElement; + } + return (

- +
diff --git a/test/regressions/site/src/tests/Radio/DisabledRadio.js b/test/regressions/site/src/tests/Radio/DisabledRadio.js index 8cc412e1e55554..667b62a36e23c1 100644 --- a/test/regressions/site/src/tests/Radio/DisabledRadio.js +++ b/test/regressions/site/src/tests/Radio/DisabledRadio.js @@ -1,12 +1,12 @@ // @flow weak import React from 'react'; -import Radio from 'material-ui/Radio'; +import Radio, { LabelRadio } from 'material-ui/Radio'; export default function DisabledRadio() { return (
- +
diff --git a/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabel.js b/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabel.js index 04bc0484001b5f..f8b522a2c5b32a 100644 --- a/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabel.js +++ b/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabel.js @@ -2,7 +2,7 @@ import React from 'react'; import { FormLabel } from 'material-ui/Form'; -import { RadioGroup, Radio } from 'material-ui/Radio'; +import { RadioGroup, LabelRadio as Radio } from 'material-ui/Radio'; export default function RadioGroupWithLabel() { return ( diff --git a/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelError.js b/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelError.js index 52ee4810574e69..eab862c7c1a969 100644 --- a/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelError.js +++ b/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelError.js @@ -2,7 +2,7 @@ import React from 'react'; import { FormLabel } from 'material-ui/Form'; -import { RadioGroup, Radio } from 'material-ui/Radio'; +import { RadioGroup, LabelRadio as Radio } from 'material-ui/Radio'; export default function RadioGroupWithLabelError() { return ( diff --git a/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelRequired.js b/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelRequired.js index d900ec4c050a41..fbd725bec4d28b 100644 --- a/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelRequired.js +++ b/test/regressions/site/src/tests/RadioGroup/RadioGroupWithLabelRequired.js @@ -2,7 +2,7 @@ import React from 'react'; import { FormLabel } from 'material-ui/Form'; -import { RadioGroup, Radio } from 'material-ui/Radio'; +import { RadioGroup, LabelRadio as Radio } from 'material-ui/Radio'; export default function RadioGroupWithLabelRequired() { return ( diff --git a/test/regressions/site/src/tests/RadioGroup/SimpleRadioGroup.js b/test/regressions/site/src/tests/RadioGroup/SimpleRadioGroup.js index 1b43d0705d4681..12b49327c926a9 100644 --- a/test/regressions/site/src/tests/RadioGroup/SimpleRadioGroup.js +++ b/test/regressions/site/src/tests/RadioGroup/SimpleRadioGroup.js @@ -1,7 +1,7 @@ // @flow weak import React from 'react'; -import { RadioGroup, Radio } from 'material-ui/Radio'; +import { RadioGroup, LabelRadio as Radio } from 'material-ui/Radio'; export default function SimpleRadioGroup() { return ( diff --git a/test/regressions/site/src/tests/Switch/DisabledSwitch.js b/test/regressions/site/src/tests/Switch/DisabledSwitch.js index 8ab92bd7f0eca1..6d93c52878a3c8 100644 --- a/test/regressions/site/src/tests/Switch/DisabledSwitch.js +++ b/test/regressions/site/src/tests/Switch/DisabledSwitch.js @@ -1,12 +1,12 @@ // @flow weak import React from 'react'; -import Switch from 'material-ui/Switch'; +import Switch, { LabelSwitch } from 'material-ui/Switch'; export default function DisabledSwitch() { return (
- +
From e4592faf86057099203d29b9cbd821c9efa65958 Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Wed, 14 Dec 2016 21:48:07 -0500 Subject: [PATCH 4/8] Remove extra whitespace --- src/Radio/RadioGroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Radio/RadioGroup.js b/src/Radio/RadioGroup.js index 4138b2188d29e3..9ad427be08c862 100644 --- a/src/Radio/RadioGroup.js +++ b/src/Radio/RadioGroup.js @@ -1,4 +1,4 @@ - // @flow weak +// @flow weak import React, { Component, Children, cloneElement, PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; From cd1978a2f7e854c3285f4d28555da6ff85158398 Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Wed, 14 Dec 2016 22:14:35 -0500 Subject: [PATCH 5/8] Use createHelper --- src/Checkbox/Checkbox.js | 2 +- src/Radio/Radio.js | 2 +- src/Switch/Switch.js | 2 +- src/internal/{SwitchLabel.js => withSwitchLabel.js} | 9 ++++----- .../{SwitchLabel.spec.js => withSwitchLabel.spec.js} | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) rename src/internal/{SwitchLabel.js => withSwitchLabel.js} (90%) rename src/internal/{SwitchLabel.spec.js => withSwitchLabel.spec.js} (97%) diff --git a/src/Checkbox/Checkbox.js b/src/Checkbox/Checkbox.js index 60c18f4127f404..e3752e0d1cdf77 100644 --- a/src/Checkbox/Checkbox.js +++ b/src/Checkbox/Checkbox.js @@ -2,7 +2,7 @@ import { createStyleSheet } from 'jss-theme-reactor'; import { createSwitch } from '../internal/SwitchBase'; -import { withSwitchLabel } from '../internal/SwitchLabel'; +import withSwitchLabel from '../internal/withSwitchLabel'; export const styleSheet = createStyleSheet('Checkbox', (theme) => { return { diff --git a/src/Radio/Radio.js b/src/Radio/Radio.js index 5a03b4c3dd87ce..4d209c9e3c227e 100644 --- a/src/Radio/Radio.js +++ b/src/Radio/Radio.js @@ -2,7 +2,7 @@ import { createStyleSheet } from 'jss-theme-reactor'; import { createSwitch } from '../internal/SwitchBase'; -import { withSwitchLabel } from '../internal/SwitchLabel'; +import withSwitchLabel from '../internal/withSwitchLabel'; export const styleSheet = createStyleSheet('Radio', (theme) => { return { diff --git a/src/Switch/Switch.js b/src/Switch/Switch.js index cd534933c03b82..6ced943db12293 100644 --- a/src/Switch/Switch.js +++ b/src/Switch/Switch.js @@ -4,7 +4,7 @@ import React, { PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; import classNames from 'classnames'; import { createSwitch } from '../internal/SwitchBase'; -import { withSwitchLabel } from '../internal/SwitchLabel'; +import withSwitchLabel from '../internal/withSwitchLabel'; export const styleSheet = createStyleSheet('Switch', (theme) => { const { palette } = theme; diff --git a/src/internal/SwitchLabel.js b/src/internal/withSwitchLabel.js similarity index 90% rename from src/internal/SwitchLabel.js rename to src/internal/withSwitchLabel.js index 1656268f563ab9..3a4d5b418c04f7 100644 --- a/src/internal/SwitchLabel.js +++ b/src/internal/withSwitchLabel.js @@ -3,6 +3,7 @@ import React, { Component, PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; +import createHelper from 'recompose/createHelper'; import classNames from 'classnames'; export const styleSheet = createStyleSheet('SwitchLabel', (theme) => { @@ -23,12 +24,8 @@ export const styleSheet = createStyleSheet('SwitchLabel', (theme) => { }; }); -export function withSwitchLabel(SwitchComponent) { - const switchComponentName = SwitchComponent.displayName || SwitchComponent.name; - +function withSwitchLabel(SwitchComponent) { return class SwitchLabel extends Component { - static displayName = `withSwitchLabel(${switchComponentName})`; - static propTypes = { /** * If `true`, the control will be disabled. @@ -93,3 +90,5 @@ export function withSwitchLabel(SwitchComponent) { } }; } + +export default createHelper(withSwitchLabel, 'withSwitchLabel', true, true); diff --git a/src/internal/SwitchLabel.spec.js b/src/internal/withSwitchLabel.spec.js similarity index 97% rename from src/internal/SwitchLabel.spec.js rename to src/internal/withSwitchLabel.spec.js index 111a9f0f47eb57..eb85b0a6217094 100644 --- a/src/internal/SwitchLabel.spec.js +++ b/src/internal/withSwitchLabel.spec.js @@ -5,7 +5,7 @@ import React from 'react'; import { assert } from 'chai'; import { spy } from 'sinon'; import { createShallowWithContext } from 'test/utils'; -import { withSwitchLabel, styleSheet } from './SwitchLabel'; +import withSwitchLabel, { styleSheet } from './withSwitchLabel'; describe('', () => { let shallow; From 5427bd69afc60af0abe0deb8549fa2c2357069f4 Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Thu, 15 Dec 2016 10:02:45 -0500 Subject: [PATCH 6/8] Fix demos using wrong components --- .../src/demos/dialogs/ConfirmationDialog.js | 4 +- .../demos/selection-controls/Checkboxes.js | 12 +-- .../selection-controls/RadioButtonsGroup.js | 10 +-- .../demos/selection-controls/SwitchLabels.js | 8 +- src/Form/FormGroup.js | 75 +++++++++++-------- src/Radio/RadioGroup.js | 5 +- 6 files changed, 63 insertions(+), 51 deletions(-) diff --git a/docs/site/src/demos/dialogs/ConfirmationDialog.js b/docs/site/src/demos/dialogs/ConfirmationDialog.js index 91a963323886fb..e0d0988930c8ea 100644 --- a/docs/site/src/demos/dialogs/ConfirmationDialog.js +++ b/docs/site/src/demos/dialogs/ConfirmationDialog.js @@ -11,7 +11,7 @@ import { DialogActions, DialogTitle, } from 'material-ui/Dialog'; -import { Radio, RadioGroup } from 'material-ui/Radio'; +import { LabelRadio as Radio, RadioGroup } from 'material-ui/Radio'; const options = [ 'None', @@ -53,7 +53,7 @@ class ConfirmationDialog extends Component { radioGroup = undefined; handleEntering = () => { - this.radioGroup.focus(); + // this.radioGroup.focus(); }; handleCancel = () => { diff --git a/docs/site/src/demos/selection-controls/Checkboxes.js b/docs/site/src/demos/selection-controls/Checkboxes.js index a04170616478e5..1f556d0b60dc1c 100644 --- a/docs/site/src/demos/selection-controls/Checkboxes.js +++ b/docs/site/src/demos/selection-controls/Checkboxes.js @@ -1,7 +1,7 @@ // @flow weak import React, { Component } from 'react'; -import Checkbox from 'material-ui/Checkbox'; +import { LabelCheckbox } from 'material-ui/Checkbox'; import { FormGroup } from 'material-ui/Form'; export default class Checkboxes extends Component { @@ -13,28 +13,28 @@ export default class Checkboxes extends Component { render() { return ( - this.setState({ checkedA: checked })} label="Option A" value="checkedA" /> - this.setState({ checkedB: checked })} label="Option B" value="checkedB" /> - - - ({ @@ -37,10 +37,10 @@ export default class RadioButtonsGroup extends Component { selectedValue={this.state.selectedValue} onChange={this.handleChange} > - - - - + + + +
); diff --git a/docs/site/src/demos/selection-controls/SwitchLabels.js b/docs/site/src/demos/selection-controls/SwitchLabels.js index 5bc362f472ca0f..aa97fb6b4bc71f 100644 --- a/docs/site/src/demos/selection-controls/SwitchLabels.js +++ b/docs/site/src/demos/selection-controls/SwitchLabels.js @@ -1,7 +1,7 @@ // @flow weak import React, { Component } from 'react'; -import Switch from 'material-ui/Switch'; +import { LabelSwitch } from 'material-ui/Switch'; export default class SwitchLabels extends Component { state = { @@ -12,17 +12,17 @@ export default class SwitchLabels extends Component { render() { return (
- this.setState({ checkedA: checked })} label="A" /> - this.setState({ checkedB: checked })} label="B" /> - +
); } diff --git a/src/Form/FormGroup.js b/src/Form/FormGroup.js index 710478634d9579..93110bd53290eb 100644 --- a/src/Form/FormGroup.js +++ b/src/Form/FormGroup.js @@ -2,6 +2,7 @@ import React, { PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; +import createHelper from 'recompose/createHelper'; import classNames from 'classnames'; export const styleSheet = createStyleSheet('FormGroup', () => { @@ -16,40 +17,48 @@ export const styleSheet = createStyleSheet('FormGroup', () => { }; }); +const formGroup = (BaseComponent) => { + function FormGroup(props, context) { + const { className, row, ...other } = props; + const classes = context.styleManager.render(styleSheet); + const rootClassName = classNames(classes.root, { + [classes.row]: row, + }, className); + + return ( + + ); + } + + FormGroup.propTypes = { + children: PropTypes.node, + /** + * The CSS class name of the root element. + */ + className: PropTypes.string, + /** + * Display group of elements in a compact row. + */ + row: PropTypes.bool, + }; + + FormGroup.defaultProps = { + row: false, + }; + + FormGroup.contextTypes = { + styleManager: PropTypes.object.isRequired, + }; + + return FormGroup; +}; + +const createFormGroup = createHelper(formGroup, 'formGroup', true, true); + +export { createFormGroup }; + /** * FormGroup wraps controls such as Checkbox and Switch. It provides compact row layout and FormLabel * awareness. Upon focusing on one of the child controls, it will propagate `focused` to the label. */ -export default function FormGroup(props, context) { - const { className, children, row } = props; - const classes = context.styleManager.render(styleSheet); - const rootClassName = classNames(classes.root, { - [classes.row]: row, - }, className); - - return ( -
- {children} -
- ); -} - -FormGroup.propTypes = { - children: PropTypes.node, - /** - * The CSS class name of the root element. - */ - className: PropTypes.string, - /** - * Display group of elements in a compact row. - */ - row: PropTypes.bool, -}; - -FormGroup.defaultProps = { - row: false, -}; - -FormGroup.contextTypes = { - styleManager: PropTypes.object.isRequired, -}; +export default createFormGroup('div'); diff --git a/src/Radio/RadioGroup.js b/src/Radio/RadioGroup.js index 9ad427be08c862..e7526563fc1e43 100644 --- a/src/Radio/RadioGroup.js +++ b/src/Radio/RadioGroup.js @@ -3,6 +3,7 @@ import React, { Component, Children, cloneElement, PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; import classNames from 'classnames'; +import { createFormGroup } from '../Form/FormGroup'; export const styleSheet = createStyleSheet('RadioGroup', () => { return { @@ -14,7 +15,7 @@ export const styleSheet = createStyleSheet('RadioGroup', () => { }; }); -export default class RadioGroup extends Component { +class RadioGroup extends Component { static propTypes = { children: PropTypes.node, /** @@ -80,3 +81,5 @@ export default class RadioGroup extends Component { ); } } + +export default createFormGroup(RadioGroup); From 69a529bf7ce1e036ffa31b8fe97dcd70cf3edfcd Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Thu, 15 Dec 2016 10:34:41 -0500 Subject: [PATCH 7/8] Fix radiogroup imperative focusing for dialog menu accessibility --- .../src/demos/dialogs/ConfirmationDialog.js | 3 +- src/Form/FormGroup.js | 75 ++++++++----------- src/Radio/RadioGroup.js | 44 +++++++---- 3 files changed, 63 insertions(+), 59 deletions(-) diff --git a/docs/site/src/demos/dialogs/ConfirmationDialog.js b/docs/site/src/demos/dialogs/ConfirmationDialog.js index e0d0988930c8ea..a57423cae8dfa6 100644 --- a/docs/site/src/demos/dialogs/ConfirmationDialog.js +++ b/docs/site/src/demos/dialogs/ConfirmationDialog.js @@ -53,7 +53,7 @@ class ConfirmationDialog extends Component { radioGroup = undefined; handleEntering = () => { - // this.radioGroup.focus(); + this.radioGroup.focus(); }; handleCancel = () => { @@ -82,6 +82,7 @@ class ConfirmationDialog extends Component { { this.radioGroup = c; }} aria-label="Gender" + name="gender" selectedValue={this.state.selectedValue} onChange={this.handleChange} > diff --git a/src/Form/FormGroup.js b/src/Form/FormGroup.js index 93110bd53290eb..710478634d9579 100644 --- a/src/Form/FormGroup.js +++ b/src/Form/FormGroup.js @@ -2,7 +2,6 @@ import React, { PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; -import createHelper from 'recompose/createHelper'; import classNames from 'classnames'; export const styleSheet = createStyleSheet('FormGroup', () => { @@ -17,48 +16,40 @@ export const styleSheet = createStyleSheet('FormGroup', () => { }; }); -const formGroup = (BaseComponent) => { - function FormGroup(props, context) { - const { className, row, ...other } = props; - const classes = context.styleManager.render(styleSheet); - const rootClassName = classNames(classes.root, { - [classes.row]: row, - }, className); - - return ( - - ); - } - - FormGroup.propTypes = { - children: PropTypes.node, - /** - * The CSS class name of the root element. - */ - className: PropTypes.string, - /** - * Display group of elements in a compact row. - */ - row: PropTypes.bool, - }; - - FormGroup.defaultProps = { - row: false, - }; - - FormGroup.contextTypes = { - styleManager: PropTypes.object.isRequired, - }; - - return FormGroup; -}; - -const createFormGroup = createHelper(formGroup, 'formGroup', true, true); - -export { createFormGroup }; - /** * FormGroup wraps controls such as Checkbox and Switch. It provides compact row layout and FormLabel * awareness. Upon focusing on one of the child controls, it will propagate `focused` to the label. */ -export default createFormGroup('div'); +export default function FormGroup(props, context) { + const { className, children, row } = props; + const classes = context.styleManager.render(styleSheet); + const rootClassName = classNames(classes.root, { + [classes.row]: row, + }, className); + + return ( +
+ {children} +
+ ); +} + +FormGroup.propTypes = { + children: PropTypes.node, + /** + * The CSS class name of the root element. + */ + className: PropTypes.string, + /** + * Display group of elements in a compact row. + */ + row: PropTypes.bool, +}; + +FormGroup.defaultProps = { + row: false, +}; + +FormGroup.contextTypes = { + styleManager: PropTypes.object.isRequired, +}; diff --git a/src/Radio/RadioGroup.js b/src/Radio/RadioGroup.js index e7526563fc1e43..00b2a0499d1a63 100644 --- a/src/Radio/RadioGroup.js +++ b/src/Radio/RadioGroup.js @@ -3,7 +3,8 @@ import React, { Component, Children, cloneElement, PropTypes } from 'react'; import { createStyleSheet } from 'jss-theme-reactor'; import classNames from 'classnames'; -import { createFormGroup } from '../Form/FormGroup'; +import FormGroup from '../Form/FormGroup'; +import { find } from '../utils/helpers'; export const styleSheet = createStyleSheet('RadioGroup', () => { return { @@ -22,12 +23,6 @@ class RadioGroup extends Component { * The CSS class name of the root element. */ className: PropTypes.string, - component: PropTypes.oneOfType([PropTypes.string, PropTypes.func]), - /** - * @ignore - * For uncontrolled support - */ - defaultValue: PropTypes.string, name: PropTypes.string, onBlur: PropTypes.func, onChange: PropTypes.func, @@ -35,14 +30,28 @@ class RadioGroup extends Component { selectedValue: PropTypes.string, }; - static defaultProps = { - component: 'div', - }; - static contextTypes = { styleManager: PropTypes.object.isRequired, }; + radios = undefined; + + focus = () => { + if (this.props.selectedValue) { + const selectedRadio = find(this.radios, (n) => n.props.checked); + if (selectedRadio) { + selectedRadio.focus(); + return; + } + } + + const focusRadio = find(this.radios, (n) => !n.props.disabled); + + if (focusRadio) { + focusRadio.focus(); + } + }; + handleRadioChange = (event, checked) => { if (checked && this.props.onChange) { this.props.onChange(event, event.target.value); @@ -53,7 +62,6 @@ class RadioGroup extends Component { const { children, className: classNameProp, - component: ComponentProp, name, selectedValue, onChange, // eslint-disable-line no-unused-vars @@ -62,24 +70,28 @@ class RadioGroup extends Component { const classes = this.context.styleManager.render(styleSheet); + this.radios = []; + return ( - {Children.map(children, (child, index) => { + const selected = selectedValue === child.props.value; return cloneElement(child, { key: index, name, - checked: selectedValue === child.props.value, + ref: (c) => { this.radios.push(c); }, + checked: selected, onChange: this.handleRadioChange, }); })} - + ); } } -export default createFormGroup(RadioGroup); +export default RadioGroup; From 97104967759100e553d698c74fd58883788af7df Mon Sep 17 00:00:00 2001 From: nathanmarks Date: Thu, 15 Dec 2016 11:26:22 -0500 Subject: [PATCH 8/8] Add test for radiogroup focus helper --- src/Form/FormGroup.js | 4 +- src/Radio/RadioGroup.js | 23 ++++++---- src/Radio/RadioGroup.spec.js | 74 +++++++++++++++++++++++++++++++-- src/internal/SwitchBase.js | 6 +++ src/internal/SwitchBase.spec.js | 51 ++++++++++++++++------- 5 files changed, 128 insertions(+), 30 deletions(-) diff --git a/src/Form/FormGroup.js b/src/Form/FormGroup.js index 710478634d9579..91c67b41727399 100644 --- a/src/Form/FormGroup.js +++ b/src/Form/FormGroup.js @@ -21,14 +21,14 @@ export const styleSheet = createStyleSheet('FormGroup', () => { * awareness. Upon focusing on one of the child controls, it will propagate `focused` to the label. */ export default function FormGroup(props, context) { - const { className, children, row } = props; + const { className, children, row, ...other } = props; const classes = context.styleManager.render(styleSheet); const rootClassName = classNames(classes.root, { [classes.row]: row, }, className); return ( -
+
{children}
); diff --git a/src/Radio/RadioGroup.js b/src/Radio/RadioGroup.js index 00b2a0499d1a63..866f1f01466e92 100644 --- a/src/Radio/RadioGroup.js +++ b/src/Radio/RadioGroup.js @@ -37,19 +37,24 @@ class RadioGroup extends Component { radios = undefined; focus = () => { - if (this.props.selectedValue) { - const selectedRadio = find(this.radios, (n) => n.props.checked); - if (selectedRadio) { - selectedRadio.focus(); - return; - } + if (!this.radios || !this.radios.length) { + return; } - const focusRadio = find(this.radios, (n) => !n.props.disabled); + const focusRadios = this.radios.filter((n) => !n.props.disabled); - if (focusRadio) { - focusRadio.focus(); + if (!focusRadios.length) { + return; } + + const selectedRadio = find(focusRadios, (n) => n.props.checked); + + if (selectedRadio) { + selectedRadio.focus(); + return; + } + + focusRadios[0].focus(); }; handleRadioChange = (event, checked) => { diff --git a/src/Radio/RadioGroup.spec.js b/src/Radio/RadioGroup.spec.js index ea695a678bbfb1..47833a12324545 100644 --- a/src/Radio/RadioGroup.spec.js +++ b/src/Radio/RadioGroup.spec.js @@ -14,14 +14,14 @@ describe('', () => { shallow = createShallowWithContext(); }); - it('should render a radiogroup div', () => { + it('should render a FormGroup with the radiogroup role', () => { const wrapper = shallow( , ); assert.strictEqual( - wrapper.is('div[role="radiogroup"]'), + wrapper.is('FormGroup[role="radiogroup"]'), true, - 'should be a div with the correct role', + 'should be a FormGroup with the correct role', ); }); @@ -46,4 +46,72 @@ describe('', () => { assert.strictEqual(handleKeyDown.callCount, 1); assert.strictEqual(handleKeyDown.args[0][0], event); }); + + describe('imperative focus()', () => { + let wrapper; + + beforeEach(() => { + wrapper = shallow( + , + ); + }); + + it('should focus the first non-disabled radio', () => { + const radios = [ + { props: { disabled: true }, focus: spy() }, + { props: { disabled: false }, focus: spy() }, + { props: { disabled: false }, focus: spy() }, + ]; + wrapper.instance().radios = radios; + wrapper.instance().focus(); + + assert.strictEqual(radios[1].focus.callCount, 1); + }); + + it('should not focus any radios if all are disabled', () => { + const radios = [ + { props: { disabled: true }, focus: spy() }, + { props: { disabled: true }, focus: spy() }, + { props: { disabled: true }, focus: spy() }, + ]; + wrapper.instance().radios = radios; + wrapper.instance().focus(); + + assert.strictEqual(radios[0].focus.callCount, 0); + assert.strictEqual(radios[1].focus.callCount, 0); + assert.strictEqual(radios[2].focus.callCount, 0); + }); + + it('should focus the selected radio', () => { + const radios = [ + { props: { disabled: true }, focus: spy() }, + { props: { disabled: false }, focus: spy() }, + { props: { disabled: false, checked: true }, focus: spy() }, + { props: { disabled: false }, focus: spy() }, + ]; + wrapper.instance().radios = radios; + wrapper.instance().focus(); + + assert.strictEqual(radios[0].focus.callCount, 0); + assert.strictEqual(radios[1].focus.callCount, 0); + assert.strictEqual(radios[2].focus.callCount, 1); + assert.strictEqual(radios[3].focus.callCount, 0); + }); + + it('should focus the non-disabled radio rather than the disabled selected radio', () => { + const radios = [ + { props: { disabled: true }, focus: spy() }, + { props: { disabled: true }, focus: spy() }, + { props: { disabled: true, checked: true }, focus: spy() }, + { props: { disabled: false }, focus: spy() }, + ]; + wrapper.instance().radios = radios; + wrapper.instance().focus(); + + assert.strictEqual(radios[0].focus.callCount, 0); + assert.strictEqual(radios[1].focus.callCount, 0); + assert.strictEqual(radios[2].focus.callCount, 0); + assert.strictEqual(radios[3].focus.callCount, 1); + }); + }); }); diff --git a/src/internal/SwitchBase.js b/src/internal/SwitchBase.js index 7b67a173882771..4f8b90c7b83cad 100644 --- a/src/internal/SwitchBase.js +++ b/src/internal/SwitchBase.js @@ -75,6 +75,10 @@ export function createSwitch({ * If false, the ripple effect will be disabled. */ ripple: PropTypes.bool, + /** + * @ignore + */ + tabIndex: PropTypes.string, value: PropTypes.string, }; @@ -135,6 +139,7 @@ export function createSwitch({ name, onChange, // eslint-disable-line no-unused-vars ripple, + tabIndex, value, ...other } = this.props; @@ -176,6 +181,7 @@ export function createSwitch({ onChange={this.handleInputChange} className={classes.input} disabled={disabled} + tabIndex={tabIndex} value={value} /> diff --git a/src/internal/SwitchBase.spec.js b/src/internal/SwitchBase.spec.js index 0bec17bb48dbb6..72e04ee07515a9 100644 --- a/src/internal/SwitchBase.spec.js +++ b/src/internal/SwitchBase.spec.js @@ -45,14 +45,33 @@ describe('', () => { assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); }); - // SHOULD APPLY CLASSNAME BASED ON STATUS - it('should spread custom props on the root node', () => { const wrapper = shallow(); assert.strictEqual(wrapper.prop('data-my-prop'), 'woof', 'custom prop should be woof'); }); - it('should set the icon to aria-hidden="true" to avoid the ligature being read by screenreaders', () => { + it('should pass tabIndex to the input so it can be taken out of focus rotation', () => { + const wrapper = shallow(); + const input = wrapper.find('input'); + assert.strictEqual(input.prop('tabIndex'), '-1'); + }); + + it('should pass value, disabled, checked, and name to the input', () => { + const props = { + name: 'gender', + disabled: true, + value: 'male', + }; + + const wrapper = shallow(); + const input = wrapper.find('input'); + + Object.keys(props).forEach((n) => { + assert.strictEqual(input.prop(n), props[n]); + }); + }); + + it('should set the icon to aria-hidden="true" to avoid being read by screenreaders', () => { const wrapper = shallow(); assert.strictEqual(wrapper.childAt(0).prop('aria-hidden'), 'true'); }); @@ -63,19 +82,19 @@ describe('', () => { assert.strictEqual(wrapper.childAt(1).prop('disabled'), true, 'should disable the input node'); }); - describe('prop: disabledClassName', () => { - it('should apply the custom disabled className when needed', () => { - const disabledClassName = 'foo'; - const wrapperA = shallow(); - assert.strictEqual(wrapperA.hasClass(disabledClassName), true, 'should have the custom disabled class'); - - const wrapperB = shallow(); - assert.strictEqual( - wrapperB.hasClass(disabledClassName), - false, - 'should not have the custom disabled class', - ); - }); + it('should apply the custom disabled className when disabled', () => { + const disabledClassName = 'foo'; + const wrapperA = shallow(); + + assert.strictEqual(wrapperA.hasClass(disabledClassName), true, 'should have the custom disabled class'); + + wrapperA.setProps({ disabled: false }); + + assert.strictEqual( + wrapperA.hasClass(disabledClassName), + false, + 'should not have the custom disabled class', + ); }); describe('controlled', () => {