Skip to content

Commit

Permalink
fix: regression on Select component when handling null values
Browse files Browse the repository at this point in the history
  • Loading branch information
diegomedina248 committed Mar 23, 2022
1 parent 54c521b commit 12fad95
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
28 changes: 28 additions & 0 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ const OPTIONS = [
{ label: 'Cher', value: 22, gender: 'Female' },
{ label: 'Her', value: 23, gender: 'Male' },
].sort((option1, option2) => option1.label.localeCompare(option2.label));
const NULL_OPTION = { label: '<NULL>', value: null } as unknown as {
label: string;
value: number;
};

const loadOptions = async (search: string, page: number, pageSize: number) => {
const totalCount = OPTIONS.length;
Expand Down Expand Up @@ -384,6 +388,30 @@ test('does not add a new option if allowNewOptions is false', async () => {
expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
});

test('adds the null option when selected in single mode', async () => {
render(<Select {...defaultProps} options={[OPTIONS[0], NULL_OPTION]} />);
await open();
userEvent.click(await findSelectOption(NULL_OPTION.label));
const values = await findAllSelectValues();
expect(values[0]).toHaveTextContent(NULL_OPTION.label);
});

test('adds the null option when selected in multiple mode', async () => {
render(
<Select
{...defaultProps}
options={[OPTIONS[0], NULL_OPTION, OPTIONS[2]]}
mode="multiple"
/>,
);
await open();
userEvent.click(await findSelectOption(OPTIONS[0].label));
userEvent.click(await findSelectOption(NULL_OPTION.label));
const values = await findAllSelectValues();
expect(values[0]).toHaveTextContent(OPTIONS[0].label);
expect(values[1]).toHaveTextContent(NULL_OPTION.label);
});

test('static - renders the select with default props', () => {
render(<Select {...defaultProps} />);
expect(getSelect()).toBeInTheDocument();
Expand Down
12 changes: 10 additions & 2 deletions superset-frontend/src/components/Select/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ import {
GroupedOptionsType,
} from 'react-select';

function isObject(value: unknown): value is Record<string, unknown> {
return (
value !== null &&
typeof value === 'object' &&
Array.isArray(value) === false
);
}

/**
* Find Option value that matches a possibly string value.
*
Expand Down Expand Up @@ -63,7 +71,7 @@ export function findValue<OptionType extends OptionTypeBase>(
export function getValue(
option: string | number | { value: string | number | null } | null,
) {
return option && typeof option === 'object' ? option.value : option;
return isObject(option) ? option.value : option;
}

type LabeledValue<V> = { label?: ReactNode; value?: V };
Expand All @@ -78,7 +86,7 @@ export function hasOption<V>(
optionsArray.find(
x =>
x === value ||
(typeof x === 'object' &&
(isObject(x) &&
(('value' in x && x.value === value) ||
(checkLabel && 'label' in x && x.label === value))),
) !== undefined
Expand Down

0 comments on commit 12fad95

Please sign in to comment.