From 282e2308f9db4c7110678c96d4e49ae42b85170b Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Wed, 20 Oct 2021 11:01:24 -0300 Subject: [PATCH 1/3] fix: Order of Select items when unselecting --- .../src/addSlice/AddSliceContainer.tsx | 14 ++-- .../src/components/DatabaseSelector/index.tsx | 16 ++-- .../src/components/Select/Select.stories.tsx | 3 +- .../src/components/Select/Select.test.tsx | 73 ++++++++++++++--- .../src/components/Select/Select.tsx | 81 ++++++++++++------- .../src/components/TableSelector/index.tsx | 6 +- .../src/components/TimezoneSelector/index.tsx | 15 ++-- .../components/RefreshIntervalModal.tsx | 3 + .../FiltersConfigForm/ColumnSelect.tsx | 1 - .../FiltersConfigForm/DatasetSelect.tsx | 6 +- .../FormattingPopoverContent.tsx | 31 ++++--- .../DateFilterControl/DateFilterLabel.tsx | 4 + .../components/CustomFrame.tsx | 16 ++++ .../controls/DateFilterControl/types.ts | 1 + .../DateFilterControl/utils/constants.ts | 44 +++++----- .../src/views/CRUD/alert/AlertReportModal.tsx | 14 ++++ 16 files changed, 220 insertions(+), 108 deletions(-) diff --git a/superset-frontend/src/addSlice/AddSliceContainer.tsx b/superset-frontend/src/addSlice/AddSliceContainer.tsx index 8dc1cab8f4f0d..db8da84beb4dd 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.tsx +++ b/superset-frontend/src/addSlice/AddSliceContainer.tsx @@ -256,15 +256,11 @@ export default class AddSliceContainer extends React.PureComponent< customLabel: ReactNode; label: string; value: string; - }[] = response.json.result - .map((item: Dataset) => ({ - value: `${item.id}__${item.datasource_type}`, - customLabel: this.newLabel(item), - label: item.table_name, - })) - .sort((a: { label: string }, b: { label: string }) => - a.label.localeCompare(b.label), - ); + }[] = response.json.result.map((item: Dataset) => ({ + value: `${item.id}__${item.datasource_type}`, + customLabel: this.newLabel(item), + label: item.table_name, + })); return { data: list, totalCount: response.json.count, diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index dd6b824c89aae..45d91f1c97662 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -201,22 +201,18 @@ export default function DatabaseSelector({ // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. SupersetClient.get({ endpoint }) .then(({ json }) => { - const options = json.result - .map((s: string) => ({ - value: s, - label: s, - title: s, - })) - .sort((a: { label: string }, b: { label: string }) => - a.label.localeCompare(b.label), - ); + const options = json.result.map((s: string) => ({ + value: s, + label: s, + title: s, + })); if (onSchemasLoad) { onSchemasLoad(options); } setSchemaOptions(options); setLoadingSchemas(false); }) - .catch(e => { + .catch(() => { setLoadingSchemas(false); handleError(t('There was an error loading the schemas')); }); diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx index 938cc040cad53..af79b175af9be 100644 --- a/superset-frontend/src/components/Select/Select.stories.tsx +++ b/superset-frontend/src/components/Select/Select.stories.tsx @@ -313,7 +313,7 @@ const USERS = [ 'Claire', 'Benedetta', 'Ilenia', -]; +].sort(); export const AsyncSelect = ({ fetchOnlyOnSearch, @@ -429,6 +429,7 @@ export const AsyncSelect = ({ }; AsyncSelect.args = { + allowClear: false, allowNewOptions: false, fetchOnlyOnSearch: false, pageSize: 10, diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index 3756d4a5d412e..c9982e5ef9827 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -52,7 +52,7 @@ const OPTIONS = [ { label: 'Irfan', value: 18, gender: 'Male' }, { label: 'George', value: 19, gender: 'Male' }, { label: 'Ashfaq', value: 20, gender: 'Male' }, -]; +].sort((option1, option2) => option1.label.localeCompare(option2.label)); const loadOptions = async (search: string, page: number, pageSize: number) => { const totalCount = OPTIONS.length; @@ -100,6 +100,8 @@ const findSelectValue = () => const findAllSelectValues = () => waitFor(() => getElementsByClassName('.ant-select-selection-item')); +const clearAll = () => userEvent.click(screen.getByLabelText('close-circle')); + const type = (text: string) => { const select = getSelect(); userEvent.clear(select); @@ -127,6 +129,37 @@ test('inverts the selection', async () => { expect(await screen.findByLabelText('stop')).toBeInTheDocument(); }); +test('sort the options by label if no sort comparator is provided', async () => { + const unsortedOptions = [...OPTIONS].sort(() => Math.random()); + render(, + ); + await open(); + const options = await findAllSelectOptions(); + const optionsPage = OPTIONS.slice(0, defaultProps.pageSize); + const sortedOptions = optionsPage.sort(sortComparator); + options.forEach((option, key) => + expect(option).toHaveTextContent(sortedOptions[key].label), + ); +}); + test('displays the selected values first', async () => { render(); + const option3 = OPTIONS[2].label; + const option8 = OPTIONS[7].label; + await open(); + userEvent.click(await findSelectOption(option3)); + userEvent.click(await findSelectOption(option8)); + await type('{esc}'); + clearAll(); + await open(); + const options = await findAllSelectOptions(); + options.forEach((option, key) => + expect(option).toHaveTextContent(OPTIONS[key].label), + ); +}); + test('searches for label or value', async () => { const option = OPTIONS[11]; render( + a.order - b.order} + sortComparator={propertyComparator('order')} /> ); diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx index 0bb1d7a180ee6..e8d389d7e7430 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx @@ -55,6 +55,8 @@ import { AdvancedFrame, } from './components'; +const { propertyComparator } = Select; + const guessFrame = (timeRange: string): FrameType => { if (COMMON_RANGE_VALUES_SET.has(timeRange)) { return 'Common'; @@ -295,10 +297,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) { options={FRAME_OPTIONS} value={frame} onChange={onChangeFrame} - sortComparator={( - a: typeof FRAME_OPTIONS[number], - b: typeof FRAME_OPTIONS[number], - ) => a.order - b.order} + sortComparator={propertyComparator('order')} /> {frame !== 'No filter' && } {frame === 'Common' && ( diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx index 0f832a13055ce..dde6524ad6d29 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx @@ -41,6 +41,9 @@ import { FrameComponentProps, } from 'src/explore/components/controls/DateFilterControl/types'; +const { propertyComparator } = Select; +const sortComparator = propertyComparator('order'); + export function CustomFrame(props: FrameComponentProps) { const { customRange, matchedFlag } = customTimeRangeDecode(props.value); if (!matchedFlag) { @@ -121,10 +124,7 @@ export function CustomFrame(props: FrameComponentProps) { options={SINCE_MODE_OPTIONS} value={sinceMode} onChange={(value: string) => onChange('sinceMode', value)} - sortComparator={( - a: typeof SINCE_MODE_OPTIONS[number], - b: typeof SINCE_MODE_OPTIONS[number], - ) => a.order - b.order} + sortComparator={sortComparator} /> {sinceMode === 'specific' && ( @@ -159,10 +159,7 @@ export function CustomFrame(props: FrameComponentProps) { options={SINCE_GRAIN_OPTIONS} value={sinceGrain} onChange={(value: string) => onChange('sinceGrain', value)} - sortComparator={( - a: typeof SINCE_GRAIN_OPTIONS[number], - b: typeof SINCE_GRAIN_OPTIONS[number], - ) => a.order - b.order} + sortComparator={sortComparator} /> @@ -181,10 +178,7 @@ export function CustomFrame(props: FrameComponentProps) { options={UNTIL_MODE_OPTIONS} value={untilMode} onChange={(value: string) => onChange('untilMode', value)} - sortComparator={( - a: typeof UNTIL_MODE_OPTIONS[number], - b: typeof UNTIL_MODE_OPTIONS[number], - ) => a.order - b.order} + sortComparator={sortComparator} /> {untilMode === 'specific' && ( @@ -218,10 +212,7 @@ export function CustomFrame(props: FrameComponentProps) { options={UNTIL_GRAIN_OPTIONS} value={untilGrain} onChange={(value: string) => onChange('untilGrain', value)} - sortComparator={( - a: typeof UNTIL_GRAIN_OPTIONS[number], - b: typeof UNTIL_GRAIN_OPTIONS[number], - ) => a.order - b.order} + sortComparator={sortComparator} /> diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index 603c42cea97be..409d89f56d7ec 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -57,6 +57,8 @@ import { import { AlertReportCronScheduler } from './components/AlertReportCronScheduler'; import { NotificationMethod } from './components/NotificationMethod'; +const { propertyComparator } = Select; + const TIMEOUT_MIN = 1; const TEXT_BASED_VISUALIZATION_TYPES = [ 'pivot_table', @@ -1154,10 +1156,7 @@ const AlertReportModal: FunctionComponent = ({ currentAlert?.validator_config_json?.op || undefined } options={CONDITIONS} - sortComparator={( - a: typeof CONDITIONS[number], - b: typeof CONDITIONS[number], - ) => a.order - b.order} + sortComparator={propertyComparator('order')} /> @@ -1225,9 +1224,7 @@ const AlertReportModal: FunctionComponent = ({ onChange={onLogRetentionChange} value={currentAlert?.log_retention || DEFAULT_RETENTION} options={RETENTION_OPTIONS} - sortComparator={(a, b) => - (a.value as number) - (b.value as number) - } + sortComparator={propertyComparator('value')} /> From 4af728607b152449f5525b79eb9e030c2f3735c1 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Fri, 22 Oct 2021 14:02:10 -0300 Subject: [PATCH 3/3] Uses a named export for propertyComparator --- superset-frontend/src/components/Select/Select.tsx | 4 +--- .../src/dashboard/components/RefreshIntervalModal.tsx | 4 +--- .../ConditionalFormattingControl/FormattingPopoverContent.tsx | 4 +--- .../components/controls/DateFilterControl/DateFilterLabel.tsx | 4 +--- .../controls/DateFilterControl/components/CustomFrame.tsx | 3 +-- .../controls/SelectAsyncControl/SelectAsyncControl.test.tsx | 1 + superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx | 4 +--- 7 files changed, 7 insertions(+), 17 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index cc351ca739500..46b96666bc433 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -183,7 +183,7 @@ const defaultSortComparator = (a: AntdLabeledValue, b: AntdLabeledValue) => { * It creates a comparator to check for a specific property. * Can be used with string and number property values. * */ -const propertyComparator = (property: string) => ( +export const propertyComparator = (property: string) => ( a: AntdLabeledValue, b: AntdLabeledValue, ) => { @@ -684,6 +684,4 @@ const Select = ({ ); }; -Select.propertyComparator = propertyComparator; - export default Select; diff --git a/superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx b/superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx index 667e4cf33eaed..3a8aef03727c9 100644 --- a/superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx +++ b/superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx @@ -17,7 +17,7 @@ * under the License. */ import React, { RefObject } from 'react'; -import { Select } from 'src/components'; +import Select, { propertyComparator } from 'src/components/Select/Select'; import { t, styled } from '@superset-ui/core'; import Alert from 'src/components/Alert'; import Button from 'src/components/Button'; @@ -25,8 +25,6 @@ import Button from 'src/components/Button'; import ModalTrigger from 'src/components/ModalTrigger'; import { FormLabel } from 'src/components/Form'; -const { propertyComparator } = Select; - export const options = [ [0, t("Don't refresh")], [10, t('10 seconds')], diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx index 191b3a769dd88..252feff4ee314 100644 --- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx +++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx @@ -19,7 +19,7 @@ import React from 'react'; import { styled, t } from '@superset-ui/core'; import { Form, FormItem, FormProps } from 'src/components/Form'; -import { Select } from 'src/components'; +import Select, { propertyComparator } from 'src/components/Select/Select'; import { Col, InputNumber, Row } from 'src/common/components'; import Button from 'src/components/Button'; import { @@ -28,8 +28,6 @@ import { MULTIPLE_VALUE_COMPARATORS, } from './types'; -const { propertyComparator } = Select; - const FullWidthInputNumber = styled(InputNumber)` width: 100%; `; diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx index e8d389d7e7430..e5324a926bdd3 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx @@ -40,7 +40,7 @@ import Label, { Type } from 'src/components/Label'; import Popover from 'src/components/Popover'; import { Divider } from 'src/common/components'; import Icons from 'src/components/Icons'; -import { Select } from 'src/components'; +import Select, { propertyComparator } from 'src/components/Select/Select'; import { Tooltip } from 'src/components/Tooltip'; import { DEFAULT_TIME_RANGE } from 'src/explore/constants'; import { useDebouncedEffect } from 'src/explore/exploreUtils'; @@ -55,8 +55,6 @@ import { AdvancedFrame, } from './components'; -const { propertyComparator } = Select; - const guessFrame = (timeRange: string): FrameType => { if (COMMON_RANGE_VALUES_SET.has(timeRange)) { return 'Common'; diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx index dde6524ad6d29..a26d7ca765574 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx @@ -23,7 +23,7 @@ import { isInteger } from 'lodash'; import { Col, InputNumber, Row } from 'src/common/components'; import { DatePicker } from 'src/components/DatePicker'; import { Radio } from 'src/components/Radio'; -import { Select } from 'src/components'; +import Select, { propertyComparator } from 'src/components/Select/Select'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { SINCE_GRAIN_OPTIONS, @@ -41,7 +41,6 @@ import { FrameComponentProps, } from 'src/explore/components/controls/DateFilterControl/types'; -const { propertyComparator } = Select; const sortComparator = propertyComparator('order'); export function CustomFrame(props: FrameComponentProps) { diff --git a/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx b/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx index 0b0b3034a6395..bc78434850818 100644 --- a/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx @@ -44,6 +44,7 @@ jest.mock('src/components/Select/Select', () => ({ ), + propertyComparator: jest.fn(), })); fetchMock.get(datasetsOwnersEndpoint, { diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index 409d89f56d7ec..c8c4df66a5dd3 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -38,7 +38,7 @@ import { Switch } from 'src/components/Switch'; import Modal from 'src/components/Modal'; import TimezoneSelector from 'src/components/TimezoneSelector'; import { Radio } from 'src/components/Radio'; -import { Select } from 'src/components'; +import Select, { propertyComparator } from 'src/components/Select/Select'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import withToasts from 'src/components/MessageToasts/withToasts'; import Owner from 'src/types/Owner'; @@ -57,8 +57,6 @@ import { import { AlertReportCronScheduler } from './components/AlertReportCronScheduler'; import { NotificationMethod } from './components/NotificationMethod'; -const { propertyComparator } = Select; - const TIMEOUT_MIN = 1; const TEXT_BASED_VISUALIZATION_TYPES = [ 'pivot_table',