Skip to content

Commit

Permalink
fix: Incorrect onChange value when an unloaded value is pasted into A…
Browse files Browse the repository at this point in the history
…syncSelect (#27996)
  • Loading branch information
michael-s-molina authored Apr 12, 2024
1 parent 6844735 commit 06077d4
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 16 deletions.
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

0 comments on commit 06077d4

Please sign in to comment.