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>
2021-09-27 17:23:52 +03:00 · 2021-09-27 17:23:52 +03:00 · 8ad03c4f25
parent 0f16177bde
commit 8ad03c4f25
4 changed files with 74 additions and 128 deletions
--- 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 });
--- 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;
--- 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<any> & {
-        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> = props => {
-  const selectProps = {
-    name: 'select-column',
-    showSearch: true,
-  };
  const {
    onSubjectChange,
    onOperatorChange,
@ -233,7 +211,6 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
    onComparatorChange,
  } = useSimpleTabFilterProps(props);
  const [suggestions, setSuggestions] = useState<Record<string, any>>([]);
-  const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState('');
  const [
    loadingComparatorSuggestions,
    setLoadingComparatorSuggestions,
@ -279,10 +256,6 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
    <FilterDefinitionOption option={option} />
  );

-  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> = 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> = 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<any> & {
-    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 (
    <>
      <Select
@ -366,81 +352,42 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
          marginTop: theme.gridUnit * 4,
          marginBottom: theme.gridUnit * 4,
        })}
-        {...selectProps}
+        options={columns.map(column => ({
+          value:
+            ('column_name' in column && column.column_name) ||
+            ('optionName' in column && column.optionName) ||
+            '',
+          label:
+            ('saved_metric_name' in column && column.saved_metric_name) ||
+            ('column_name' in column && column.column_name) ||
+            ('label' in column && column.label),
+          key:
+            ('id' in column && column.id) ||
+            ('optionName' in column && column.optionName) ||
+            undefined,
+          customLabel: renderSubjectOptionLabel(column),
+        }))}
        {...subjectSelectProps}
-        filterOption={(input, option) =>
-          option && option.filterBy
-            ? option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0
-            : false
-        }
-        getPopupContainer={triggerNode => triggerNode.parentNode}
-      >
-        {columns.map(column => (
-          <Select.Option
-            value={
-              ('id' in column && column.id) ||
-              ('optionName' in column && column.optionName) ||
-              ''
-            }
-            filterBy={
-              ('saved_metric_name' in column && column.saved_metric_name) ||
-              ('column_name' in column && column.column_name) ||
-              ('label' in column && column.label)
-            }
-            key={
-              ('id' in column && column.id) ||
-              ('optionName' in column && column.optionName) ||
-              undefined
-            }
-          >
-            {renderSubjectOptionLabel(column)}
-          </Select.Option>
-        ))}
-      </Select>
+      />
      <Select
        css={theme => ({ marginBottom: theme.gridUnit * 4 })}
-        {...selectProps}
+        options={OPERATORS_OPTIONS.filter(op =>
+          isOperatorRelevant(op, subject),
+        ).map(option => ({
+          value: option,
+          label: OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display,
+          key: option,
+        }))}
        {...operatorSelectProps}
-        filterOption={(input, option) =>
-          option && option.children
-            ? option.children.toLowerCase().indexOf(input.toLowerCase()) >= 0
-            : false
-        }
-        getPopupContainer={triggerNode => triggerNode.parentNode}
-      >
-        {OPERATORS_OPTIONS.filter(op => isOperatorRelevant(op, subject)).map(
-          option => (
-            <Select.Option value={option} key={option}>
-              {OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display}
-            </Select.Option>
-          ),
-        )}
-      </Select>
+      />
      {MULTI_OPERATORS.has(operatorId) || suggestions.length > 0 ? (
        <SelectWithLabel
-          data-test="adhoc-filter-simple-value"
+          options={suggestions.map((suggestion: string) => ({
+            value: suggestion,
+            label: String(suggestion),
+          }))}
          {...comparatorSelectProps}
-          getPopupContainer={triggerNode => triggerNode.parentNode}
-          onSearch={val => setCurrentSuggestionSearch(val)}
-          onSelect={clearSuggestionSearch}
-          onBlur={clearSuggestionSearch}
-        >
-          {suggestions.map((suggestion: string) => (
-            <Select.Option value={suggestion} key={suggestion}>
-              {String(suggestion)}
-            </Select.Option>
-          ))}
-
-          {/* enable selecting an option not included in suggestions */}
-          {currentSuggestionSearch &&
-            !suggestions.some(
-              (suggestion: string) => suggestion === currentSuggestionSearch,
-            ) && (
-              <Select.Option value={currentSuggestionSearch}>
-                {currentSuggestionSearch}
-              </Select.Option>
-            )}
-        </SelectWithLabel>
+        />
      ) : (
        <Input
          data-test="adhoc-filter-simple-value"
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx
@ -18,8 +18,8 @@
 */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { NativeSelect as Select } from 'src/components/Select';
-import { t } from '@superset-ui/core';
+import { Select } from 'src/components';
+import { styled, t } from '@superset-ui/core';
 import { SQLEditor } from 'src/components/AsyncAceEditor';
 import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';

@ -44,6 +44,13 @@ const propTypes = {
  activeKey: PropTypes.string.isRequired,
 };

+const StyledSelect = styled(Select)`
+  ${({ theme }) => `
+    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 (
      <span>
        <div className="filter-edit-clause-section">
-          <Select
+          <StyledSelect
+            options={selectOptions}
            {...this.selectProps}
            {...clauseSelectProps}
-            className="filter-edit-clause-dropdown"
-            getPopupContainer={triggerNode => triggerNode.parentNode}
-          >
-            {Object.keys(CLAUSES).map(clause => (
-              <Select.Option value={clause} key={clause}>
-                {clause}
-              </Select.Option>
-            ))}
-          </Select>
+          />
          <span className="filter-edit-clause-info">
            <strong>WHERE</strong> {t('Filters by columns')}
            <br />