From 78f2799bca61eb6232a323b90628961c3c7bef6d Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Thu, 16 Jun 2022 09:05:44 -0700
Subject: [PATCH] Add NumberInputWidget component (#23247)
* First pass at NumberWidget
* Share styled components
* Remove TokenField prop for now
* Second pass
* Add to ParameterValueWidget
* Add placeholder text
* Rename to NumberInputWidget
* Only use new widget for field filter parameters for now
* Fix placeholder
* Fix button text
* Fix placeholder text
* Add unit tests
* Fix type
---
.../components/ParameterValueWidget.jsx | 39 +++-
.../NumberInputWidget.stories.tsx | 48 +++++
.../NumberInputWidget/NumberInputWidget.tsx | 109 +++++++++++
.../NumberInputWidget.unit.spec.tsx | 184 ++++++++++++++++++
.../widgets/NumberInputWidget/index.ts | 1 +
.../metabase/parameters/utils/operators.js | 12 ++
.../parameters/utils/parameter-type.ts | 5 +
7 files changed, 394 insertions(+), 4 deletions(-)
create mode 100644 frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.stories.tsx
create mode 100644 frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx
create mode 100644 frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx
create mode 100644 frontend/src/metabase/parameters/components/widgets/NumberInputWidget/index.ts
diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
index c045eff320a..381761e28d0 100644
--- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
+++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
@@ -7,7 +7,11 @@ import _ from "underscore";
import { getParameterIconName } from "metabase/parameters/utils/ui";
import { isDashboardParameterWithoutMapping } from "metabase/parameters/utils/dashboards";
import { isOnlyMappedToFields } from "metabase/parameters/utils/fields";
-import { isDateParameter } from "metabase/parameters/utils/parameter-type";
+import {
+ isDateParameter,
+ isNumberParameter,
+} from "metabase/parameters/utils/parameter-type";
+import { getNumberParameterArity } from "metabase/parameters/utils/operators";
import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
import Icon from "metabase/components/Icon";
import DateSingleWidget from "metabase/components/DateSingleWidget";
@@ -20,6 +24,7 @@ import Tooltip from "metabase/components/Tooltip";
import TextWidget from "metabase/components/TextWidget";
import WidgetStatusIcon from "metabase/parameters/components/WidgetStatusIcon";
import FormattedParameterValue from "metabase/parameters/components/FormattedParameterValue";
+import NumberInputWidget from "metabase/parameters/components/widgets/NumberInputWidget";
import ParameterFieldWidget from "./widgets/ParameterFieldWidget/ParameterFieldWidget";
import S from "./ParameterWidget.css";
@@ -230,12 +235,35 @@ function Widget({
);
}
- const DateWidget = DATE_WIDGETS[parameter.type];
- if (DateWidget) {
+ if (isDateParameter(parameter)) {
+ const DateWidget = DATE_WIDGETS[parameter.type];
return (
<DateWidget value={value} setValue={setValue} onClose={onPopoverClose} />
);
} else if (isOnlyMappedToFields(parameter)) {
+ const normalizedValue = Array.isArray(value)
+ ? value
+ : [value].filter(v => v != null);
+
+ if (isNumberParameter(parameter)) {
+ const arity = getNumberParameterArity(parameter);
+ return (
+ <NumberInputWidget
+ value={normalizedValue}
+ setValue={value => {
+ setValue(value);
+ onPopoverClose();
+ }}
+ arity={arity}
+ infixText={
+ typeof arity === "number" && arity > 1 ? t`and` : undefined
+ }
+ autoFocus
+ placeholder={isEditing ? t`Enter a default value…` : undefined}
+ />
+ );
+ }
+
return (
<ParameterFieldWidget
target={target}
@@ -243,7 +271,7 @@ function Widget({
parameters={parameters}
dashboard={dashboard}
placeholder={placeholder}
- value={value}
+ value={normalizedValue}
fields={parameter.fields}
setValue={value => {
setValue(value);
@@ -278,6 +306,9 @@ function getWidgetDefinition(parameter) {
if (DATE_WIDGETS[parameter.type]) {
return DATE_WIDGETS[parameter.type];
} else if (isOnlyMappedToFields(parameter)) {
+ if (isNumberParameter(parameter)) {
+ return NumberInputWidget;
+ }
return ParameterFieldWidget;
} else {
return TextWidget;
diff --git a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.stories.tsx b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.stories.tsx
new file mode 100644
index 00000000000..b518fbdcfaa
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.stories.tsx
@@ -0,0 +1,48 @@
+import React from "react";
+import { ComponentStory } from "@storybook/react";
+import { useArgs } from "@storybook/client-api";
+import NumberInputWidget from "./NumberInputWidget";
+
+export default {
+ title: "Parameters/NumberInputWidget",
+ component: NumberInputWidget,
+};
+
+const Template: ComponentStory<typeof NumberInputWidget> = args => {
+ const [{ value }, updateArgs] = useArgs();
+
+ const handleSetValue = (v: number[] | undefined) => {
+ updateArgs({ value: v });
+ };
+
+ return (
+ <NumberInputWidget {...args} value={value} setValue={handleSetValue} />
+ );
+};
+
+export const Default = Template.bind({});
+Default.args = {
+ value: [1],
+};
+
+export const TwoArgs = Template.bind({});
+TwoArgs.args = {
+ value: [1, 2],
+ arity: 2,
+ infixText: "and",
+};
+
+export const ThreeArgs = Template.bind({});
+ThreeArgs.args = {
+ value: [1, 2],
+ arity: 3,
+ infixText: "foo",
+ autoFocus: true,
+};
+
+export const NArgs = Template.bind({});
+NArgs.args = {
+ value: [1, 2, 3, 4, 5, 6],
+ arity: "n",
+ autoFocus: true,
+};
diff --git a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx
new file mode 100644
index 00000000000..468cd39e04d
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx
@@ -0,0 +1,109 @@
+import React, { useState } from "react";
+import { t } from "ttag";
+import { times, isEqual, isNumber, isUndefined } from "lodash";
+
+import TokenField, { parseNumberValue } from "metabase/components/TokenField";
+import NumericInput from "metabase/core/components/NumericInput";
+import {
+ WidgetRoot,
+ Footer,
+ UpdateButton,
+} from "metabase/parameters/components/widgets/Widget.styled";
+
+export type NumberInputWidgetProps = {
+ value: number[] | undefined;
+ setValue: (value: number[] | undefined) => void;
+ className?: string;
+ arity?: "n" | number;
+ infixText?: string;
+ autoFocus?: boolean;
+ placeholder?: string;
+};
+
+const OPTIONS: any[] = [];
+
+function NumberInputWidget({
+ value,
+ setValue,
+ className,
+ arity = 1,
+ infixText,
+ autoFocus,
+ placeholder = t`Enter a number`,
+}: NumberInputWidgetProps) {
+ const arrayValue = normalize(value);
+ const [unsavedArrayValue, setUnsavedArrayValue] = useState<
+ (number | undefined)[]
+ >(arrayValue);
+ const hasValueChanged = !isEqual(arrayValue, unsavedArrayValue);
+ const allValuesUnset = unsavedArrayValue.every(isUndefined);
+ const allValuesSet = unsavedArrayValue.every(isNumber);
+ const isValid =
+ (arity === "n" || unsavedArrayValue.length === arity) &&
+ (allValuesUnset || allValuesSet);
+
+ const onClick = () => {
+ if (isValid) {
+ if (allValuesUnset || unsavedArrayValue.length === 0) {
+ setValue(undefined);
+ } else {
+ setValue(unsavedArrayValue);
+ }
+ }
+ };
+
+ return (
+ <WidgetRoot className={className}>
+ {arity === "n" ? (
+ <TokenField
+ multi
+ updateOnInputChange
+ autoFocus={autoFocus}
+ value={unsavedArrayValue}
+ parseFreeformValue={parseNumberValue}
+ onChange={newValue => {
+ setUnsavedArrayValue(newValue);
+ }}
+ options={OPTIONS}
+ placeholder={placeholder}
+ />
+ ) : (
+ times(arity, i => (
+ <div className="inline-block" key={i}>
+ <NumericInput
+ className="p1"
+ autoFocus={autoFocus && i === 0}
+ value={unsavedArrayValue[i]}
+ onChange={newValue => {
+ setUnsavedArrayValue(unsavedArrayValue => {
+ const newUnsavedValue = [...unsavedArrayValue];
+ newUnsavedValue[i] = newValue;
+ return newUnsavedValue;
+ });
+ }}
+ placeholder={placeholder}
+ />
+ {infixText && i !== arity - 1 && (
+ <span className="px1">{infixText}</span>
+ )}
+ </div>
+ ))
+ )}
+ <Footer>
+ <UpdateButton disabled={!isValid || !hasValueChanged} onClick={onClick}>
+ {arrayValue.length ? t`Update filter` : t`Add filter`}
+ </UpdateButton>
+ </Footer>
+ </WidgetRoot>
+ );
+}
+
+export default NumberInputWidget;
+
+function normalize(value: number[] | undefined): number[] {
+ if (Array.isArray(value)) {
+ return value;
+ } else {
+ return [];
+ }
+}
diff --git a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx
new file mode 100644
index 00000000000..793f58ab021
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx
@@ -0,0 +1,184 @@
+import React from "react";
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+
+import NumberInputWidget from "./NumberInputWidget";
+
+const mockSetValue = jest.fn();
+
+describe("NumberInputWidget", () => {
+ beforeEach(() => {
+ jest.resetAllMocks();
+ });
+
+ describe("arity of 1", () => {
+ it("should render an input populated with a value", () => {
+ render(<NumberInputWidget value={[123]} setValue={mockSetValue} />);
+
+ const textbox = screen.getByRole("textbox");
+ expect(textbox).toBeInTheDocument();
+ expect(textbox).toHaveValue("123");
+ });
+
+ it("should render an empty input", () => {
+ render(<NumberInputWidget value={undefined} setValue={mockSetValue} />);
+
+ const textbox = screen.getByRole("textbox");
+ expect(textbox).toBeInTheDocument();
+ expect(textbox).toHaveAttribute("placeholder", "Enter a number");
+ });
+
+ it("should render a disabled update button, until the value is changed", () => {
+ render(<NumberInputWidget value={[123]} setValue={mockSetValue} />);
+
+ const button = screen.getByRole("button", { name: "Update filter" });
+ expect(button).toBeInTheDocument();
+ expect(button).toHaveAttribute("disabled");
+
+ userEvent.type(screen.getByRole("textbox"), "456");
+ expect(button).not.toHaveAttribute("disabled");
+ });
+
+ it("should let you update the input with a new value", () => {
+ render(<NumberInputWidget value={[123]} setValue={mockSetValue} />);
+
+ const textbox = screen.getByRole("textbox");
+ userEvent.clear(textbox);
+ userEvent.type(textbox, "456");
+ const button = screen.getByRole("button", { name: "Update filter" });
+ userEvent.click(button);
+ expect(mockSetValue).toHaveBeenCalledWith([456]);
+ });
+
+ it("should let you update the input with an undefined value", () => {
+ render(<NumberInputWidget value={[1]} setValue={mockSetValue} />);
+
+ const textbox = screen.getByRole("textbox");
+ const button = screen.getByRole("button", { name: "Update filter" });
+ userEvent.type(textbox, "{backspace}");
+ userEvent.click(button);
+ expect(mockSetValue).toHaveBeenCalledWith(undefined);
+ });
+ });
+
+ describe("arity of 2", () => {
+ it("should render an input populated with a value", () => {
+ render(
+ <NumberInputWidget
+ arity={2}
+ value={[123, 456]}
+ setValue={mockSetValue}
+ />,
+ );
+
+ const [textbox1, textbox2] = screen.getAllByRole("textbox");
+ expect(textbox1).toBeInTheDocument();
+ expect(textbox1).toHaveValue("123");
+
+ expect(textbox2).toBeInTheDocument();
+ expect(textbox2).toHaveValue("456");
+ });
+
+ it("should be invalid when one of the inputs is empty", () => {
+ render(
+ <NumberInputWidget
+ arity={2}
+ value={[123, 456]}
+ setValue={mockSetValue}
+ />,
+ );
+
+ const [textbox1] = screen.getAllByRole("textbox");
+ userEvent.clear(textbox1);
+ const button = screen.getByRole("button", { name: "Update filter" });
+ expect(button).toHaveAttribute("disabled");
+ });
+
+ it("should be settable", () => {
+ render(
+ <NumberInputWidget
+ arity={2}
+ value={undefined}
+ setValue={mockSetValue}
+ />,
+ );
+
+ const [textbox1, textbox2] = screen.getAllByRole("textbox");
+ userEvent.type(textbox1, "1");
+ userEvent.type(textbox2, "2");
+
+ const button = screen.getByRole("button", { name: "Add filter" });
+ userEvent.click(button);
+
+ expect(mockSetValue).toHaveBeenCalledWith([1, 2]);
+ });
+
+ it("should be clearable by emptying all inputs", () => {
+ render(
+ <NumberInputWidget
+ arity={2}
+ value={[123, 456]}
+ setValue={mockSetValue}
+ />,
+ );
+
+ const [textbox1, textbox2] = screen.getAllByRole("textbox");
+ userEvent.clear(textbox1);
+ userEvent.clear(textbox2);
+
+ const button = screen.getByRole("button", { name: "Update filter" });
+ userEvent.click(button);
+
+ expect(mockSetValue).toHaveBeenCalledWith(undefined);
+ });
+ });
+
+ describe("arity of n", () => {
+ it("should render a token field input", () => {
+ render(
+ <NumberInputWidget
+ arity="n"
+ value={[1, 2, 3, 4]}
+ setValue={mockSetValue}
+ />,
+ );
+
+ const values = screen.getAllByRole("list")[0];
+ expect(values.textContent).toEqual("1234");
+ });
+
+ it("should correctly parse number inputs", () => {
+ render(
+ <NumberInputWidget
+ arity="n"
+ value={undefined}
+ setValue={mockSetValue}
+ />,
+ );
+
+ const input = screen.getByRole("textbox");
+ userEvent.type(input, "foo{enter}123abc{enter}456{enter}");
+
+ const values = screen.getAllByRole("list")[0];
+ expect(values.textContent).toEqual("123456");
+
+ const button = screen.getByRole("button", { name: "Add filter" });
+ userEvent.click(button);
+ expect(mockSetValue).toHaveBeenCalledWith([123, 456]);
+ });
+
+ it("should be unsettable", () => {
+ render(
+ <NumberInputWidget arity="n" value={[1, 2]} setValue={mockSetValue} />,
+ );
+
+ const input = screen.getByRole("textbox");
+ userEvent.type(input, "{backspace}{backspace}");
+
+ const button = screen.getByRole("button", { name: "Update filter" });
+
+ userEvent.click(button);
+ expect(mockSetValue).toHaveBeenCalledWith(undefined);
+ });
+ });
+});
diff --git a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/index.ts b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/index.ts
new file mode 100644
index 00000000000..087e5e0a76c
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/index.ts
@@ -0,0 +1 @@
+export { default } from "./NumberInputWidget";
diff --git a/frontend/src/metabase/parameters/utils/operators.js b/frontend/src/metabase/parameters/utils/operators.js
index 3a4b4a580cd..1d2d239f169 100644
--- a/frontend/src/metabase/parameters/utils/operators.js
+++ b/frontend/src/metabase/parameters/utils/operators.js
@@ -64,3 +64,15 @@ export function buildTypedOperatorOptions(
};
});
}
+
+export function getNumberParameterArity(parameter) {
+ switch (parameter.type) {
+ case "number/=":
+ case "number/!=":
+ return "n";
+ case "number/between":
+ return 2;
+ default:
+ return 1;
+ }
+}
diff --git a/frontend/src/metabase/parameters/utils/parameter-type.ts b/frontend/src/metabase/parameters/utils/parameter-type.ts
index 4da9d9a7b03..5100ee88129 100644
--- a/frontend/src/metabase/parameters/utils/parameter-type.ts
+++ b/frontend/src/metabase/parameters/utils/parameter-type.ts
@@ -28,6 +28,11 @@ export function isDateParameter(parameter: Parameter | string) {
return type === "date";
}
+export function isNumberParameter(parameter: Parameter) {
+ const type = getParameterType(parameter);
+ return type === "number";
+}
+
export function isFieldFilterParameter(
parameter: Parameter,
): parameter is FieldFilterUiParameter {
--
GitLab