From 344faf7fcf51235e81f479a06f4c2c9dffe81086 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Thu, 12 Sep 2024 14:37:18 -0400
Subject: [PATCH] Add Select all in ListValuePicker (#47865)
* Unify FilterPickerForm
* Unify FilterPickerForm
* Add Select all in ListValuePicker
* Layout changes
* Layout changes
* Add tests
* Add tests
* Revert "Unify FilterPickerForm"
This reverts commit a7dfe8848d9b62d9e8371ddb47451c8d45002cbc.
* Revert "Unify FilterPickerForm"
This reverts commit 8f002b87212936996b7dc1cd04f16e06a7b0c967.
---
.../FilterValuePicker.unit.spec.tsx | 48 ++++---
.../ListValuePicker/ListValuePicker.tsx | 56 +++++---
.../ListValuePicker.unit.spec.tsx | 129 ++++++++++++++++++
3 files changed, 193 insertions(+), 40 deletions(-)
create mode 100644 frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.unit.spec.tsx
diff --git a/frontend/src/metabase/querying/filters/components/FilterValuePicker/FilterValuePicker.unit.spec.tsx b/frontend/src/metabase/querying/filters/components/FilterValuePicker/FilterValuePicker.unit.spec.tsx
index df14c65d0e7..293a9fe2ce2 100644
--- a/frontend/src/metabase/querying/filters/components/FilterValuePicker/FilterValuePicker.unit.spec.tsx
+++ b/frontend/src/metabase/querying/filters/components/FilterValuePicker/FilterValuePicker.unit.spec.tsx
@@ -369,12 +369,13 @@ describe("StringFilterValuePicker", () => {
});
const checkboxes = screen.getAllByRole("checkbox");
- expect(checkboxes[0]).toHaveAccessibleName("In-progress");
- expect(checkboxes[0]).toBeChecked();
- expect(checkboxes[1]).toHaveAccessibleName("To-do");
- expect(checkboxes[1]).not.toBeChecked();
- expect(checkboxes[2]).toHaveAccessibleName("Completed");
+ expect(checkboxes[0]).toHaveAccessibleName("Select all");
+ expect(checkboxes[1]).toHaveAccessibleName("In-progress");
+ expect(checkboxes[1]).toBeChecked();
+ expect(checkboxes[2]).toHaveAccessibleName("To-do");
expect(checkboxes[2]).not.toBeChecked();
+ expect(checkboxes[3]).toHaveAccessibleName("Completed");
+ expect(checkboxes[3]).not.toBeChecked();
});
it("should not elevate selected field values after checking an item", async () => {
@@ -404,12 +405,13 @@ describe("StringFilterValuePicker", () => {
/>,
);
const checkboxes = screen.getAllByRole("checkbox");
- expect(checkboxes[0]).toHaveAccessibleName("In-progress");
- expect(checkboxes[0]).toBeChecked();
- expect(checkboxes[1]).toHaveAccessibleName("To-do");
- expect(checkboxes[1]).not.toBeChecked();
- expect(checkboxes[2]).toHaveAccessibleName("Completed");
- expect(checkboxes[2]).toBeChecked();
+ expect(checkboxes[0]).toHaveAccessibleName("Select all");
+ expect(checkboxes[1]).toHaveAccessibleName("In-progress");
+ expect(checkboxes[1]).toBeChecked();
+ expect(checkboxes[2]).toHaveAccessibleName("To-do");
+ expect(checkboxes[2]).not.toBeChecked();
+ expect(checkboxes[3]).toHaveAccessibleName("Completed");
+ expect(checkboxes[3]).toBeChecked();
});
it("should not elevate selected field values after unchecking an item", async () => {
@@ -439,12 +441,13 @@ describe("StringFilterValuePicker", () => {
/>,
);
const checkboxes = screen.getAllByRole("checkbox");
- expect(checkboxes[0]).toHaveAccessibleName("In-progress");
- expect(checkboxes[0]).not.toBeChecked();
- expect(checkboxes[1]).toHaveAccessibleName("Completed");
- expect(checkboxes[1]).toBeChecked();
- expect(checkboxes[2]).toHaveAccessibleName("To-do");
- expect(checkboxes[2]).not.toBeChecked();
+ expect(checkboxes[0]).toHaveAccessibleName("Select all");
+ expect(checkboxes[1]).toHaveAccessibleName("In-progress");
+ expect(checkboxes[1]).not.toBeChecked();
+ expect(checkboxes[2]).toHaveAccessibleName("Completed");
+ expect(checkboxes[2]).toBeChecked();
+ expect(checkboxes[3]).toHaveAccessibleName("To-do");
+ expect(checkboxes[3]).not.toBeChecked();
});
it("should handle empty field values", async () => {
@@ -1024,12 +1027,13 @@ describe("NumberFilterValuePicker", () => {
});
const checkboxes = screen.getAllByRole("checkbox");
- expect(checkboxes[0]).toHaveAccessibleName("In-progress");
- expect(checkboxes[0]).toBeChecked();
- expect(checkboxes[1]).toHaveAccessibleName("To-do");
- expect(checkboxes[1]).not.toBeChecked();
- expect(checkboxes[2]).toHaveAccessibleName("Completed");
+ expect(checkboxes[0]).toHaveAccessibleName("Select all");
+ expect(checkboxes[1]).toHaveAccessibleName("In-progress");
+ expect(checkboxes[1]).toBeChecked();
+ expect(checkboxes[2]).toHaveAccessibleName("To-do");
expect(checkboxes[2]).not.toBeChecked();
+ expect(checkboxes[3]).toHaveAccessibleName("Completed");
+ expect(checkboxes[3]).not.toBeChecked();
});
});
diff --git a/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.tsx b/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.tsx
index 169d53a99f7..2ab69dfede9 100644
--- a/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.tsx
+++ b/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.tsx
@@ -57,11 +57,21 @@ function CheckboxListPicker({
elevatedValues,
);
const visibleOptions = searchOptions(options, searchValue);
+ const isAll = options.length === selectedValues.length;
+ const isNone = selectedValues.length === 0;
const handleInputChange = (event: ChangeEvent<HTMLInputElement>) => {
setSearchValue(event.currentTarget.value);
};
+ const handleToggleAll = () => {
+ if (isAll) {
+ onChange([]);
+ } else {
+ onChange(options.map(option => option.value));
+ }
+ };
+
return (
<Stack>
<TextInput
@@ -70,24 +80,34 @@ function CheckboxListPicker({
autoFocus={autoFocus}
onChange={handleInputChange}
/>
- <Checkbox.Group value={selectedValues} onChange={onChange}>
- {visibleOptions.length > 0 ? (
- <Stack>
- {visibleOptions.map(option => (
- <Checkbox
- key={option.value}
- value={option.value}
- label={option.label}
- />
- ))}
- </Stack>
- ) : (
- <Stack c="text-light" justify="center" align="center">
- <Icon name="search" size={40} />
- <Text c="text-medium" fw="bold">{t`Didn't find anything`}</Text>
- </Stack>
- )}
- </Checkbox.Group>
+ {visibleOptions.length > 0 ? (
+ <Stack>
+ <Checkbox
+ variant="stacked"
+ label={isAll ? `Select none` : t`Select all`}
+ checked={isAll}
+ indeterminate={!isAll && !isNone}
+ fw="bold"
+ onChange={handleToggleAll}
+ />
+ <Checkbox.Group value={selectedValues} onChange={onChange}>
+ <Stack>
+ {visibleOptions.map(option => (
+ <Checkbox
+ key={option.value}
+ value={option.value}
+ label={option.label}
+ />
+ ))}
+ </Stack>
+ </Checkbox.Group>
+ </Stack>
+ ) : (
+ <Stack c="text-light" justify="center" align="center">
+ <Icon name="search" size={40} />
+ <Text c="text-medium" fw="bold">{t`Didn't find anything`}</Text>
+ </Stack>
+ )}
</Stack>
);
}
diff --git a/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.unit.spec.tsx b/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.unit.spec.tsx
new file mode 100644
index 00000000000..06835d917b3
--- /dev/null
+++ b/frontend/src/metabase/querying/filters/components/FilterValuePicker/ListValuePicker/ListValuePicker.unit.spec.tsx
@@ -0,0 +1,129 @@
+import userEvent from "@testing-library/user-event";
+
+import { renderWithProviders, screen } from "__support__/ui";
+import type { FieldValue } from "metabase-types/api";
+import { PRODUCT_CATEGORY_VALUES } from "metabase-types/api/mocks/presets";
+
+import { ListValuePicker } from "./ListValuePicker";
+
+type SetupOpts = {
+ fieldValues?: FieldValue[];
+ selectedValues?: string[];
+ placeholder?: string;
+ shouldCreate?: (query: string, values: string[]) => boolean;
+ autoFocus?: boolean;
+ compact?: boolean;
+};
+
+function setup({
+ fieldValues = [],
+ selectedValues = [],
+ placeholder = "Search the list",
+ shouldCreate,
+ autoFocus,
+ compact,
+}: SetupOpts) {
+ const onChange = jest.fn();
+ const onFocus = jest.fn();
+ const onBlur = jest.fn();
+
+ renderWithProviders(
+ <ListValuePicker
+ fieldValues={fieldValues}
+ selectedValues={selectedValues}
+ placeholder={placeholder}
+ shouldCreate={shouldCreate}
+ autoFocus={autoFocus}
+ compact={compact}
+ onChange={onChange}
+ onFocus={onFocus}
+ onBlur={onBlur}
+ />,
+ );
+
+ return { onChange, onFocus, onBlur };
+}
+
+describe("ListValuePicker", () => {
+ describe("checkbox list mode", () => {
+ const allOptions = PRODUCT_CATEGORY_VALUES.values;
+ const allValues = allOptions.map(([value]) => String(value));
+
+ it("should allow to select all options", async () => {
+ const { onChange } = setup({
+ fieldValues: allOptions,
+ selectedValues: [],
+ });
+
+ const checkbox = screen.getByLabelText("Select all");
+ expect(checkbox).not.toBeChecked();
+ await userEvent.click(checkbox);
+
+ expect(onChange).toHaveBeenCalledWith(allValues);
+ });
+
+ it("should allow to select all options when some are selected", async () => {
+ const { onChange } = setup({
+ fieldValues: allOptions,
+ selectedValues: [allValues[0]],
+ });
+
+ const checkbox = screen.getByLabelText("Select all");
+ expect(checkbox).not.toBeChecked();
+ await userEvent.click(checkbox);
+
+ expect(onChange).toHaveBeenCalledWith(allValues);
+ });
+
+ it("should allow to select all options after search", async () => {
+ const { onChange } = setup({
+ fieldValues: allOptions,
+ selectedValues: [],
+ });
+
+ await userEvent.type(
+ screen.getByPlaceholderText("Search the list"),
+ allValues[0],
+ );
+ expect(screen.getByLabelText(allValues[0])).toBeInTheDocument();
+ expect(screen.queryByLabelText(allValues[1])).not.toBeInTheDocument();
+
+ const checkbox = screen.getByLabelText("Select all");
+ expect(checkbox).not.toBeChecked();
+ await userEvent.click(checkbox);
+ expect(onChange).toHaveBeenCalledWith(allValues);
+ });
+
+ it("should allow to deselect all options", async () => {
+ const { onChange } = setup({
+ fieldValues: allOptions,
+ selectedValues: allValues,
+ });
+
+ const checkbox = screen.getByLabelText("Select none");
+ expect(checkbox).toBeChecked();
+ await userEvent.click(checkbox);
+
+ expect(onChange).toHaveBeenCalledWith([]);
+ });
+
+ it("should allow to deselect all options after search", async () => {
+ const { onChange } = setup({
+ fieldValues: allOptions,
+ selectedValues: allValues,
+ });
+
+ await userEvent.type(
+ screen.getByPlaceholderText("Search the list"),
+ allValues[0],
+ );
+ expect(screen.getByLabelText(allValues[0])).toBeInTheDocument();
+ expect(screen.queryByLabelText(allValues[1])).not.toBeInTheDocument();
+
+ const checkbox = screen.getByLabelText("Select none");
+ expect(checkbox).toBeChecked();
+ await userEvent.click(checkbox);
+ expect(onChange).toHaveBeenCalledWith([]);
+ });
+ });
+});
--
GitLab