From 26423c63becace5a22847a4537f179974820bf88 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Wed, 11 Jan 2023 16:11:16 +0200
Subject: [PATCH] Add single selection mode to picker (#27624)
---
.../DataPicker/CardPicker/CardPickerContainer.tsx | 4 +++-
.../RawDataPicker/RawDataPickerContainer.tsx | 4 +++-
.../tests/DataPicker-Models.unit.spec.ts | 15 ++++++++-------
.../tests/DataPicker-Questions.unit.spec.ts | 15 ++++++++-------
.../tests/DataPicker-RawData.unit.spec.ts | 11 ++++++-----
.../containers/DataPicker/tests/common.tsx | 6 ++++++
.../src/metabase/containers/DataPicker/types.ts | 1 +
.../containers/DataPicker/useSelectedTables.ts | 13 ++++++-------
8 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerContainer.tsx b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerContainer.tsx
index a4f57a06a65..c1cdaff22ff 100644
--- a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerContainer.tsx
+++ b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerContainer.tsx
@@ -27,6 +27,7 @@ import CardPickerView from "./CardPickerView";
interface CardPickerOwnProps extends DataPickerProps {
targetModel: "model" | "question";
+ isMultiSelect?: boolean;
onBack?: () => void;
}
@@ -64,6 +65,7 @@ function CardPickerContainer({
schema: selectedSchema,
currentUser,
targetModel,
+ isMultiSelect,
allLoading,
onChange,
onBack,
@@ -72,7 +74,7 @@ function CardPickerContainer({
const { selectedTableIds, toggleTableIdSelection } = useSelectedTables({
initialValues: value.tableIds,
- mode: "multiple",
+ isMultiSelect,
});
const collectionsMap = useMemo(
diff --git a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerContainer.tsx b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerContainer.tsx
index 47f0d0a7ee9..8580aecb05c 100644
--- a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerContainer.tsx
+++ b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerContainer.tsx
@@ -29,6 +29,7 @@ interface TableListLoaderProps {
}
interface RawDataPickerOwnProps extends DataPickerProps {
+ isMultiSelect?: boolean;
onBack?: () => void;
}
@@ -37,6 +38,7 @@ type RawDataPickerProps = RawDataPickerOwnProps & DatabaseListLoaderProps;
function RawDataPicker({
value,
databases,
+ isMultiSelect,
allLoading,
onChange,
onBack,
@@ -45,7 +47,7 @@ function RawDataPicker({
const { selectedTableIds, toggleTableIdSelection } = useSelectedTables({
initialValues: value.tableIds,
- mode: "multiple",
+ isMultiSelect,
});
const selectedDatabase = useMemo(() => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
index 44ac210af3e..61d6e277854 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
@@ -80,23 +80,24 @@ describe("DataPicker — picking models", () => {
const { onChange } = await setup();
userEvent.click(screen.getByText(/Models/i));
- const listItem = await screen.findByRole("menuitem", {
- name: SAMPLE_MODEL.name,
- });
- userEvent.click(listItem);
+ userEvent.click(await screen.findByText(SAMPLE_MODEL.name));
+ userEvent.click(screen.getByText(SAMPLE_MODEL_2.name));
- expect(listItem).toHaveAttribute("aria-selected", "true");
+ const selectedItem = screen.getByRole("menuitem", {
+ name: SAMPLE_MODEL_2.name,
+ });
+ expect(selectedItem).toHaveAttribute("aria-selected", "true");
expect(onChange).toHaveBeenCalledWith({
type: "models",
databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
schemaId: ROOT_COLLECTION_MODEL_VIRTUAL_SCHEMA_ID,
collectionId: "root",
- tableIds: [getQuestionVirtualTableId(SAMPLE_MODEL.id)],
+ tableIds: [getQuestionVirtualTableId(SAMPLE_MODEL_2.id)],
});
});
it("allows to pick multiple models", async () => {
- const { onChange } = await setup();
+ const { onChange } = await setup({ isMultiSelect: true });
userEvent.click(screen.getByText(/Models/i));
userEvent.click(await screen.findByText(SAMPLE_MODEL.name));
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
index 320c707e4a1..00eec337014 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
@@ -78,23 +78,24 @@ describe("DataPicker — picking questions", () => {
const { onChange } = await setup();
userEvent.click(screen.getByText(/Saved Questions/i));
- const listItem = await screen.findByRole("menuitem", {
- name: SAMPLE_QUESTION.name,
- });
- userEvent.click(listItem);
+ userEvent.click(await screen.findByText(SAMPLE_QUESTION.name));
+ userEvent.click(screen.getByText(SAMPLE_QUESTION_2.name));
- expect(listItem).toHaveAttribute("aria-selected", "true");
+ const selectedItem = screen.getByRole("menuitem", {
+ name: SAMPLE_QUESTION_2.name,
+ });
+ expect(selectedItem).toHaveAttribute("aria-selected", "true");
expect(onChange).toHaveBeenCalledWith({
type: "questions",
databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
schemaId: ROOT_COLLECTION_QUESTIONS_VIRTUAL_SCHEMA_ID,
collectionId: "root",
- tableIds: [getQuestionVirtualTableId(SAMPLE_QUESTION.id)],
+ tableIds: [getQuestionVirtualTableId(SAMPLE_QUESTION_2.id)],
});
});
it("allows to pick multiple questions", async () => {
- const { onChange } = await setup();
+ const { onChange } = await setup({ isMultiSelect: true });
userEvent.click(screen.getByText(/Saved Questions/i));
userEvent.click(await screen.findByText(SAMPLE_QUESTION.name));
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts
index f05e431e73e..c509e7b51d4 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts
@@ -47,7 +47,7 @@ describe("DataPicker — picking raw data", () => {
});
it("allows to pick multiple tables", async () => {
- const { onChange } = await setup();
+ const { onChange } = await setup({ isMultiSelect: true });
userEvent.click(screen.getByText(/Raw Data/i));
userEvent.click(await screen.findByText(/Orders/i));
@@ -91,12 +91,13 @@ describe("DataPicker — picking raw data", () => {
const { onChange } = await setup();
userEvent.click(screen.getByText(/Raw Data/i));
- const tableListItem = await screen.findByRole("menuitem", {
+ userEvent.click(await screen.findByText("Orders"));
+ userEvent.click(screen.getByText("Products"));
+
+ const selectedItem = screen.getByRole("menuitem", {
name: /Products/i,
});
- userEvent.click(tableListItem);
-
- expect(tableListItem).toHaveAttribute("aria-selected", "true");
+ expect(selectedItem).toHaveAttribute("aria-selected", "true");
expect(onChange).toHaveBeenCalledWith({
type: "raw-data",
databaseId: SAMPLE_DATABASE.id,
diff --git a/frontend/src/metabase/containers/DataPicker/tests/common.tsx b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
index 7846c37c9a9..6dd81f5005b 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/common.tsx
+++ b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
@@ -82,10 +82,12 @@ export const SAMPLE_QUESTION_3 = createMockCard({
function DataPickerWrapper({
value: initialValue,
filters,
+ isMultiSelect,
onChange,
}: {
value: DataPickerValue;
filters?: DataPickerFiltersProp;
+ isMultiSelect?: boolean;
onChange: (value: DataPickerValue) => void;
}) {
const [value, setValue] = useDataPickerValue(initialValue);
@@ -93,6 +95,7 @@ function DataPickerWrapper({
<DataPicker
value={value}
filters={filters}
+ isMultiSelect={isMultiSelect}
onChange={(value: DataPickerValue) => {
setValue(value);
onChange(value);
@@ -104,6 +107,7 @@ function DataPickerWrapper({
interface SetupOpts {
initialValue?: DataPickerValue;
filters?: DataPickerFiltersProp;
+ isMultiSelect?: boolean;
hasDataAccess?: boolean;
hasEmptyDatabase?: boolean;
hasMultiSchemaDatabase?: boolean;
@@ -114,6 +118,7 @@ interface SetupOpts {
export async function setup({
initialValue = { tableIds: [] },
filters,
+ isMultiSelect = false,
hasDataAccess = true,
hasEmptyDatabase = false,
hasMultiSchemaDatabase = false,
@@ -174,6 +179,7 @@ export async function setup({
<DataPickerWrapper
value={initialValue}
filters={filters}
+ isMultiSelect={isMultiSelect}
onChange={onChange}
/>,
{
diff --git a/frontend/src/metabase/containers/DataPicker/types.ts b/frontend/src/metabase/containers/DataPicker/types.ts
index fa00a86605c..b8b84a512e8 100644
--- a/frontend/src/metabase/containers/DataPicker/types.ts
+++ b/frontend/src/metabase/containers/DataPicker/types.ts
@@ -35,6 +35,7 @@ export interface DataPickerProps {
value: DataPickerValue;
onChange: (value: DataPickerValue) => void;
filters?: DataPickerFiltersProp;
+ isMultiSelect?: boolean;
}
export type DataPickerSelectedItem = {
diff --git a/frontend/src/metabase/containers/DataPicker/useSelectedTables.ts b/frontend/src/metabase/containers/DataPicker/useSelectedTables.ts
index 74bc6ea9b78..3624bdeb8ae 100644
--- a/frontend/src/metabase/containers/DataPicker/useSelectedTables.ts
+++ b/frontend/src/metabase/containers/DataPicker/useSelectedTables.ts
@@ -4,12 +4,12 @@ import type { TableId } from "metabase-types/api";
interface SelectedTablesHookOpts {
initialValues?: TableId[];
- mode?: "single" | "multiple";
+ isMultiSelect?: boolean;
}
function useSelectedTables({
initialValues = [],
- mode = "single",
+ isMultiSelect,
}: SelectedTablesHookOpts = {}) {
const [selectedTableIds, setSelectedTableIds] = useState(
new Set(initialValues),
@@ -21,14 +21,13 @@ function useSelectedTables({
const addSelectedTableId = useCallback(
(id: TableId) => {
- const nextState =
- mode === "multiple"
- ? new Set([...selectedTableIds, id])
- : new Set([id]);
+ const nextState = isMultiSelect
+ ? new Set([...selectedTableIds, id])
+ : new Set([id]);
setSelectedTableIds(nextState);
return Array.from(nextState);
},
- [selectedTableIds, mode],
+ [selectedTableIds, isMultiSelect],
);
const removeSelectedTableId = useCallback(
--
GitLab