From f43a15076d26c02681d0a4579c4d703c7ea6b9ca Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Tue, 25 Jun 2024 11:17:18 +0200
Subject: [PATCH] Refactor: Time-series chrome component (#44568)
* Move SimpleSpecificDatePicker to the form
* Extract `isSpecificValue` helper
* Integrate the toggle into `SimpleDateIntervalPicker`
* Make DateOperatorPicker do only one thing
Separation of concerns - lift SimpleRelativeDatePicker up to the form.
* Fix unit tests
* Add back the test for the `SimpleSpecificDatePicker`
* Keep the rendering logic in the `SimpleDatePicker`
---
.../DateOperatorPicker/DateOperatorPicker.tsx | 33 ++---
.../DateOperatorPicker.unit.spec.tsx | 119 ------------------
.../SimpleDateIntervalPicker.tsx | 32 ++---
.../SimpleDateIntervalPicker.unit.spec.tsx | 47 +++++++
.../SimpleRelativeDatePicker.tsx | 20 ---
.../SimpleRelativeDatePicker/index.ts | 1 -
.../DatePicker/RelativeDatePicker/index.ts | 1 -
.../SimpleDatePicker/SimpleDatePicker.tsx | 14 +++
.../SimpleDatePicker.unit.spec.tsx | 36 ++++++
.../SimpleSpecificDatePicker.unit.spec.tsx | 41 ++++++
.../DatePicker/SpecificDatePicker/utils.ts | 7 ++
11 files changed, 172 insertions(+), 179 deletions(-)
delete mode 100644 frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/SimpleRelativeDatePicker.tsx
delete mode 100644 frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/index.ts
create mode 100644 frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/SimpleSpecificDatePicker/SimpleSpecificDatePicker.unit.spec.tsx
diff --git a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx
index f3a24b60e3e..4055cf49f3d 100644
--- a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx
+++ b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx
@@ -1,11 +1,7 @@
import { useMemo } from "react";
-import { Stack, Select } from "metabase/ui";
+import { Select } from "metabase/ui";
-import { SimpleRelativeDatePicker } from "../RelativeDatePicker";
-import { IncludeCurrentSwitch } from "../RelativeDatePicker/IncludeCurrentSwitch";
-import { isIntervalValue, isRelativeValue } from "../RelativeDatePicker/utils";
-import { SimpleSpecificDatePicker } from "../SpecificDatePicker";
import type { DatePickerOperator, DatePickerValue } from "../types";
import { getAvailableOptions, getOptionType, setOptionType } from "./utils";
@@ -37,24 +33,13 @@ export function DateOperatorPicker({
};
return (
- <Stack>
- <Select
- data={options}
- value={optionType}
- onChange={handleChange}
- style={{
- flex: 1,
- }}
- />
- {isRelativeValue(value) && (
- <SimpleRelativeDatePicker value={value} onChange={onChange} />
- )}
- {isRelativeValue(value) && isIntervalValue(value) && (
- <IncludeCurrentSwitch value={value} onChange={onChange} />
- )}
- {value?.type === "specific" && (
- <SimpleSpecificDatePicker value={value} onChange={onChange} />
- )}
- </Stack>
+ <Select
+ data={options}
+ value={optionType}
+ onChange={handleChange}
+ style={{
+ flex: 1,
+ }}
+ />
);
}
diff --git a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx
index f0b99d7fe52..ad5ad59824d 100644
--- a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx
+++ b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx
@@ -42,123 +42,4 @@ describe("DateOperatorPicker", () => {
unit: "day",
});
});
-
- it("should be able to change a specific date value", async () => {
- const { onChange } = setup({
- value: {
- type: "specific",
- operator: "=",
- values: [new Date(2015, 1, 10)],
- },
- });
-
- await userEvent.click(screen.getByText("15"));
-
- expect(onChange).toHaveBeenCalledWith({
- type: "specific",
- operator: "=",
- values: [new Date(2015, 1, 15)],
- });
- });
-
- it("should be able to change a relative date value", async () => {
- const { onChange } = setup({
- value: {
- type: "relative",
- value: 1,
- unit: "month",
- },
- });
-
- await userEvent.type(screen.getByLabelText("Interval"), "2");
-
- expect(onChange).toHaveBeenCalledWith({
- type: "relative",
- value: 12,
- unit: "month",
- });
- });
-
- describe("Include current switch", () => {
- it("should not show 'Include current' switch by default", async () => {
- setup();
-
- await userEvent.click(screen.getByDisplayValue("All time"));
- expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument();
- });
-
- it("should not show 'Include current' switch by for the specific date value", async () => {
- setup();
-
- await userEvent.click(screen.getByDisplayValue("All time"));
- await userEvent.click(screen.getByText("Between"));
- expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument();
- });
-
- it("should not show 'Include current' switch by for the `Current` relative date", async () => {
- setup();
-
- await userEvent.click(screen.getByDisplayValue("All time"));
- await userEvent.click(screen.getByText("Current"));
- expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument();
- });
-
- it("should show 'Include current' switch and work for the relative `Previous` or `Next`", async () => {
- setup({
- value: {
- type: "relative",
- value: 1,
- unit: "month",
- },
- });
-
- expect(screen.getByDisplayValue("Next")).toBeInTheDocument();
- expect(screen.getByLabelText("Include this month")).toBeInTheDocument();
- });
-
- it("should turn the 'Include current' switch on and register that change", async () => {
- const { onChange } = setup({
- value: {
- type: "relative",
- value: 1,
- unit: "month",
- },
- });
-
- await userEvent.click(screen.getByLabelText("Include this month"));
-
- expect(onChange).toHaveBeenCalledWith({
- options: {
- "include-current": true,
- },
- type: "relative",
- value: 1,
- unit: "month",
- });
- });
-
- it("should turn the 'Include current' switch off and register that change", async () => {
- const { onChange } = setup({
- value: {
- options: {
- "include-current": true,
- },
- type: "relative",
- value: 1,
- unit: "month",
- },
- });
-
- await userEvent.click(screen.getByLabelText("Include this month"));
-
- expect(onChange).toHaveBeenCalledWith({
- options: {
- "include-current": false,
- },
- type: "relative",
- value: 1,
- unit: "month",
- });
- });
- });
});
diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.tsx b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.tsx
index 873046bcfc0..8603e99fa1c 100644
--- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.tsx
+++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.tsx
@@ -2,6 +2,7 @@ import { t } from "ttag";
import { Group, NumberInput, Select } from "metabase/ui";
+import { IncludeCurrentSwitch } from "../../IncludeCurrentSwitch";
import type { DateIntervalValue } from "../../types";
import { getInterval, getUnitOptions, setInterval } from "../../utils";
import { setUnit } from "../utils";
@@ -32,19 +33,22 @@ export function SimpleDateIntervalPicker({
};
return (
- <Group>
- <NumberInput
- value={interval}
- aria-label={t`Interval`}
- w="4rem"
- onChange={handleIntervalChange}
- />
- <Select
- data={unitOptions}
- value={value.unit}
- aria-label={t`Unit`}
- onChange={handleUnitChange}
- />
- </Group>
+ <>
+ <Group>
+ <NumberInput
+ value={interval}
+ aria-label={t`Interval`}
+ w="4rem"
+ onChange={handleIntervalChange}
+ />
+ <Select
+ data={unitOptions}
+ value={value.unit}
+ aria-label={t`Unit`}
+ onChange={handleUnitChange}
+ />
+ </Group>
+ <IncludeCurrentSwitch value={value} onChange={onChange} />
+ </>
);
}
diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.unit.spec.tsx b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.unit.spec.tsx
index b438a2e685e..6486018db97 100644
--- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.unit.spec.tsx
+++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/SimpleDateIntervalPicker/SimpleDateIntervalPicker.unit.spec.tsx
@@ -132,4 +132,51 @@ describe("SimpleDateIntervalPicker", () => {
});
},
);
+
+ describe("Include current switch", () => {
+ it("should turn the 'Include current' switch on and register that change", async () => {
+ const { onChange } = setup({
+ value: {
+ type: "relative",
+ value: 1,
+ unit: "month",
+ },
+ });
+
+ await userEvent.click(screen.getByLabelText("Include this month"));
+
+ expect(onChange).toHaveBeenCalledWith({
+ options: {
+ "include-current": true,
+ },
+ type: "relative",
+ value: 1,
+ unit: "month",
+ });
+ });
+
+ it("should turn the 'Include current' switch off and register that change", async () => {
+ const { onChange } = setup({
+ value: {
+ options: {
+ "include-current": true,
+ },
+ type: "relative",
+ value: 1,
+ unit: "month",
+ },
+ });
+
+ await userEvent.click(screen.getByLabelText("Include this month"));
+
+ expect(onChange).toHaveBeenCalledWith({
+ options: {
+ "include-current": false,
+ },
+ type: "relative",
+ value: 1,
+ unit: "month",
+ });
+ });
+ });
});
diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/SimpleRelativeDatePicker.tsx b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/SimpleRelativeDatePicker.tsx
deleted file mode 100644
index 394a37cefb8..00000000000
--- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/SimpleRelativeDatePicker.tsx
+++ /dev/null
@@ -1,20 +0,0 @@
-import type { RelativeDatePickerValue } from "../../types";
-import { CurrentDatePicker } from "../CurrentDatePicker";
-import { SimpleDateIntervalPicker } from "../DateIntervalPicker";
-import { isIntervalValue } from "../utils";
-
-interface SimpleRelativeDatePickerProps {
- value: RelativeDatePickerValue;
- onChange: (value: RelativeDatePickerValue) => void;
-}
-
-export function SimpleRelativeDatePicker({
- value,
- onChange,
-}: SimpleRelativeDatePickerProps) {
- return isIntervalValue(value) ? (
- <SimpleDateIntervalPicker value={value} onChange={onChange} />
- ) : (
- <CurrentDatePicker value={value} onChange={onChange} />
- );
-}
diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/index.ts b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/index.ts
deleted file mode 100644
index 05933eeb45b..00000000000
--- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/index.ts
+++ /dev/null
@@ -1 +0,0 @@
-export * from "./SimpleRelativeDatePicker";
diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/index.ts b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/index.ts
index fe4844a2d02..0ffc957112a 100644
--- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/index.ts
+++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/index.ts
@@ -1,2 +1 @@
export * from "./RelativeDatePicker";
-export * from "./SimpleRelativeDatePicker";
diff --git a/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.tsx b/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.tsx
index 7db8d57aabd..87bd3bbfbd5 100644
--- a/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.tsx
+++ b/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.tsx
@@ -5,6 +5,11 @@ import { t } from "ttag";
import { Button, Stack } from "metabase/ui";
import { DateOperatorPicker } from "../DateOperatorPicker";
+import { CurrentDatePicker } from "../RelativeDatePicker/CurrentDatePicker";
+import { SimpleDateIntervalPicker } from "../RelativeDatePicker/DateIntervalPicker";
+import { isIntervalValue, isRelativeValue } from "../RelativeDatePicker/utils";
+import { SimpleSpecificDatePicker } from "../SpecificDatePicker";
+import { isSpecificValue } from "../SpecificDatePicker/utils";
import { DATE_PICKER_OPERATORS } from "../constants";
import type { DatePickerOperator, DatePickerValue } from "../types";
@@ -34,6 +39,15 @@ export function SimpleDatePicker({
availableOperators={availableOperators}
onChange={setValue}
/>
+ {isRelativeValue(value) && isIntervalValue(value) && (
+ <SimpleDateIntervalPicker value={value} onChange={setValue} />
+ )}
+ {isRelativeValue(value) && !isIntervalValue(value) && (
+ <CurrentDatePicker value={value} onChange={setValue} />
+ )}
+ {isSpecificValue(value) && (
+ <SimpleSpecificDatePicker value={value} onChange={setValue} />
+ )}
<Button type="submit" variant="filled">{t`Apply`}</Button>
</Stack>
</form>
diff --git a/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.unit.spec.tsx b/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.unit.spec.tsx
index a7a062afb06..790b1d015f6 100644
--- a/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.unit.spec.tsx
+++ b/frontend/src/metabase/querying/components/DatePicker/SimpleDatePicker/SimpleDatePicker.unit.spec.tsx
@@ -45,4 +45,40 @@ describe("SimpleDatePicker", () => {
unit: "month",
});
});
+
+ it("should not show 'Include current' switch by default", async () => {
+ setup();
+
+ await userEvent.click(screen.getByDisplayValue("All time"));
+ expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument();
+ });
+
+ it("should not show 'Include current' switch by for the specific date value", async () => {
+ setup();
+
+ await userEvent.click(screen.getByDisplayValue("All time"));
+ await userEvent.click(screen.getByText("Between"));
+ expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument();
+ });
+
+ it("should not show 'Include current' switch by for the `Current` relative date", async () => {
+ setup();
+
+ await userEvent.click(screen.getByDisplayValue("All time"));
+ await userEvent.click(screen.getByText("Current"));
+ expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument();
+ });
+
+ it("should show 'Include current' switch and work for the relative `Previous` or `Next`", async () => {
+ setup({
+ value: {
+ type: "relative",
+ value: 1,
+ unit: "month",
+ },
+ });
+
+ expect(screen.getByDisplayValue("Next")).toBeInTheDocument();
+ expect(screen.getByLabelText("Include this month")).toBeInTheDocument();
+ });
});
diff --git a/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/SimpleSpecificDatePicker/SimpleSpecificDatePicker.unit.spec.tsx b/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/SimpleSpecificDatePicker/SimpleSpecificDatePicker.unit.spec.tsx
new file mode 100644
index 00000000000..f192e4281b2
--- /dev/null
+++ b/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/SimpleSpecificDatePicker/SimpleSpecificDatePicker.unit.spec.tsx
@@ -0,0 +1,41 @@
+import userEvent from "@testing-library/user-event";
+
+import { renderWithProviders, screen } from "__support__/ui";
+
+import type { SpecificDatePickerValue } from "../../types";
+
+import { SimpleSpecificDatePicker } from "./SimpleSpecificDatePicker";
+
+interface SetupOpts {
+ value: SpecificDatePickerValue;
+}
+
+function setup({ value }: SetupOpts) {
+ const onChange = jest.fn();
+
+ renderWithProviders(
+ <SimpleSpecificDatePicker value={value} onChange={onChange} />,
+ );
+
+ return { onChange };
+}
+
+describe("SimpleSpecificDatePicker", () => {
+ it("should be able to change a specific date value", async () => {
+ const { onChange } = setup({
+ value: {
+ type: "specific",
+ operator: "=",
+ values: [new Date(2015, 1, 10)],
+ },
+ });
+
+ await userEvent.click(screen.getByText("15"));
+
+ expect(onChange).toHaveBeenCalledWith({
+ type: "specific",
+ operator: "=",
+ values: [new Date(2015, 1, 15)],
+ });
+ });
+});
diff --git a/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/utils.ts b/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/utils.ts
index d50dfef7ed9..eb5c24ddeec 100644
--- a/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/utils.ts
+++ b/frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/utils.ts
@@ -2,6 +2,7 @@ import dayjs from "dayjs";
import type {
DatePickerOperator,
+ DatePickerValue,
SpecificDatePickerOperator,
SpecificDatePickerValue,
} from "../types";
@@ -9,6 +10,12 @@ import type {
import { TABS } from "./constants";
import type { Tab } from "./types";
+export function isSpecificValue(
+ value?: DatePickerValue,
+): value is SpecificDatePickerValue {
+ return value?.type === "specific";
+}
+
export function getTabs(
availableOperators: ReadonlyArray<DatePickerOperator>,
): Tab[] {
--
GitLab