You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/09/28 17:38:58 UTC
[superset] 08/08: fix: Duplicate items when pasting into Select (#25447)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 58778a78b24f12c07c8fdfc70fc228bfeed9e11d
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Thu Sep 28 14:11:58 2023 -0300
fix: Duplicate items when pasting into Select (#25447)
(cherry picked from commit 7cf96cd8436c782090336ae8a9581bd3898a4e6a)
---
.../src/components/Select/AsyncSelect.test.tsx | 39 ++++++++++++++++++++++
.../src/components/Select/AsyncSelect.tsx | 26 ++++++++++++---
.../src/components/Select/Select.test.tsx | 39 ++++++++++++++++++++++
superset-frontend/src/components/Select/Select.tsx | 33 +++++++++++++-----
superset-frontend/src/components/Select/utils.tsx | 28 ++++++++++------
5 files changed, 141 insertions(+), 24 deletions(-)
diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
index b964f48ee7..e49f00be53 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
@@ -858,6 +858,45 @@ test('does not duplicate options when using numeric values', async () => {
await waitFor(() => expect(getAllSelectOptions().length).toBe(1));
});
+test('pasting an existing option does not duplicate it', async () => {
+ const options = jest.fn(async () => ({
+ data: [OPTIONS[0]],
+ totalCount: 1,
+ }));
+ render(<AsyncSelect {...defaultProps} options={options} />);
+ await open();
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => OPTIONS[0].label,
+ },
+ });
+ fireEvent(input, paste);
+ expect(await findAllSelectOptions()).toHaveLength(1);
+});
+
+test('pasting an existing option does not duplicate it in multiple mode', async () => {
+ const options = jest.fn(async () => ({
+ data: [
+ { label: 'John', value: 1 },
+ { label: 'Liam', value: 2 },
+ { label: 'Olivia', value: 3 },
+ ],
+ totalCount: 3,
+ }));
+ render(<AsyncSelect {...defaultProps} options={options} mode="multiple" />);
+ await open();
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => 'John,Liam,Peter',
+ },
+ });
+ fireEvent(input, paste);
+ // Only Peter should be added
+ expect(await findAllSelectOptions()).toHaveLength(4);
+});
+
/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx
index 320d6ec3bd..20de7bb591 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.tsx
@@ -49,6 +49,8 @@ import {
dropDownRenderHelper,
handleFilterOptionHelper,
mapOptions,
+ getOption,
+ isObject,
} from './utils';
import {
AsyncSelectProps,
@@ -523,19 +525,33 @@ const AsyncSelect = forwardRef(
[ref],
);
+ const getPastedTextValue = useCallback(
+ (text: string) => {
+ const option = getOption(text, fullSelectOptions, true);
+ const value: AntdLabeledValue = {
+ label: text,
+ value: text,
+ };
+ if (option) {
+ value.label = isObject(option) ? option.label : option;
+ value.value = isObject(option) ? option.value! : option;
+ }
+ return value;
+ },
+ [fullSelectOptions],
+ );
+
const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
const pastedText = e.clipboardData.getData('text');
if (isSingleMode) {
- setSelectValue({ label: pastedText, value: pastedText });
+ setSelectValue(getPastedTextValue(pastedText));
} else {
const token = tokenSeparators.find(token => pastedText.includes(token));
const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ const values = array.map(item => getPastedTextValue(item));
setSelectValue(previous => [
...((previous || []) as AntdLabeledValue[]),
- ...array.map<AntdLabeledValue>(value => ({
- label: value,
- value,
- })),
+ ...values,
]);
}
};
diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
index 2b204cec1c..52e566df17 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -972,6 +972,45 @@ test('does not duplicate options when using numeric values', async () => {
await waitFor(() => expect(getAllSelectOptions().length).toBe(1));
});
+test('pasting an existing option does not duplicate it', async () => {
+ render(<Select {...defaultProps} options={[OPTIONS[0]]} />);
+ await open();
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => OPTIONS[0].label,
+ },
+ });
+ fireEvent(input, paste);
+ expect(await findAllSelectOptions()).toHaveLength(1);
+});
+
+test('pasting an existing option does not duplicate it in multiple mode', async () => {
+ const options = [
+ { label: 'John', value: 1 },
+ { label: 'Liam', value: 2 },
+ { label: 'Olivia', value: 3 },
+ ];
+ render(
+ <Select
+ {...defaultProps}
+ options={options}
+ mode="multiple"
+ allowSelectAll={false}
+ />,
+ );
+ await open();
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => 'John,Liam,Peter',
+ },
+ });
+ fireEvent(input, paste);
+ // Only Peter should be added
+ expect(await findAllSelectOptions()).toHaveLength(4);
+});
+
/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
index 89c62ef8bd..907850a456 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -51,6 +51,8 @@ import {
mapValues,
mapOptions,
hasCustomLabels,
+ getOption,
+ isObject,
} from './utils';
import { RawValue, SelectOptionsType, SelectProps } from './types';
import {
@@ -530,27 +532,42 @@ const Select = forwardRef(
actualMaxTagCount -= 1;
}
+ const getPastedTextValue = useCallback(
+ (text: string) => {
+ const option = getOption(text, fullSelectOptions, true);
+ if (labelInValue) {
+ const value: AntdLabeledValue = {
+ label: text,
+ value: text,
+ };
+ if (option) {
+ value.label = isObject(option) ? option.label : option;
+ value.value = isObject(option) ? option.value! : option;
+ }
+ return value;
+ }
+ return option ? (isObject(option) ? option.value! : option) : text;
+ },
+ [fullSelectOptions, labelInValue],
+ );
+
const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
const pastedText = e.clipboardData.getData('text');
if (isSingleMode) {
- setSelectValue(
- labelInValue ? { label: pastedText, value: pastedText } : pastedText,
- );
+ setSelectValue(getPastedTextValue(pastedText));
} else {
const token = tokenSeparators.find(token => pastedText.includes(token));
const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ const values = array.map(item => getPastedTextValue(item));
if (labelInValue) {
setSelectValue(previous => [
...((previous || []) as AntdLabeledValue[]),
- ...array.map<AntdLabeledValue>(value => ({
- label: value,
- value,
- })),
+ ...(values as AntdLabeledValue[]),
]);
} else {
setSelectValue(previous => [
...((previous || []) as string[]),
- ...array,
+ ...(values as string[]),
]);
}
}
diff --git a/superset-frontend/src/components/Select/utils.tsx b/superset-frontend/src/components/Select/utils.tsx
index 7de201caeb..0b638f4f01 100644
--- a/superset-frontend/src/components/Select/utils.tsx
+++ b/superset-frontend/src/components/Select/utils.tsx
@@ -49,27 +49,33 @@ export function getValue(
return isLabeledValue(option) ? option.value : option;
}
-export function hasOption(
+export function getOption(
value: V,
options?: V | LabeledValue | (V | LabeledValue)[],
checkLabel = false,
-): boolean {
+): V | LabeledValue {
const optionsArray = ensureIsArray(options);
// When comparing the values we use the equality
// operator to automatically convert different types
- return (
- optionsArray.find(
- x =>
+ return optionsArray.find(
+ x =>
+ // eslint-disable-next-line eqeqeq
+ x == value ||
+ (isObject(x) &&
// eslint-disable-next-line eqeqeq
- x == value ||
- (isObject(x) &&
- // eslint-disable-next-line eqeqeq
- (('value' in x && x.value == value) ||
- (checkLabel && 'label' in x && x.label === value))),
- ) !== undefined
+ (('value' in x && x.value == value) ||
+ (checkLabel && 'label' in x && x.label === value))),
);
}
+export function hasOption(
+ value: V,
+ options?: V | LabeledValue | (V | LabeledValue)[],
+ checkLabel = false,
+): boolean {
+ return getOption(value, options, checkLabel) !== undefined;
+}
+
/**
* It creates a comparator to check for a specific property.
* Can be used with string and number property values.