This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3f0756f chore: Select component refactoring - SelectControl -
Iteration 5 (#16510)
3f0756f is described below
commit 3f0756f637660cb721285e645e419ad835f688b4
Author: Geido <[email protected]>
AuthorDate: Mon Oct 4 18:24:41 2021 +0300
chore: Select component refactoring - SelectControl - Iteration 5 (#16510)
* Refactor Select DatasourceEditor
* Fire onChange with allowNewOptions
* Clean up
* Refactor Select in AnnotationLayer
* Handle on clear
* Update tests
* Refactor Select in SpatialControl
* Show search
* Refactor Select in FilterBox
* Remove search where unnecessary
* Update SelectControl - WIP
* Refactor Controls
* Update SelectControl tests
* Clean up
* Test allowNewOptions false
* Use SelectControl AnnotationLayer
* Use SelectControl SpatialControl
* Clean up
* Render custom label
* Show search
* Implement filterOption
* Improve filterOption
* Update Cypress
* Update Cypress table test
* Use value for defaultValue
* Merge with latest changes
* Reconcile with latest Select changes
* Update
superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
Co-authored-by: Michael S. Molina
<[email protected]>
* Update
superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
Co-authored-by: Michael S. Molina
<[email protected]>
* Revert changes to test
* Call onPopoverClear when v value is undefined
Co-authored-by: Michael S. Molina
<[email protected]>
---
.../integration/explore/advanced_analytics.test.ts | 10 +-
.../cypress/integration/explore/control.test.ts | 4 +-
.../explore/visualizations/table.test.ts | 2 +-
.../explore/components/SelectControl_spec.jsx | 137 ++---------
.../explore/components/TextArea_spec.jsx | 2 +-
.../spec/javascripts/explore/fixtures.tsx | 3 -
.../src/datasource/DatasourceEditor.jsx | 17 +-
.../AnnotationLayerControl/AnnotationLayer.jsx | 89 +++----
.../AnnotationLayer.test.tsx | 53 +++-
.../controls/AnnotationLayerControl/index.jsx | 19 +-
.../controls/FilterBoxItemControl/index.jsx | 13 +-
.../explore/components/controls/SelectControl.jsx | 268 ++++++++-------------
.../explore/components/controls/SpatialControl.jsx | 1 +
superset-frontend/src/explore/controls.jsx | 7 +-
14 files changed, 223 insertions(+), 402 deletions(-)
diff --git
a/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts
b/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts
index 77ebfbe..ff88c33 100644
---
a/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts
+++
b/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts
@@ -29,13 +29,13 @@ describe('Advanced analytics', () => {
cy.get('.ant-collapse-header').contains('Advanced Analytics').click();
- cy.get('[data-test=time_compare]').find('.Select__control').click();
+ cy.get('[data-test=time_compare]').find('.ant-select').click();
cy.get('[data-test=time_compare]')
- .find('input[type=text]')
+ .find('input[type=search]')
.type('28 days{enter}');
cy.get('[data-test=time_compare]')
- .find('input[type=text]')
+ .find('input[type=search]')
.type('1 year{enter}');
cy.get('button[data-test="run-query-button"]').click();
@@ -48,10 +48,10 @@ describe('Advanced analytics', () => {
cy.get('.ant-collapse-header').contains('Advanced Analytics').click();
cy.get('[data-test=time_compare]')
- .find('.Select__multi-value__label')
+ .find('.ant-select-selector')
.contains('28 days');
cy.get('[data-test=time_compare]')
- .find('.Select__multi-value__label')
+ .find('.ant-select-selector')
.contains('1 year');
});
});
diff --git
a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
index f9eeecb..c57369e 100644
--- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
@@ -235,8 +235,8 @@ describe('Groupby control', () => {
cy.verifySliceSuccess({ waitAlias: '@chartData' });
cy.get('[data-test=groupby]').within(() => {
- cy.get('.Select__control').click();
- cy.get('input[type=text]').type('state{enter}');
+ cy.get('.ant-select').click();
+ cy.get('input[type=search]').type('state{enter}');
});
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'svg' });
diff --git
a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
index 95915fb..2a4258b 100644
---
a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
+++
b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
@@ -54,7 +54,7 @@ describe('Visualization > Table', () => {
granularity_sqla: undefined,
metrics: ['count'],
});
- cy.get('input[name="select-granularity_sqla"]').should('have.value', 'ds');
+ cy.get('[data-test=granularity_sqla] .column-option-label').contains('ds');
});
it('Format non-numeric metrics correctly', () => {
diff --git
a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
index b9c76e3..bc16485 100644
---
a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
+++
b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
@@ -20,8 +20,7 @@
import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
-import { Select, CreatableSelect } from 'src/components/Select';
-import OnPasteSelect from 'src/components/Select/OnPasteSelect';
+import { Select as SelectComponent } from 'src/components';
import SelectControl from 'src/explore/components/controls/SelectControl';
import { styledMount as mount } from 'spec/helpers/theming';
@@ -48,59 +47,35 @@ describe('SelectControl', () => {
wrapper = shallow(<SelectControl {...defaultProps} />);
});
- it('uses Select in onPasteSelect when freeForm=false', () => {
- wrapper = shallow(<SelectControl {...defaultProps} multi />);
- const select = wrapper.find(OnPasteSelect);
- expect(select.props().selectWrap).toBe(Select);
- });
-
- it('uses Creatable in onPasteSelect when freeForm=true', () => {
- wrapper = shallow(<SelectControl {...defaultProps} multi freeForm />);
- const select = wrapper.find(OnPasteSelect);
- expect(select.props().selectWrap).toBe(CreatableSelect);
- });
-
it('calls props.onChange when select', () => {
const select = wrapper.instance();
- select.onChange({ value: 50 });
+ select.onChange(50);
expect(defaultProps.onChange.calledWith(50)).toBe(true);
});
- it('returns all options on select all', () => {
- const expectedValues = ['one', 'two'];
- const selectAllProps = {
- multi: true,
- allowAll: true,
- choices: expectedValues,
- name: 'row_limit',
- label: 'Row Limit',
- valueKey: 'value',
- onChange: sinon.spy(),
- };
- wrapper.setProps(selectAllProps);
- wrapper.instance().onChange([{ meta: true, value: 'Select all' }]);
- expect(selectAllProps.onChange.calledWith(expectedValues)).toBe(true);
- });
-
describe('render', () => {
it('renders with Select by default', () => {
- expect(wrapper.find(OnPasteSelect)).not.toExist();
- expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(1);
+ expect(wrapper.find(SelectComponent)).toExist();
});
- it('renders with OnPasteSelect when multi', () => {
+ it('renders as mode multiple', () => {
wrapper.setProps({ multi: true });
- expect(wrapper.find(OnPasteSelect)).toExist();
- expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(0);
+ expect(wrapper.find(SelectComponent)).toExist();
+ expect(wrapper.find(SelectComponent).prop('mode')).toBe('multiple');
});
- it('renders with Creatable when freeForm', () => {
+ it('renders with allowNewOptions when freeForm', () => {
wrapper.setProps({ freeForm: true });
- expect(wrapper.find(OnPasteSelect)).not.toExist();
- expect(wrapper.findWhere(x => x.type() ===
CreatableSelect)).toHaveLength(
- 1,
- );
+ expect(wrapper.find(SelectComponent)).toExist();
+ expect(wrapper.find(SelectComponent).prop('allowNewOptions')).toBe(true);
+ });
+
+ it('renders with allowNewOptions=false when freeForm=false', () => {
+ wrapper.setProps({ freeForm: false });
+ expect(wrapper.find(SelectComponent)).toExist();
+
expect(wrapper.find(SelectComponent).prop('allowNewOptions')).toBe(false);
});
+
describe('empty placeholder', () => {
describe('withMulti', () => {
it('does not show a placeholder if there are no choices', () => {
@@ -161,16 +136,6 @@ describe('SelectControl', () => {
);
expect(wrapper.html()).not.toContain('add something');
});
- it('shows numbers of options as a placeholder by default', () => {
- wrapper = mount(<SelectControl {...defaultProps} multi />);
- expect(wrapper.html()).toContain('2 option(s');
- });
- it('reduces the number of options in the placeholder by the value
length', () => {
- wrapper = mount(
- <SelectControl {...defaultProps} multi value={['today']} />,
- );
- expect(wrapper.html()).toContain('1 option(s');
- });
});
describe('when select is single', () => {
it('does not render the placeholder when a selection has been made', ()
=> {
@@ -186,82 +151,12 @@ describe('SelectControl', () => {
});
});
- describe('optionsRemaining', () => {
- describe('isMulti', () => {
- it('returns the options minus selected values', () => {
- const wrapper = mount(
- <SelectControl {...defaultProps} multi value={['today']} />,
- );
- expect(wrapper.instance().optionsRemaining()).toEqual(1);
- });
- });
- describe('is not multi', () => {
- it('returns the length of all options', () => {
- wrapper = mount(
- <SelectControl
- {...defaultProps}
- value={50}
- placeholder="add something"
- />,
- );
- expect(wrapper.instance().optionsRemaining()).toEqual(2);
- });
- });
- describe('with Select all', () => {
- it('does not count it', () => {
- const props = { ...defaultProps, multi: true, allowAll: true };
- const wrapper = mount(<SelectControl {...props} />);
- expect(wrapper.instance().getOptions(props).length).toEqual(3);
- expect(wrapper.instance().optionsRemaining()).toEqual(2);
- });
- });
- });
-
describe('getOptions', () => {
it('returns the correct options', () => {
wrapper.setProps(defaultProps);
expect(wrapper.instance().getOptions(defaultProps)).toEqual(options);
});
-
- it('shows Select-All when enabled', () => {
- const selectAllProps = {
- choices: ['one', 'two'],
- name: 'name',
- freeForm: true,
- allowAll: true,
- multi: true,
- valueKey: 'value',
- };
- wrapper.setProps(selectAllProps);
- expect(wrapper.instance().getOptions(selectAllProps)).toContainEqual({
- label: 'Select all',
- meta: true,
- value: 'Select all',
- });
- });
-
- it('returns the correct options when freeform is set to true', () => {
- const freeFormProps = {
- choices: [],
- freeForm: true,
- value: ['one', 'two'],
- name: 'row_limit',
- label: 'Row Limit',
- valueKey: 'custom_value_key',
- onChange: sinon.spy(),
- };
- // the last added option is at the top
- const expectedNewOptions = [
- { custom_value_key: 'two', label: 'two' },
- { custom_value_key: 'one', label: 'one' },
- ];
- wrapper.setProps(freeFormProps);
- expect(wrapper.instance().getOptions(freeFormProps)).toEqual(
- expectedNewOptions,
- );
- });
});
-
describe('UNSAFE_componentWillReceiveProps', () => {
it('sets state.options if props.choices has changed', () => {
const updatedOptions = [
diff --git
a/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx
b/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx
index fe804b5..6dbc4b5 100644
--- a/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx
@@ -31,7 +31,7 @@ const defaultProps = {
onChange: sinon.spy(),
};
-describe('SelectControl', () => {
+describe('TextArea', () => {
let wrapper;
beforeEach(() => {
wrapper = shallow(<TextAreaControl {...defaultProps} />);
diff --git a/superset-frontend/spec/javascripts/explore/fixtures.tsx
b/superset-frontend/spec/javascripts/explore/fixtures.tsx
index d9185cf..e4327cc 100644
--- a/superset-frontend/spec/javascripts/explore/fixtures.tsx
+++ b/superset-frontend/spec/javascripts/explore/fixtures.tsx
@@ -93,13 +93,10 @@ export const controlPanelSectionsChartOptionsTable:
ControlPanelSectionConfig[]
default: [],
description: t('Columns to display'),
optionRenderer: c => <ColumnOption column={c} showType />,
- valueRenderer: c => <ColumnOption column={c} />,
valueKey: 'column_name',
- allowAll: true,
mapStateToProps: stateRef => ({
options: stateRef.datasource ? stateRef.datasource.columns : [],
}),
- commaChoosesOption: false,
freeForm: true,
} as ControlConfig<'SelectControl', ColumnMeta>,
},
diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx
b/superset-frontend/src/datasource/DatasourceEditor.jsx
index a848176..e502a3e 100644
--- a/superset-frontend/src/datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/datasource/DatasourceEditor.jsx
@@ -40,7 +40,7 @@ import { getClientErrorObject } from
'src/utils/getClientErrorObject';
import CheckboxControl from 'src/explore/components/controls/CheckboxControl';
import TextControl from 'src/explore/components/controls/TextControl';
-import SelectControl from 'src/explore/components/controls/SelectControl';
+import { Select } from 'src/components';
import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
import SelectAsyncControl from
'src/explore/components/controls/SelectAsyncControl';
import SpatialControl from 'src/explore/components/controls/SpatialControl';
@@ -121,7 +121,12 @@ const StyledLabelWrapper = styled.div`
const checkboxGenerator = (d, onChange) => (
<CheckboxControl value={d} onChange={onChange} />
);
-const DATA_TYPES = ['STRING', 'NUMERIC', 'DATETIME', 'BOOLEAN'];
+const DATA_TYPES = [
+ { value: 'STRING', label: 'STRING' },
+ { value: 'NUMERIC', label: 'NUMERIC' },
+ { value: 'DATETIME', label: 'DATETIME' },
+ { value: 'BOOLEAN', label: 'BOOLEAN' },
+];
const DATASOURCE_TYPES_ARR = [
{ key: 'physical', label: t('Physical (table or view)') },
@@ -207,7 +212,13 @@ function ColumnCollectionTable({
fieldKey="type"
label={t('Data type')}
control={
- <SelectControl choices={DATA_TYPES} name="type" freeForm />
+ <Select
+ ariaLabel={t('Data type')}
+ options={DATA_TYPES}
+ name="type"
+ allowNewOptions
+ allowClear
+ />
}
/>
)}
diff --git
a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
index 2042cb8..355b38d 100644
---
a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
+++
b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
@@ -70,8 +70,6 @@ const propTypes = {
addAnnotationLayer: PropTypes.func,
removeAnnotationLayer: PropTypes.func,
close: PropTypes.func,
-
- onPopoverClear: PropTypes.func,
};
const defaultProps = {
@@ -95,7 +93,6 @@ const defaultProps = {
addAnnotationLayer: () => {},
removeAnnotationLayer: () => {},
close: () => {},
- onPopoverClear: () => {},
};
export default class AnnotationLayer extends React.PureComponent {
@@ -172,7 +169,6 @@ export default class AnnotationLayer extends
React.PureComponent {
);
this.handleValue = this.handleValue.bind(this);
this.isValidForm = this.isValidForm.bind(this);
- this.popoverClearWrapper = this.popoverClearWrapper.bind(this);
}
componentDidMount() {
@@ -238,15 +234,6 @@ export default class AnnotationLayer extends
React.PureComponent {
return !errors.filter(x => x).length;
}
- popoverClearWrapper(value, actionMeta, callback) {
- if (callback) {
- callback(value);
- }
- if (actionMeta?.action === 'clear') {
- this.props.onPopoverClear(true);
- }
- }
-
handleAnnotationType(annotationType) {
this.setState({
annotationType,
@@ -266,7 +253,7 @@ export default class AnnotationLayer extends
React.PureComponent {
handleValue(value) {
this.setState({
value,
- descriptionColumns: null,
+ descriptionColumns: [],
intervalEndColumn: null,
timeColumn: null,
titleColumn: null,
@@ -409,6 +396,7 @@ export default class AnnotationLayer extends
React.PureComponent {
if (requiresQuery(sourceType)) {
return (
<SelectControl
+ ariaLabel={t('Annotation layer value')}
name="annotation-layer-value"
showHeader
hovered
@@ -418,9 +406,7 @@ export default class AnnotationLayer extends
React.PureComponent {
options={valueOptions}
isLoading={isLoadingOptions}
value={value}
- onChange={(value, _, actionMeta) =>
- this.popoverClearWrapper(value, actionMeta, this.handleValue)
- }
+ onChange={this.handleValue}
validationErrors={!value ? ['Mandatory'] : []}
optionRenderer={this.renderOption}
/>
@@ -479,14 +465,17 @@ export default class AnnotationLayer extends
React.PureComponent {
{(annotationType === ANNOTATION_TYPES.EVENT ||
annotationType === ANNOTATION_TYPES.INTERVAL) && (
<SelectControl
+ ariaLabel={t('Annotation layer time column')}
hovered
name="annotation-layer-time-column"
label={
annotationType === ANNOTATION_TYPES.INTERVAL
- ? 'Interval start column'
- : 'Event time column'
+ ? t('Interval start column')
+ : t('Event time column')
}
- description="This column must contain date/time information."
+ description={t(
+ 'This column must contain date/time information.',
+ )}
validationErrors={!timeColumn ? ['Mandatory'] : []}
clearable={false}
options={timeColumnOptions}
@@ -496,48 +485,42 @@ export default class AnnotationLayer extends
React.PureComponent {
)}
{annotationType === ANNOTATION_TYPES.INTERVAL && (
<SelectControl
+ ariaLabel={t('Annotation layer interval end')}
hovered
name="annotation-layer-intervalEnd"
- label="Interval End column"
- description="This column must contain date/time information."
+ label={t('Interval End column')}
+ description={t(
+ 'This column must contain date/time information.',
+ )}
validationErrors={!intervalEndColumn ? ['Mandatory'] : []}
options={columns}
value={intervalEndColumn}
- onChange={(value, _, actionMeta) =>
- this.popoverClearWrapper(value, actionMeta, v =>
- this.setState({ intervalEndColumn: v }),
- )
- }
+ onChange={value => this.setState({ intervalEndColumn: value })}
/>
)}
<SelectControl
+ ariaLabel={t('Annotation layer title column')}
hovered
name="annotation-layer-title"
- label="Title Column"
- description="Pick a title for you annotation."
+ label={t('Title Column')}
+ description={t('Pick a title for you annotation.')}
options={[{ value: '', label: 'None' }].concat(columns)}
value={titleColumn}
- onChange={(value, _, actionMeta) =>
- this.popoverClearWrapper(value, actionMeta, v =>
- this.setState({ titleColumn: v }),
- )
- }
+ onChange={value => this.setState({ titleColumn: value })}
/>
{annotationType !== ANNOTATION_TYPES.TIME_SERIES && (
<SelectControl
+ ariaLabel={t('Annotation layer description columns')}
hovered
name="annotation-layer-title"
- label="Description Columns"
- description={`Pick one or more columns that should be shown in
the
- annotation. If you don't select a column all of them will be
shown.`}
+ label={t('Description Columns')}
+ description={t(
+ "Pick one or more columns that should be shown in the
annotation. If you don't select a column all of them will be shown.",
+ )}
multi
options={columns}
value={descriptionColumns}
- onChange={(value, _, actionMeta) =>
- this.popoverClearWrapper(value, actionMeta, v =>
- this.setState({ descriptionColumns: v }),
- )
- }
+ onChange={value => this.setState({ descriptionColumns: value
})}
/>
)}
<div style={{ marginTop: '1rem' }}>
@@ -629,6 +612,7 @@ export default class AnnotationLayer extends
React.PureComponent {
info={t('Configure your how you overlay is displayed here.')}
>
<SelectControl
+ ariaLabel={t('Annotation layer stroke')}
name="annotation-layer-stroke"
label={t('Style')}
// see '../../../visualizations/nvd3_vis.css'
@@ -643,6 +627,7 @@ export default class AnnotationLayer extends
React.PureComponent {
onChange={v => this.setState({ style: v })}
/>
<SelectControl
+ ariaLabel={t('Annotation layer opacity')}
name="annotation-layer-opacity"
label={t('Opacity')}
// see '../../../visualizations/nvd3_vis.css'
@@ -653,11 +638,7 @@ export default class AnnotationLayer extends
React.PureComponent {
{ value: 'opacityHigh', label: '0.8' },
]}
value={opacity}
- onChange={(value, _, actionMeta) =>
- this.popoverClearWrapper(value, actionMeta, v =>
- this.setState({ opacity: v }),
- )
- }
+ onChange={value => this.setState({ opacity: value })}
/>
<div>
<ControlHeader label={t('Color')} />
@@ -746,6 +727,7 @@ export default class AnnotationLayer extends
React.PureComponent {
onChange={v => this.setState({ show: !v })}
/>
<SelectControl
+ ariaLabel={t('Annotation layer type')}
hovered
description={t('Choose the annotation layer type')}
label={t('Annotation layer type')}
@@ -757,19 +739,14 @@ export default class AnnotationLayer extends
React.PureComponent {
/>
{supportedSourceTypes.length > 0 && (
<SelectControl
+ ariaLabel={t('Annotation source type')}
hovered
- description="Choose the source of your annotations"
- label="Annotation Source"
+ description={t('Choose the source of your annotations')}
+ label={t('Annotation Source')}
name="annotation-source-type"
options={supportedSourceTypes}
value={sourceType}
- onChange={(value, _, actionMeta) =>
- this.popoverClearWrapper(
- value,
- actionMeta,
- this.handleAnnotationSourceType,
- )
- }
+ onChange={this.handleAnnotationSourceType}
validationErrors={!sourceType ? [t('Mandatory')] : []}
/>
)}
diff --git
a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
index beec955..ed8ae44 100644
---
a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
+++
b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
@@ -84,10 +84,22 @@ test('renders extra checkboxes when type is time series',
async () => {
});
test('enables apply and ok buttons', async () => {
- await waitForRender();
- userEvent.type(screen.getByLabelText('Name'), 'Test');
- userEvent.type(screen.getByLabelText('Formula'), '2x');
- await waitFor(() => {
+ const { container } = render(<AnnotationLayer {...defaultProps} />);
+
+ waitFor(() => {
+ expect(container).toBeInTheDocument();
+ });
+
+ const nameInput = screen.getByRole('textbox', { name: 'Name' });
+ const formulaInput = screen.getByRole('textbox', { name: 'Formula' });
+
+ expect(nameInput).toBeInTheDocument();
+ expect(formulaInput).toBeInTheDocument();
+
+ userEvent.type(nameInput, 'Name');
+ userEvent.type(formulaInput, '2x');
+
+ waitFor(() => {
expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
});
@@ -134,12 +146,17 @@ test('renders chart options', async () => {
await waitForRender({
annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
});
- userEvent.click(screen.getByText('2 option(s)'));
- userEvent.click(screen.getByText('Superset annotation'));
- expect(await screen.findByLabelText('Annotation layer')).toBeInTheDocument();
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation source type' }),
+ );
userEvent.click(screen.getByText('Superset annotation'));
+ expect(screen.getByText('Annotation layer')).toBeInTheDocument();
+
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation source type' }),
+ );
userEvent.click(screen.getByText('Table'));
- expect(await screen.findByLabelText('Chart')).toBeInTheDocument();
+ expect(screen.getByText('Chart')).toBeInTheDocument();
});
test('keeps apply disabled when missing required fields', async () => {
@@ -147,18 +164,28 @@ test('keeps apply disabled when missing required fields',
async () => {
annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
sourceType: 'Table',
});
- userEvent.click(screen.getByText('1 option(s)'));
- await waitFor(() => userEvent.click(screen.getByText('Chart A')));
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation layer value' }),
+ );
+ userEvent.click(await screen.findByText('Chart A'));
expect(
screen.getByText('Annotation Slice Configuration'),
).toBeInTheDocument();
userEvent.click(screen.getByRole('button', { name: 'Automatic Color' }));
- userEvent.click(screen.getByLabelText('Title Column'));
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation layer title column' }),
+ );
userEvent.click(screen.getByText('None'));
- userEvent.click(screen.getByLabelText('Style'));
+ userEvent.click(screen.getByText('Style'));
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation layer stroke' }),
+ );
userEvent.click(screen.getByText('Dashed'));
- userEvent.click(screen.getByLabelText('Opacity'));
+ userEvent.click(screen.getByText('Opacity'));
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation layer opacity' }),
+ );
userEvent.click(screen.getByText('0.5'));
const checkboxes = screen.getAllByRole('checkbox');
diff --git
a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx
b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx
index c94fe88..dc0130f 100644
---
a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx
+++
b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx
@@ -62,12 +62,10 @@ class AnnotationLayerControl extends React.PureComponent {
this.state = {
popoverVisible: {},
addedAnnotationIndex: null,
- popoverClear: false,
};
this.addAnnotationLayer = this.addAnnotationLayer.bind(this);
this.removeAnnotationLayer = this.removeAnnotationLayer.bind(this);
this.handleVisibleChange = this.handleVisibleChange.bind(this);
- this.handlePopoverClear = this.handlePopoverClear.bind(this);
}
componentDidMount() {
@@ -105,19 +103,9 @@ class AnnotationLayerControl extends React.PureComponent {
}
handleVisibleChange(visible, popoverKey) {
- if (!this.state.popoverClear) {
- this.setState(prevState => ({
- popoverVisible: { ...prevState.popoverVisible, [popoverKey]: visible },
- }));
- } else {
- this.handlePopoverClear(false);
- }
- }
-
- handlePopoverClear(popoverClear) {
- this.setState({
- popoverClear,
- });
+ this.setState(prevState => ({
+ popoverVisible: { ...prevState.popoverVisible, [popoverKey]: visible },
+ }));
}
removeAnnotationLayer(annotation) {
@@ -143,7 +131,6 @@ class AnnotationLayerControl extends React.PureComponent {
this.handleVisibleChange(false, popoverKey);
this.setState({ addedAnnotationIndex: null });
}}
- onPopoverClear={this.handlePopoverClear}
/>
</div>
);
diff --git
a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx
b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx
index e04ce8b..25aa79f 100644
---
a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx
+++
b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx
@@ -23,7 +23,7 @@ import { InfoTooltipWithTrigger } from
'@superset-ui/chart-controls';
import Popover from 'src/components/Popover';
import FormRow from 'src/components/FormRow';
-import SelectControl from 'src/explore/components/controls/SelectControl';
+import { Select } from 'src/components';
import CheckboxControl from 'src/explore/components/controls/CheckboxControl';
import TextControl from 'src/explore/components/controls/TextControl';
import { FILTER_CONFIG_ATTRIBUTES } from 'src/explore/constants';
@@ -136,12 +136,12 @@ export default class FilterBoxItemControl extends
React.Component {
<FormRow
label={t('Column')}
control={
- <SelectControl
+ <Select
+ ariaLabel={t('Column')}
value={this.state.column}
name="column"
- clearable={false}
options={this.props.datasource.columns
- .filter(col => col !== this.state.column)
+ .filter(col => col.column_name !== this.state.column)
.map(col => ({
value: col.column_name,
label: col.column_name,
@@ -184,11 +184,12 @@ export default class FilterBoxItemControl extends
React.Component {
label={t('Sort metric')}
tooltip={t('Metric to sort the results by')}
control={
- <SelectControl
+ <Select
+ ariaLabel={t('Sort metric')}
value={this.state.metric}
name="column"
options={this.props.datasource.metrics
- .filter(metric => metric !== this.state.metric)
+ .filter(m => m.metric_name !== this.state.metric)
.map(m => ({
value: m.metric_name,
label: m.metric_name,
diff --git
a/superset-frontend/src/explore/components/controls/SelectControl.jsx
b/superset-frontend/src/explore/components/controls/SelectControl.jsx
index f14c748..4342485 100644
--- a/superset-frontend/src/explore/components/controls/SelectControl.jsx
+++ b/superset-frontend/src/explore/components/controls/SelectControl.jsx
@@ -18,11 +18,12 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
-import { t, css } from '@superset-ui/core';
-import { Select, CreatableSelect, OnPasteSelect } from 'src/components/Select';
+import { css, t } from '@superset-ui/core';
+import { Select } from 'src/components';
import ControlHeader from 'src/explore/components/ControlHeader';
const propTypes = {
+ ariaLabel: PropTypes.string,
autoFocus: PropTypes.bool,
choices: PropTypes.array,
clearable: PropTypes.bool,
@@ -30,10 +31,8 @@ const propTypes = {
disabled: PropTypes.bool,
freeForm: PropTypes.bool,
isLoading: PropTypes.bool,
- label: PropTypes.string,
multi: PropTypes.bool,
isMulti: PropTypes.bool,
- allowAll: PropTypes.bool,
name: PropTypes.string.isRequired,
onChange: PropTypes.func,
onFocus: PropTypes.func,
@@ -42,21 +41,29 @@ const propTypes = {
PropTypes.number,
PropTypes.array,
]),
+ default: PropTypes.oneOfType([
+ PropTypes.string,
+ PropTypes.number,
+ PropTypes.array,
+ ]),
showHeader: PropTypes.bool,
optionRenderer: PropTypes.func,
- valueRenderer: PropTypes.func,
valueKey: PropTypes.string,
options: PropTypes.array,
placeholder: PropTypes.string,
- noResultsText: PropTypes.string,
- selectRef: PropTypes.func,
filterOption: PropTypes.func,
- promptTextCreator: PropTypes.func,
- commaChoosesOption: PropTypes.bool,
- menuPortalTarget: PropTypes.element,
- menuPosition: PropTypes.string,
- menuPlacement: PropTypes.string,
- forceOverflow: PropTypes.bool,
+
+ // ControlHeader props
+ label: PropTypes.string,
+ renderTrigger: PropTypes.bool,
+ validationErrors: PropTypes.array,
+ rightNode: PropTypes.node,
+ leftNode: PropTypes.node,
+ onClick: PropTypes.func,
+ hovered: PropTypes.bool,
+ tooltipOnClick: PropTypes.func,
+ warning: PropTypes.string,
+ danger: PropTypes.string,
};
const defaultProps = {
@@ -73,10 +80,6 @@ const defaultProps = {
onFocus: () => {},
showHeader: true,
valueKey: 'value',
- noResultsText: t('No results found'),
- promptTextCreator: label => `Create Option ${label}`,
- commaChoosesOption: true,
- allowAll: false,
};
export default class SelectControl extends React.PureComponent {
@@ -84,13 +87,9 @@ export default class SelectControl extends
React.PureComponent {
super(props);
this.state = {
options: this.getOptions(props),
- value: this.props.value,
};
this.onChange = this.onChange.bind(this);
- this.createMetaSelectAllOption = this.createMetaSelectAllOption.bind(this);
- this.select = null; // pointer to the react-select instance
- this.getSelectRef = this.getSelectRef.bind(this);
- this.handleKeyDownForCreate = this.handleKeyDownForCreate.bind(this);
+ this.handleFilterOptions = this.handleFilterOptions.bind(this);
}
UNSAFE_componentWillReceiveProps(nextProps) {
@@ -105,194 +104,130 @@ export default class SelectControl extends
React.PureComponent {
// Beware: This is acting like an on-click instead of an on-change
// (firing every time user chooses vs firing only if a new option is chosen).
- onChange(opt, actionMeta) {
- let optionValue = this.props.multi ? [] : null;
- if (opt) {
- if (this.props.multi) {
- opt.forEach(o => {
- // select all options
- if (o.meta === true) {
- optionValue = this.getOptions(this.props)
- .filter(x => !x.meta)
- .map(x => x[this.props.valueKey]);
- return;
- }
- optionValue.push(o[this.props.valueKey] || o);
- });
- } else if (opt.meta === true) {
- return;
- } else {
- optionValue = opt[this.props.valueKey];
- }
- }
+ onChange(val) {
// will eventually call `exploreReducer`: SET_FIELD_VALUE
- this.props.onChange(optionValue, [], actionMeta);
- }
+ const { valueKey } = this.props;
+ let onChangeVal = val;
- getSelectRef(instance) {
- this.select = instance;
- if (this.props.selectRef) {
- this.props.selectRef(instance);
+ if (Array.isArray(val)) {
+ const values = val.map(v => v?.[valueKey] || v);
+ onChangeVal = values;
+ }
+ if (typeof val === 'object' && val?.[valueKey]) {
+ onChangeVal = val[valueKey];
}
+ this.props.onChange(onChangeVal, []);
}
getOptions(props) {
+ const { choices, optionRenderer, valueKey } = props;
let options = [];
if (props.options) {
- options = props.options.map(x => x);
- } else if (props.choices) {
+ options = props.options.map(o => ({
+ ...o,
+ value: o[valueKey],
+ label: o.label || o[valueKey],
+ customLabel: optionRenderer ? optionRenderer(o) : undefined,
+ }));
+ } else if (choices) {
// Accepts different formats of input
- options = props.choices.map(c => {
+ options = choices.map(c => {
if (Array.isArray(c)) {
const [value, label] = c.length > 1 ? c : [c[0], c[0]];
- return { label, [props.valueKey]: value };
+ return {
+ value,
+ label,
+ };
}
if (Object.is(c)) {
- return c;
+ return {
+ ...c,
+ value: c[valueKey],
+ label: c.label || c[valueKey],
+ };
}
- return { label: c, [props.valueKey]: c };
+ return { value: c, label: c };
});
}
- // For FreeFormSelect, insert newly created values into options
- if (props.freeForm && props.value) {
- const existingOptionValues = new Set(options.map(c =>
c[props.valueKey]));
- const selectedValues = Array.isArray(props.value)
- ? props.value
- : [props.value];
- selectedValues.forEach(v => {
- if (!existingOptionValues.has(v)) {
- // place the newly created options at the top
- options.unshift({ label: v, [props.valueKey]: v });
- }
- });
- }
- if (props.allowAll === true && props.multi === true) {
- if (!this.optionsIncludesSelectAll(options)) {
- options.unshift(this.createMetaSelectAllOption());
- }
- } else {
- options = options.filter(o => !this.isMetaSelectAllOption(o));
- }
return options;
}
- handleKeyDownForCreate(event) {
- const { key } = event;
- if (key === 'Tab' || (this.props.commaChoosesOption && key === ',')) {
- // simulate an Enter event
- if (this.select) {
- this.select.onKeyDown({ ...event, key: 'Enter' });
- }
- }
- }
-
- isMetaSelectAllOption(o) {
- return o.meta && o.meta === true && o.label === 'Select all';
- }
-
- optionsIncludesSelectAll(o) {
- return o.findIndex(o => this.isMetaSelectAllOption(o)) >= 0;
- }
-
- optionsRemaining() {
- const { options } = this.state;
- const { value } = this.props;
- // if select is multi/value is array, we show the options not selected
- let remainingOptions = Array.isArray(value)
- ? options.length - value.length
- : options.length;
- if (this.optionsIncludesSelectAll(options)) {
- remainingOptions -= 1;
- }
- return remainingOptions < 0 ? 0 : remainingOptions;
- }
-
- createMetaSelectAllOption() {
- const option = { label: 'Select all', meta: true };
- option[this.props.valueKey] = 'Select all';
- return option;
+ handleFilterOptions(text, option) {
+ const { filterOption } = this.props;
+ return filterOption({ data: option }, text);
}
render() {
- // Tab, comma or Enter will trigger a new option created for
FreeFormSelect
const {
+ ariaLabel,
autoFocus,
clearable,
disabled,
filterOption,
+ freeForm,
isLoading,
+ isMulti,
label,
- menuPlacement,
+ multi,
name,
- noResultsText,
+ placeholder,
onFocus,
optionRenderer,
- promptTextCreator,
+ showHeader,
value,
- valueKey,
- valueRenderer,
- forceOverflow,
- menuPortalTarget,
- menuPosition,
+ // ControlHeader props
+ description,
+ renderTrigger,
+ rightNode,
+ leftNode,
+ validationErrors,
+ onClick,
+ hovered,
+ tooltipOnClick,
+ warning,
+ danger,
} = this.props;
- const optionsRemaining = this.optionsRemaining();
- const optionRemaingText = optionsRemaining
- ? t('%s option(s)', optionsRemaining)
- : '';
- const placeholder = this.props.placeholder || optionRemaingText;
- const isMulti = this.props.isMulti || this.props.multi;
-
- let assistiveText;
- if (
- isMulti &&
- optionsRemaining &&
- Array.isArray(this.state.value) &&
- Array.isArray(value) &&
- !!value.length
- ) {
- assistiveText = optionRemaingText;
- }
+ const headerProps = {
+ name,
+ label,
+ description,
+ renderTrigger,
+ rightNode,
+ leftNode,
+ validationErrors,
+ onClick,
+ hovered,
+ tooltipOnClick,
+ warning,
+ danger,
+ };
const selectProps = {
+ allowNewOptions: freeForm,
autoFocus,
- 'aria-label': label,
- clearable,
+ ariaLabel:
+ ariaLabel || (typeof label === 'string' ? label : t('Select ...')),
+ allowClear: clearable,
disabled,
- filterOption,
- ignoreAccents: false,
- isLoading,
- isMulti,
- labelKey: 'label',
- menuPlacement,
- forceOverflow,
- menuPortalTarget,
- menuPosition,
+ filterOption:
+ filterOption && typeof filterOption === 'function'
+ ? this.handleFilterOptions
+ : true,
+ header: showHeader && <ControlHeader {...headerProps} />,
+ loading: isLoading,
+ mode: isMulti || multi ? 'multiple' : 'single',
name: `select-${name}`,
- noResultsText,
onChange: this.onChange,
onFocus,
optionRenderer,
- value,
options: this.state.options,
placeholder,
- assistiveText,
- promptTextCreator,
- selectRef: this.getSelectRef,
- valueKey,
- valueRenderer,
+ value:
+ value ||
+ (this.props.default !== undefined ? this.props.default : undefined),
};
- let SelectComponent;
- if (this.props.freeForm) {
- SelectComponent = CreatableSelect;
- // Don't create functions in `render` because React relies on shallow
- // compare to decide weathere to rerender child components.
- selectProps.onKeyDown = this.handleKeyDownForCreate;
- } else {
- SelectComponent = Select;
- }
-
return (
<div
css={theme => css`
@@ -307,12 +242,7 @@ export default class SelectControl extends
React.PureComponent {
}
`}
>
- {this.props.showHeader && <ControlHeader {...this.props} />}
- {isMulti ? (
- <OnPasteSelect {...selectProps} selectWrap={SelectComponent} />
- ) : (
- <SelectComponent {...selectProps} />
- )}
+ <Select {...selectProps} />
</div>
);
}
diff --git
a/superset-frontend/src/explore/components/controls/SpatialControl.jsx
b/superset-frontend/src/explore/components/controls/SpatialControl.jsx
index cf9ae6b..23456db 100644
--- a/superset-frontend/src/explore/components/controls/SpatialControl.jsx
+++ b/superset-frontend/src/explore/components/controls/SpatialControl.jsx
@@ -134,6 +134,7 @@ export default class SpatialControl extends React.Component
{
renderSelect(name, type) {
return (
<SelectControl
+ ariaLabel={name}
name={name}
choices={this.props.choices}
value={this.state[name]}
diff --git a/superset-frontend/src/explore/controls.jsx
b/superset-frontend/src/explore/controls.jsx
index e54f7ab..f28a197 100644
--- a/superset-frontend/src/explore/controls.jsx
+++ b/superset-frontend/src/explore/controls.jsx
@@ -125,15 +125,12 @@ const groupByControl = {
includeTime: false,
description: t('One or many controls to group by'),
optionRenderer: c => <StyledColumnOption column={c} showType />,
- valueRenderer: c => <StyledColumnOption column={c} />,
valueKey: 'column_name',
- allowAll: true,
filterOption: ({ data: opt }, text) =>
(opt.column_name &&
opt.column_name.toLowerCase().indexOf(text.toLowerCase()) >= 0) ||
(opt.verbose_name &&
opt.verbose_name.toLowerCase().indexOf(text.toLowerCase()) >= 0),
- promptTextCreator: label => label,
mapStateToProps: (state, control) => {
const newState = {};
if (state.datasource) {
@@ -144,7 +141,6 @@ const groupByControl = {
}
return newState;
},
- commaChoosesOption: false,
};
const metrics = {
@@ -266,7 +262,7 @@ export const controls = {
type: 'SelectControl',
freeForm: true,
label: TIME_FILTER_LABELS.granularity,
- default: 'one day',
+ default: 'P1D',
choices: [
[null, 'all'],
['PT5S', '5 seconds'],
@@ -304,7 +300,6 @@ export const controls = {
),
clearable: false,
optionRenderer: c => <StyledColumnOption column={c} showType />,
- valueRenderer: c => <StyledColumnOption column={c} />,
valueKey: 'column_name',
mapStateToProps: state => {
const props = {};
