Skip to content

Commit

Permalink
chore: Select component refactoring - SelectAsyncControl - Iteration 5 (
Browse files Browse the repository at this point in the history
#16609)

* Refactor Select

* Implement onError

* Separate async request

* Lint

* Allow clear

* Improve type

* Reconcile with Select latest changes

* Fix handleOnChange

* Clean up

* Add placeholder

* Accept null values

* Update superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* Clean up

* Implement type guard

* Fix lint

* Catch error within loadOptions

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>
  • Loading branch information
geido and suddjian authored Oct 7, 2021
1 parent 5fc9970 commit a782a62
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 113 deletions.
5 changes: 5 additions & 0 deletions superset-frontend/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export interface SelectProps extends PickedSelectProps {
pageSize?: number;
invertSelection?: boolean;
fetchOnlyOnSearch?: boolean;
onError?: (error: string) => void;
}

const StyledContainer = styled.div`
Expand Down Expand Up @@ -331,6 +332,10 @@ const Select = ({
getClientErrorObject(response).then(e => {
const { error } = e;
setError(error);

if (props.onError) {
props.onError(error);
}
});

const handleData = (data: OptionsType) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,21 @@
* under the License.
*/
import userEvent from '@testing-library/user-event';
import fetchMock from 'fetch-mock';
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import SelectAsyncControl from '.';

jest.mock('src/components/AsyncSelect', () => ({
const datasetsOwnersEndpoint = 'glob:*/api/v1/dataset/related/owners*';

jest.mock('src/components/Select/Select', () => ({
__esModule: true,
default: (props: any) => (
<div
data-test="select-test"
data-data-endpoint={props.dataEndpoint}
data-value={JSON.stringify(props.value)}
data-placeholder={props.placeholder}
data-multi={props.multi ? 'true' : 'false'}
data-multi={props.mode}
>
<button
type="button"
Expand All @@ -40,19 +42,19 @@ jest.mock('src/components/AsyncSelect', () => ({
<button type="button" onClick={() => props.mutator()}>
mutator
</button>

<div data-test="valueRenderer">
{props.valueRenderer({ label: 'valueRenderer' })}
</div>
</div>
),
}));

fetchMock.get(datasetsOwnersEndpoint, {
result: [],
});

const createProps = () => ({
ariaLabel: 'SelectAsyncControl',
value: [],
dataEndpoint: 'api/v1/dataset/related/owners',
dataEndpoint: datasetsOwnersEndpoint,
multi: true,
onAsyncErrorMessage: 'Error while fetching data',
placeholder: 'Select ...',
onChange: jest.fn(),
mutator: jest.fn(),
Expand All @@ -65,17 +67,13 @@ beforeEach(() => {
test('Should render', () => {
const props = createProps();
render(<SelectAsyncControl {...props} />, { useRedux: true });
expect(screen.getByTestId('SelectAsyncControl')).toBeInTheDocument();
expect(screen.getByTestId('select-test')).toBeInTheDocument();
});

test('Should send correct props to AsyncSelect component - value props', () => {
test('Should send correct props to Select component - value props', () => {
const props = createProps();
render(<SelectAsyncControl {...props} />, { useRedux: true });

expect(screen.getByTestId('select-test')).toHaveAttribute(
'data-data-endpoint',
props.dataEndpoint,
);
expect(screen.getByTestId('select-test')).toHaveAttribute(
'data-value',
JSON.stringify(props.value),
Expand All @@ -86,22 +84,19 @@ test('Should send correct props to AsyncSelect component - value props', () => {
);
expect(screen.getByTestId('select-test')).toHaveAttribute(
'data-multi',
'true',
);
expect(screen.getByTestId('valueRenderer')).toHaveTextContent(
'valueRenderer',
'multiple',
);
});

test('Should send correct props to AsyncSelect component - function onChange multi:true', () => {
test('Should send correct props to Select component - function onChange multi:true', () => {
const props = createProps();
render(<SelectAsyncControl {...props} />, { useRedux: true });
expect(props.onChange).toBeCalledTimes(0);
userEvent.click(screen.getByText('onChange'));
expect(props.onChange).toBeCalledTimes(1);
});

test('Should send correct props to AsyncSelect component - function onChange multi:false', () => {
test('Should send correct props to Select component - function onChange multi:false', () => {
const props = createProps();
render(<SelectAsyncControl {...{ ...props, multi: false }} />, {
useRedux: true,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import React, { useEffect, useState } from 'react';
import { t, SupersetClient } from '@superset-ui/core';
import ControlHeader from 'src/explore/components/ControlHeader';
import { Select } from 'src/components';
import { SelectProps, OptionsType } from 'src/components/Select/Select';
import { SelectValue, LabeledValue } from 'antd/lib/select';
import withToasts from 'src/components/MessageToasts/withToasts';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';

type SelectAsyncProps = Omit<SelectProps, 'options' | 'ariaLabel' | 'onChange'>;

interface SelectAsyncControlProps extends SelectAsyncProps {
addDangerToast: (error: string) => void;
ariaLabel?: string;
dataEndpoint: string;
default?: SelectValue;
mutator?: (response: Record<string, any>) => OptionsType;
multi?: boolean;
onChange: (val: SelectValue) => void;
// ControlHeader related props
description?: string;
hovered?: boolean;
label?: string;
}

function isLabeledValue(arg: any): arg is LabeledValue {
return arg.value !== undefined;
}

const SelectAsyncControl = ({
addDangerToast,
allowClear = true,
ariaLabel,
dataEndpoint,
multi = true,
mutator,
onChange,
placeholder,
value,
...props
}: SelectAsyncControlProps) => {
const [options, setOptions] = useState<OptionsType>([]);

const handleOnChange = (val: SelectValue) => {
let onChangeVal = val;
if (Array.isArray(val)) {
const values = val.map(v => (isLabeledValue(v) ? v.value : v));
onChangeVal = values;
}
if (isLabeledValue(val)) {
onChangeVal = val.value;
}
onChange(onChangeVal);
};

useEffect(() => {
const onError = (response: Response) =>
getClientErrorObject(response).then(e => {
const { error } = e;
addDangerToast(`${t('Error while fetching data')}: ${error}`);
});
const loadOptions = () =>
SupersetClient.get({
endpoint: dataEndpoint,
})
.then(response => {
const data = mutator ? mutator(response.json) : response.json.result;
setOptions(data);
})
.catch(onError);
loadOptions();
}, [addDangerToast, dataEndpoint, mutator]);

return (
<Select
allowClear={allowClear}
ariaLabel={ariaLabel || t('Select ...')}
value={value || (props.default !== undefined ? props.default : undefined)}
header={<ControlHeader {...props} />}
mode={multi ? 'multiple' : 'single'}
onChange={handleOnChange}
options={options}
placeholder={placeholder}
/>
);
};

export default withToasts(SelectAsyncControl);

0 comments on commit a782a62

Please sign in to comment.