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: Incorrect onChange value when an unloaded value is pasted into AsyncSelect #27996

Merged
merged 1 commit into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 33 additions & 9 deletions superset-frontend/src/components/Select/AsyncSelect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,14 @@ test('removes duplicated values', async () => {
},
});
fireEvent(input, paste);
const values = await findAllSelectValues();
expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a');
expect(values[1]).toHaveTextContent('b');
expect(values[2]).toHaveTextContent('c');
expect(values[3]).toHaveTextContent('d');
await waitFor(async () => {
const values = await findAllSelectValues();
expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a');
expect(values[1]).toHaveTextContent('b');
expect(values[2]).toHaveTextContent('c');
expect(values[3]).toHaveTextContent('d');
});
});

test('renders a custom label', async () => {
Expand Down Expand Up @@ -879,7 +881,7 @@ test('fires onChange when pasting a selection', async () => {
},
});
fireEvent(input, paste);
expect(onChange).toHaveBeenCalledTimes(1);
await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1));
});

test('does not duplicate options when using numeric values', async () => {
Expand Down Expand Up @@ -935,8 +937,30 @@ test('pasting an existing option does not duplicate it in multiple mode', async
},
});
fireEvent(input, paste);
// Only Peter should be added
expect(await findAllSelectOptions()).toHaveLength(4);
await waitFor(async () =>
// Only Peter should be added
expect(await findAllSelectOptions()).toHaveLength(4),
);
});

test('onChange is called with the value property when pasting an option that was not loaded yet', async () => {
const onChange = jest.fn();
render(<AsyncSelect {...defaultProps} onChange={onChange} />);
await open();
const input = getElementByClassName('.ant-select-selection-search-input');
const lastOption = OPTIONS[OPTIONS.length - 1];
const paste = createEvent.paste(input, {
clipboardData: {
getData: () => lastOption.label,
},
});
fireEvent(input, paste);
await waitFor(() =>
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({ value: lastOption.value }),
expect.anything(),
),
);
});

test('does not fire onChange if the same value is selected in single mode', async () => {
Expand Down
23 changes: 16 additions & 7 deletions superset-frontend/src/components/Select/AsyncSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,15 @@ const AsyncSelect = forwardRef(
);

const getPastedTextValue = useCallback(
(text: string) => {
const option = getOption(text, fullSelectOptions, true);
async (text: string) => {
let option = getOption(text, fullSelectOptions, true);
if (!option && !allValuesLoaded) {
const fetchOptions = options as SelectOptionsPagePromise;
option = await fetchOptions(text, 0, pageSize).then(
({ data }: SelectOptionsTypePage) =>
data.find(item => item.label === text),
);
}
const value: AntdLabeledValue = {
label: text,
value: text,
Expand All @@ -550,20 +557,22 @@ const AsyncSelect = forwardRef(
}
return value;
},
[fullSelectOptions],
[allValuesLoaded, fullSelectOptions, options, pageSize],
);

const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
const onPaste = async (e: ClipboardEvent<HTMLInputElement>) => {
const pastedText = e.clipboardData.getData('text');
if (isSingleMode) {
setSelectValue(getPastedTextValue(pastedText));
setSelectValue(await 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));
const values = await Promise.all(
array.map(item => getPastedTextValue(item)),
);
setSelectValue(previous => [
...((previous || []) as AntdLabeledValue[]),
...values,
...values.filter(value => !hasOption(value.value, previous)),
]);
}
fireOnChange();
Expand Down
Loading