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`

frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx

 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,
+      }}
+    />
   );
 }

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",
-      });
-    });
-  });
 });

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} />
+    </>
   );
 }

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",
+      });
+    });
+  });
 });

frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/SimpleRelativeDatePicker.tsx

-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} />
-  );
-}

frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/SimpleRelativeDatePicker/index.ts

-export * from "./SimpleRelativeDatePicker";

frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/index.ts

 export * from "./RelativeDatePicker";
-export * from "./SimpleRelativeDatePicker";

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>

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();
+  });
 });

frontend/src/metabase/querying/components/DatePicker/SpecificDatePicker/SimpleSpecificDatePicker/SimpleSpecificDatePicker.unit.spec.tsx

+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)],
+    });
+  });
+});

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[] {