QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-04-18 15:44:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: upgrade react-select and make multi-select sortable (#9628)

Browse Source 
* feat: upgrade react-select v1.3.0 to v3.1.0

Upgrade `react-select`, replace `react-virtualized-select` with a custom
solution implemented with `react-window`.

Future plans include deprecate `react-virtualized` used in other places, too.

Migrate all react-select related components to `src/Components/Select`.

* Fix new list view

* Fix tests

* Address PR comments

* Fix a flacky Cypress test

* Adjust styles for Select in CRUD ListView

* Fix loadOptions for owners select in chart PropertiesModal

TODO: add typing support for AsyncSelect props.

* Address PR comments; allow isMulti in SelectControl, too

* Clean up NaN in table filter values

* Fix flacky test
This commit is contained in:
Jesse Yang
2020-05-19 16:59:49 -07:00
committed by GitHub
parent 68832d2fa5
commit 81ab8dd8b4
97 changed files with 2027 additions and 1234 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx
View File

@@ -21,6 +21,7 @@ import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import OnPasteSelect from 'src/components/Select/OnPasteSelect';
import AdhocFilter, {
EXPRESSION_TYPES,
CLAUSES,
@@ -28,7 +29,6 @@ import AdhocFilter, {
import AdhocFilterControl from 'src/explore/components/controls/AdhocFilterControl';
import AdhocMetric from 'src/explore/AdhocMetric';
import { AGGREGATES, OPERATORS } from 'src/explore/constants';
import OnPasteSelect from 'src/components/OnPasteSelect';
const simpleAdhocFilter = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,

73
superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx
View File

@@ -95,58 +95,50 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => {
.instance()
.onSubjectChange({ type: 'VARCHAR(255)', column_name: 'source' });
expect(onChange.calledOnce).toBe(true);
expect(
onChange.lastCall.args[0].equals(
simpleAdhocFilter.duplicateWith({ subject: 'source' }),
),
).toBe(true);
expect(onChange.lastCall.args[0]).toEqual(
simpleAdhocFilter.duplicateWith({ subject: 'source' }),
);
});
it('may alter the clause in onSubjectChange if the old clause is not appropriate', () => {
const { wrapper, onChange } = setup();
wrapper.instance().onSubjectChange(sumValueAdhocMetric);
expect(onChange.calledOnce).toBe(true);
expect(
onChange.lastCall.args[0].equals(
simpleAdhocFilter.duplicateWith({
subject: sumValueAdhocMetric.label,
clause: CLAUSES.HAVING,
}),
),
).toBe(true);
expect(onChange.lastCall.args[0]).toEqual(
simpleAdhocFilter.duplicateWith({
subject: sumValueAdhocMetric.label,
clause: CLAUSES.HAVING,
}),
);
});
it('will convert from individual comparator to array if the operator changes to multi', () => {
const { wrapper, onChange } = setup();
wrapper.instance().onOperatorChange({ operator: 'in' });
wrapper.instance().onOperatorChange('in');
expect(onChange.calledOnce).toBe(true);
expect(onChange.lastCall.args[0].comparator).toHaveLength(1);
expect(onChange.lastCall.args[0].comparator[0]).toBe('10');
expect(onChange.lastCall.args[0].operator).toBe('in');
expect(onChange.lastCall.args[0]).toEqual(
simpleAdhocFilter.duplicateWith({ operator: 'in', comparator: ['10'] }),
);
});
it('will convert from array to individual comparators if the operator changes from multi', () => {
const { wrapper, onChange } = setup({
adhocFilter: simpleMultiAdhocFilter,
});
wrapper.instance().onOperatorChange({ operator: '<' });
wrapper.instance().onOperatorChange('<');
expect(onChange.calledOnce).toBe(true);
expect(
onChange.lastCall.args[0].equals(
simpleAdhocFilter.duplicateWith({ operator: '<', comparator: '10' }),
),
).toBe(true);
expect(onChange.lastCall.args[0]).toEqual(
simpleMultiAdhocFilter.duplicateWith({ operator: '<', comparator: '10' }),
);
});
it('passes the new adhocFilter to onChange after onComparatorChange', () => {
const { wrapper, onChange } = setup();
wrapper.instance().onComparatorChange('20');
expect(onChange.calledOnce).toBe(true);
expect(
onChange.lastCall.args[0].equals(
simpleAdhocFilter.duplicateWith({ comparator: '20' }),
),
).toBe(true);
expect(onChange.lastCall.args[0]).toEqual(
simpleAdhocFilter.duplicateWith({ comparator: '20' }),
);
});
it('will filter operators for table datasources', () => {
@@ -195,20 +187,17 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => {
partitionColumn: 'ds',
});
wrapper.instance().onOperatorChange({ operator: 'LATEST PARTITION' });
expect(
onChange.lastCall.args[0].equals(
testAdhocFilter.duplicateWith({
subject: 'ds',
operator: 'LATEST PARTITION',
comparator: null,
clause: 'WHERE',
expressionType: 'SQL',
sqlExpression:
"ds = '{{ presto.latest_partition('schema.table1') }}' ",
}),
),
).toBe(true);
wrapper.instance().onOperatorChange('LATEST PARTITION');
expect(onChange.lastCall.args[0]).toEqual(
testAdhocFilter.duplicateWith({
subject: 'ds',
operator: 'LATEST PARTITION',
comparator: null,
clause: 'WHERE',
expressionType: 'SQL',
sqlExpression: "ds = '{{ presto.latest_partition('schema.table1') }}' ",
}),
);
});
it('expands when its multi comparator input field expands', () => {

10
superset-frontend/spec/javascripts/explore/components/AdhocFilterOption_spec.jsx
View File

@@ -52,7 +52,15 @@ function setup(overrides) {
describe('AdhocFilterOption', () => {
it('renders an overlay trigger wrapper for the label', () => {
const { wrapper } = setup();
expect(wrapper.find(OverlayTrigger)).toHaveLength(1);
const overlay = wrapper.find(OverlayTrigger);
expect(overlay).toHaveLength(1);
expect(overlay.props().defaultOverlayShown).toBe(false);
expect(wrapper.find(Label)).toHaveLength(1);
});
it('should open new filter popup by default', () => {
const { wrapper } = setup({
adhocFilter: simpleAdhocFilter.duplicateWith({ isNew: true }),
});
expect(wrapper.find(OverlayTrigger).props().defaultOverlayShown).toBe(true);
});
});

2
superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
View File

@@ -75,7 +75,7 @@ describe('AdhocMetricEditPopover', () => {
it('overwrites the adhocMetric in state with onAggregateChange', () => {
const { wrapper } = setup();
wrapper.instance().onAggregateChange({ aggregate: AGGREGATES.AVG });
wrapper.instance().onAggregateChange(AGGREGATES.AVG);
expect(wrapper.state('adhocMetric')).toEqual(
sumValueAdhocMetric.duplicateWith({ aggregate: AGGREGATES.AVG }),
);

7
superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx
View File

@@ -55,4 +55,11 @@ describe('AdhocMetricOption', () => {
expect(wrapper.find(OverlayTrigger)).toHaveLength(1);
expect(wrapper.find(Label)).toHaveLength(1);
});
it('overlay should open if metric is new', () => {
const { wrapper } = setup({
adhocMetric: sumValueAdhocMetric.duplicateWith({ isNew: true }),
});
expect(wrapper.find(OverlayTrigger).props().defaultOverlayShown).toBe(true);
});
});

2
superset-frontend/spec/javascripts/explore/components/ColorScheme_spec.jsx
View File

@@ -19,7 +19,7 @@
/* eslint-disable no-unused-expressions */
import React from 'react';
import { mount } from 'enzyme';
import { Creatable } from 'react-select';
import Creatable from 'react-select/creatable';
import { getCategoricalSchemeRegistry } from '@superset-ui/color';
import ColorSchemeControl from 'src/explore/components/controls/ColorSchemeControl';

2
superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx
View File

@@ -18,8 +18,8 @@
*/
import React from 'react';
import { mount } from 'enzyme';
import { DisplayQueryButton } from 'src/explore/components/DisplayQueryButton';
import ModalTrigger from 'src/components/ModalTrigger';
import { DisplayQueryButton } from 'src/explore/components/DisplayQueryButton';
describe('DisplayQueryButton', () => {
const defaultProps = {

129
superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx
View File

@@ -23,7 +23,7 @@ import { shallow } from 'enzyme';
import MetricsControl from 'src/explore/components/controls/MetricsControl';
import { AGGREGATES } from 'src/explore/constants';
import OnPasteSelect from 'src/components/OnPasteSelect';
import OnPasteSelect from 'src/components/Select/OnPasteSelect';
import AdhocMetric, { EXPRESSION_TYPES } from 'src/explore/AdhocMetric';
const defaultProps = {
@@ -138,11 +138,11 @@ describe('MetricsControl', () => {
expressionType: EXPRESSION_TYPES.SIMPLE,
column: { type: 'double', column_name: 'value' },
aggregate: AGGREGATES.SUM,
fromFormData: true,
label: 'SUM(value)',
hasCustomLabel: false,
optionName: 'blahblahblah',
sqlExpression: null,
isNew: false,
},
'avg__value',
]);
@@ -163,7 +163,8 @@ describe('MetricsControl', () => {
select.simulate('change', [valueColumn]);
const adhocMetric = onChange.lastCall.args[0][0];
expect(adhocMetric instanceof AdhocMetric).toBe(true);
expect(adhocMetric).toBeInstanceOf(AdhocMetric);
expect(adhocMetric.isNew).toBe(true);
expect(onChange.lastCall.args).toEqual([
[
{
@@ -171,35 +172,46 @@ describe('MetricsControl', () => {
column: valueColumn,
aggregate: AGGREGATES.SUM,
label: 'SUM(value)',
fromFormData: false,
hasCustomLabel: false,
optionName: adhocMetric.optionName,
sqlExpression: null,
isNew: true,
},
],
]);
});
it('handles aggregates being selected', () => {
it('handles aggregates being selected', done => {
const { wrapper, onChange } = setup();
const select = wrapper.find(OnPasteSelect);
// mock out the Select ref
const setInputSpy = sinon.spy();
const handleInputSpy = sinon.spy();
wrapper.instance().select = {
setInputValue: setInputSpy,
handleInputChange: handleInputSpy,
input: { input: {} },
};
const instance = wrapper.instance();
const handleInputChangeSpy = jest.fn();
const focusInputSpy = jest.fn();
// simulate react-select StateManager
instance.selectRef({
select: {
handleInputChange: handleInputChangeSpy,
inputRef: { value: '' },
focusInput: focusInputSpy,
},
});
select.simulate('change', [{ aggregate_name: 'SUM', optionName: 'SUM' }]);
expect(setInputSpy.calledWith('SUM()')).toBe(true);
expect(handleInputSpy.calledWith({ target: { value: 'SUM()' } })).toBe(
true,
);
expect(onChange.lastCall.args).toEqual([[]]);
expect(instance.select.inputRef.value).toBe('SUM()');
expect(handleInputChangeSpy).toHaveBeenCalledWith({
currentTarget: { value: 'SUM()' },
});
expect(onChange.calledOnceWith([])).toBe(true);
expect(focusInputSpy).toHaveBeenCalledTimes(0);
setTimeout(() => {
expect(focusInputSpy).toHaveBeenCalledTimes(1);
expect(instance.select.inputRef.selectionStart).toBe(4);
expect(instance.select.inputRef.selectionEnd).toBe(4);
done();
});
});
it('preserves existing selected AdhocMetrics', () => {
@@ -255,9 +267,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'a_metric',
optionName: 'a_metric',
expression: 'SUM(FANCY(metric))',
data: {
metric_name: 'a_metric',
optionName: 'a_metric',
expression: 'SUM(FANCY(metric))',
},
},
'a',
),
@@ -270,9 +284,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'avg__metric',
optionName: 'avg__metric',
expression: 'AVG(metric)',
data: {
metric_name: 'avg__metric',
optionName: 'avg__metric',
expression: 'AVG(metric)',
},
},
'a',
),
@@ -285,9 +301,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
type: 'VARCHAR(255)',
column_name: 'source',
optionName: '_col_source',
data: {
type: 'VARCHAR(255)',
column_name: 'source',
optionName: '_col_source',
},
},
'sou',
),
@@ -297,7 +315,7 @@ describe('MetricsControl', () => {
!!wrapper
.instance()
.selectFilterOption(
{ aggregate_name: 'AVG', optionName: '_aggregate_AVG' },
{ data: { aggregate_name: 'AVG', optionName: '_aggregate_AVG' } },
'av',
),
).toBe(true);
@@ -309,9 +327,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'sum__num',
verbose_name: 'babies',
optionName: '_col_sum_num',
data: {
metric_name: 'sum__num',
verbose_name: 'babies',
optionName: '_col_sum_num',
},
},
'bab',
),
@@ -324,9 +344,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'avg__metric',
optionName: 'avg__metric',
expression: 'AVG(metric)',
data: {
metric_name: 'avg__metric',
optionName: 'avg__metric',
expression: 'AVG(metric)',
},
},
'a',
),
@@ -339,9 +361,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'my_fancy_sum_metric',
optionName: 'my_fancy_sum_metric',
expression: 'SUM(value)',
data: {
metric_name: 'my_fancy_sum_metric',
optionName: 'my_fancy_sum_metric',
expression: 'SUM(value)',
},
},
'sum',
),
@@ -354,9 +378,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'sum__value',
optionName: 'sum__value',
expression: 'SUM(value)',
data: {
metric_name: 'sum__value',
optionName: 'sum__value',
expression: 'SUM(value)',
},
},
'sum',
),
@@ -365,9 +391,11 @@ describe('MetricsControl', () => {
expect(
!!wrapper.instance().selectFilterOption(
{
metric_name: 'sum__value',
optionName: 'sum__value',
expression: 'SUM("table"."value")',
data: {
metric_name: 'sum__value',
optionName: 'sum__value',
expression: 'SUM("table"."value")',
},
},
'sum',
),
@@ -379,12 +407,12 @@ describe('MetricsControl', () => {
wrapper.setState({ aggregateInInput: true });
expect(
!!wrapper
.instance()
.selectFilterOption(
{ metric_name: 'metric', expression: 'SUM(FANCY(metric))' },
'SUM(',
),
!!wrapper.instance().selectFilterOption(
{
data: { metric_name: 'metric', expression: 'SUM(FANCY(metric))' },
},
'SUM(',
),
).toBe(false);
});
@@ -395,7 +423,10 @@ describe('MetricsControl', () => {
expect(
!!wrapper
.instance()
.selectFilterOption({ type: 'DOUBLE', column_name: 'value' }, 'SUM('),
.selectFilterOption(
{ data: { type: 'DOUBLE', column_name: 'value' } },
'SUM(',
),
).toBe(true);
});

81
superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
View File

@@ -18,12 +18,10 @@
*/
/* eslint-disable no-unused-expressions */
import React from 'react';
import Select, { Creatable } from 'react-select';
import VirtualizedSelect from 'react-virtualized-select';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import OnPasteSelect from 'src/components/OnPasteSelect';
import VirtualizedRendererWrap from 'src/components/VirtualizedRendererWrap';
import { Select, CreatableSelect } from 'src/components/Select';
import OnPasteSelect from 'src/components/Select/OnPasteSelect';
import SelectControl from 'src/explore/components/controls/SelectControl';
const defaultProps = {
@@ -47,16 +45,42 @@ describe('SelectControl', () => {
beforeEach(() => {
wrapper = shallow(<SelectControl {...defaultProps} />);
wrapper.setProps(defaultProps);
});
it('renders an OnPasteSelect', () => {
it('renders with Select by default', () => {
expect(wrapper.find(OnPasteSelect)).toHaveLength(0);
expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(1);
});
it('renders with OnPasteSelect when multi', () => {
wrapper.setProps({ multi: true });
expect(wrapper.find(OnPasteSelect)).toHaveLength(1);
expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(0);
});
it('calls onChange when toggled', () => {
it('renders with Creatable when freeForm', () => {
wrapper.setProps({ freeForm: true });
expect(wrapper.find(OnPasteSelect)).toHaveLength(0);
expect(wrapper.findWhere(x => x.type() === CreatableSelect)).toHaveLength(
1,
);
});
it('uses Select in onPasteSelect when freeForm=false', () => {
wrapper = shallow(<SelectControl {...defaultProps} multi />);
const select = wrapper.find(OnPasteSelect);
select.simulate('change', { value: 50 });
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 });
expect(defaultProps.onChange.calledWith(50)).toBe(true);
});
@@ -72,36 +96,10 @@ describe('SelectControl', () => {
onChange: sinon.spy(),
};
wrapper.setProps(selectAllProps);
const select = wrapper.find(OnPasteSelect);
select.simulate('change', [{ meta: true, value: 'Select All' }]);
wrapper.instance().onChange([{ meta: true, value: 'Select All' }]);
expect(selectAllProps.onChange.calledWith(expectedValues)).toBe(true);
});
it('passes VirtualizedSelect as selectWrap', () => {
const select = wrapper.find(OnPasteSelect);
expect(select.props().selectWrap).toBe(VirtualizedSelect);
});
it('passes Creatable as selectComponent when freeForm=true', () => {
wrapper = shallow(<SelectControl {...defaultProps} freeForm />);
const select = wrapper.find(OnPasteSelect);
expect(select.props().selectComponent).toBe(Creatable);
});
it('passes Select as selectComponent when freeForm=false', () => {
const select = wrapper.find(OnPasteSelect);
expect(select.props().selectComponent).toBe(Select);
});
it('wraps optionRenderer in a VirtualizedRendererWrap', () => {
const select = wrapper.find(OnPasteSelect);
const defaultOptionRenderer = SelectControl.defaultProps.optionRenderer;
const wrappedRenderer = VirtualizedRendererWrap(defaultOptionRenderer);
expect(typeof select.props().optionRenderer).toBe('function');
// different instances of wrapper with same inner renderer are unequal
expect(select.props().optionRenderer.name).toBe(wrappedRenderer.name);
});
describe('getOptions', () => {
it('returns the correct options', () => {
wrapper.setProps(defaultProps);
@@ -132,15 +130,18 @@ describe('SelectControl', () => {
value: ['one', 'two'],
name: 'row_limit',
label: 'Row Limit',
valueKey: 'value',
valueKey: 'custom_value_key',
onChange: sinon.spy(),
};
const newOptions = [
{ value: 'one', label: 'one' },
{ value: 'two', label: 'two' },
// 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(newOptions);
expect(wrapper.instance().getOptions(freeFormProps)).toEqual(
expectedNewOptions,
);
});
});