Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Checkbox.Group onChange should not pass removed value #16392

Merged
merged 3 commits into from Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 38 additions & 1 deletion components/checkbox/Checkbox.tsx
@@ -1,5 +1,6 @@
import * as React from 'react';
import * as PropTypes from 'prop-types';
import { polyfill } from 'react-lifecycles-compat';
import classNames from 'classnames';
import RcCheckbox from 'rc-checkbox';
import shallowEqual from 'shallowequal';
Expand Down Expand Up @@ -40,7 +41,7 @@ export interface CheckboxChangeEvent {
nativeEvent: MouseEvent;
}

export default class Checkbox extends React.Component<CheckboxProps, {}> {
class Checkbox extends React.Component<CheckboxProps, {}> {
static Group: typeof CheckboxGroup;
static defaultProps = {
indeterminate: false,
Expand All @@ -54,6 +55,38 @@ export default class Checkbox extends React.Component<CheckboxProps, {}> {

private rcCheckbox: any;

constructor(props: CheckboxProps) {
super(props);
this.state = {
prevValue: props.value,
};
}

componentDidMount() {
const { value } = this.props;
const { checkboxGroup = {} } = this.context || {};
if (checkboxGroup.registerValue) {
checkboxGroup.registerValue(value);
}
}

componentDidUpdate({ value: prevValue }: CheckboxProps) {
const { value } = this.props;
const { checkboxGroup = {} } = this.context || {};
if (value !== prevValue && checkboxGroup.registerValue && checkboxGroup.cancelValue) {
checkboxGroup.cancelValue(prevValue);
checkboxGroup.registerValue(value);
}
}

componentWillUnmount() {
const { value } = this.props;
const { checkboxGroup = {} } = this.context || {};
if (checkboxGroup.cancelValue) {
checkboxGroup.cancelValue(value);
}
}

shouldComponentUpdate(
nextProps: CheckboxProps,
nextState: {},
Expand Down Expand Up @@ -134,3 +167,7 @@ export default class Checkbox extends React.Component<CheckboxProps, {}> {
return <ConfigConsumer>{this.renderCheckbox}</ConfigConsumer>;
}
}

polyfill(Checkbox);

export default Checkbox;
22 changes: 20 additions & 2 deletions components/checkbox/Group.tsx
Expand Up @@ -32,7 +32,8 @@ export interface CheckboxGroupProps extends AbstractCheckboxGroupProps {
}

export interface CheckboxGroupState {
value: any;
value: CheckboxValueType[];
registeredValues: CheckboxValueType[];
}

export interface CheckboxGroupContext {
Expand Down Expand Up @@ -72,6 +73,7 @@ class CheckboxGroup extends React.Component<CheckboxGroupProps, CheckboxGroupSta
super(props);
this.state = {
value: props.value || props.defaultValue || [],
registeredValues: [],
};
}

Expand All @@ -82,6 +84,10 @@ class CheckboxGroup extends React.Component<CheckboxGroupProps, CheckboxGroupSta
value: this.state.value,
disabled: this.props.disabled,
name: this.props.name,

// https://github.com/ant-design/ant-design/issues/16376
registerValue: this.registerValue,
cancelValue: this.cancelValue,
},
};
}
Expand All @@ -90,6 +96,17 @@ class CheckboxGroup extends React.Component<CheckboxGroupProps, CheckboxGroupSta
return !shallowEqual(this.props, nextProps) || !shallowEqual(this.state, nextState);
}

registerValue = (value: string) => {
this.setState(({ registeredValues }) => ({
registeredValues: [...registeredValues, value],
}));
};
cancelValue = (value: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

空一行 一起了

this.setState(({ registeredValues }) => ({
registeredValues: registeredValues.filter(val => val !== value),
}));
};

getOptions() {
const { options } = this.props;
// https://github.com/Microsoft/TypeScript/issues/7960
Expand All @@ -105,6 +122,7 @@ class CheckboxGroup extends React.Component<CheckboxGroupProps, CheckboxGroupSta
}

toggleOption = (option: CheckboxOptionType) => {
const { registeredValues } = this.state;
const optionIndex = this.state.value.indexOf(option.value);
const value = [...this.state.value];
if (optionIndex === -1) {
Expand All @@ -117,7 +135,7 @@ class CheckboxGroup extends React.Component<CheckboxGroupProps, CheckboxGroupSta
}
const onChange = this.props.onChange;
if (onChange) {
onChange(value);
onChange(value.filter(val => registeredValues.indexOf(val) !== -1));
}
};

Expand Down
22 changes: 22 additions & 0 deletions components/checkbox/__tests__/group.test.js
Expand Up @@ -111,4 +111,26 @@ describe('CheckboxGroup', () => {
expect(onChange).toHaveBeenCalled();
expect(onChange.mock.calls[0][0].target.value).toEqual('my');
});

// https://github.com/ant-design/ant-design/issues/16376
it('onChange should filter removed value', () => {
const onChange = jest.fn();
const wrapper = mount(
<Checkbox.Group defaultValue={[1]} onChange={onChange}>
<Checkbox key={1} value={1} />
<Checkbox key={2} value={2} />
</Checkbox.Group>,
);

wrapper.setProps({
children: [<Checkbox key={2} value={2} />],
});

wrapper
.find('.ant-checkbox-input')
.at(0)
.simulate('change');

expect(onChange).toHaveBeenCalledWith([2]);
});
});