From 55be249870251b4fa6186d2fa25177e77719335c Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Mon, 25 Oct 2021 08:47:01 -0300 Subject: [PATCH] fix: Order of Select items when unselecting (#17169) * fix: Order of Select items when unselecting * Adds a property comparator that supports strings and numbers * Uses a named export for propertyComparator --- .../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 | 93 +++++++++++++------ .../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 | 30 +++--- .../DateFilterControl/DateFilterLabel.tsx | 3 +- .../components/CustomFrame.tsx | 8 +- .../controls/DateFilterControl/types.ts | 1 + .../DateFilterControl/utils/constants.ts | 44 +++++---- .../SelectAsyncControl.test.tsx | 1 + .../src/views/CRUD/alert/AlertReportModal.tsx | 11 ++- 17 files changed, 216 insertions(+), 112 deletions(-) diff --git a/superset-frontend/src/addSlice/AddSliceContainer.tsx b/superset-frontend/src/addSlice/AddSliceContainer.tsx index 8dc1cab8f4..db8da84beb 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 dd6b824c89..45d91f1c97 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 938cc040ca..af79b175af 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 3756d4a5d4..c9982e5ef9 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( +