Skip to content

Commit

Permalink
fix: Should not trigger onChange multiple times (#603)
Browse files Browse the repository at this point in the history 
* fix: Should not trigger onChange multiple times

* test: Add test case

* fix lint
  • Loading branch information
@zombieJ
zombieJ authored Mar 3, 2021
1 parent 65c2e47 commit da4781b
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 46 deletions.
10 changes: 5 additions & 5 deletions examples/custom-icon.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable no-console */
/* eslint-disable no-console, max-classes-per-file */
import React from 'react';
import Select, { Option } from '../src';
import '../assets/index.less';
Expand All @@ -22,7 +22,7 @@ const clearPath =
' 0-8 3.6-8 8v60c0 4.4 3.6 8 8 8h618c35.3 0 64-' +
'28.7 64-64V306c0-35.3-28.7-64-64-64z';

const menuItemSelectedIcon = props => {
const menuItemSelectedIcon = (props) => {
const { ...p } = props;
return <span style={{ position: 'absolute', right: 0 }}>{p.isSelected ? '🌹' : '☑️'}</span>;
};
Expand All @@ -33,7 +33,7 @@ const singleItemIcon = (
</span>
);

const getSvg = path => (
const getSvg = (path) => (
<i>
<svg
viewBox="0 0 1024 1024"
Expand All @@ -60,7 +60,7 @@ class CustomIconComponent extends React.Component {
});
};

onKeyDown = e => {
onKeyDown = (e) => {
const { value } = this.state;
if (e.keyCode === 13) {
console.log('onEnter', value);
Expand Down Expand Up @@ -158,7 +158,7 @@ class Test extends React.Component {
console.log(args);
};

useAnim = e => {
useAnim = (e) => {
this.setState({
useAnim: e.target.checked,
});
Expand Down
22 changes: 13 additions & 9 deletions src/Selector/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
import * as React from 'react';
import { useRef } from 'react';
import KeyCode from 'rc-util/lib/KeyCode';
import { ScrollTo } from 'rc-virtual-list/lib/List';
import type { ScrollTo } from 'rc-virtual-list/lib/List';
import MultipleSelector from './MultipleSelector';
import SingleSelector from './SingleSelector';
import { LabelValueType, RawValueType, CustomTagProps } from '../interface/generator';
import { RenderNode, Mode } from '../interface';
import type { LabelValueType, RawValueType, CustomTagProps } from '../interface/generator';
import type { RenderNode, Mode } from '../interface';
import useLock from '../hooks/useLock';

export interface InnerSelectorProps {
Expand Down Expand Up @@ -129,7 +129,7 @@ const Selector: React.RefForwardingComponent<RefSelectorProps, SelectorProps> =
// ====================== Input ======================
const [getInputMouseDown, setInputMouseDown] = useLock(0);

const onInternalInputKeyDown: React.KeyboardEventHandler<HTMLInputElement> = event => {
const onInternalInputKeyDown: React.KeyboardEventHandler<HTMLInputElement> = (event) => {
const { which } = event;

if (which === KeyCode.UP || which === KeyCode.DOWN) {
Expand Down Expand Up @@ -172,12 +172,16 @@ const Selector: React.RefForwardingComponent<RefSelectorProps, SelectorProps> =
compositionStatusRef.current = true;
};

const onInputCompositionEnd: React.CompositionEventHandler<HTMLInputElement> = e => {
const onInputCompositionEnd: React.CompositionEventHandler<HTMLInputElement> = (e) => {
compositionStatusRef.current = false;
triggerOnSearch((e.target as HTMLInputElement).value);

// Trigger search again to support `tokenSeparators` with typewriting
if (mode !== 'combobox') {
triggerOnSearch((e.target as HTMLInputElement).value);
}
};

const onInputChange: React.ChangeEventHandler<HTMLInputElement> = event => {
const onInputChange: React.ChangeEventHandler<HTMLInputElement> = (event) => {
let {
target: { value },
} = event;
Expand All @@ -197,7 +201,7 @@ const Selector: React.RefForwardingComponent<RefSelectorProps, SelectorProps> =
triggerOnSearch(value);
};

const onInputPaste: React.ClipboardEventHandler = e => {
const onInputPaste: React.ClipboardEventHandler = (e) => {
const { clipboardData } = e;
const value = clipboardData.getData('text');

Expand All @@ -218,7 +222,7 @@ const Selector: React.RefForwardingComponent<RefSelectorProps, SelectorProps> =
}
};

const onMouseDown: React.MouseEventHandler<HTMLElement> = event => {
const onMouseDown: React.MouseEventHandler<HTMLElement> = (event) => {
const inputMouseDown = getInputMouseDown();
if (event.target !== inputRef.current && !inputMouseDown) {
event.preventDefault();
Expand Down
44 changes: 25 additions & 19 deletions src/generate.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import { useState, useRef, useEffect, useMemo } from 'react';
import KeyCode from 'rc-util/lib/KeyCode';
import classNames from 'classnames';
import useMergedState from 'rc-util/lib/hooks/useMergedState';
import { ScrollTo } from 'rc-virtual-list/lib/List';
import Selector, { RefSelectorProps } from './Selector';
import SelectTrigger, { RefTriggerProps } from './SelectTrigger';
import { RenderNode, Mode, RenderDOMFunc, OnActiveValue } from './interface';
import {
import type { ScrollTo } from 'rc-virtual-list/lib/List';
import type { RefSelectorProps } from './Selector';
import Selector from './Selector';
import type { RefTriggerProps } from './SelectTrigger';
import SelectTrigger from './SelectTrigger';
import type { RenderNode, Mode, RenderDOMFunc, OnActiveValue } from './interface';
import type {
GetLabeledValue,
FilterOptions,
FilterFunc,
Expand All @@ -29,11 +31,12 @@ import {
FlattenOptionsType,
SingleType,
OnClear,
INTERNAL_PROPS_MARK,
SelectSource,
CustomTagProps,
CustomTagProps} from './interface/generator';
import {
INTERNAL_PROPS_MARK
} from './interface/generator';
import { OptionListProps, RefOptionListProps } from './OptionList';
import type { OptionListProps, RefOptionListProps } from './OptionList';
import { toInnerValue, toOuterValues, removeLastEnabledValue, getUUID } from './utils/commonUtil';
import TransBtn from './TransBtn';
import useLock from './hooks/useLock';
Expand All @@ -59,7 +62,7 @@ const DEFAULT_OMIT_PROPS = [
export interface RefSelectProps {
focus: () => void;
blur: () => void;
scrollTo?: ScrollTo,
scrollTo?: ScrollTo;
}

export interface SelectProps<OptionsType extends object[], ValueType> extends React.AriaAttributes {
Expand Down Expand Up @@ -194,7 +197,7 @@ export interface GenerateConfig<OptionsType extends object[]> {
getLabeledValue: GetLabeledValue<FlattenOptionsType<OptionsType>>;
filterOptions: FilterOptions<OptionsType>;
findValueOption: // Need still support legacy ts api
| ((values: RawValueType[], options: FlattenOptionsType<OptionsType>) => OptionsType)
| ((values: RawValueType[], options: FlattenOptionsType<OptionsType>) => OptionsType)
// New API add prevValueOptions support
| ((
values: RawValueType[],
Expand Down Expand Up @@ -327,7 +330,7 @@ export default function generateSelector<
const useInternalProps = internalProps.mark === INTERNAL_PROPS_MARK;

const domProps = omitDOMProps ? omitDOMProps(restProps) : restProps;
DEFAULT_OMIT_PROPS.forEach(prop => {
DEFAULT_OMIT_PROPS.forEach((prop) => {
delete domProps[prop];
});

Expand All @@ -337,7 +340,8 @@ export default function generateSelector<
const listRef = useRef<RefOptionListProps>(null);

const tokenWithEnter = useMemo(
() => (tokenSeparators || []).some(tokenSeparator => ['\n', '\r\n'].includes(tokenSeparator)),
() =>
(tokenSeparators || []).some((tokenSeparator) => ['\n', '\r\n'].includes(tokenSeparator)),
[tokenSeparators],
);

Expand Down Expand Up @@ -446,7 +450,7 @@ export default function generateSelector<
});
if (
mode === 'tags' &&
filteredOptions.every(opt => opt[optionFilterProp] !== mergedSearchValue)
filteredOptions.every((opt) => opt[optionFilterProp] !== mergedSearchValue)
) {
filteredOptions.unshift({
value: mergedSearchValue,
Expand Down Expand Up @@ -682,7 +686,7 @@ export default function generateSelector<

if (mode !== 'tags') {
patchRawValues = patchLabels
.map(label => {
.map((label) => {
const item = mergedFlattenOptions.find(
({ data }) => data[mergedOptionLabelProp] === label,
);
Expand All @@ -695,7 +699,7 @@ export default function generateSelector<
new Set<RawValueType>([...mergedRawValue, ...patchRawValues]),
);
triggerChange(newRawValues);
newRawValues.forEach(newRawValue => {
newRawValues.forEach((newRawValue) => {
triggerSelect(newRawValue, true, 'input');
});

Expand All @@ -719,9 +723,11 @@ export default function generateSelector<
// If menu is open, OptionList will take charge
// If mode isn't tags, press enter is not meaningful when you can't see any option
const onSearchSubmit = (searchText: string) => {
const newRawValues = Array.from(new Set<RawValueType>([...mergedRawValue, searchText]));
const newRawValues = Array.from(
new Set<RawValueType>([...mergedRawValue, searchText]),
);
triggerChange(newRawValues);
newRawValues.forEach(newRawValue => {
newRawValues.forEach((newRawValue) => {
triggerSelect(newRawValue, true, 'input');
});
setInnerSearchValue('');
Expand Down Expand Up @@ -845,10 +851,10 @@ export default function generateSelector<
}
};

const activeTimeoutIds: number[] = [];
const activeTimeoutIds: any[] = [];
useEffect(
() => () => {
activeTimeoutIds.forEach(timeoutId => clearTimeout(timeoutId));
activeTimeoutIds.forEach((timeoutId) => clearTimeout(timeoutId));
activeTimeoutIds.splice(0, activeTimeoutIds.length);
},
[],
Expand Down
52 changes: 39 additions & 13 deletions tests/Combobox.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* eslint-disable max-classes-per-file */

import { mount } from 'enzyme';
import KeyCode from 'rc-util/lib/KeyCode';
import React from 'react';
import { resetWarned } from 'rc-util/lib/warning';
import Select, { Option, SelectProps } from '../src';
import type { SelectProps } from '../src';
import Select, { Option } from '../src';
import focusTest from './shared/focusTest';
import keyDownTest from './shared/keyDownTest';
import openControlledTest from './shared/openControlledTest';
Expand All @@ -11,7 +14,7 @@ import allowClearTest from './shared/allowClearTest';
import throwOptionValue from './shared/throwOptionValue';

async function delay(timeout = 0) {
return new Promise(resolve => {
return new Promise((resolve) => {
setTimeout(resolve, timeout);
});
}
Expand Down Expand Up @@ -139,13 +142,16 @@ describe('Select.Combobox', () => {
public handleChange = () => {
setTimeout(() => {
this.setState({
data: [{ key: '1', label: '1' }, { key: '2', label: '2' }],
data: [
{ key: '1', label: '1' },
{ key: '2', label: '2' },
],
});
}, 500);
};

public render() {
const options = this.state.data.map(item => (
const options = this.state.data.map((item) => (
<Option value={item.key}>{item.label}</Option>
));
return (
Expand Down Expand Up @@ -174,19 +180,25 @@ describe('Select.Combobox', () => {
jest.useFakeTimers();
class AsyncCombobox extends React.Component {
public state = {
data: [{ key: '1', label: '1' }, { key: '2', label: '2' }],
data: [
{ key: '1', label: '1' },
{ key: '2', label: '2' },
],
};

public onSelect = () => {
setTimeout(() => {
this.setState({
data: [{ key: '3', label: '3' }, { key: '4', label: '4' }],
data: [
{ key: '3', label: '3' },
{ key: '4', label: '4' },
],
});
}, 500);
};

public render() {
const options = this.state.data.map(item => (
const options = this.state.data.map((item) => (
<Option value={item.key}>{item.label}</Option>
));
return (
Expand Down Expand Up @@ -243,10 +255,7 @@ describe('Select.Combobox', () => {
</Select>,
);

wrapper
.find('.rc-select-item-option')
.first()
.simulate('mouseMove');
wrapper.find('.rc-select-item-option').first().simulate('mouseMove');

expect(wrapper.find('input').props().value).toBeFalsy();
});
Expand Down Expand Up @@ -341,7 +350,7 @@ describe('Select.Combobox', () => {
options: [],
};

public updateOptions = value => {
public updateOptions = (value) => {
const options = [value, value + value, value + value + value];
this.setState({
options,
Expand All @@ -351,7 +360,7 @@ describe('Select.Combobox', () => {
public render() {
return (
<Select mode="combobox" onChange={this.updateOptions}>
{this.state.options.map(opt => (
{this.state.options.map((opt) => (
<Option value={opt}>{opt}</Option>
))}
</Select>
Expand Down Expand Up @@ -459,4 +468,21 @@ describe('Select.Combobox', () => {
expect(wrapper.find('input').props().maxLength).toBe(6);
});
});

it('typewriting should not trigger onChange multiple times', () => {
const onChange = jest.fn();
const wrapper = mount(<Select mode="combobox" onChange={onChange} />);

wrapper.find('input').simulate('compositionStart', { target: { value: '' } });
wrapper.find('input').simulate('change', { target: { value: 'a' } });
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenLastCalledWith('a', expect.anything());

wrapper.find('input').simulate('change', { target: { value: '啊' } });
expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveBeenLastCalledWith('啊', expect.anything());

wrapper.find('input').simulate('compositionEnd', { target: { value: '啊' } });
expect(onChange).toHaveBeenCalledTimes(2);
});
});
16 changes: 16 additions & 0 deletions tsconfig.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"compilerOptions": {
"target": "esnext",
"moduleResolution": "node",
"baseUrl": "./",
"jsx": "preserve",
"declaration": true,
"skipLibCheck": true,
"esModuleInterop": true,
"paths": {
"@/*": ["src/*"],
"@@/*": ["src/.umi/*"],
"rc-table": ["src/index.ts"]
}
}
}

1 comment on commit da4781b

@vercel
Copy link

@vercel vercel bot commented on da4781b Mar 3, 2021

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

Please sign in to comment.