From c6ca28fa5dab1818631cf119c9f277ae9a34d27e Mon Sep 17 00:00:00 2001 From: zombiej Date: Mon, 18 May 2020 14:50:14 +0800 Subject: [PATCH 1/4] refactor: Use rc-util to min fianl dist size --- package.json | 5 +- src/index.tsx | 199 +++++++++++++++++++++----------------------- tests/index.spec.js | 36 ++++++-- 3 files changed, 125 insertions(+), 115 deletions(-) diff --git a/package.json b/package.json index 90c16d0..74e6c54 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,8 @@ "coverage": "father test --coverage" }, "devDependencies": { + "@types/classnames": "^2.2.10", + "@types/jest": "^25.2.2", "coveralls": "^3.0.6", "enzyme": "^3.0.0", "enzyme-adapter-react-16": "^1.0.1", @@ -52,6 +54,7 @@ "react-dom": "^16.0.0" }, "dependencies": { - "classnames": "^2.2.1" + "classnames": "^2.2.1", + "rc-util": "^4.20.5" } } diff --git a/src/index.tsx b/src/index.tsx index 6786927..b715de4 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,17 +1,24 @@ import * as React from 'react'; import classNames from 'classnames'; - -export type SwitchChangeEventHandler = (checked: boolean, event: MouseEvent) => void; +import useMergedState from 'rc-util/lib/hooks/useMergedState'; +import { composeRef } from 'rc-util/lib/ref'; +import KeyCode from 'rc-util/lib/KeyCode'; + +export type SwitchChangeEventHandler = ( + checked: boolean, + event: React.MouseEvent | React.KeyboardEvent, +) => void; export type SwitchClickEventHandler = SwitchChangeEventHandler; -interface SwitchProps { +interface SwitchProps + extends Omit, 'onChange' | 'onClick'> { className?: string; prefixCls?: string; disabled?: boolean; checkedChildren?: React.ReactNode; unCheckedChildren?: React.ReactNode; onChange?: SwitchChangeEventHandler; - onMouseUp: React.MouseEventHandler; + onKeyDown?: React.KeyboardEventHandler; onClick?: SwitchClickEventHandler; tabIndex?: number; checked?: boolean; @@ -22,115 +29,95 @@ interface SwitchProps { title?: string; } -const Switch = React.forwardRef((props, ref) => { - const mergedRef = (ref as any) || React.createRef(); - - let initChecked = false; - if ('checked' in props) { - initChecked = !!props.checked; - } else { - initChecked = !!props.defaultChecked; - } - const [checked, setChecked] = React.useState(initChecked); - - React.useEffect(() => { - const { autoFocus, disabled } = props; - if (autoFocus && !disabled) { - focus(); - } - }, [props.autoFocus, props.disabled]); - - React.useEffect(() => { - if ('checked' in props) { - setChecked(!!props.checked); - } - }, [props.checked]); - - const setInternalChecked = (checked, e) => { - const { disabled, onChange } = props; - if (disabled) { - return; +const Switch = React.forwardRef( + ( + { + prefixCls = 'rc-switch', + className, + autoFocus, + checked, + defaultChecked, + disabled, + loadingIcon, + checkedChildren, + unCheckedChildren, + onClick, + onChange, + onKeyDown, + ...restProps + }, + ref, + ) => { + const buttonRef = React.useRef(); + const mergedRef = composeRef(buttonRef, ref); + + const [innerChecked, setInnerChecked] = useMergedState(false, { + value: checked, + defaultValue: defaultChecked, + }); + + React.useEffect(() => { + if (autoFocus && !disabled) { + buttonRef.current.focus(); + } + }, []); + + function triggerChange( + newChecked: boolean, + event: React.MouseEvent | React.KeyboardEvent, + ) { + let mergedChecked = innerChecked; + + if (!disabled) { + mergedChecked = newChecked; + setInnerChecked(mergedChecked); + onChange?.(mergedChecked, event); + } + + return mergedChecked; } - if (!('checked' in props)) { - setChecked(checked); - } - if (onChange) { - onChange(checked, e); - } - }; - - const handleClick = e => { - const { onClick } = props; - const newChecked = !checked; - setInternalChecked(newChecked, e); - if (onClick) { - onClick(newChecked, e); - } - }; - const handleKeyDown = e => { - if (e.keyCode === 37) { - // Left - setInternalChecked(false, e); - } else if (e.keyCode === 39) { - // Right - setInternalChecked(true, e); + function onInternalKeyDown(e: React.KeyboardEvent) { + if (e.which === KeyCode.LEFT) { + triggerChange(false, e); + } else if (e.which === KeyCode.RIGHT) { + triggerChange(true, e); + } + onKeyDown?.(e); } - }; - // Handle auto focus when click switch in Chrome - const handleMouseUp = e => { - (mergedRef.current as any).blur(); - if (props.onMouseUp) { - props.onMouseUp(e); + function onInternalClick(e: React.MouseEvent) { + const ret = triggerChange(!innerChecked, e); + // [Legacy] trigger onClick with value + onClick?.(ret, e); } - }; - const { - className, - prefixCls, - disabled, - loadingIcon, - checkedChildren, - unCheckedChildren, - onChange, - ...restProps - } = props; - - const switchClassName = classNames({ - [className]: !!className, - [prefixCls]: true, - [`${prefixCls}-checked`]: checked, - [`${prefixCls}-disabled`]: disabled, - }); - - return ( - - ); -}); + const switchClassName = classNames(prefixCls, className, { + [`${prefixCls}-checked`]: innerChecked, + [`${prefixCls}-disabled`]: disabled, + }); + + return ( + + ); + }, +); Switch.displayName = 'Switch'; -Switch.defaultProps = { - prefixCls: 'rc-switch', - checkedChildren: null, - unCheckedChildren: null, - className: '', - defaultChecked: false, -}; - export default Switch; diff --git a/tests/index.spec.js b/tests/index.spec.js index c88692d..a64b58b 100644 --- a/tests/index.spec.js +++ b/tests/index.spec.js @@ -1,4 +1,5 @@ import React from 'react'; +import KeyCode from 'rc-util/lib/KeyCode'; // eslint-disable-next-line import/no-extraneous-dependencies import { mount } from 'enzyme'; import Switch from '../index'; @@ -24,9 +25,9 @@ describe('rc-switch', () => { it('should be checked upon right key and unchecked on left key', () => { const wrapper = createSwitch(); expect(wrapper.exists('.unchecked')).toBeTruthy(); - wrapper.simulate('keydown', { keyCode: 39 }); + wrapper.simulate('keydown', { which: KeyCode.RIGHT }); expect(wrapper.exists('.checked')).toBeTruthy(); - wrapper.simulate('keydown', { keyCode: 37 }); + wrapper.simulate('keydown', { which: KeyCode.LEFT }); expect(wrapper.exists('.unchecked')).toBeTruthy(); }); @@ -89,12 +90,22 @@ describe('rc-switch', () => { expect(handleBlur).toHaveBeenCalled(); }); - it('autoFocus', () => { - const container = document.createElement('div'); - document.body.appendChild(container); - const handleFocus = jest.fn(); - mount(, { attachTo: container }); - expect(handleFocus).toHaveBeenCalled(); + describe('autoFocus', () => { + it('basic', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const handleFocus = jest.fn(); + mount(, { attachTo: container }); + expect(handleFocus).toHaveBeenCalled(); + }); + + it('not work when disabled', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const handleFocus = jest.fn(); + mount(, { attachTo: container }); + expect(handleFocus).not.toHaveBeenCalled(); + }); }); it('disabled', () => { @@ -110,4 +121,13 @@ describe('rc-switch', () => { wrapper.simulate('mouseup'); expect(onMouseUp).toHaveBeenCalled(); }); + + it('disabled should click not to change', () => { + const onChange = jest.fn(); + const onClick = jest.fn(); + const wrapper = createSwitch({ disabled: true, onChange, onClick, checked: true }); + + wrapper.simulate('click'); + expect(onChange).not.toHaveBeenCalled(); + }); }); From cb4c12ad3715dcd7212682d4a19c710f9cf8964c Mon Sep 17 00:00:00 2001 From: zombiej Date: Mon, 18 May 2020 14:53:42 +0800 Subject: [PATCH 2/4] fix lint --- examples/simple.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/simple.js b/examples/simple.js index e7c87dc..1ae9dca 100644 --- a/examples/simple.js +++ b/examples/simple.js @@ -1,6 +1,6 @@ import '../assets/index.less'; import React from 'react'; -import Switch from '../src/index'; +import Switch from '../src'; function onChange(value, event) { // eslint-disable-next-line no-console From ff47a2763fed7d3ff5d5acae5843bfa7ef817d5f Mon Sep 17 00:00:00 2001 From: zombiej Date: Mon, 18 May 2020 15:18:07 +0800 Subject: [PATCH 3/4] update eslint --- .eslintrc.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index f30d7a7..5e01f7c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -8,5 +8,7 @@ module.exports = { 'prefer-promise-reject-errors': 0, 'react/no-array-index-key': 0, 'react/sort-comp': 0, + 'import/no-named-as-default': 0, + 'import/no-named-as-default-member': 0, }, }; From fc94c31217f458ec3501b3e40167da9c80d19888 Mon Sep 17 00:00:00 2001 From: zombiej Date: Mon, 18 May 2020 15:23:15 +0800 Subject: [PATCH 4/4] rm autoFocus logic --- src/index.tsx | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/index.tsx b/src/index.tsx index b715de4..271eeee 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,7 +1,6 @@ import * as React from 'react'; import classNames from 'classnames'; import useMergedState from 'rc-util/lib/hooks/useMergedState'; -import { composeRef } from 'rc-util/lib/ref'; import KeyCode from 'rc-util/lib/KeyCode'; export type SwitchChangeEventHandler = ( @@ -23,7 +22,6 @@ interface SwitchProps tabIndex?: number; checked?: boolean; defaultChecked?: boolean; - autoFocus?: boolean; loadingIcon: React.ReactNode; style?: React.CSSProperties; title?: string; @@ -34,7 +32,6 @@ const Switch = React.forwardRef( { prefixCls = 'rc-switch', className, - autoFocus, checked, defaultChecked, disabled, @@ -48,20 +45,11 @@ const Switch = React.forwardRef( }, ref, ) => { - const buttonRef = React.useRef(); - const mergedRef = composeRef(buttonRef, ref); - const [innerChecked, setInnerChecked] = useMergedState(false, { value: checked, defaultValue: defaultChecked, }); - React.useEffect(() => { - if (autoFocus && !disabled) { - buttonRef.current.focus(); - } - }, []); - function triggerChange( newChecked: boolean, event: React.MouseEvent | React.KeyboardEvent, @@ -105,7 +93,7 @@ const Switch = React.forwardRef( aria-checked={innerChecked} disabled={disabled} className={switchClassName} - ref={mergedRef} + ref={ref} onKeyDown={onInternalKeyDown} onClick={onInternalClick} >