Merge pull request #5163 from Sage/FE-5153_decimal-props-value-not-re…

…liable fix(decimal): ensure that value prop is reliably displayed - FE-5153
Sage · Jun 1, 2022 · f6230db · f6230db
2 parents 4c086fb + 850aa95
commit f6230db
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 21 deletions.
diff --git a/src/components/decimal/decimal-test.stories.mdx b/src/components/decimal/decimal-test.stories.mdx
@@ -119,6 +119,29 @@ export const NewValidationStory = (args) => {
   );
 };
 
+export const DecimalCustomOnChangeStory = (args) => {
+  const [state, setState] = useState("0.01");
+  const handleChange = (e) => {
+    let newValue = e.target.value.rawValue;
+    if (newValue.startsWith("22.22")) newValue = "22.22";
+    action("onChange")(e, newValue);
+    setState(newValue);
+  };
+  return (
+    <div>
+      If you try to type "22.222", the onChange should block the last "2" from being entered and you should see "22.22" in the textbox.
+      The recommended approach for manipulating input values is to use validation. However, it is also possible to manipulate this via the onChange function like so:
+      <Decimal
+        mt={2}
+        value={state}
+        onChange={handleChange}
+        onBlur={action("onBlur")}
+        {...getCommonTextboxArgsWithSpecialCaracters(args)}
+      />
+    </div>
+  );
+};
+
 # Decimal
 
 ### Default
@@ -169,3 +192,11 @@ export const NewValidationStory = (args) => {
     {NewValidationStory.bind({})}
   </Story>
 </Canvas>
+
+### Custom onChange
+
+<Canvas>
+  <Story name="custom onChange" args={commonArgs}>
+    {DecimalCustomOnChangeStory.bind({})}
+  </Story>
+</Canvas>
diff --git a/src/components/decimal/decimal.component.js b/src/components/decimal/decimal.component.js
@@ -38,21 +38,24 @@ const Decimal = ({
 
   const emptyValue = allowEmptyValue ? "" : "0.00";
 
-  const getSafeValueProp = (initialValue) => {
-    // We're intentionally preventing the use of number values to help prevent any unintentional rounding issues
-    invariant(
-      typeof initialValue === "string",
-      "Decimal `value` prop must be a string"
-    );
-
-    if (initialValue && !allowEmptyValue) {
+  const getSafeValueProp = useCallback(
+    (initialValue) => {
+      // We're intentionally preventing the use of number values to help prevent any unintentional rounding issues
       invariant(
-        initialValue !== "",
-        "Decimal `value` must not be an empty string. Please use `allowEmptyValue` or `0.00`"
+        typeof initialValue === "string",
+        "Decimal `value` prop must be a string"
       );
-    }
-    return initialValue;
-  };
+
+      if (initialValue && !allowEmptyValue) {
+        invariant(
+          initialValue !== "",
+          "Decimal `value` must not be an empty string. Please use `allowEmptyValue` or `0.00`"
+        );
+      }
+      return initialValue;
+    },
+    [allowEmptyValue]
+  );
 
   const getSeparator = useCallback(
     (separatorType) => {
@@ -227,17 +230,31 @@ const Decimal = ({
     prevControlledRef.current = isControlled;
   }, [isControlled]);
 
+  const prevValue = usePrevious(value);
+
   useEffect(() => {
-    const unformattedValue = toStandardDecimal(stateValue);
+    const standardDecimalValue = toStandardDecimal(stateValue);
 
     if (isControlled) {
       const valueProp = getSafeValueProp(value);
-      if (unformattedValue !== valueProp) {
-        setStateValue(formatValue(value));
+      if (standardDecimalValue !== valueProp) {
+        if (valueProp === "" && prevValue === "") {
+          setStateValue(formatValue(emptyValue));
+        } else {
+          setStateValue(formatValue(valueProp));
+        }
       }
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [value]);
+  }, [
+    emptyValue,
+    formatValue,
+    getSafeValueProp,
+    isControlled,
+    prevValue,
+    stateValue,
+    toStandardDecimal,
+    value,
+  ]);
 
   return (
     <>

diff --git a/src/components/decimal/decimal.spec.js b/src/components/decimal/decimal.spec.js
@@ -27,13 +27,13 @@ const DOM = (jsx) => {
   ReactDOM.render(jsx, container);
 };
 
-function render(props = {}, renderer = mount) {
+function render(props = {}, renderer = mount, mockComponent) {
   const defaultProps = {
     onChange,
     ...props,
   };
 
-  renderer(<Decimal {...defaultProps} />);
+  renderer(mockComponent || <Decimal {...defaultProps} />);
 }
 
 describe("Decimal", () => {
@@ -1117,7 +1117,19 @@ describe("Decimal", () => {
     });
 
     it("typing a negative value does not revert to the default value", () => {
-      render({ value: "123" });
+      const Foo = () => {
+        const [val, setVal] = React.useState("123");
+        return (
+          <Decimal
+            value={val}
+            onChange={(e) => {
+              onChange(e.target.value);
+              setVal(e.target.value.rawValue);
+            }}
+          />
+        );
+      };
+      render({}, mount, <Foo />);
       type("-");
       expect(onChange).toHaveBeenCalled();
       expect(value()).toBe("-");

diff --git a/src/components/decimal/decimal.test.js b/src/components/decimal/decimal.test.js
@@ -255,6 +255,26 @@ context("Tests for Decimal component", () => {
         });
     });
 
+    it("can have a custom onChange handler", () => {
+      const CustomDecimalComponent = ({ onChange, value, ...props }) => {
+        const [state, setState] = React.useState("0.01");
+        const handleChange = ({ target }) => {
+          let newValue = target.value.rawValue;
+          if (newValue.startsWith("22.22")) newValue = "22.22";
+          setState(newValue);
+        };
+
+        return <Decimal onChange={handleChange} value={state} {...props} />;
+      };
+
+      CypressMountWithProviders(<CustomDecimalComponent />);
+
+      commonDataElementInputPreview().type("22.222");
+      expect(
+        commonDataElementInputPreview().should("have.attr", "value", "22.22")
+      );
+    });
+
     it("should call onBlur callback when a blur event is triggered", () => {
       CypressMountWithProviders(<Decimal onBlur={callback} />);