From da88c04e66174d0581c24066affa81c2ddfb36fb Mon Sep 17 00:00:00 2001 From: Christian Vuerings Date: Wed, 4 Dec 2019 13:29:52 -0800 Subject: [PATCH] Checkbox: Convert examples & component to use hooks (#600) --- .eslintrc.json | 9 + CHANGELOG.md | 1 + docs/src/Checkbox.doc.js | 135 ++++++------ packages/gestalt/src/Checkbox.js | 220 ++++++++------------ packages/gestalt/src/Checkbox.jsdom.test.js | 23 ++ packages/gestalt/src/Checkbox.test.js | 13 -- 6 files changed, 187 insertions(+), 214 deletions(-) create mode 100644 packages/gestalt/src/Checkbox.jsdom.test.js diff --git a/.eslintrc.json b/.eslintrc.json index 05bd891e73..72be1eec54 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -34,6 +34,15 @@ "error", { "components": ["label"], "allowChildren": true } ], + "jsx-a11y/label-has-associated-control": [ + "error", + { + "labelComponents": ["Label"], + "labelAttributes": ["htmlFor"], + "controlComponents": ["CheckBox"], + "depth": 3 + } + ], "no-await-in-loop": "off", "no-return-await": "off", "prettier/prettier": [ diff --git a/CHANGELOG.md b/CHANGELOG.md index b2e1e227e7..f47b921c9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Avatar: Convert component to use hooks (#598) - Card: Convert examples & component to use hooks (#597) +- Checkbox: Convert examples & component to use hooks (#600) - Internal: Add `react-testing-library` (#598) - Internal: Enable `react-hooks/exhaustive-deps` lint rule (#598) diff --git a/docs/src/Checkbox.doc.js b/docs/src/Checkbox.doc.js index df62eed82e..06c1cd87b9 100644 --- a/docs/src/Checkbox.doc.js +++ b/docs/src/Checkbox.doc.js @@ -84,32 +84,25 @@ card( `} name="Example: Accessibility" defaultCode={` -class CheckboxExample extends React.Component { - constructor(props) { - super(props); - this.state = { checked: true }; - this.handleChecked = this._handleChecked.bind(this); - } - _handleChecked({ checked }) { - this.setState({ checked }); - } - render() { - return ( - - - - - ); - } +function CheckboxExample() { + const [checked, setChecked] = React.useState(true); + return ( + + { + setChecked(checked); + }} + /> + + + ); } `} /> @@ -122,37 +115,35 @@ card( " name="Example: Labeled stack" defaultCode={` -class CheckboxExample extends React.Component { - render() { - const CheckboxWithLabel = ({ id, label }) => ( - - {}} - /> - - - ); - - return ( - - - - - - - - - +function CheckboxExample() { + const CheckboxWithLabel = ({ id, label }) => ( + + {}} + /> + + + ); + + return ( + + + + + + + + + - ); - } + + ); } `} /> @@ -163,24 +154,22 @@ card( id="hasError" name="Example: Error state" defaultCode={` -class CheckboxExample extends React.Component { - render() { - return ( - - {}} - /> - - - ); - } +function CheckboxExample() { + return ( + + {}} + /> + + + ); } `} /> diff --git a/packages/gestalt/src/Checkbox.js b/packages/gestalt/src/Checkbox.js index 6585f3f0ff..88f61d0b75 100644 --- a/packages/gestalt/src/Checkbox.js +++ b/packages/gestalt/src/Checkbox.js @@ -22,142 +22,106 @@ type Props = {| size?: 'sm' | 'md', |}; -type State = {| - focused: boolean, -|}; - -export default class Checkbox extends React.Component { - input: ?HTMLInputElement; - - static propTypes = { - checked: PropTypes.bool, - disabled: PropTypes.bool, - hasError: PropTypes.bool, - id: PropTypes.string.isRequired, - indeterminate: PropTypes.bool, - name: PropTypes.string, - onChange: PropTypes.func.isRequired, - onClick: PropTypes.func, - size: PropTypes.oneOf(['sm', 'md']), - }; - - static defaultProps = { - checked: false, - disabled: false, - hasError: false, - indeterminate: false, - size: 'md', - }; - - state = { - focused: false, - }; - - componentDidMount() { - if (this.props.indeterminate) { - this.setIndeterminate(!!this.props.indeterminate); - } - } - - componentDidUpdate(previousProps: Props) { - if (previousProps.indeterminate !== this.props.indeterminate) { - this.setIndeterminate(!!this.props.indeterminate); +export default function Checkbox({ + checked = false, + disabled = false, + hasError = false, + id, + indeterminate = false, + name, + onChange, + onClick, + size = 'md', +}: Props) { + const inputElement = React.useRef(null); + const [focused, setFocused] = React.useState(false); + + React.useEffect(() => { + if (inputElement && inputElement.current) { + inputElement.current.indeterminate = indeterminate; } - } + }, [indeterminate]); - setIndeterminate(indeterminate: boolean) { - if (this.input) { - this.input.indeterminate = indeterminate; + const handleChange = (event: SyntheticInputEvent<>) => { + if (onChange) { + onChange({ event, checked: event.target.checked }); } - } - - handleChange = (event: SyntheticInputEvent<>) => { - const { checked } = event.target; - this.props.onChange({ event, checked }); }; - handleClick = (event: SyntheticInputEvent) => { - const { onClick } = this.props; + const handleClick = (event: SyntheticInputEvent) => { if (onClick) { - const { checked } = event.currentTarget; - onClick({ event, checked }); + onClick({ event, checked: event.currentTarget.checked }); } }; - handleBlur = () => this.setState({ focused: false }); - - handleFocus = () => this.setState({ focused: true }); - - render() { - const { - checked, - disabled, - hasError, - id, - indeterminate, - name, - size, - } = this.props; - - let borderStyle = styles.border; - if (!disabled && (checked || indeterminate)) { - borderStyle = styles.borderDark; - } else if (hasError) { - borderStyle = styles.borderError; - } - - return ( - - { - this.input = el; - }} - type="checkbox" - /> -
- {(checked || indeterminate) && ( - - )} -
-
- ); + let borderStyle = styles.border; + if (!disabled && (checked || indeterminate)) { + borderStyle = styles.borderDark; + } else if (hasError) { + borderStyle = styles.borderError; } + + return ( + + setFocused(false)} + onChange={handleChange} + onClick={handleClick} + onFocus={() => setFocused(true)} + ref={inputElement} + type="checkbox" + /> +
+ {(checked || indeterminate) && ( + + )} +
+
+ ); } + +Checkbox.propTypes = { + checked: PropTypes.bool, + disabled: PropTypes.bool, + hasError: PropTypes.bool, + id: PropTypes.string.isRequired, + indeterminate: PropTypes.bool, + name: PropTypes.string, + onChange: PropTypes.func.isRequired, + onClick: PropTypes.func, + size: PropTypes.oneOf(['sm', 'md']), +}; diff --git a/packages/gestalt/src/Checkbox.jsdom.test.js b/packages/gestalt/src/Checkbox.jsdom.test.js new file mode 100644 index 0000000000..aaa2a87278 --- /dev/null +++ b/packages/gestalt/src/Checkbox.jsdom.test.js @@ -0,0 +1,23 @@ +// @flow +import React from 'react'; +import { render } from '@testing-library/react'; +import Checkbox from './Checkbox.js'; + +test('Checkbox handles click', () => { + const mockOnClick = jest.fn(); + const mockOnChange = jest.fn(); + const { getByLabelText } = render( +
+ + + + ); + getByLabelText('Label').click({ currentTarget: { checked: true } }); + expect(mockOnClick).toHaveBeenCalled(); + expect(mockOnChange).toHaveBeenCalled(); +}); diff --git a/packages/gestalt/src/Checkbox.test.js b/packages/gestalt/src/Checkbox.test.js index fdff814064..3f0924af23 100644 --- a/packages/gestalt/src/Checkbox.test.js +++ b/packages/gestalt/src/Checkbox.test.js @@ -1,7 +1,6 @@ // @flow import React from 'react'; import { create } from 'react-test-renderer'; -import { shallow } from 'enzyme'; import Checkbox from './Checkbox.js'; test('Checkbox', () => { @@ -50,15 +49,3 @@ test('Checkbox with error', () => { ).toJSON(); expect(tree).toMatchSnapshot(); }); - -test('Checkbox handles click', () => { - const mockOnClick = jest.fn(); - const wrapper = shallow( - {}} onClick={mockOnClick} /> - ); - wrapper.find('input').simulate('click', { currentTarget: { checked: true } }); - expect(mockOnClick).toHaveBeenCalledWith({ - event: { currentTarget: { checked: true } }, - checked: true, - }); -});