This is an automated email from the ASF dual-hosted git repository. erikrit pushed a commit to branch revert-11181-elizabeth/multiple-filter-options in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit 5d4e7c0865f30b2c2d3c45c698910faa3c60967a Author: Erik Ritter <[email protected]> AuthorDate: Wed Oct 14 10:40:12 2020 -0700 Revert "fix: keep placeholder in multivalue select when a value exists (#11181)" This reverts commit 31cc4155b7e195e6aee1874a68577e0d55d10d77. --- .../explore/components/SelectControl_spec.jsx | 67 +++++++--------------- superset-frontend/src/components/Select/styles.tsx | 44 +------------- .../components/controls/AdhocFilterControl.jsx | 2 +- .../explore/components/controls/MetricsControl.jsx | 6 +- 4 files changed, 25 insertions(+), 94 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx index 76ca694..5409da2 100644 --- 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, mount } from 'enzyme'; +import { shallow } 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,6 +47,25 @@ 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); @@ -81,52 +100,6 @@ 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 index 73436f7..e5f844a 100644 --- 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, ComponentType } from 'react'; +import React, { CSSProperties } from 'react'; import { css, SerializedStyles, ClassNames } from '@emotion/core'; import { supersetTheme } from '@superset-ui/core'; import { @@ -241,39 +241,9 @@ export const DEFAULT_STYLES: PartialStylesConfig = { }), }; -const multiInputSpanStyle = css` - color: '#879399', - position: 'absolute', - top: '2px', - left: '4px', - width: '100vw', -`; +const { ClearIndicator, DropdownIndicator, Option } = defaultComponents; -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 = { +export const DEFAULT_COMPONENTS: SelectComponentsConfig<any> = { Option: ({ children, innerProps, data, ...props }) => ( <ClassNames> {({ css }) => ( @@ -302,14 +272,6 @@ export const DEFAULT_COMPONENTS: SelectComponentsType = { /> </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 index e0a5779..13f8d87 100644 --- 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 one or more column or metric')} + placeholder={t('choose a 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 index a73159e..ad1a320 100644 --- a/superset-frontend/src/explore/components/controls/MetricsControl.jsx +++ b/superset-frontend/src/explore/components/controls/MetricsControl.jsx @@ -337,11 +337,7 @@ export default class MetricsControl extends React.PureComponent { <OnPasteSelect isMulti={this.props.multi} name={`select-${this.props.name}`} - placeholder={ - this.props.multi - ? t('choose one or more column or aggregate function') - : t('choose a column or aggregate function') - } + placeholder={t('choose a column or aggregate function')} options={this.state.options} value={this.state.value} labelKey="label"
