Select: Preserve value when allowCustomValue is set (#87843)

* initial working poc with some better types

* move logic inside SelectBase

* add unit tests

* cleaner logic

* simpler

* update comments

* more comments

* use onMenuClose

* undo changes to cleanValue

* fix unit tests

---------

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
This commit is contained in:
Ashley Harrison 2024-05-20 11:01:38 +01:00 committed by GitHub
parent cf407fe8de
commit 429bcbe67f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 81 additions and 31 deletions

View File

@ -682,11 +682,7 @@ exports[`better eslint`] = {
[0, 0, 0, "Unexpected any. Specify a different type.", "2"],
[0, 0, 0, "Unexpected any. Specify a different type.", "3"],
[0, 0, 0, "Do not use any type assertions.", "4"],
[0, 0, 0, "Unexpected any. Specify a different type.", "5"],
[0, 0, 0, "Do not use any type assertions.", "6"],
[0, 0, 0, "Unexpected any. Specify a different type.", "7"],
[0, 0, 0, "Unexpected any. Specify a different type.", "8"],
[0, 0, 0, "Unexpected any. Specify a different type.", "9"]
[0, 0, 0, "Unexpected any. Specify a different type.", "5"]
],
"packages/grafana-ui/src/components/Select/SelectOptionGroup.tsx:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"],

View File

@ -174,35 +174,20 @@ describe('PromQueryBuilder', () => {
});
it('shows hints for histogram metrics', async () => {
const { container } = setup({
setup({
metric: 'histogram_metric_bucket',
labels: [],
operations: [],
});
await openMetricSelect(container);
await userEvent.click(screen.getByText('histogram_metric_bucket'));
await waitFor(() => expect(screen.getByText('hint: add histogram_quantile')).toBeInTheDocument());
});
it('shows hints for counter metrics', async () => {
const { container } = setup({
setup({
metric: 'histogram_metric_sum',
labels: [],
operations: [],
});
await openMetricSelect(container);
await userEvent.click(screen.getByText('histogram_metric_sum'));
await waitFor(() => expect(screen.getByText('hint: add rate')).toBeInTheDocument());
});
it('shows hints for counter metrics', async () => {
const { container } = setup({
metric: 'histogram_metric_sum',
labels: [],
operations: [],
});
await openMetricSelect(container);
await userEvent.click(screen.getByText('histogram_metric_sum'));
await waitFor(() => expect(screen.getByText('hint: add rate')).toBeInTheDocument());
});
@ -215,7 +200,7 @@ describe('PromQueryBuilder', () => {
for (let i = 0; i < 25; i++) {
data.series.push(new MutableDataFrame());
}
const { container } = setup(
setup(
{
metric: 'histogram_metric_sum',
labels: [],
@ -223,8 +208,6 @@ describe('PromQueryBuilder', () => {
},
data
);
await openMetricSelect(container);
await userEvent.click(screen.getByText('histogram_metric_sum'));
await waitFor(() => expect(screen.getAllByText(/hint:/)).toHaveLength(2));
});

View File

@ -64,6 +64,39 @@ describe('SelectBase', () => {
expect(screen.queryByText('Test label')).not.toBeInTheDocument();
});
describe('with custom values', () => {
it('allows editing a custom SelectableValue', async () => {
render(
<SelectBase
onChange={onChangeHandler}
allowCustomValue
value={{
label: 'my custom value',
value: 'my custom value',
}}
/>
);
await userEvent.click(screen.getByRole('combobox'));
await userEvent.type(screen.getByRole('combobox'), '{backspace}{backspace}{enter}');
expect(onChangeHandler).toHaveBeenCalled();
expect(onChangeHandler.mock.calls[0][0]).toEqual(
expect.objectContaining({ label: 'my custom val', value: 'my custom val' })
);
});
it('allows editing a custom basic value', async () => {
render(<SelectBase onChange={onChangeHandler} allowCustomValue value="my custom value" />);
await userEvent.click(screen.getByRole('combobox'));
await userEvent.type(screen.getByRole('combobox'), '{backspace}{backspace}{enter}');
expect(onChangeHandler).toHaveBeenCalled();
expect(onChangeHandler.mock.calls[0][0]).toEqual(
expect.objectContaining({ label: 'my custom val', value: 'my custom val' })
);
});
});
describe('when openMenuOnFocus prop', () => {
describe('is provided', () => {
it('opens on focus', () => {

View File

@ -1,6 +1,11 @@
import { t } from 'i18next';
import React, { ComponentProps, useCallback, useEffect, useRef, useState } from 'react';
import { default as ReactSelect, IndicatorsContainerProps, Props as ReactSelectProps } from 'react-select';
import {
default as ReactSelect,
IndicatorsContainerProps,
Props as ReactSelectProps,
ClearIndicatorProps,
} from 'react-select';
import { default as ReactAsyncSelect } from 'react-select/async';
import { default as AsyncCreatable } from 'react-select/async-creatable';
import Creatable from 'react-select/creatable';
@ -19,7 +24,7 @@ import { MultiValueContainer, MultiValueRemove } from './MultiValue';
import { SelectContainer } from './SelectContainer';
import { SelectMenu, SelectMenuOptions, VirtualizedSelectMenu } from './SelectMenu';
import { SelectOptionGroup } from './SelectOptionGroup';
import { SingleValue } from './SingleValue';
import { Props, SingleValue } from './SingleValue';
import { ValueContainer } from './ValueContainer';
import { getSelectStyles } from './getSelectStyles';
import { useCustomSelectStyles } from './resetSelectStyles';
@ -199,6 +204,8 @@ export function SelectBase<T, Rest = {}>({
}
}
const [internalInputValue, setInternalInputValue] = useState('');
const commonSelectProps = {
'aria-label': ariaLabel,
'data-testid': dataTestid,
@ -268,12 +275,41 @@ export function SelectBase<T, Rest = {}>({
};
if (allowCustomValue) {
ReactSelectComponent = Creatable as any;
ReactSelectComponent = Creatable;
creatableProps.allowCreateWhileLoading = allowCreateWhileLoading;
creatableProps.formatCreateLabel = formatCreateLabel ?? defaultFormatCreateLabel;
creatableProps.onCreateOption = onCreateOption;
creatableProps.createOptionPosition = createOptionPosition;
creatableProps.isValidNewOption = isValidNewOption;
// code needed to allow editing a custom value once entered
// we only want to do this for single selects, not multi
if (!isMulti) {
creatableProps.inputValue = internalInputValue;
creatableProps.onMenuOpen = () => {
// make sure we call the base onMenuOpen if it exists
commonSelectProps.onMenuOpen?.();
// restore the input state to the selected value
setInternalInputValue(selectedValue?.[0]?.label ?? '');
};
creatableProps.onInputChange = (val, actionMeta) => {
// make sure we call the base onInputChange
commonSelectProps.onInputChange(val, actionMeta);
// update the input value state on change since we're explicitly controlling it
if (actionMeta.action === 'input-change') {
setInternalInputValue(val);
}
};
creatableProps.onMenuClose = () => {
// make sure we call the base onMenuClose if it exists
commonSelectProps.onMenuClose?.();
// clear the input state on menu close, else react-select won't show the SingleValue component correctly
setInternalInputValue('');
};
}
}
// Instead of having AsyncSelect, as a separate component we render ReactAsyncSelect
@ -300,7 +336,7 @@ export function SelectBase<T, Rest = {}>({
IndicatorSeparator: IndicatorSeparator,
Control: CustomControl,
Option: SelectMenuOptions,
ClearIndicator(props: any) {
ClearIndicator(props: ClearIndicatorProps) {
const { clearValue } = props;
return (
<Icon
@ -330,7 +366,7 @@ export function SelectBase<T, Rest = {}>({
);
},
DropdownIndicator: DropdownIndicator,
SingleValue(props: any) {
SingleValue(props: Props<T>) {
return <SingleValue {...props} isDisabled={disabled} />;
},
SelectContainer,

View File

@ -111,6 +111,7 @@ describe('useCreatableSelectPersistedBehaviour', () => {
await userEvent.click(input);
// we expect 2 elemnts having "Option 2": the input itself and the option.
expect(screen.getAllByText('Option 2')).toHaveLength(2);
expect(screen.getByText('Option 2')).toBeInTheDocument();
expect(screen.getByRole('combobox')).toHaveValue('Option 2');
});
});

View File

@ -157,6 +157,7 @@ describe('SearchForm', () => {
await user.click(asyncOperationSelect);
jest.advanceTimersByTime(3000);
await user.clear(asyncOperationSelect);
await user.type(asyncOperationSelect, '$');
const operationOption = await screen.findByText('$operation');
expect(operationOption).toBeDefined();