mirror of
https://github.com/apache/superset.git
synced 2024-09-17 11:09:47 -04:00
fix: Select onChange is fired when the same item is selected in single mode (#27706)
(cherry picked from commit d69a1870a0
)
This commit is contained in:
parent
b8e556d6f2
commit
b7fa3edcf4
@ -939,6 +939,18 @@ test('pasting an existing option does not duplicate it in multiple mode', async
|
|||||||
expect(await findAllSelectOptions()).toHaveLength(4);
|
expect(await findAllSelectOptions()).toHaveLength(4);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('does not fire onChange if the same value is selected in single mode', async () => {
|
||||||
|
const onChange = jest.fn();
|
||||||
|
render(<AsyncSelect {...defaultProps} onChange={onChange} />);
|
||||||
|
const optionText = 'Emma';
|
||||||
|
await open();
|
||||||
|
expect(onChange).toHaveBeenCalledTimes(0);
|
||||||
|
userEvent.click(await findSelectOption(optionText));
|
||||||
|
expect(onChange).toHaveBeenCalledTimes(1);
|
||||||
|
userEvent.click(await findSelectOption(optionText));
|
||||||
|
expect(onChange).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
/*
|
/*
|
||||||
TODO: Add tests that require scroll interaction. Needs further investigation.
|
TODO: Add tests that require scroll interaction. Needs further investigation.
|
||||||
- Fetches more data when scrolling and more data is available
|
- Fetches more data when scrolling and more data is available
|
||||||
|
@ -50,10 +50,12 @@ import {
|
|||||||
mapOptions,
|
mapOptions,
|
||||||
getOption,
|
getOption,
|
||||||
isObject,
|
isObject,
|
||||||
|
isEqual as utilsIsEqual,
|
||||||
} from './utils';
|
} from './utils';
|
||||||
import {
|
import {
|
||||||
AsyncSelectProps,
|
AsyncSelectProps,
|
||||||
AsyncSelectRef,
|
AsyncSelectRef,
|
||||||
|
RawValue,
|
||||||
SelectOptionsPagePromise,
|
SelectOptionsPagePromise,
|
||||||
SelectOptionsType,
|
SelectOptionsType,
|
||||||
SelectOptionsTypePage,
|
SelectOptionsTypePage,
|
||||||
@ -220,7 +222,16 @@ const AsyncSelect = forwardRef(
|
|||||||
|
|
||||||
const handleOnSelect: SelectProps['onSelect'] = (selectedItem, option) => {
|
const handleOnSelect: SelectProps['onSelect'] = (selectedItem, option) => {
|
||||||
if (isSingleMode) {
|
if (isSingleMode) {
|
||||||
|
// on select is fired in single value mode if the same value is selected
|
||||||
|
const valueChanged = !utilsIsEqual(
|
||||||
|
selectedItem,
|
||||||
|
selectValue as RawValue | AntdLabeledValue,
|
||||||
|
'value',
|
||||||
|
);
|
||||||
setSelectValue(selectedItem);
|
setSelectValue(selectedItem);
|
||||||
|
if (valueChanged) {
|
||||||
|
fireOnChange();
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
setSelectValue(previousState => {
|
setSelectValue(previousState => {
|
||||||
const array = ensureIsArray(previousState);
|
const array = ensureIsArray(previousState);
|
||||||
@ -234,8 +245,8 @@ const AsyncSelect = forwardRef(
|
|||||||
}
|
}
|
||||||
return previousState;
|
return previousState;
|
||||||
});
|
});
|
||||||
|
fireOnChange();
|
||||||
}
|
}
|
||||||
fireOnChange();
|
|
||||||
onSelect?.(selectedItem, option);
|
onSelect?.(selectedItem, option);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -1053,6 +1053,18 @@ test('pasting an existing option does not duplicate it in multiple mode', async
|
|||||||
expect(await findAllSelectOptions()).toHaveLength(4);
|
expect(await findAllSelectOptions()).toHaveLength(4);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('does not fire onChange if the same value is selected in single mode', async () => {
|
||||||
|
const onChange = jest.fn();
|
||||||
|
render(<Select {...defaultProps} onChange={onChange} />);
|
||||||
|
const optionText = 'Emma';
|
||||||
|
await open();
|
||||||
|
expect(onChange).toHaveBeenCalledTimes(0);
|
||||||
|
userEvent.click(await findSelectOption(optionText));
|
||||||
|
expect(onChange).toHaveBeenCalledTimes(1);
|
||||||
|
userEvent.click(await findSelectOption(optionText));
|
||||||
|
expect(onChange).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
/*
|
/*
|
||||||
TODO: Add tests that require scroll interaction. Needs further investigation.
|
TODO: Add tests that require scroll interaction. Needs further investigation.
|
||||||
- Fetches more data when scrolling and more data is available
|
- Fetches more data when scrolling and more data is available
|
||||||
|
@ -53,6 +53,7 @@ import {
|
|||||||
hasCustomLabels,
|
hasCustomLabels,
|
||||||
getOption,
|
getOption,
|
||||||
isObject,
|
isObject,
|
||||||
|
isEqual as utilsIsEqual,
|
||||||
} from './utils';
|
} from './utils';
|
||||||
import { RawValue, SelectOptionsType, SelectProps } from './types';
|
import { RawValue, SelectOptionsType, SelectProps } from './types';
|
||||||
import {
|
import {
|
||||||
@ -227,7 +228,16 @@ const Select = forwardRef(
|
|||||||
|
|
||||||
const handleOnSelect: SelectProps['onSelect'] = (selectedItem, option) => {
|
const handleOnSelect: SelectProps['onSelect'] = (selectedItem, option) => {
|
||||||
if (isSingleMode) {
|
if (isSingleMode) {
|
||||||
|
// on select is fired in single value mode if the same value is selected
|
||||||
|
const valueChanged = !utilsIsEqual(
|
||||||
|
selectedItem,
|
||||||
|
selectValue as RawValue | AntdLabeledValue,
|
||||||
|
'value',
|
||||||
|
);
|
||||||
setSelectValue(selectedItem);
|
setSelectValue(selectedItem);
|
||||||
|
if (valueChanged) {
|
||||||
|
fireOnChange();
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
setSelectValue(previousState => {
|
setSelectValue(previousState => {
|
||||||
const array = ensureIsArray(previousState);
|
const array = ensureIsArray(previousState);
|
||||||
@ -259,8 +269,8 @@ const Select = forwardRef(
|
|||||||
}
|
}
|
||||||
return previousState;
|
return previousState;
|
||||||
});
|
});
|
||||||
|
fireOnChange();
|
||||||
}
|
}
|
||||||
fireOnChange();
|
|
||||||
onSelect?.(selectedItem, option);
|
onSelect?.(selectedItem, option);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -49,22 +49,24 @@ export function getValue(
|
|||||||
return isLabeledValue(option) ? option.value : option;
|
return isLabeledValue(option) ? option.value : option;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function isEqual(a: V | LabeledValue, b: V | LabeledValue, key: string) {
|
||||||
|
const actualA = isObject(a) && key in a ? a[key] : a;
|
||||||
|
const actualB = isObject(b) && key in b ? b[key] : b;
|
||||||
|
// When comparing the values we use the equality
|
||||||
|
// operator to automatically convert different types
|
||||||
|
// eslint-disable-next-line eqeqeq
|
||||||
|
return actualA == actualB;
|
||||||
|
}
|
||||||
|
|
||||||
export function getOption(
|
export function getOption(
|
||||||
value: V,
|
value: V,
|
||||||
options?: V | LabeledValue | (V | LabeledValue)[],
|
options?: V | LabeledValue | (V | LabeledValue)[],
|
||||||
checkLabel = false,
|
checkLabel = false,
|
||||||
): V | LabeledValue {
|
): V | LabeledValue {
|
||||||
const optionsArray = ensureIsArray(options);
|
const optionsArray = ensureIsArray(options);
|
||||||
// When comparing the values we use the equality
|
|
||||||
// operator to automatically convert different types
|
|
||||||
return optionsArray.find(
|
return optionsArray.find(
|
||||||
x =>
|
x =>
|
||||||
// eslint-disable-next-line eqeqeq
|
isEqual(x, value, 'value') || (checkLabel && isEqual(x, value, 'label')),
|
||||||
x == value ||
|
|
||||||
(isObject(x) &&
|
|
||||||
// eslint-disable-next-line eqeqeq
|
|
||||||
(('value' in x && x.value == value) ||
|
|
||||||
(checkLabel && 'label' in x && x.label === value))),
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user