Skip to content

Commit

Permalink
feat: Select all for synchronous select (#22084)
Browse files Browse the repository at this point in the history
Co-authored-by: GITHUB_USERNAME <EMAIL>
  • Loading branch information
cccs-RyanK authored Jan 18, 2023
1 parent ad758c0 commit 02c9242
Show file tree
Hide file tree
Showing 10 changed files with 429 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,9 @@ describe('Horizontal FilterBar', () => {
saveNativeFilterSettings([SAMPLE_CHART]);
cy.getBySel('filter-bar').within(() => {
cy.get(nativeFilters.filterItem).contains('Albania').should('be.visible');
cy.get(nativeFilters.filterItem).contains('+1').should('be.visible');
cy.get(nativeFilters.filterItem).contains('+ 1 ...').should('be.visible');
cy.get('.ant-select-selection-search-input').click();
cy.get(nativeFilters.filterItem).contains('+2').should('be.visible');
cy.get(nativeFilters.filterItem).contains('+ 2 ...').should('be.visible');
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/components/Select/AsyncSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import {
TOKEN_SEPARATORS,
DEFAULT_SORT_COMPARATOR,
} from './constants';
import { oneLineTagRender } from './CustomTag';
import { customTagRender } from './CustomTag';

const Error = ({ error }: { error: string }) => (
<StyledError>
Expand Down Expand Up @@ -517,7 +517,7 @@ const AsyncSelect = forwardRef(
)
}
oneLine={oneLine}
tagRender={oneLine ? oneLineTagRender : undefined}
tagRender={customTagRender}
{...props}
ref={ref}
>
Expand Down
21 changes: 13 additions & 8 deletions superset-frontend/src/components/Select/CustomTag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { styled } from '@superset-ui/core';
import { useCSSTextTruncation } from 'src/hooks/useTruncation';
import { Tooltip } from '../Tooltip';
import { CustomTagProps } from './types';
import { SELECT_ALL_VALUE } from './utils';
import { NoElement } from './styles';

const StyledTag = styled(AntdTag)`
& .ant-tag-close-icon {
Expand Down Expand Up @@ -51,10 +53,10 @@ const Tag = (props: any) => {
};

/**
* Custom tag renderer dedicated for oneLine mode
* Custom tag renderer
*/
export const oneLineTagRender = (props: CustomTagProps) => {
const { label } = props;
export const customTagRender = (props: CustomTagProps) => {
const { label, value } = props;

const onPreventMouseDown = (event: React.MouseEvent<HTMLElement>) => {
// if close icon is clicked, stop propagation to avoid opening the dropdown
Expand All @@ -69,9 +71,12 @@ export const oneLineTagRender = (props: CustomTagProps) => {
}
};

return (
<Tag onMouseDown={onPreventMouseDown} {...props}>
{label}
</Tag>
);
if (value !== SELECT_ALL_VALUE) {
return (
<Tag onMouseDown={onPreventMouseDown} {...(props as object)}>
{label}
</Tag>
);
}
return <NoElement />;
};
8 changes: 8 additions & 0 deletions superset-frontend/src/components/Select/Select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ const ARG_TYPES = {
Requires '"mode=multiple"'.
`,
},
maxTagCount: {
defaultValue: 4,
description: `Sets maxTagCount attribute. The overflow tag is displayed in
place of the remaining items.
Requires '"mode=multiple"'.
`,
},
};

const mountHeader = (type: String) => {
Expand Down Expand Up @@ -207,6 +214,7 @@ InteractiveSelect.args = {
placeholder: 'Select ...',
optionFilterProps: ['value', 'label', 'custom'],
oneLine: false,
maxTagCount: 4,
};

InteractiveSelect.argTypes = {
Expand Down
225 changes: 215 additions & 10 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
import React from 'react';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { Select } from 'src/components';
import Select from 'src/components/Select/Select';
import { SELECT_ALL_VALUE } from './utils';

const ARIA_LABEL = 'Test';
const NEW_OPTION = 'Kyle';
Expand Down Expand Up @@ -64,6 +65,9 @@ const defaultProps = {
showSearch: true,
};

const selectAllOptionLabel = (numOptions: number) =>
`${String(SELECT_ALL_VALUE)} (${numOptions})`;

const getElementByClassName = (className: string) =>
document.querySelector(className)! as HTMLElement;

Expand All @@ -89,7 +93,12 @@ const findSelectValue = () =>
waitFor(() => getElementByClassName('.ant-select-selection-item'));

const findAllSelectValues = () =>
waitFor(() => getElementsByClassName('.ant-select-selection-item'));
waitFor(() => [...getElementsByClassName('.ant-select-selection-item')]);

const findAllCheckedValues = () =>
waitFor(() => [
...getElementsByClassName('.ant-select-item-option-selected'),
]);

const clearAll = () => userEvent.click(screen.getByLabelText('close-circle'));

Expand Down Expand Up @@ -209,26 +218,37 @@ test('should sort selected to the top when in multi mode', async () => {
let labels = originalLabels.slice();

await open();
userEvent.click(await findSelectOption(labels[1]));
expect(await matchOrder(labels)).toBe(true);
userEvent.click(await findSelectOption(labels[2]));
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
).toBe(true);

await type('{esc}');
await open();
labels = labels.splice(1, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true);
labels = labels.splice(2, 1).concat(labels);
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
).toBe(true);

await open();
userEvent.click(await findSelectOption(labels[5]));
await type('{esc}');
await open();
labels = [labels.splice(0, 1)[0], labels.splice(4, 1)[0]].concat(labels);
expect(await matchOrder(labels)).toBe(true);
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
).toBe(true);

// should revert to original order
clearAll();
await type('{esc}');
await open();
expect(await matchOrder(originalLabels)).toBe(true);
expect(
await matchOrder([
selectAllOptionLabel(originalLabels.length),
...originalLabels,
]),
).toBe(true);
});

test('searches for label or value', async () => {
Expand Down Expand Up @@ -440,15 +460,15 @@ test('changes the selected item in single mode', async () => {
label: firstOption.label,
value: firstOption.value,
}),
firstOption,
expect.objectContaining(firstOption),
);
userEvent.click(await findSelectOption(secondOption.label));
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
label: secondOption.label,
value: secondOption.value,
}),
secondOption,
expect.objectContaining(secondOption),
);
expect(await findSelectValue()).toHaveTextContent(secondOption.label);
});
Expand Down Expand Up @@ -566,6 +586,136 @@ test('finds an element with a numeric value and does not duplicate the options',
expect(await querySelectOption('11')).not.toBeInTheDocument();
});

test('render "Select all" for multi select', async () => {
render(<Select {...defaultProps} mode="multiple" options={OPTIONS} />);
await open();
const options = await findAllSelectOptions();
expect(options[0]).toHaveTextContent(selectAllOptionLabel(OPTIONS.length));
});

test('does not render "Select all" for single select', async () => {
render(<Select {...defaultProps} options={OPTIONS} mode="single" />);
await open();
expect(
screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
).not.toBeInTheDocument();
});

test('does not render "Select all" for an empty multiple select', async () => {
render(<Select {...defaultProps} options={[]} mode="multiple" />);
await open();
expect(
screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
).not.toBeInTheDocument();
});

test('does not render "Select all" when searching', async () => {
render(<Select {...defaultProps} options={OPTIONS} mode="multiple" />);
await open();
await type('Select');
expect(
screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
).not.toBeInTheDocument();
});

test('does not render "Select all" as one of the tags after selection', async () => {
render(<Select {...defaultProps} options={OPTIONS} mode="multiple" />);
await open();
userEvent.click(await findSelectOption(selectAllOptionLabel(OPTIONS.length)));
const values = await findAllSelectValues();
expect(values[0]).not.toHaveTextContent(selectAllOptionLabel(OPTIONS.length));
});

test('keeps "Select all" at the top after a selection', async () => {
const selected = OPTIONS[2];
render(
<Select
{...defaultProps}
options={OPTIONS.slice(0, 10)}
mode="multiple"
value={[selected]}
/>,
);
await open();
const options = await findAllSelectOptions();
expect(options[0]).toHaveTextContent(selectAllOptionLabel(10));
expect(options[1]).toHaveTextContent(selected.label);
});

test('selects all values', async () => {
render(
<Select
{...defaultProps}
options={OPTIONS}
mode="multiple"
maxTagCount={0}
/>,
);
await open();
userEvent.click(await findSelectOption(selectAllOptionLabel(OPTIONS.length)));
const values = await findAllSelectValues();
expect(values.length).toBe(1);
expect(values[0]).toHaveTextContent(`+ ${OPTIONS.length} ...`);
});

test('unselects all values', async () => {
render(
<Select
{...defaultProps}
options={OPTIONS}
mode="multiple"
maxTagCount={0}
/>,
);
await open();
userEvent.click(await findSelectOption(selectAllOptionLabel(OPTIONS.length)));
let values = await findAllSelectValues();
expect(values.length).toBe(1);
expect(values[0]).toHaveTextContent(`+ ${OPTIONS.length} ...`);
userEvent.click(await findSelectOption(selectAllOptionLabel(OPTIONS.length)));
values = await findAllSelectValues();
expect(values.length).toBe(0);
});

test('deselecting a value also deselects "Select all"', async () => {
render(
<Select
{...defaultProps}
options={OPTIONS.slice(0, 10)}
mode="multiple"
maxTagCount={0}
/>,
);
await open();
userEvent.click(await findSelectOption(selectAllOptionLabel(10)));
let values = await findAllCheckedValues();
expect(values[0]).toHaveTextContent(selectAllOptionLabel(10));
userEvent.click(await findSelectOption(OPTIONS[0].label));
values = await findAllCheckedValues();
expect(values[0]).not.toHaveTextContent(selectAllOptionLabel(10));
});

test('selecting all values also selects "Select all"', async () => {
render(
<Select
{...defaultProps}
options={OPTIONS.slice(0, 10)}
mode="multiple"
maxTagCount={0}
/>,
);
await open();
const options = await findAllSelectOptions();
options.forEach((option, index) => {
// skip select all
if (index > 0) {
userEvent.click(option);
}
});
const values = await findAllSelectValues();
expect(values[0]).toHaveTextContent(`+ 10 ...`);
});

test('Renders only 1 tag and an overflow tag in oneLine mode', () => {
render(
<Select
Expand Down Expand Up @@ -614,6 +764,61 @@ test('Renders only an overflow tag if dropdown is open in oneLine mode', async (
expect(withinSelector.getByText('+ 2 ...')).toBeVisible();
});

test('+N tag does not count the "Select All" option', async () => {
render(
<Select
{...defaultProps}
options={OPTIONS.slice(0, 10)}
mode="multiple"
maxTagCount={0}
/>,
);
await open();
userEvent.click(await findSelectOption(selectAllOptionLabel(10)));
const values = await findAllSelectValues();
// maxTagCount is 0 so the +N tag should be + 10 ...
expect(values[0]).toHaveTextContent('+ 10 ...');
});

test('"Select All" is checked when unchecking a newly added option and all the other options are still selected', async () => {
render(
<Select
{...defaultProps}
options={OPTIONS.slice(0, 10)}
mode="multiple"
allowNewOptions
/>,
);
await open();
userEvent.click(await findSelectOption(selectAllOptionLabel(10)));
expect(await findSelectOption(selectAllOptionLabel(10))).toBeInTheDocument();
// add a new option
await type(`${NEW_OPTION}{enter}`);
expect(await findSelectOption(selectAllOptionLabel(11))).toBeInTheDocument();
expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
// select all should be selected
let values = await findAllCheckedValues();
expect(values[0]).toHaveTextContent(selectAllOptionLabel(11));
// remove new option
userEvent.click(await findSelectOption(NEW_OPTION));
// select all should still be selected
values = await findAllCheckedValues();
expect(values[0]).toHaveTextContent(selectAllOptionLabel(10));
expect(await findSelectOption(selectAllOptionLabel(10))).toBeInTheDocument();
});

test('does not render "Select All" when there are 0 or 1 options', async () => {
render(
<Select {...defaultProps} options={[]} mode="multiple" allowNewOptions />,
);
await open();
expect(screen.queryByText(selectAllOptionLabel(0))).not.toBeInTheDocument();
await type(`${NEW_OPTION}{enter}`);
expect(screen.queryByText(selectAllOptionLabel(1))).not.toBeInTheDocument();
await type(`Kyle2{enter}`);
expect(screen.queryByText(selectAllOptionLabel(2))).toBeInTheDocument();
});

/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
Expand Down
Loading

0 comments on commit 02c9242

Please sign in to comment.