mirror of https://github.com/apache/superset.git
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>
This commit is contained in:
parent
3a42071e0f
commit
f476ba23a2
|
@ -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 ' +
|
'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.',
|
'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} />,
|
optionRenderer: c => <ColumnOption showType column={c} />,
|
||||||
valueRenderer: c => <ColumnOption column={c} />,
|
valueRenderer: c => <ColumnOption column={c} />,
|
||||||
valueKey: 'column_name',
|
valueKey: 'column_name',
|
||||||
|
|
|
@ -114,7 +114,7 @@ const config: ControlPanelConfig = {
|
||||||
label: t('XScale Interval'),
|
label: t('XScale Interval'),
|
||||||
renderTrigger: true,
|
renderTrigger: true,
|
||||||
choices: formatSelectOptionsForRange(1, 50),
|
choices: formatSelectOptionsForRange(1, 50),
|
||||||
default: '1',
|
default: 1,
|
||||||
clearable: false,
|
clearable: false,
|
||||||
description: t(
|
description: t(
|
||||||
'Number of steps to take between ticks when displaying the X scale',
|
'Number of steps to take between ticks when displaying the X scale',
|
||||||
|
@ -129,7 +129,7 @@ const config: ControlPanelConfig = {
|
||||||
type: 'SelectControl',
|
type: 'SelectControl',
|
||||||
label: t('YScale Interval'),
|
label: t('YScale Interval'),
|
||||||
choices: formatSelectOptionsForRange(1, 50),
|
choices: formatSelectOptionsForRange(1, 50),
|
||||||
default: '1',
|
default: 1,
|
||||||
clearable: false,
|
clearable: false,
|
||||||
renderTrigger: true,
|
renderTrigger: true,
|
||||||
description: t(
|
description: t(
|
||||||
|
|
|
@ -116,6 +116,8 @@ const all_columns: typeof sharedControls.groupby = {
|
||||||
? [t('must have a value')]
|
? [t('must have a value')]
|
||||||
: [],
|
: [],
|
||||||
}),
|
}),
|
||||||
|
sortComparator: (a: { label: string }, b: { label: string }) =>
|
||||||
|
a.label.localeCompare(b.label),
|
||||||
visibility: isRawMode,
|
visibility: isRawMode,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -276,6 +278,8 @@ const config: ControlPanelConfig = {
|
||||||
choices: datasource?.order_by_choices || [],
|
choices: datasource?.order_by_choices || [],
|
||||||
}),
|
}),
|
||||||
visibility: isRawMode,
|
visibility: isRawMode,
|
||||||
|
sortComparator: (a: { label: string }, b: { label: string }) =>
|
||||||
|
a.label.localeCompare(b.label),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
],
|
],
|
||||||
|
|
|
@ -27,6 +27,7 @@ import { styledMount as mount } from 'spec/helpers/theming';
|
||||||
const defaultProps = {
|
const defaultProps = {
|
||||||
choices: [
|
choices: [
|
||||||
['1 year ago', '1 year ago'],
|
['1 year ago', '1 year ago'],
|
||||||
|
['1 week ago', '1 week ago'],
|
||||||
['today', 'today'],
|
['today', 'today'],
|
||||||
],
|
],
|
||||||
name: 'row_limit',
|
name: 'row_limit',
|
||||||
|
@ -36,8 +37,9 @@ const defaultProps = {
|
||||||
};
|
};
|
||||||
|
|
||||||
const options = [
|
const options = [
|
||||||
{ value: '1 year ago', label: '1 year ago' },
|
{ value: '1 year ago', label: '1 year ago', order: 0 },
|
||||||
{ value: 'today', label: 'today' },
|
{ value: '1 week ago', label: '1 week ago', order: 1 },
|
||||||
|
{ value: 'today', label: 'today', order: 2 },
|
||||||
];
|
];
|
||||||
|
|
||||||
describe('SelectControl', () => {
|
describe('SelectControl', () => {
|
||||||
|
@ -149,6 +151,37 @@ describe('SelectControl', () => {
|
||||||
expect(wrapper.html()).not.toContain('add something');
|
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', () => {
|
describe('getOptions', () => {
|
||||||
|
@ -160,8 +193,8 @@ describe('SelectControl', () => {
|
||||||
describe('UNSAFE_componentWillReceiveProps', () => {
|
describe('UNSAFE_componentWillReceiveProps', () => {
|
||||||
it('sets state.options if props.choices has changed', () => {
|
it('sets state.options if props.choices has changed', () => {
|
||||||
const updatedOptions = [
|
const updatedOptions = [
|
||||||
{ value: 'three', label: 'three' },
|
{ value: 'three', label: 'three', order: 0 },
|
||||||
{ value: 'four', label: 'four' },
|
{ value: 'four', label: 'four', order: 1 },
|
||||||
];
|
];
|
||||||
const newProps = {
|
const newProps = {
|
||||||
choices: [
|
choices: [
|
||||||
|
|
|
@ -171,7 +171,6 @@ const StyledSelect = styled(AntdSelect)`
|
||||||
&& .ant-select-selector {
|
&& .ant-select-selector {
|
||||||
border-radius: ${theme.gridUnit}px;
|
border-radius: ${theme.gridUnit}px;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Open the dropdown when clicking on the suffix
|
// Open the dropdown when clicking on the suffix
|
||||||
// This is fixed in version 4.16
|
// This is fixed in version 4.16
|
||||||
.ant-select-arrow .anticon:not(.ant-select-suffix) {
|
.ant-select-arrow .anticon:not(.ant-select-suffix) {
|
||||||
|
@ -196,7 +195,6 @@ const StyledError = styled.div`
|
||||||
width: 100%;
|
width: 100%;
|
||||||
padding: ${theme.gridUnit * 2}px;
|
padding: ${theme.gridUnit * 2}px;
|
||||||
color: ${theme.colors.error.base};
|
color: ${theme.colors.error.base};
|
||||||
|
|
||||||
& svg {
|
& svg {
|
||||||
margin-right: ${theme.gridUnit * 2}px;
|
margin-right: ${theme.gridUnit * 2}px;
|
||||||
}
|
}
|
||||||
|
@ -298,8 +296,9 @@ const Select = ({
|
||||||
const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
|
const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
|
||||||
const initialOptions =
|
const initialOptions =
|
||||||
options && Array.isArray(options) ? options : EMPTY_OPTIONS;
|
options && Array.isArray(options) ? options : EMPTY_OPTIONS;
|
||||||
const [selectOptions, setSelectOptions] =
|
const [selectOptions, setSelectOptions] = useState<OptionsType>(
|
||||||
useState<OptionsType>(initialOptions);
|
initialOptions.sort(sortComparator),
|
||||||
|
);
|
||||||
const shouldUseChildrenOptions = !!selectOptions.find(
|
const shouldUseChildrenOptions = !!selectOptions.find(
|
||||||
opt => opt?.customLabel,
|
opt => opt?.customLabel,
|
||||||
);
|
);
|
||||||
|
|
|
@ -37,6 +37,7 @@ import AdhocFilter, {
|
||||||
CLAUSES,
|
CLAUSES,
|
||||||
} from 'src/explore/components/controls/FilterControl/AdhocFilter';
|
} from 'src/explore/components/controls/FilterControl/AdhocFilter';
|
||||||
import { Input } from 'src/common/components';
|
import { Input } from 'src/common/components';
|
||||||
|
import { propertyComparator } from 'src/components/Select/Select';
|
||||||
|
|
||||||
const StyledInput = styled(Input)`
|
const StyledInput = styled(Input)`
|
||||||
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
|
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
|
||||||
|
@ -387,12 +388,14 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
|
||||||
css={theme => ({ marginBottom: theme.gridUnit * 4 })}
|
css={theme => ({ marginBottom: theme.gridUnit * 4 })}
|
||||||
options={(props.operators ?? OPERATORS_OPTIONS)
|
options={(props.operators ?? OPERATORS_OPTIONS)
|
||||||
.filter(op => isOperatorRelevant(op, subject))
|
.filter(op => isOperatorRelevant(op, subject))
|
||||||
.map(option => ({
|
.map((option, index) => ({
|
||||||
value: option,
|
value: option,
|
||||||
label: OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display,
|
label: OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display,
|
||||||
key: option,
|
key: option,
|
||||||
|
order: index,
|
||||||
}))}
|
}))}
|
||||||
{...operatorSelectProps}
|
{...operatorSelectProps}
|
||||||
|
sortComparator={propertyComparator('order')}
|
||||||
/>
|
/>
|
||||||
{MULTI_OPERATORS.has(operatorId) || suggestions.length > 0 ? (
|
{MULTI_OPERATORS.has(operatorId) || suggestions.length > 0 ? (
|
||||||
<SelectWithLabel
|
<SelectWithLabel
|
||||||
|
@ -402,6 +405,9 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
|
||||||
label: String(suggestion),
|
label: String(suggestion),
|
||||||
}))}
|
}))}
|
||||||
{...comparatorSelectProps}
|
{...comparatorSelectProps}
|
||||||
|
sortComparator={propertyComparator(
|
||||||
|
typeof suggestions[0] === 'number' ? 'value' : 'label',
|
||||||
|
)}
|
||||||
/>
|
/>
|
||||||
) : (
|
) : (
|
||||||
<StyledInput
|
<StyledInput
|
||||||
|
|
|
@ -19,7 +19,7 @@
|
||||||
import React from 'react';
|
import React from 'react';
|
||||||
import PropTypes from 'prop-types';
|
import PropTypes from 'prop-types';
|
||||||
import { css, t } from '@superset-ui/core';
|
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';
|
import ControlHeader from 'src/explore/components/ControlHeader';
|
||||||
|
|
||||||
const propTypes = {
|
const propTypes = {
|
||||||
|
@ -133,9 +133,10 @@ export default class SelectControl extends React.PureComponent {
|
||||||
}));
|
}));
|
||||||
} else if (choices) {
|
} else if (choices) {
|
||||||
// Accepts different formats of input
|
// Accepts different formats of input
|
||||||
options = choices.map(c => {
|
options = choices.map((c, i) => {
|
||||||
if (Array.isArray(c)) {
|
if (Array.isArray(c)) {
|
||||||
const [value, label] = c.length > 1 ? c : [c[0], c[0]];
|
const [value, label] = c.length > 1 ? c : [c[0], c[0]];
|
||||||
|
if (!this.props.sortComparator) return { value, label, order: i };
|
||||||
return {
|
return {
|
||||||
value,
|
value,
|
||||||
label,
|
label,
|
||||||
|
@ -240,6 +241,7 @@ export default class SelectControl extends React.PureComponent {
|
||||||
optionRenderer,
|
optionRenderer,
|
||||||
options: this.state.options,
|
options: this.state.options,
|
||||||
placeholder,
|
placeholder,
|
||||||
|
sortComparator: this.props.sortComparator || propertyComparator('order'),
|
||||||
value: getValue(),
|
value: getValue(),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue