chore: Select component refactoring - MetricControl - Iteration 5 (ap…

…ache#16423) * Refactor Select * Render saved metric option * Update tests * Fix Cypress and ariaLabels * Fix lint * Lint Cypress * Reconcile with latest Select changes
opus-42 · Nov 14, 2021 · d3ddae7 · d3ddae7
1 parent 7f67049
commit d3ddae7
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 58 deletions.
diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts
@@ -42,8 +42,12 @@ describe('AdhocMetrics', () => {
     cy.get('[data-test="AdhocMetricEditTitle#trigger"]').click();
     cy.get('[data-test="AdhocMetricEditTitle#input"]').type(metricName);
 
-    cy.get('[name="select-column"]').click().type('num_girls{enter}');
-    cy.get('[name="select-aggregate"]').click().type('sum{enter}');
+    cy.get('input[aria-label="Select column"]')
+      .click()
+      .type('num_girls{enter}');
+    cy.get('input[aria-label="Select aggregate options"]')
+      .click()
+      .type('sum{enter}');
 
     cy.get('[data-test="AdhocMetricEdit#save"]').contains('Save').click();
 

diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
@@ -45,8 +45,10 @@ describe('Visualization > Line', () => {
     // Title edit for saved metrics is disabled - switch to Simple
     cy.get('[id="adhoc-metric-edit-tabs-tab-SIMPLE"]').click();
 
-    cy.get('[name="select-column"]').click().type('num{enter}');
-    cy.get('[name="select-aggregate"]').click().type('sum{enter}');
+    cy.get('input[aria-label="Select column"]').click().type('num{enter}');
+    cy.get('input[aria-label="Select aggregate options"]')
+      .click()
+      .type('sum{enter}');
     cy.get('[data-test="AdhocMetricEdit#save"]').contains('Save').click();
 
     cy.get('.text-danger').should('not.exist');

diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
@@ -73,7 +73,7 @@ describe('AdhocMetricEditPopover', () => {
 
   it('overwrites the adhocMetric in state with onColumnChange', () => {
     const { wrapper } = setup();
-    wrapper.instance().onColumnChange(columns[0].id);
+    wrapper.instance().onColumnChange(columns[0].column_name);
     expect(wrapper.state('adhocMetric')).toEqual(
       sumValueAdhocMetric.duplicateWith({ column: columns[0] }),
     );
@@ -100,7 +100,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(columns[0].id);
+    wrapper.instance().onColumnChange(columns[0].column_name);
     expect(wrapper.find(Button).find({ disabled: true })).not.toExist();
     wrapper.instance().onAggregateChange(null);
     expect(wrapper.find(Button).find({ disabled: true })).toExist();
@@ -109,7 +109,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(columns[1].id);
+    wrapper.instance().onColumnChange(columns[1].column_name);
     expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).toExist();
   });
 

diff --git a/...t-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx b/...t-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
@@ -21,7 +21,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import Tabs from 'src/components/Tabs';
 import Button from 'src/components/Button';
-import { NativeSelect as Select } from 'src/components/Select';
+import { Select } from 'src/components';
 import { t, styled } from '@superset-ui/core';
 
 import { Form, FormItem } from 'src/components/Form';
@@ -162,8 +162,10 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
     );
   }
 
-  onColumnChange(columnId) {
-    const column = this.props.columns.find(column => column.id === columnId);
+  onColumnChange(columnName) {
+    const column = this.props.columns.find(
+      column => column.column_name === columnName,
+    );
     this.setState(prevState => ({
       adhocMetric: prevState.adhocMetric.duplicateWith({
         column,
@@ -184,9 +186,9 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
     }));
   }
 
-  onSavedMetricChange(savedMetricId) {
+  onSavedMetricChange(savedMetricName) {
     const savedMetric = this.props.savedMetricsOptions.find(
-      metric => metric.id === savedMetricId,
+      metric => metric.metric_name === savedMetricName,
     );
     this.setState(prevState => ({
       savedMetric,
@@ -262,6 +264,10 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
     return <StyledColumnOption column={column} showType />;
   }
 
+  renderMetricOption(savedMetric) {
+    return <StyledMetricOption metric={savedMetric} showType />;
+  }
+
   render() {
     const {
       adhocMetric: propsAdhocMetric,
@@ -290,34 +296,30 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
 
     // autofocus on column if there's no value in column; otherwise autofocus on aggregate
     const columnSelectProps = {
+      ariaLabel: t('Select column'),
       placeholder: t('%s column(s)', columns.length),
       value: columnValue,
       onChange: this.onColumnChange,
       allowClear: true,
-      showSearch: true,
       autoFocus: !columnValue,
-      filterOption: (input, option) =>
-        option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
     };
 
     const aggregateSelectProps = {
+      ariaLabel: t('Select aggregate options'),
       placeholder: t('%s aggregates(s)', AGGREGATES_OPTIONS.length),
       value: adhocMetric.aggregate || adhocMetric.inferSqlExpressionAggregate(),
       onChange: this.onAggregateChange,
       allowClear: true,
       autoFocus: !!columnValue,
-      showSearch: true,
     };
 
     const savedSelectProps = {
+      ariaLabel: t('Select saved metrics'),
       placeholder: t('%s saved metric(s)', savedMetricsOptions?.length ?? 0),
-      value: savedMetric?.verbose_name || savedMetric?.metric_name,
+      value: savedMetric?.metric_name,
       onChange: this.onSavedMetricChange,
       allowClear: true,
-      showSearch: true,
       autoFocus: true,
-      filterOption: (input, option) =>
-        option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
     };
 
     if (this.props.datasourceType === 'druid' && aggregateSelectProps.options) {
@@ -354,55 +356,41 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
           <Tabs.TabPane key={SAVED_TAB_KEY} tab={t('Saved')}>
             <FormItem label={t('Saved metric')}>
               <StyledSelect
+                options={
+                  Array.isArray(savedMetricsOptions)
+                    ? savedMetricsOptions.map(savedMetric => ({
+                        value: savedMetric.metric_name,
+                        label: savedMetric.metric_name,
+                        customLabel: this.renderMetricOption(savedMetric),
+                        key: savedMetric.id,
+                      }))
+                    : []
+                }
                 {...savedSelectProps}
-                name="select-saved"
-                getPopupContainer={triggerNode => triggerNode.parentNode}
-              >
-                {Array.isArray(savedMetricsOptions) &&
-                  savedMetricsOptions.map(savedMetric => (
-                    <Select.Option
-                      value={savedMetric.id}
-                      filterBy={
-                        savedMetric.verbose_name || savedMetric.metric_name
-                      }
-                      key={savedMetric.id}
-                    >
-                      <StyledMetricOption metric={savedMetric} showType />
-                    </Select.Option>
-                  ))}
-              </StyledSelect>
+              />
             </FormItem>
           </Tabs.TabPane>
           <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
             <FormItem label={t('column')}>
               <Select
+                options={columns.map(column => ({
+                  value: column.column_name,
+                  label: column.verbose_name || column.column_name,
+                  key: column.id,
+                  customLabel: this.renderColumnOption(column),
+                }))}
                 {...columnSelectProps}
-                name="select-column"
-                getPopupContainer={triggerNode => triggerNode.parentNode}
-              >
-                {columns.map(column => (
-                  <Select.Option
-                    value={column.id}
-                    filterBy={column.verbose_name || column.column_name}
-                    key={column.id}
-                  >
-                    {this.renderColumnOption(column)}
-                  </Select.Option>
-                ))}
-              </Select>
+              />
             </FormItem>
             <FormItem label={t('aggregate')}>
               <Select
+                options={AGGREGATES_OPTIONS.map(option => ({
+                  value: option,
+                  label: option,
+                  key: option,
+                }))}
                 {...aggregateSelectProps}
-                name="select-aggregate"
-                getPopupContainer={triggerNode => triggerNode.parentNode}
-              >
-                {AGGREGATES_OPTIONS.map(option => (
-                  <Select.Option value={option} key={option}>
-                    {option}
-                  </Select.Option>
-                ))}
-              </Select>
+              />
             </FormItem>
           </Tabs.TabPane>
           <Tabs.TabPane