You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by er...@apache.org on 2020/10/14 17:40:39 UTC
[incubator-superset] 01/01: Revert "fix: keep placeholder in
multivalue select when a value exists (#11181)"
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 <er...@airbnb.com>
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"