chore: Select component refactoring - TimeSeriesColumnControl - Itera…

…tion 5 (apache#16442) * Refactor Select * Update superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Fix tests * Mock debounce * Add debounce Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
opus-42 · Nov 14, 2021 · 16ced23 · 16ced23
1 parent adae67a
commit 16ced23
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 17 deletions.
diff --git a/.../src/explore/components/controls/TimeSeriesColumnControl/TimeSeriesColumnControl.test.tsx b/.../src/explore/components/controls/TimeSeriesColumnControl/TimeSeriesColumnControl.test.tsx
@@ -21,7 +21,11 @@ import { render, screen } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import TimeSeriesColumnControl from '.';
 
-jest.mock('lodash/debounce', () => jest.fn(fn => fn));
+jest.mock('lodash/debounce', () => (fn: Function & { cancel: Function }) => {
+  // eslint-disable-next-line no-param-reassign
+  fn.cancel = jest.fn();
+  return fn;
+});
 
 test('renders with default props', () => {
   render(<TimeSeriesColumnControl />);
@@ -78,7 +82,7 @@ test('triggers onChange when type changes', () => {
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
-  userEvent.click(screen.getByText('Select...'));
+  userEvent.click(screen.getByText('Select ...'));
   userEvent.click(screen.getByText('Time comparison'));
   expect(onChange).not.toHaveBeenCalled();
   userEvent.click(screen.getByRole('button', { name: 'Save' }));
@@ -121,7 +125,7 @@ test('triggers onChange when time type changes', () => {
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="time" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
-  userEvent.click(screen.getByText('Select...'));
+  userEvent.click(screen.getByText('Select ...'));
   userEvent.click(screen.getByText('Difference'));
   expect(onChange).not.toHaveBeenCalled();
   userEvent.click(screen.getByRole('button', { name: 'Save' }));

diff --git a/superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx b/superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx
@@ -21,7 +21,7 @@ import PropTypes from 'prop-types';
 import { Row, Col, Input } from 'src/common/components';
 import Button from 'src/components/Button';
 import Popover from 'src/components/Popover';
-import Select from 'src/components/Select';
+import { Select } from 'src/components';
 import { t, styled } from '@superset-ui/core';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
 import BoundsControl from '../BoundsControl';
@@ -61,17 +61,17 @@ const defaultProps = {
 };
 
 const comparisonTypeOptions = [
-  { value: 'value', label: 'Actual value' },
-  { value: 'diff', label: 'Difference' },
-  { value: 'perc', label: 'Percentage' },
-  { value: 'perc_change', label: 'Percentage change' },
+  { value: 'value', label: 'Actual value', key: 'value' },
+  { value: 'diff', label: 'Difference', key: 'diff' },
+  { value: 'perc', label: 'Percentage', key: 'perc' },
+  { value: 'perc_change', label: 'Percentage change', key: 'perc_change' },
 ];
 
 const colTypeOptions = [
-  { value: 'time', label: 'Time comparison' },
-  { value: 'contrib', label: 'Contribution' },
-  { value: 'spark', label: 'Sparkline' },
-  { value: 'avg', label: 'Period average' },
+  { value: 'time', label: 'Time comparison', key: 'time' },
+  { value: 'contrib', label: 'Contribution', key: 'contrib' },
+  { value: 'spark', label: 'Sparkline', key: 'spark' },
+  { value: 'avg', label: 'Period average', key: 'avg' },
 ];
 
 const StyledRow = styled(Row)`
@@ -143,7 +143,7 @@ export default class TimeSeriesColumnControl extends React.Component {
   }
 
   onSelectChange(attr, opt) {
-    this.setState({ [attr]: opt.value });
+    this.setState({ [attr]: opt });
   }
 
   onTextInputChange(attr, event) {
@@ -216,8 +216,8 @@ export default class TimeSeriesColumnControl extends React.Component {
           'Type of comparison, value difference or percentage',
           'col-type',
           <Select
-            value={this.state.colType}
-            clearable={false}
+            ariaLabel={t('Type')}
+            value={this.state.colType || undefined}
             onChange={this.onSelectChange.bind(this, 'colType')}
             options={colTypeOptions}
           />,
@@ -273,8 +273,8 @@ export default class TimeSeriesColumnControl extends React.Component {
             'Type of comparison, value difference or percentage',
             'comp-type',
             <Select
-              value={this.state.comparisonType}
-              clearable={false}
+              ariaLabel={t('Type')}
+              value={this.state.comparisonType || undefined}
               onChange={this.onSelectChange.bind(this, 'comparisonType')}
               options={comparisonTypeOptions}
             />,