From 0def2f8136833b3c3bdd497313c79dc29c0b799b Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 14 Dec 2020 16:11:13 +0100 Subject: [PATCH 1/5] Migrate react-select to Antd select in Metrics popover --- .../src/common/components/Select.tsx | 8 +++ .../components/AdhocMetricEditPopover.jsx | 59 ++++++++----------- .../src/views/CRUD/alert/AlertReportModal.tsx | 2 +- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/superset-frontend/src/common/components/Select.tsx b/superset-frontend/src/common/components/Select.tsx index ca2262197e701..75bd073912e6e 100644 --- a/superset-frontend/src/common/components/Select.tsx +++ b/superset-frontend/src/common/components/Select.tsx @@ -20,6 +20,10 @@ import { styled } from '@superset-ui/core'; import { Select as BaseSelect } from 'src/common/components'; const StyledSelect = styled(BaseSelect)` + display: block; +`; + +const StyledGraySelect = styled(StyledSelect)` &.ant-select-single { .ant-select-selector { height: 36px; @@ -44,3 +48,7 @@ const StyledOption = BaseSelect.Option; export const Select = Object.assign(StyledSelect, { Option: StyledOption, }); + +export const GraySelect = Object.assign(StyledGraySelect, { + Option: StyledOption, +}); diff --git a/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx b/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx index da1cde11cca9a..17ec4d0045617 100644 --- a/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx +++ b/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx @@ -21,7 +21,7 @@ import PropTypes from 'prop-types'; import { FormGroup } from 'react-bootstrap'; import Tabs from 'src/common/components/Tabs'; import Button from 'src/components/Button'; -import Select from 'src/components/Select'; +import { Select } from 'src/common/components/Select'; import { styled, t } from '@superset-ui/core'; import { ColumnOption } from '@superset-ui/chart-controls'; @@ -70,27 +70,12 @@ export default class AdhocMetricEditPopover extends React.Component { this.handleAceEditorRef = this.handleAceEditorRef.bind(this); this.refreshAceEditor = this.refreshAceEditor.bind(this); - this.popoverRef = React.createRef(); - this.state = { adhocMetric: this.props.adhocMetric, width: startingWidth, height: startingHeight, }; - this.selectProps = { - labelKey: 'label', - isMulti: false, - autosize: false, - clearable: true, - }; - - this.menuPortalProps = { - menuPosition: 'fixed', - menuPlacement: 'bottom', - menuPortalTarget: this.popoverRef.current, - }; - document.addEventListener('mouseup', this.onMouseUp); } @@ -118,7 +103,8 @@ export default class AdhocMetricEditPopover extends React.Component { this.props.onClose(); } - onColumnChange(column) { + onColumnChange(columnId) { + const column = this.props.columns.find(column => column.id === columnId); this.setState(prevState => ({ adhocMetric: prevState.adhocMetric.duplicateWith({ column, @@ -213,20 +199,24 @@ export default class AdhocMetricEditPopover extends React.Component { const columnSelectProps = { placeholder: t('%s column(s)', columns.length), - options: columns, value: (adhocMetric.column && adhocMetric.column.column_name) || adhocMetric.inferSqlExpressionColumn(), onChange: this.onColumnChange, optionRenderer: this.renderColumnOption, - valueKey: 'column_name', + allowClear: true, + showSearch: true, + filterOption: (input, option) => + option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0, }; const aggregateSelectProps = { placeholder: t('%s aggregates(s)', AGGREGATES_OPTIONS.length), - options: AGGREGATES_OPTIONS, value: adhocMetric.aggregate || adhocMetric.inferSqlExpressionAggregate(), onChange: this.onAggregateChange, + allowClear: true, + autoFocus: true, + showSearch: true, }; if (this.props.datasourceType === 'druid') { @@ -241,7 +231,6 @@ export default class AdhocMetricEditPopover extends React.Component {
column - + {columns.map(column => ( + + {this.renderColumnOption(column)} + + ))} + aggregate - + {AGGREGATES_OPTIONS.map(option => ( + {option} + ))} + Date: Mon, 14 Dec 2020 17:10:26 +0100 Subject: [PATCH 2/5] Fix tests --- .../components/AdhocMetricEditPopover_spec.jsx | 12 ++++++------ .../explore/components/AdhocMetricEditPopover.jsx | 8 +++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx index 5aff466065f2e..98a62bd84abf7 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx @@ -28,9 +28,9 @@ import AdhocMetricEditPopover from 'src/explore/components/AdhocMetricEditPopove import { AGGREGATES } from 'src/explore/constants'; const columns = [ - { type: 'VARCHAR(255)', column_name: 'source' }, - { type: 'VARCHAR(255)', column_name: 'target' }, - { type: 'DOUBLE', column_name: 'value' }, + { type: 'VARCHAR(255)', column_name: 'source', id: 1 }, + { type: 'VARCHAR(255)', column_name: 'target', id: 2 }, + { type: 'DOUBLE', column_name: 'value', id: 3 }, ]; const sumValueAdhocMetric = new AdhocMetric({ @@ -68,7 +68,7 @@ describe('AdhocMetricEditPopover', () => { it('overwrites the adhocMetric in state with onColumnChange', () => { const { wrapper } = setup(); - wrapper.instance().onColumnChange(columns[0]); + wrapper.instance().onColumnChange(columns[0].id); expect(wrapper.state('adhocMetric')).toEqual( sumValueAdhocMetric.duplicateWith({ column: columns[0] }), ); @@ -95,7 +95,7 @@ describe('AdhocMetricEditPopover', () => { expect(wrapper.find(Button).find({ disabled: true })).not.toExist(); wrapper.instance().onColumnChange(null); expect(wrapper.find(Button).find({ disabled: true })).toExist(); - wrapper.instance().onColumnChange({ column: columns[0] }); + wrapper.instance().onColumnChange(columns[0].id); expect(wrapper.find(Button).find({ disabled: true })).not.toExist(); wrapper.instance().onAggregateChange(null); expect(wrapper.find(Button).find({ disabled: true })).toExist(); @@ -104,7 +104,7 @@ describe('AdhocMetricEditPopover', () => { it('highlights save if changes are present', () => { const { wrapper } = setup(); expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).not.toExist(); - wrapper.instance().onColumnChange({ column: columns[1] }); + wrapper.instance().onColumnChange(columns[1].id); expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).toExist(); }); diff --git a/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx b/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx index 17ec4d0045617..a75406fcf183e 100644 --- a/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx +++ b/superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx @@ -203,7 +203,6 @@ export default class AdhocMetricEditPopover extends React.Component { (adhocMetric.column && adhocMetric.column.column_name) || adhocMetric.inferSqlExpressionColumn(), onChange: this.onColumnChange, - optionRenderer: this.renderColumnOption, allowClear: true, showSearch: true, filterOption: (input, option) => @@ -254,7 +253,8 @@ export default class AdhocMetricEditPopover extends React.Component { {columns.map(column => ( {this.renderColumnOption(column)} @@ -267,7 +267,9 @@ export default class AdhocMetricEditPopover extends React.Component { From 12ec04f51862405ba7c6b43d0fc9325fcfabef2c Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 14 Dec 2020 18:17:12 +0100 Subject: [PATCH 3/5] Migrate react-select to Antd select in Filters popover --- ...AdhocFilterEditPopoverSimpleTabContent.jsx | 57 ++++++++++++------- .../AdhocFilterEditPopoverSqlTabContent.jsx | 15 ++--- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx index a0402193978d6..e14ec8e5a14dd 100644 --- a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx @@ -19,7 +19,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FormGroup } from 'react-bootstrap'; -import { Select } from 'src/components/Select'; +import { Select } from 'src/common/components/Select'; import { t, SupersetClient } from '@superset-ui/core'; import AdhocFilter, { EXPRESSION_TYPES, CLAUSES } from '../AdhocFilter'; @@ -92,11 +92,8 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon }; this.selectProps = { - isMulti: false, name: 'select-column', - labelKey: 'label', - autosize: false, - clearable: false, + showSearch: true, }; this.menuPortalProps = { @@ -116,7 +113,9 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon } } - onSubjectChange(option) { + onSubjectChange(optionId) { + const option = this.props.options.find(option => option.id === optionId); + let subject; let clause; // infer the new clause based on what subject was selected. @@ -259,15 +258,13 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon const { adhocFilter, options: columns, datasource } = this.props; const { subject, operator, comparator } = adhocFilter; const subjectSelectProps = { - options: columns, - value: subject ? { value: subject } : undefined, + value: subject ?? undefined, onChange: this.onSubjectChange, - optionRenderer: this.renderSubjectOptionLabel, - valueRenderer: this.renderSubjectOptionValue, - valueKey: 'filterOptionName', noResultsText: t( 'No such column found. To filter on a metric, try the Custom SQL tab.', ), + filterOption: (input, option) => + option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0, }; if (datasource.type === 'druid') { @@ -283,19 +280,15 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon adhocFilter.clause === CLAUSES.WHERE ? t('%s column(s)', columns.length) : t('To filter on a metric, use Custom SQL tab.'); - // make sure options have `column_name` - subjectSelectProps.options = columns.filter(option => option.column_name); } const operatorSelectProps = { placeholder: t('%s operators(s)', OPERATORS_OPTIONS.length), // like AGGREGTES_OPTIONS, operator options are string - options: OPERATORS_OPTIONS.filter(op => - this.isOperatorRelevant(op, subject), - ), value: operator, onChange: this.onOperatorChange, - getOptionLabel: translateOperator, + filterOption: (input, option) => + option.value.toLowerCase().indexOf(input.toLowerCase()) >= 0, }; return ( @@ -303,18 +296,40 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon {MULTI_OPERATORS.has(operator) || diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx index 2a982bacec978..ffd01751f5c15 100644 --- a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx @@ -19,7 +19,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FormGroup } from 'react-bootstrap'; -import Select from 'src/components/Select'; +import { Select } from 'src/common/components/Select'; import { t } from '@superset-ui/core'; import { SQLEditor } from 'src/components/AsyncAceEditor'; import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; @@ -51,11 +51,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component this.handleAceEditorRef = this.handleAceEditorRef.bind(this); this.selectProps = { - isMulti: false, name: 'select-column', - labelKey: 'label', - autosize: false, - clearable: false, }; } @@ -94,7 +90,6 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component const clauseSelectProps = { placeholder: t('choose WHERE or HAVING...'), - options: Object.keys(CLAUSES), value: adhocFilter.clause, onChange: this.onSqlExpressionClauseChange, }; @@ -121,7 +116,13 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component {...this.selectProps} {...clauseSelectProps} className="filter-edit-clause-dropdown" - /> + > + {Object.keys(CLAUSES).map(clause => ( + + {clause} + + ))} + WHERE {t('filters by columns')}
From 1a44ce96c0043e11d9a0b5d57963c5fb2b6f31f0 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 14 Dec 2020 19:21:17 +0100 Subject: [PATCH 4/5] Migrate SelectControl to Antd Select --- ...FilterEditPopoverSimpleTabContent_spec.jsx | 14 ++-- ...AdhocFilterEditPopoverSimpleTabContent.jsx | 74 ++++++++++--------- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx index 442527b77b4aa..fc13220940ce7 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx @@ -59,10 +59,10 @@ const simpleCustomFilter = new AdhocFilter({ }); const options = [ - { type: 'VARCHAR(255)', column_name: 'source' }, - { type: 'VARCHAR(255)', column_name: 'target' }, - { type: 'DOUBLE', column_name: 'value' }, - { saved_metric_name: 'my_custom_metric' }, + { type: 'VARCHAR(255)', column_name: 'source', id: 1 }, + { type: 'VARCHAR(255)', column_name: 'target', id: 2 }, + { type: 'DOUBLE', column_name: 'value', id: 3 }, + { saved_metric_name: 'my_custom_metric', id: 4 }, sumValueAdhocMetric, ]; @@ -91,9 +91,7 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => { it('passes the new adhocFilter to onChange after onSubjectChange', () => { const { wrapper, onChange } = setup(); - wrapper - .instance() - .onSubjectChange({ type: 'VARCHAR(255)', column_name: 'source' }); + wrapper.instance().onSubjectChange(1); expect(onChange.calledOnce).toBe(true); expect(onChange.lastCall.args[0]).toEqual( simpleAdhocFilter.duplicateWith({ subject: 'source' }), @@ -102,7 +100,7 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => { it('may alter the clause in onSubjectChange if the old clause is not appropriate', () => { const { wrapper, onChange } = setup(); - wrapper.instance().onSubjectChange(sumValueAdhocMetric); + wrapper.instance().onSubjectChange(sumValueAdhocMetric.optionName); expect(onChange.calledOnce).toBe(true); expect(onChange.lastCall.args[0]).toEqual( simpleAdhocFilter.duplicateWith({ diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx index e14ec8e5a14dd..87fc769756755 100644 --- a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx @@ -36,7 +36,6 @@ import { DISABLE_INPUT_OPERATORS, } from '../constants'; import FilterDefinitionOption from './FilterDefinitionOption'; -import SelectControl from './controls/SelectControl'; const propTypes = { adhocFilter: PropTypes.instanceOf(AdhocFilter).isRequired, @@ -113,8 +112,10 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon } } - onSubjectChange(optionId) { - const option = this.props.options.find(option => option.id === optionId); + onSubjectChange(id) { + const option = this.props.options.find( + option => option.id === id || option.optionName === id, + ); let subject; let clause; @@ -246,21 +247,25 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon } } - renderSubjectOptionLabel(option) { - return ; + createSuggestionsPlaceholder() { + const suggestionsLength = this.state.suggestions.length; + return suggestionsLength + ? t('%s option(s)', this.state.suggestions.length) + : ''; } - renderSubjectOptionValue({ value }) { - return {value}; + renderSubjectOptionLabel(option) { + return ; } render() { - const { adhocFilter, options: columns, datasource } = this.props; + const { adhocFilter, options, datasource } = this.props; + let columns = options; const { subject, operator, comparator } = adhocFilter; const subjectSelectProps = { value: subject ?? undefined, onChange: this.onSubjectChange, - noResultsText: t( + notFoundContent: t( 'No such column found. To filter on a metric, try the Custom SQL tab.', ), filterOption: (input, option) => @@ -280,6 +285,7 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon adhocFilter.clause === CLAUSES.WHERE ? t('%s column(s)', columns.length) : t('To filter on a metric, use Custom SQL tab.'); + columns = options.filter(option => option.column_name); } const operatorSelectProps = { @@ -299,21 +305,17 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon {...subjectSelectProps} name="filter-column" > - {columns - .filter(column => !!column.column_name) - .map(column => ( - - {this.renderSubjectOptionLabel(column)} - - ))} + {columns.map(column => ( + + {this.renderSubjectOptionLabel(column)} + + ))} @@ -334,20 +336,26 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon {MULTI_OPERATORS.has(operator) || this.state.suggestions.length > 0 ? ( - + placeholder={this.createSuggestionsPlaceholder()} + > + {this.state.suggestions.map(suggestion => ( + + {suggestion} + + ))} + ) : ( Date: Tue, 15 Dec 2020 09:07:34 +0100 Subject: [PATCH 5/5] Add label with number of options left --- ...AdhocFilterEditPopoverSimpleTabContent.jsx | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx index 87fc769756755..fa241914fc543 100644 --- a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx @@ -20,7 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FormGroup } from 'react-bootstrap'; import { Select } from 'src/common/components/Select'; -import { t, SupersetClient } from '@superset-ui/core'; +import { t, SupersetClient, styled } from '@superset-ui/core'; import AdhocFilter, { EXPRESSION_TYPES, CLAUSES } from '../AdhocFilter'; import adhocMetricType from '../propTypes/adhocMetricType'; @@ -37,6 +37,15 @@ import { } from '../constants'; import FilterDefinitionOption from './FilterDefinitionOption'; +const SelectWithLabel = styled(Select)` + .ant-select-selector::after { + content: '${({ labelText }) => labelText || '\\A0'}'; + display: inline-block; + white-space: nowrap; + color: ${({ theme }) => theme.colors.grayscale.light1}; + } +`; + const propTypes = { adhocFilter: PropTypes.instanceOf(AdhocFilter).isRequired, onChange: PropTypes.func.isRequired, @@ -247,11 +256,20 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon } } + optionsRemaining() { + const { suggestions } = this.state; + const { comparator } = this.props.adhocFilter; + // if select is multi/value is array, we show the options not selected + const valuesFromSuggestionsLength = Array.isArray(comparator) + ? comparator.filter(v => suggestions.includes(v)).length + : 0; + return suggestions?.length - valuesFromSuggestionsLength ?? 0; + } + createSuggestionsPlaceholder() { - const suggestionsLength = this.state.suggestions.length; - return suggestionsLength - ? t('%s option(s)', this.state.suggestions.length) - : ''; + const optionsRemaining = this.optionsRemaining(); + const placeholder = t('%s option(s)', optionsRemaining); + return optionsRemaining ? placeholder : ''; } renderSubjectOptionLabel(option) { @@ -336,7 +354,7 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon {MULTI_OPERATORS.has(operator) || this.state.suggestions.length > 0 ? ( - + ) : (