fix `ChartSettingInputNumeric` not accepting decimals on safari (#33056)

* fix `ChartSettingInputNumeric` decimals on safari

* add unit test

frontend/src/metabase/visualizations/components/settings/ChartSettingInputNumeric.tsx

@@ -3,31 +3,51 @@ import * as React from "react";
 import _ from "underscore";
 import { ChartSettingNumericInput } from "./ChartSettingInputNumeric.styled";
 
+const ALLOWED_CHARS = [
+  "0",
+  "1",
+  "2",
+  "3",
+  "4",
+  "5",
+  "6",
+  "7",
+  "8",
+  "9",
+  ".",
+  "-",
+];
+
 interface ChartSettingInputProps {
-  value: number;
+  value: number | undefined;
   onChange: (value: number | undefined) => void;
   onChangeSettings: () => void;
 }
 
-const ChartSettingInputNumeric = ({
+export const ChartSettingInputNumeric = ({
   onChange,
   value,
   ...props
 }: ChartSettingInputProps) => {
-  const [internalValue, setInternalValue] = useState(value);
+  const [internalValue, setInternalValue] = useState(value?.toString() ?? "");
 
   return (
     <ChartSettingNumericInput
-      type="number"
+      type="text"
       {..._.omit(props, "onChangeSettings")}
-      error={!!internalValue && isNaN(internalValue)}
+      error={internalValue !== "" && isNaN(Number(internalValue))}
       value={internalValue}
       onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
-        const num = parseFloat(e.target.value);
-        setInternalValue(num);
+        const everyCharValid = e.target.value
+          .split("")
+          .every(char => ALLOWED_CHARS.includes(char));
+
+        if (everyCharValid) {
+          setInternalValue(e.target.value);
+        }
       }}
       onBlur={(e: React.ChangeEvent<HTMLInputElement>) => {
-        const num = parseFloat(e.target.value);
+        const num = e.target.value !== "" ? Number(e.target.value) : Number.NaN;
         if (isNaN(num)) {
           onChange(undefined);
         } else {
@@ -37,6 +57,3 @@ const ChartSettingInputNumeric = ({
     />
   );
 };
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default ChartSettingInputNumeric;

frontend/src/metabase/visualizations/components/settings/ChartSettingInputNumeric.unit.spec.tsx

+import userEvent from "@testing-library/user-event";
+
+import { fireEvent, renderWithProviders, screen } from "__support__/ui";
+import { ChartSettingInputNumeric } from "./ChartSettingInputNumeric";
+
+function setup({
+  value,
+}: {
+  value?: number;
+} = {}) {
+  const onChange = jest.fn();
+
+  renderWithProviders(
+    <ChartSettingInputNumeric
+      value={value}
+      onChange={onChange}
+      onChangeSettings={() => null}
+    />,
+  );
+
+  const input = screen.getByRole("textbox");
+
+  return { input, onChange };
+}
+
+function type({ input, value }: { input: HTMLElement; value: string }) {
+  userEvent.clear(input);
+  userEvent.type(input, value);
+  fireEvent.blur(input);
+}
+
+describe("ChartSettingInputNumber", () => {
+  it("allows integer values", () => {
+    const { input, onChange } = setup();
+
+    type({ input, value: "123" });
+    expect(input).toHaveDisplayValue("123");
+    expect(onChange).toHaveBeenCalledWith(123);
+
+    type({ input, value: "-456" });
+    expect(input).toHaveDisplayValue("-456");
+    expect(onChange).toHaveBeenCalledWith(-456);
+  });
+
+  it("allows decimal values", () => {
+    const { input, onChange } = setup();
+
+    type({ input, value: "1.23" });
+    expect(input).toHaveDisplayValue("1.23");
+    expect(onChange).toHaveBeenCalledWith(1.23);
+
+    type({ input, value: "-4.56" });
+    expect(input).toHaveDisplayValue("-4.56");
+    expect(onChange).toHaveBeenCalledWith(-4.56);
+
+    // multiple decimal places should call onChange with
+    // undefined since it's an invalid value
+    type({ input, value: "1.2.3" });
+    expect(input).toHaveDisplayValue("1.2.3");
+    expect(onChange).toHaveBeenCalledWith(undefined);
+  });
+
+  it("does not allow non-alphanumeric values", () => {
+    const { input, onChange } = setup();
+
+    type({ input, value: "asdf" });
+    expect(input).toHaveDisplayValue("");
+    expect(onChange).toHaveBeenCalledWith(undefined);
+  });
+
+  it("renders the `value` prop on load", () => {
+    const { input } = setup({ value: 123 });
+
+    expect(input).toHaveDisplayValue("123");
+  });
+});

frontend/src/metabase/visualizations/lib/settings.js

@@ -3,7 +3,7 @@ import { getIn } from "icepick";
 import _ from "underscore";
 import ChartSettingInput from "metabase/visualizations/components/settings/ChartSettingInput";
 import ChartSettingInputGroup from "metabase/visualizations/components/settings/ChartSettingInputGroup";
-import ChartSettingInputNumeric from "metabase/visualizations/components/settings/ChartSettingInputNumeric";
+import { ChartSettingInputNumeric } from "metabase/visualizations/components/settings/ChartSettingInputNumeric";
 import ChartSettingRadio from "metabase/visualizations/components/settings/ChartSettingRadio";
 import ChartSettingSelect from "metabase/visualizations/components/settings/ChartSettingSelect";
 import ChartSettingToggle from "metabase/visualizations/components/settings/ChartSettingToggle";