From 8ad03c4f2518835818c5ead116ddf69b41ed342d Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Mon, 27 Sep 2021 17:23:52 +0300 Subject: [PATCH] chore: Select component refactoring - FilterControl - Iteration 5 (#15777) * Refactor Select for AdhocFilterEditPopoverSqlTabContent * Refactor Selects * Fix numeric options * Clean up * Fix Select value * Add showSearch * Display null label * Implement StyledSelect * Update superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Fix aria-label * Revert MetricControls changes * Update ariaLabel * Reconcile with latest Select changes Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- .../integration/explore/AdhocFilters.test.ts | 2 +- .../AdhocFilterEditPopover/index.jsx | 5 - .../index.tsx | 165 ++++++------------ .../index.jsx | 30 ++-- 4 files changed, 74 insertions(+), 128 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts index eaa691185b..2a0d142914 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts @@ -52,7 +52,7 @@ describe('AdhocFilters', () => { }); xit('Set simple adhoc filter', () => { - cy.get('[data-test=adhoc-filter-simple-value] .Select__control').click(); + cy.get('[aria-label="Comparator option"] .Select__control').click(); cy.get('[data-test=adhoc-filter-simple-value] input[type=text]') .focus() .type('Jack{enter}', { delay: 20 }); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx index ba4a63601e..2fd29a6ced 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx @@ -68,11 +68,6 @@ const FilterPopoverContentContainer = styled.div` max-width: none; } - .filter-edit-clause-dropdown { - width: ${({ theme }) => theme.gridUnit * 30}px; - margin-right: ${({ theme }) => theme.gridUnit}px; - } - .filter-edit-clause-info { font-size: ${({ theme }) => theme.typography.sizes.xs}px; padding-left: ${({ theme }) => theme.gridUnit}px; diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index f6ef44b5a1..fd97963d30 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -17,7 +17,7 @@ * under the License. */ import React, { useEffect, useState } from 'react'; -import { NativeSelect as Select } from 'src/components/Select'; +import { Select } from 'src/components'; import { t, SupersetClient, styled } from '@superset-ui/core'; import { Operators, @@ -36,25 +36,7 @@ import AdhocFilter, { EXPRESSION_TYPES, CLAUSES, } from 'src/explore/components/controls/FilterControl/AdhocFilter'; -import { Input, SelectProps } from 'src/common/components'; - -const SelectWithLabel = styled(Select)` - .ant-select-selector { - margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; - } - - .ant-select-selector::after { - content: '${( - pr: SelectProps & { - labelText: string | boolean; - }, - ) => pr.labelText || '\\A0'}'; - display: inline-block; - white-space: nowrap; - color: ${({ theme }) => theme.colors.grayscale.light1}; - width: max-content; - } -`; +import { Input } from 'src/common/components'; export interface SimpleColumnType { id: number; @@ -132,10 +114,10 @@ export const useSimpleTabFilterProps = (props: Props) => { HAVING_OPERATORS.indexOf(operator) === -1) ); }; - const onSubjectChange = (id: string | number) => { + const onSubjectChange = (id: string) => { const option = props.options.find( option => - ('id' in option && option.id === id) || + ('column_name' in option && option.column_name === id) || ('optionName' in option && option.optionName === id), ); @@ -222,10 +204,6 @@ export const useSimpleTabFilterProps = (props: Props) => { }; const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { - const selectProps = { - name: 'select-column', - showSearch: true, - }; const { onSubjectChange, onOperatorChange, @@ -233,7 +211,6 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { onComparatorChange, } = useSimpleTabFilterProps(props); const [suggestions, setSuggestions] = useState>([]); - const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState(''); const [ loadingComparatorSuggestions, setLoadingComparatorSuggestions, @@ -279,10 +256,6 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { ); - const clearSuggestionSearch = () => { - setCurrentSuggestionSearch(''); - }; - const getOptionsRemaining = () => { const { comparator } = props.adhocFilter; // if select is multi/value is array, we show the options not selected @@ -299,7 +272,9 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { let columns = props.options; const { subject, operator, comparator, operatorId } = props.adhocFilter; + const subjectSelectProps = { + ariaLabel: t('Select subject'), value: subject ?? undefined, onChange: onSubjectChange, notFoundContent: t( @@ -332,33 +307,44 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { '%s operator(s)', OPERATORS_OPTIONS.filter(op => isOperatorRelevant(op, subject)).length, ), - value: OPERATOR_ENUM_TO_OPERATOR_TYPE[operatorId]?.display, + value: operatorId, onChange: onOperatorChange, autoFocus: !!subjectSelectProps.value && !operator, - name: 'select-operator', + ariaLabel: t('Select operator'), }; const shouldFocusComparator = !!subjectSelectProps.value && !!operatorSelectProps.value; - const comparatorSelectProps: SelectProps & { - labelText: string | boolean; - } = { + const comparatorSelectProps = { allowClear: true, - showSearch: true, - mode: MULTI_OPERATORS.has(operatorId) ? 'tags' : undefined, - tokenSeparators: [',', '\n', '\t', ';'], + allowNewOptions: true, + ariaLabel: t('Comparator option'), + mode: MULTI_OPERATORS.has(operatorId) + ? ('multiple' as const) + : ('single' as const), loading: loadingComparatorSuggestions, value: comparator, onChange: onComparatorChange, notFoundContent: t('Type a value here'), disabled: DISABLE_INPUT_OPERATORS.includes(operatorId), placeholder: createSuggestionsPlaceholder(), - labelText: - comparator && comparator.length > 0 && createSuggestionsPlaceholder(), autoFocus: shouldFocusComparator, }; + const labelText = + comparator && comparator.length > 0 && createSuggestionsPlaceholder(); + + const SelectWithLabel = styled(Select)` + .ant-select-selector::after { + content: ${() => labelText || '\\A0'}; + display: inline-block; + white-space: nowrap; + color: ${({ theme }) => theme.colors.grayscale.light1}; + width: max-content; + } + `; + return ( <> + /> + /> {MULTI_OPERATORS.has(operatorId) || suggestions.length > 0 ? ( ({ + value: suggestion, + label: String(suggestion), + }))} {...comparatorSelectProps} - getPopupContainer={triggerNode => triggerNode.parentNode} - onSearch={val => setCurrentSuggestionSearch(val)} - onSelect={clearSuggestionSearch} - onBlur={clearSuggestionSearch} - > - {suggestions.map((suggestion: string) => ( - - {String(suggestion)} - - ))} - - {/* enable selecting an option not included in suggestions */} - {currentSuggestionSearch && - !suggestions.some( - (suggestion: string) => suggestion === currentSuggestionSearch, - ) && ( - - {currentSuggestionSearch} - - )} - + /> ) : ( ` + width: ${theme.gridUnit * 30}px; + marginRight: ${theme.gridUnit}px; + `} +`; + export default class AdhocFilterEditPopoverSqlTabContent extends React.Component { constructor(props) { super(props); @@ -54,7 +61,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component this.handleAceEditorRef = this.handleAceEditorRef.bind(this); this.selectProps = { - name: 'select-column', + ariaLabel: t('Select column'), }; } @@ -111,22 +118,19 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component }) .filter(Boolean), ); + const selectOptions = Object.keys(CLAUSES).map(clause => ({ + label: clause, + value: clause, + })); return (
- + /> WHERE {t('Filters by columns')}