chore: Select component refactoring - SelectControl - Iteration 5 (ap…

…ache#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 <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Revert changes to test * Call onPopoverClear when v value is undefined Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
opus-42 · Nov 14, 2021 · 8616060 · 8616060
1 parent ae661db
commit 8616060
Show file tree

Hide file tree

Showing 14 changed files with 223 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+                  />
                 }
               />
             )}