Add StringInputWidget component (#23419)

* Add StringInputWidget component

* Second pass, add tests, storybook

* Use StringInputWidget as default widget

* use label, not title

* Remove disabled widget logic

* Prettier nonsense

* Fix default widget TextWidget usage in query builder

* Update text in e2e test

* Update e2e text selector

frontend/src/metabase/parameters/components/ParameterValueWidget.jsx

@@ -8,12 +8,14 @@ import {
   getParameterIconName,
   getParameterWidgetTitle,
 } from "metabase/parameters/utils/ui";
-import { isDashboardParameterWithoutMapping } from "metabase/parameters/utils/dashboards";
 import {
   isDateParameter,
   isNumberParameter,
 } from "metabase/parameters/utils/parameter-type";
-import { getNumberParameterArity } from "metabase/parameters/utils/operators";
+import {
+  getNumberParameterArity,
+  getStringParameterArity,
+} from "metabase/parameters/utils/operators";
 
 import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
 import Icon from "metabase/components/Icon";
@@ -23,11 +25,11 @@ import DateRelativeWidget from "metabase/components/DateRelativeWidget";
 import DateMonthYearWidget from "metabase/components/DateMonthYearWidget";
 import DateQuarterYearWidget from "metabase/components/DateQuarterYearWidget";
 import DateAllOptionsWidget from "metabase/components/DateAllOptionsWidget";
-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 StringInputWidget from "metabase/parameters/components/widgets/StringInputWidget";
 
 import ParameterFieldWidget from "./widgets/ParameterFieldWidget/ParameterFieldWidget";
 import S from "./ParameterWidget.css";
@@ -95,56 +97,44 @@ class ParameterValueWidget extends Component {
       isFullscreen,
       noReset,
       className,
-      dashboard,
     } = this.props;
     const { isFocused } = this.state;
     const hasValue = value != null;
-    const isDashParamWithoutMapping = isDashboardParameterWithoutMapping(
-      parameter,
-      dashboard,
-    );
-    const isDashParamWithoutMappingText = t`This filter needs to be connected to a card.`;
     const { noPopover } = getWidgetDefinition(parameter);
     const parameterTypeIcon = getParameterIconName(parameter);
     const showTypeIcon = !isEditing && !hasValue && !isFocused;
 
     if (noPopover) {
       return (
-        <Tooltip
-          tooltip={isDashParamWithoutMappingText}
-          isEnabled={isDashParamWithoutMapping}
+        <div
+          ref={this.trigger}
+          className={cx(S.parameter, S.noPopover, className, {
+            [S.selected]: hasValue,
+            [S.isEditing]: isEditing,
+          })}
         >
-          <div
-            ref={this.trigger}
-            className={cx(S.parameter, S.noPopover, className, {
-              [S.selected]: hasValue,
-              [S.isEditing]: isEditing,
-            })}
-          >
-            {showTypeIcon && (
-              <Icon
-                name={parameterTypeIcon}
-                className="flex-align-left mr1 flex-no-shrink"
-                size={14}
-              />
-            )}
-            <Widget
-              {...this.props}
-              target={this.getTargetRef()}
-              onFocusChanged={this.onFocusChanged}
-              onPopoverClose={this.onPopoverClose}
-              disabled={isDashParamWithoutMapping}
-            />
-            <WidgetStatusIcon
-              isFullscreen={isFullscreen}
-              hasValue={hasValue}
-              noReset={noReset}
-              noPopover={!!noPopover}
-              isFocused={isFocused}
-              setValue={setValue}
+          {showTypeIcon && (
+            <Icon
+              name={parameterTypeIcon}
+              className="flex-align-left mr1 flex-no-shrink"
+              size={14}
             />
-          </div>
-        </Tooltip>
+          )}
+          <Widget
+            {...this.props}
+            target={this.getTargetRef()}
+            onFocusChanged={this.onFocusChanged}
+            onPopoverClose={this.onPopoverClose}
+          />
+          <WidgetStatusIcon
+            isFullscreen={isFullscreen}
+            hasValue={hasValue}
+            noReset={noReset}
+            noPopover={!!noPopover}
+            isFocused={isFocused}
+            setValue={setValue}
+          />
+        </div>
       );
     } else {
       const placeholderText = isEditing
@@ -154,58 +144,50 @@ class ParameterValueWidget extends Component {
         : placeholder || t`Select…`;
 
       return (
-        <Tooltip
-          tooltip={isDashParamWithoutMappingText}
-          isEnabled={isDashParamWithoutMapping}
-        >
-          <PopoverWithTrigger
-            ref={this.valuePopover}
-            disabled={isDashParamWithoutMapping}
-            triggerElement={
-              <div
-                ref={this.trigger}
-                className={cx(S.parameter, className, {
-                  [S.selected]: hasValue,
-                  "cursor-not-allowed": isDashParamWithoutMapping,
-                })}
-              >
-                {showTypeIcon && (
-                  <Icon
-                    name={parameterTypeIcon}
-                    className="flex-align-left mr1 flex-no-shrink"
-                    size={14}
-                  />
-                )}
-                <div className="mr1 text-nowrap">
-                  <FormattedParameterValue
-                    parameter={parameter}
-                    value={value}
-                    placeholder={placeholderText}
-                  />
-                </div>
-                <WidgetStatusIcon
-                  isFullscreen={isFullscreen}
-                  hasValue={hasValue}
-                  noReset={noReset}
-                  noPopover={!!noPopover}
-                  isFocused={isFocused}
-                  setValue={setValue}
+        <PopoverWithTrigger
+          ref={this.valuePopover}
+          triggerElement={
+            <div
+              ref={this.trigger}
+              className={cx(S.parameter, className, {
+                [S.selected]: hasValue,
+              })}
+            >
+              {showTypeIcon && (
+                <Icon
+                  name={parameterTypeIcon}
+                  className="flex-align-left mr1 flex-no-shrink"
+                  size={14}
+                />
+              )}
+              <div className="mr1 text-nowrap">
+                <FormattedParameterValue
+                  parameter={parameter}
+                  value={value}
+                  placeholder={placeholderText}
                 />
               </div>
-            }
-            target={this.getTargetRef}
-            // make sure the full date picker will expand to fit the dual calendars
-            autoWidth={parameter.type === "date/all-options"}
-          >
-            <Widget
-              {...this.props}
-              target={this.getTargetRef()}
-              onFocusChanged={this.onFocusChanged}
-              onPopoverClose={this.onPopoverClose}
-              disabled={isDashParamWithoutMapping}
-            />
-          </PopoverWithTrigger>
-        </Tooltip>
+              <WidgetStatusIcon
+                isFullscreen={isFullscreen}
+                hasValue={hasValue}
+                noReset={noReset}
+                noPopover={!!noPopover}
+                isFocused={isFocused}
+                setValue={setValue}
+              />
+            </div>
+          }
+          target={this.getTargetRef}
+          // make sure the full date picker will expand to fit the dual calendars
+          autoWidth={parameter.type === "date/all-options"}
+        >
+          <Widget
+            {...this.props}
+            target={this.getTargetRef()}
+            onFocusChanged={this.onFocusChanged}
+            onPopoverClose={this.onPopoverClose}
+          />
+        </PopoverWithTrigger>
       );
     }
   }
@@ -225,20 +207,8 @@ function Widget({
   onFocusChanged,
   parameters,
   dashboard,
-  disabled,
   target,
 }) {
-  if (disabled) {
-    return (
-      <TextWidget
-        className={cx(className, "cursor-not-allowed")}
-        value={value}
-        placeholder={placeholder}
-        disabled={disabled}
-      />
-    );
-  }
-
   const normalizedValue = Array.isArray(value)
     ? value
     : [value].filter(v => v != null);
@@ -296,14 +266,17 @@ function Widget({
     );
   } else {
     return (
-      <TextWidget
-        value={value}
-        setValue={setValue}
+      <StringInputWidget
+        value={normalizedValue}
+        setValue={value => {
+          setValue(value);
+          onPopoverClose();
+        }}
         className={className}
-        isEditing={isEditing}
-        commitImmediately={commitImmediately}
-        placeholder={placeholder}
-        focusChanged={onFocusChanged}
+        autoFocus
+        placeholder={isEditing ? t`Enter a default value…` : undefined}
+        arity={getStringParameterArity(parameter)}
+        label={getParameterWidgetTitle(parameter)}
       />
     );
   }
@@ -325,6 +298,6 @@ function getWidgetDefinition(parameter) {
   } else if (!_.isEmpty(parameter.fields)) {
     return ParameterFieldWidget;
   } else {
-    return TextWidget;
+    return StringInputWidget;
   }
 }

frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.stories.tsx

+import React from "react";
+import { ComponentStory } from "@storybook/react";
+import { useArgs } from "@storybook/client-api";
+import StringInputWidget from "./StringInputWidget";
+
+export default {
+  title: "Parameters/StringInputWidget",
+  component: StringInputWidget,
+};
+
+const Template: ComponentStory<typeof StringInputWidget> = args => {
+  const [{ value }, updateArgs] = useArgs();
+
+  const handleSetValue = (v: string[] | undefined) => {
+    updateArgs({ value: v });
+  };
+
+  return (
+    <StringInputWidget {...args} value={value} setValue={handleSetValue} />
+  );
+};
+
+export const Default = Template.bind({});
+Default.args = {
+  value: ["foo"],
+};
+
+export const NArgs = Template.bind({});
+NArgs.args = {
+  value: ["foo", "bar", "baz"],
+  arity: "n",
+  autoFocus: true,
+};

frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.tsx

+import React, { useState } from "react";
+import { t } from "ttag";
+import { isEqual, isString, isEmpty } from "lodash";
+
+import TokenField, { parseStringValue } from "metabase/components/TokenField";
+import {
+  WidgetRoot,
+  WidgetLabel,
+  Footer,
+  UpdateButton,
+  TokenFieldWrapper,
+} from "metabase/parameters/components/widgets/Widget.styled";
+
+type StringInputWidgetProps = {
+  value: string[] | undefined;
+  setValue: (value: string[] | undefined) => void;
+  className?: string;
+  autoFocus?: boolean;
+  placeholder?: string;
+  arity?: 1 | "n";
+  label?: string;
+};
+
+const OPTIONS: any[] = [];
+
+function StringInputWidget({
+  value,
+  setValue,
+  className,
+  autoFocus,
+  arity = 1,
+  placeholder = t`Enter some text`,
+  label,
+}: StringInputWidgetProps) {
+  const arrayValue = normalize(value);
+  const [unsavedArrayValue, setUnsavedArrayValue] =
+    useState<string[]>(arrayValue);
+  const multi = arity === "n";
+  const hasValueChanged = !isEqual(arrayValue, unsavedArrayValue);
+  const isValid = unsavedArrayValue.every(isString);
+
+  const onClick = () => {
+    if (isEmpty(unsavedArrayValue)) {
+      setValue(undefined);
+    } else {
+      setValue(unsavedArrayValue);
+    }
+  };
+
+  return (
+    <WidgetRoot className={className}>
+      {label && <WidgetLabel>{label}</WidgetLabel>}
+      <TokenFieldWrapper>
+        <TokenField
+          value={unsavedArrayValue}
+          onChange={setUnsavedArrayValue}
+          placeholder={placeholder}
+          options={OPTIONS}
+          autoFocus={autoFocus}
+          multi={multi}
+          parseFreeformValue={parseStringValue}
+          updateOnInputChange
+        />
+      </TokenFieldWrapper>
+      <Footer>
+        <UpdateButton disabled={!isValid || !hasValueChanged} onClick={onClick}>
+          {arrayValue.length ? t`Update filter` : t`Add filter`}
+        </UpdateButton>
+      </Footer>
+    </WidgetRoot>
+  );
+}
+
+export default StringInputWidget;
+
+function normalize(value: string[] | undefined): string[] {
+  if (Array.isArray(value)) {
+    return value;
+  } else {
+    return [];
+  }
+}

frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.unit.spec.tsx

+import React from "react";
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+
+import StringInputWidget from "./StringInputWidget";
+
+const mockSetValue = jest.fn();
+
+describe("StringInputWidget", () => {
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("arity of 1", () => {
+    it("should render an input populated with a value", () => {
+      render(<StringInputWidget value={["foo"]} setValue={mockSetValue} />);
+
+      const textbox = screen.getByRole("textbox");
+      expect(textbox).toBeInTheDocument();
+      const values = screen.getAllByRole("list")[0];
+      expect(values.textContent).toEqual("foo");
+    });
+
+    it("should render an empty input", () => {
+      render(<StringInputWidget value={undefined} setValue={mockSetValue} />);
+
+      const textbox = screen.getByRole("textbox");
+      expect(textbox).toBeInTheDocument();
+      expect(textbox).toHaveAttribute("placeholder", "Enter some text");
+    });
+
+    it("should render a disabled update button, until the value is changed", () => {
+      render(<StringInputWidget value={["foo"]} setValue={mockSetValue} />);
+
+      const button = screen.getByRole("button", { name: "Update filter" });
+      expect(button).toBeInTheDocument();
+      expect(button).toHaveAttribute("disabled");
+
+      userEvent.type(screen.getByRole("textbox"), "bar");
+      expect(button).not.toHaveAttribute("disabled");
+    });
+
+    it("should let you update the input with a new value", () => {
+      render(<StringInputWidget value={["foo"]} setValue={mockSetValue} />);
+
+      const textbox = screen.getByRole("textbox");
+      userEvent.type(textbox, "{backspace}{backspace}{backspace}bar");
+      const button = screen.getByRole("button", { name: "Update filter" });
+      userEvent.click(button);
+      expect(mockSetValue).toHaveBeenCalledWith(["bar"]);
+    });
+
+    it("should let you update the input with an undefined value", () => {
+      render(<StringInputWidget value={["a"]} setValue={mockSetValue} />);
+
+      const textbox = screen.getByRole("textbox");
+      const button = screen.getByRole("button", { name: "Update filter" });
+      userEvent.type(textbox, "{backspace}{enter}");
+      userEvent.click(button);
+      expect(mockSetValue).toHaveBeenCalledWith(undefined);
+    });
+  });
+
+  describe("arity of n", () => {
+    it("should render a token field input", () => {
+      render(
+        <StringInputWidget
+          arity="n"
+          value={["foo", "bar"]}
+          setValue={mockSetValue}
+        />,
+      );
+
+      const values = screen.getAllByRole("list")[0];
+      expect(values.textContent).toEqual("foobar");
+    });
+
+    it("should correctly parse number inputs", () => {
+      render(
+        <StringInputWidget
+          arity="n"
+          value={undefined}
+          setValue={mockSetValue}
+        />,
+      );
+
+      const input = screen.getByRole("textbox");
+      userEvent.type(input, "foo{enter}bar{enter}baz{enter}");
+
+      const values = screen.getAllByRole("list")[0];
+      expect(values.textContent).toEqual("foobarbaz");
+
+      const button = screen.getByRole("button", { name: "Add filter" });
+      userEvent.click(button);
+      expect(mockSetValue).toHaveBeenCalledWith(["foo", "bar", "baz"]);
+    });
+
+    it("should be unsettable", () => {
+      render(
+        <StringInputWidget
+          arity="n"
+          value={["foo", "bar"]}
+          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);
+    });
+  });
+});

frontend/src/metabase/parameters/components/widgets/StringInputWidget/index.ts

+export { default } from "./StringInputWidget";

frontend/src/metabase/parameters/utils/operators.js

@@ -76,3 +76,13 @@ export function getNumberParameterArity(parameter) {
       return 1;
   }
 }
+
+export function getStringParameterArity(parameter) {
+  switch (parameter.type) {
+    case "string/=":
+    case "string/!=":
+      return "n";
+    default:
+      return 1;
+  }
+}

frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx

@@ -273,6 +273,7 @@ class TagEditorParam extends Component {
                     }
                   : {
                       fields: [],
+                      hasVariableTemplateTagTarget: true,
                       type:
                         tag["widget-type"] ||
                         (tag.type === "date" ? "date/single" : null),

frontend/test/metabase/scenarios/dashboard-filters/reproductions/15279-gracefully-deal-with-corrupted-filter.cy.spec.js

@@ -96,6 +96,6 @@ describe("issue 15279", () => {
 
     // The corrupted filter is now present in the UI, but it doesn't work (as expected)
     // People can now easily remove it
-    cy.findByPlaceholderText("Enter a value...");
+    cy.findByText("Select…");
   });
 });

frontend/test/metabase/scenarios/dashboard/text-box.cy.spec.js

@@ -142,6 +142,6 @@ describe("scenarios > dashboard > text-box", () => {
 
     // confirm text box and filter are still there
     cy.findByText("text text text");
-    cy.findByPlaceholderText("Text");
+    cy.findByText("Text");
   });
 });

frontend/test/metabase/scenarios/native-filters/helpers/e2e-field-filter-helpers.js

@@ -149,6 +149,7 @@ function addSimpleNumberFilter(value) {
  * @param {string} value
  */
 function enterDefaultValue(value) {
+  cy.findByText("Enter a default value…").click();
   cy.findByPlaceholderText("Enter a default value…").type(`${value}{enter}`);
   cy.button("Add filter").click();
 }
@@ -158,6 +159,7 @@ function enterDefaultValue(value) {
  * @param {string} result
  */
 export function pickDefaultValue(searchTerm, result) {
+  cy.findByText("Enter a default value…").click();
   cy.findByPlaceholderText("Enter a default value…").type(searchTerm);
   popover().findByText(result).click();