fix: keep placeholder in multivalue select when a value exists (apach…

…e#11181)
zhaowufu · Oct 13, 2020 · 31cc415 · 31cc415
1 parent d7eb1d4
commit 31cc415
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 25 deletions.
diff --git a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
@@ -19,7 +19,7 @@
 /* eslint-disable no-unused-expressions */
 import React from 'react';
 import sinon from 'sinon';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
 import { Select, CreatableSelect } from 'src/components/Select';
 import OnPasteSelect from 'src/components/Select/OnPasteSelect';
 import SelectControl from 'src/explore/components/controls/SelectControl';
@@ -47,25 +47,6 @@ describe('SelectControl', () => {
     wrapper = shallow(<SelectControl {...defaultProps} />);
   });
 
-  it('renders with Select by default', () => {
-    expect(wrapper.find(OnPasteSelect)).not.toExist();
-    expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(1);
-  });
-
-  it('renders with OnPasteSelect when multi', () => {
-    wrapper.setProps({ multi: true });
-    expect(wrapper.find(OnPasteSelect)).toExist();
-    expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(0);
-  });
-
-  it('renders with Creatable when freeForm', () => {
-    wrapper.setProps({ freeForm: true });
-    expect(wrapper.find(OnPasteSelect)).not.toExist();
-    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);
@@ -100,6 +81,52 @@ describe('SelectControl', () => {
     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);
+    });
+
+    it('renders with OnPasteSelect when multi', () => {
+      wrapper.setProps({ multi: true });
+      expect(wrapper.find(OnPasteSelect)).toExist();
+      expect(wrapper.findWhere(x => x.type() === Select)).toHaveLength(0);
+    });
+
+    it('renders with Creatable when freeForm', () => {
+      wrapper.setProps({ freeForm: true });
+      expect(wrapper.find(OnPasteSelect)).not.toExist();
+      expect(wrapper.findWhere(x => x.type() === CreatableSelect)).toHaveLength(
+        1,
+      );
+    });
+    describe('when select is multi', () => {
+      it('renders the placeholder when a selection has been made', () => {
+        wrapper = mount(
+          <SelectControl
+            {...defaultProps}
+            multi
+            value={50}
+            placeholder="add something"
+          />,
+        );
+        expect(wrapper.html()).toContain('add something');
+      });
+    });
+    describe('when select is single', () => {
+      it('does not render the placeholder when a selection has been made', () => {
+        wrapper = mount(
+          <SelectControl
+            {...defaultProps}
+            value={50}
+            placeholder="add something"
+          />,
+        );
+        expect(wrapper.html()).not.toContain('add something');
+      });
+    });
+  });
+
   describe('getOptions', () => {
     it('returns the correct options', () => {
       wrapper.setProps(defaultProps);

diff --git a/superset-frontend/src/components/Select/styles.tsx b/superset-frontend/src/components/Select/styles.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { CSSProperties } from 'react';
+import React, { CSSProperties, ComponentType } from 'react';
 import { css, SerializedStyles, ClassNames } from '@emotion/core';
 import { supersetTheme } from '@superset-ui/core';
 import {
@@ -241,9 +241,39 @@ export const DEFAULT_STYLES: PartialStylesConfig = {
   }),
 };
 
-const { ClearIndicator, DropdownIndicator, Option } = defaultComponents;
+const multiInputSpanStyle = css`
+  color: '#879399',
+  position: 'absolute',
+  top: '2px',
+  left: '4px',
+  width: '100vw',
+`;
 
-export const DEFAULT_COMPONENTS: SelectComponentsConfig<any> = {
+const { ClearIndicator, DropdownIndicator, Option, Input } = defaultComponents;
+
+type SelectComponentsType = SelectComponentsConfig<any> & {
+  Input: ComponentType<InputProps>;
+};
+
+// react-select is missing selectProps from their props type
+// so overwriting it here to avoid errors
+type InputProps = {
+  selectProps: {
+    isMulti: boolean;
+    value: {
+      length: number;
+    };
+    placeholder: string;
+  };
+  cx: (a: string | null, b: any, c: string) => string | void;
+  innerRef: (element: React.Ref<any>) => void;
+  isHidden: boolean;
+  isDisabled?: boolean | undefined;
+  className?: string | undefined;
+  getStyles: any;
+};
+
+export const DEFAULT_COMPONENTS: SelectComponentsType = {
   Option: ({ children, innerProps, data, ...props }) => (
     <ClassNames>
       {({ css }) => (
@@ -272,6 +302,14 @@ export const DEFAULT_COMPONENTS: SelectComponentsConfig<any> = {
       />
     </DropdownIndicator>
   ),
+  Input: (props: InputProps) => (
+    <div style={{ position: 'relative' }}>
+      <Input {...props} />
+      {!!(props.selectProps.isMulti && props.selectProps.value.length) && (
+        <span css={multiInputSpanStyle}>{props.selectProps.placeholder}</span>
+      )}
+    </div>
+  ),
 };
 
 export const VALUE_LABELED_STYLES: PartialStylesConfig = {

diff --git a/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx b/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx
@@ -267,7 +267,7 @@ export default class AdhocFilterControl extends React.Component {
         <OnPasteSelect
           isMulti
           name={`select-${this.props.name}`}
-          placeholder={t('choose a column or metric')}
+          placeholder={t('choose one or more column or metric')}
           options={this.state.options}
           value={this.state.values}
           labelKey="label"

diff --git a/superset-frontend/src/explore/components/controls/MetricsControl.jsx b/superset-frontend/src/explore/components/controls/MetricsControl.jsx
@@ -337,7 +337,11 @@ export default class MetricsControl extends React.PureComponent {
         <OnPasteSelect
           isMulti={this.props.multi}
           name={`select-${this.props.name}`}
-          placeholder={t('choose a column or aggregate function')}
+          placeholder={
+            this.props.multi
+              ? t('choose one or more column or aggregate function')
+              : t('choose a column or aggregate function')
+          }
           options={this.state.options}
           value={this.state.value}
           labelKey="label"