[incubator-superset] 33/34: fix: ColorSchemeControl should not use CreatableSelect (#10814)

[vi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

villebro pushed a commit to branch 0.38
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit d61ffb2523ffe5097b58f9dd25a9f7ca73b18a46
Author: Jesse Yang <je...@airbnb.com>
AuthorDate: Tue Sep 8 18:38:50 2020 -0700

    fix: ColorSchemeControl should not use CreatableSelect (#10814)
    
    * fix: ColorSchemeControl should not be CreatableSelect
    
       Currently if you type to search in ColorSchemeControl it crashes the
    whole page.
    
    * Make it possible to filter by label
    
    * Fix ColorSchemeControl unit test
---
 .../explore/visualizations/line.test.ts            | 16 +++++++++++--
 .../explore/components/ColorScheme_spec.jsx        |  6 +++--
 .../components/controls/ColorSchemeControl.jsx     | 26 ++++++++++++----------
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
index 5c64007..34f80f2 100644
--- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
@@ -46,12 +46,24 @@ describe('Visualization > Line', () => {
   });
 
   it('should allow negative values in Y bounds', () => {
-    cy.get('#controlSections-tab-display').click().wait(1000);
+    cy.get('#controlSections-tab-display').click();
     cy.get('span').contains('Y Axis Bounds').scrollIntoView();
-    cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 }).wait(1000);
+    cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 });
     cy.get('.alert-warning').should('not.exist');
   });
 
+  it('should allow type to search color schemes', () => {
+    cy.get('#controlSections-tab-display').click();
+    cy.get('.Control[data-test="color_scheme"]').scrollIntoView();
+    cy.get('.Control[data-test="color_scheme"] input[type="text"]')
+      .focus()
+      .type('air{enter}');
+    cy.get('input[name="select-color_scheme"]').should(
+      'have.value',
+      'bnbColors',
+    );
+  });
+
   it('should work with adhoc metric', () => {
     const formData = { ...LINE_CHART_DEFAULTS, metrics: [NUM_METRIC] };
     cy.visitChartByParams(JSON.stringify(formData));
diff --git a/superset-frontend/spec/javascripts/explore/components/ColorScheme_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ColorScheme_spec.jsx
index fcd0f9d..b778519 100644
--- a/superset-frontend/spec/javascripts/explore/components/ColorScheme_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/ColorScheme_spec.jsx
@@ -19,12 +19,14 @@
 /* eslint-disable no-unused-expressions */
 import React from 'react';
 import { mount } from 'enzyme';
-import Creatable from 'react-select/creatable';
+import { Select } from 'src/components/Select';
 import { getCategoricalSchemeRegistry } from '@superset-ui/color';
 
 import ColorSchemeControl from 'src/explore/components/controls/ColorSchemeControl';
 
 const defaultProps = {
+  name: 'color_scheme',
+  label: 'Color Scheme',
   options: getCategoricalSchemeRegistry()
     .keys()
     .map(s => [s, s]),
@@ -37,6 +39,6 @@ describe('ColorSchemeControl', () => {
   });
 
   it('renders a Creatable', () => {
-    expect(wrapper.find(Creatable)).toExist();
+    expect(wrapper.find(Select)).toExist();
   });
 });
diff --git a/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx b/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx
index 428cfe7..60f91ac 100644
--- a/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx
+++ b/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx
@@ -19,7 +19,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { isFunction } from 'lodash';
-import { CreatableSelect } from 'src/components/Select';
+import { Select } from 'src/components/Select';
 import ControlHeader from '../ControlHeader';
 import TooltipWrapper from '../../../components/TooltipWrapper';
 import './ColorSchemeControl.less';
@@ -53,7 +53,6 @@ export default class ColorSchemeControl extends React.PureComponent {
     this.state = {
       scheme: this.props.value,
     };
-
     this.onChange = this.onChange.bind(this);
     this.renderOption = this.renderOption.bind(this);
   }
@@ -65,9 +64,8 @@ export default class ColorSchemeControl extends React.PureComponent {
   }
 
   renderOption(key) {
-    const { isLinear, schemes } = this.props;
-    const schemeLookup = isFunction(schemes) ? schemes() : schemes;
-    const currentScheme = schemeLookup[key.value || defaultProps.value];
+    const { isLinear } = this.props;
+    const currentScheme = this.schemes[key.value];
 
     // For categorical scheme, display all the colors
     // For sequential scheme, show 10 or interpolate to 10.
@@ -100,12 +98,16 @@ export default class ColorSchemeControl extends React.PureComponent {
   }
 
   render() {
-    const { choices } = this.props;
-    const options = (isFunction(choices) ? choices() : choices).map(choice => ({
-      value: choice[0],
-      label: choice[1],
-    }));
-
+    const { schemes, choices } = this.props;
+    // save parsed schemes for later
+    this.schemes = isFunction(schemes) ? schemes() : schemes;
+    const options = (isFunction(choices) ? choices() : choices).map(
+      ([value, label]) => ({
+        value,
+        // use scheme label if available
+        label: this.schemes[value]?.label || label,
+      }),
+    );
     const selectProps = {
       multi: false,
       name: `select-${this.props.name}`,
@@ -122,7 +124,7 @@ export default class ColorSchemeControl extends React.PureComponent {
     return (
       <div>
         <ControlHeader {...this.props} />
-        <CreatableSelect {...selectProps} />
+        <Select {...selectProps} />
       </div>
     );
   }