fix(select): select component sort functionality on certain options (#…

…17638) * fix: Select component sort function sorting by label instead of value on numbers * fix: change select component default sorting to sort by the initial index rather than a property like value or label * fix: select sorting add sortOptions to select components using sortByProperty * fix: change select component back, add order to options coming in from SelectControl * fix: select component options intitial sort bug * fix: add test cases for select fix Co-authored-by: Corbin Robb <corbin@Corbins-MacBook-Pro.local>
apache · Dec 9, 2021 · f476ba2 · f476ba2
1 parent 3a42071
commit f476ba2
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 13 deletions.
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
@@ -106,6 +106,8 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
     'One or many columns to group by. High cardinality groupings should include a sort by metric ' +
       'and series limit to limit the number of fetched and rendered series.',
   ),
+  sortComparator: (a: { label: string }, b: { label: string }) =>
+    a.label.localeCompare(b.label),
   optionRenderer: c => <ColumnOption showType column={c} />,
   valueRenderer: c => <ColumnOption column={c} />,
   valueKey: 'column_name',

diff --git a/superset-frontend/plugins/legacy-plugin-chart-heatmap/src/controlPanel.ts b/superset-frontend/plugins/legacy-plugin-chart-heatmap/src/controlPanel.ts
@@ -114,7 +114,7 @@ const config: ControlPanelConfig = {
               label: t('XScale Interval'),
               renderTrigger: true,
               choices: formatSelectOptionsForRange(1, 50),
-              default: '1',
+              default: 1,
               clearable: false,
               description: t(
                 'Number of steps to take between ticks when displaying the X scale',
@@ -129,7 +129,7 @@ const config: ControlPanelConfig = {
               type: 'SelectControl',
               label: t('YScale Interval'),
               choices: formatSelectOptionsForRange(1, 50),
-              default: '1',
+              default: 1,
               clearable: false,
               renderTrigger: true,
               description: t(

diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
@@ -116,6 +116,8 @@ const all_columns: typeof sharedControls.groupby = {
         ? [t('must have a value')]
         : [],
   }),
+  sortComparator: (a: { label: string }, b: { label: string }) =>
+    a.label.localeCompare(b.label),
   visibility: isRawMode,
 };
 
@@ -276,6 +278,8 @@ const config: ControlPanelConfig = {
                 choices: datasource?.order_by_choices || [],
               }),
               visibility: isRawMode,
+              sortComparator: (a: { label: string }, b: { label: string }) =>
+                a.label.localeCompare(b.label),
             },
           },
         ],

diff --git a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
@@ -27,6 +27,7 @@ import { styledMount as mount } from 'spec/helpers/theming';
 const defaultProps = {
   choices: [
     ['1 year ago', '1 year ago'],
+    ['1 week ago', '1 week ago'],
     ['today', 'today'],
   ],
   name: 'row_limit',
@@ -36,8 +37,9 @@ const defaultProps = {
 };
 
 const options = [
-  { value: '1 year ago', label: '1 year ago' },
-  { value: 'today', label: 'today' },
+  { value: '1 year ago', label: '1 year ago', order: 0 },
+  { value: '1 week ago', label: '1 week ago', order: 1 },
+  { value: 'today', label: 'today', order: 2 },
 ];
 
 describe('SelectControl', () => {
@@ -149,6 +151,37 @@ describe('SelectControl', () => {
         expect(wrapper.html()).not.toContain('add something');
       });
     });
+
+    describe('when select has a sortComparator prop', () => {
+      it('does not add add order key and sorts by sortComparator', () => {
+        const sortComparator = (a, b) => a.label.localeCompare(b.label);
+        const optionsSortedByLabel = options
+          .map(opt => ({ label: opt.label, value: opt.value }))
+          .sort(sortComparator);
+        wrapper = mount(
+          <SelectControl
+            {...defaultProps}
+            sortComparator={sortComparator}
+            value={50}
+            placeholder="add something"
+          />,
+        );
+        expect(wrapper.state().options).toEqual(optionsSortedByLabel);
+      });
+    });
+
+    describe('when select does not have a sortComparator prop', () => {
+      it('adds an order key and maintains its intial order', () => {
+        wrapper = mount(
+          <SelectControl
+            {...defaultProps}
+            value={50}
+            placeholder="add something"
+          />,
+        );
+        expect(wrapper.state().options).toEqual(options);
+      });
+    });
   });
 
   describe('getOptions', () => {
@@ -160,8 +193,8 @@ describe('SelectControl', () => {
   describe('UNSAFE_componentWillReceiveProps', () => {
     it('sets state.options if props.choices has changed', () => {
       const updatedOptions = [
-        { value: 'three', label: 'three' },
-        { value: 'four', label: 'four' },
+        { value: 'three', label: 'three', order: 0 },
+        { value: 'four', label: 'four', order: 1 },
       ];
       const newProps = {
         choices: [

diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
@@ -171,7 +171,6 @@ const StyledSelect = styled(AntdSelect)`
     && .ant-select-selector {
       border-radius: ${theme.gridUnit}px;
     }
-
     // Open the dropdown when clicking on the suffix
     // This is fixed in version 4.16
     .ant-select-arrow .anticon:not(.ant-select-suffix) {
@@ -196,7 +195,6 @@ const StyledError = styled.div`
     width: 100%;
     padding: ${theme.gridUnit * 2}px;
     color: ${theme.colors.error.base};
-
     & svg {
       margin-right: ${theme.gridUnit * 2}px;
     }
@@ -298,8 +296,9 @@ const Select = ({
   const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
   const initialOptions =
     options && Array.isArray(options) ? options : EMPTY_OPTIONS;
-  const [selectOptions, setSelectOptions] =
-    useState<OptionsType>(initialOptions);
+  const [selectOptions, setSelectOptions] = useState<OptionsType>(
+    initialOptions.sort(sortComparator),
+  );
   const shouldUseChildrenOptions = !!selectOptions.find(
     opt => opt?.customLabel,
   );

diff --git a/...xplore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/...xplore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
@@ -37,6 +37,7 @@ import AdhocFilter, {
   CLAUSES,
 } from 'src/explore/components/controls/FilterControl/AdhocFilter';
 import { Input } from 'src/common/components';
+import { propertyComparator } from 'src/components/Select/Select';
 
 const StyledInput = styled(Input)`
   margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
@@ -387,12 +388,14 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
         css={theme => ({ marginBottom: theme.gridUnit * 4 })}
         options={(props.operators ?? OPERATORS_OPTIONS)
           .filter(op => isOperatorRelevant(op, subject))
-          .map(option => ({
+          .map((option, index) => ({
             value: option,
             label: OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display,
             key: option,
+            order: index,
           }))}
         {...operatorSelectProps}
+        sortComparator={propertyComparator('order')}
       />
       {MULTI_OPERATORS.has(operatorId) || suggestions.length > 0 ? (
         <SelectWithLabel
@@ -402,6 +405,9 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
             label: String(suggestion),
           }))}
           {...comparatorSelectProps}
+          sortComparator={propertyComparator(
+            typeof suggestions[0] === 'number' ? 'value' : 'label',
+          )}
         />
       ) : (
         <StyledInput

diff --git a/superset-frontend/src/explore/components/controls/SelectControl.jsx b/superset-frontend/src/explore/components/controls/SelectControl.jsx
@@ -19,7 +19,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { css, t } from '@superset-ui/core';
-import { Select } from 'src/components';
+import Select, { propertyComparator } from 'src/components/Select/Select';
 import ControlHeader from 'src/explore/components/ControlHeader';
 
 const propTypes = {
@@ -133,9 +133,10 @@ export default class SelectControl extends React.PureComponent {
       }));
     } else if (choices) {
       // Accepts different formats of input
-      options = choices.map(c => {
+      options = choices.map((c, i) => {
         if (Array.isArray(c)) {
           const [value, label] = c.length > 1 ? c : [c[0], c[0]];
+          if (!this.props.sortComparator) return { value, label, order: i };
           return {
             value,
             label,
@@ -240,6 +241,7 @@ export default class SelectControl extends React.PureComponent {
       optionRenderer,
       options: this.state.options,
       placeholder,
+      sortComparator: this.props.sortComparator || propertyComparator('order'),
       value: getValue(),
     };