react/fix inputNumber handleAdjust logic (#518)

* fix inputNumber handleAdjust logic * fix input currency min/max logic * refactor * add changelog * Update react/src/components/atoms/forms/InputNumber/index.js Co-Authored-By: clairesunstudio <[email protected]> * simplify input number handle change logic * simply handle blur and handle adjust on input number * wip * update input currency * Update changelogs/DP-13167.md * Update react/src/components/atoms/forms/InputCurrency/index.js Co-Authored-By: clairesunstudio <[email protected]> * Update react/src/components/atoms/forms/InputCurrency/index.js * Update react/src/components/atoms/forms/InputCurrency/index.js * Update react/src/components/atoms/forms/InputCurrency/index.js Co-Authored-By: avrilpearl <[email protected]> * cast to fixed back to number
massgov · Apr 1, 2019 · 15ea3e9 · 15ea3e9
1 parent bb40f5d
commit 15ea3e9
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 125 deletions.
diff --git a/changelogs/DP-13167.md b/changelogs/DP-13167.md
@@ -0,0 +1,4 @@
+Patch
+Fixed
+- (React) [InputCurrency, InputNumber] DP-13167: Fixed handleAdjust logic so that min/max are not required for up/down buttons to work. #518
+- (React) [InputNumber] DP-13167: Fixed the initial steps when using up/down without a default value and decimal formatting onBlur. #518
diff --git a/react/src/components/atoms/forms/InputCurrency/index.js b/react/src/components/atoms/forms/InputCurrency/index.js
@@ -47,148 +47,131 @@ const Currency = (props) => {
             }
             return number;
           };
-          const hasProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && !is.nil(obj[property]);
+          const hasNumberProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && is.number(obj[property]);
+          const greaterThanMin = (val) => !hasNumberProperty(props, 'min') || (val >= props.min);
+          const lessThanMax = (val) => !hasNumberProperty(props, 'max') || (val <= props.max);
+
+          const displayErrorMessage = (val) => {
+            const { min, max, required } = props;
+            if (required && !is.number(val)) {
+              const errorMsg = 'Please enter a value.';
+              return{
+                showError: true,
+                errorMsg
+              };
+            } else if (is.number(val)) {
+              const { showError, errorMsg } = validNumber(val, min, max);
+              return{
+                showError, errorMsg
+              };
+            }
+            return{
+              showError: false,
+              errorMsg: ''
+            };
+          };
+
           const handleChange = (e) => {
             const { type } = e;
             const stringValue = ref.current.value;
-            let numberValue;
-            const update = {
-              value: stringValue
-            };
-            if (is.empty(stringValue)) {
-              numberValue = 0;
-            } else {
-              numberValue = Number(numbro.unformat(stringValue));
-            }
-            // This validation is needed here as onKeyDown does not
-            // get the new value in the input after a key press.
-            if (props.required && is.empty(stringValue)) {
-              errorMsg = 'Please enter a value.';
-              update.showError = true;
-              update.errorMsg = errorMsg;
-            } else if (is.number(numberValue) && !is.empty(stringValue)) {
-              const validate = validNumber(numberValue, props.min, props.max);
-              update.showError = validate.showError;
-              update.errorMsg = validate.errorMsg;
-            } else {
-              errorMsg = '';
-              update.showError = false;
-              update.errorMsg = errorMsg;
-            }
-            context.updateState(update, () => {
-              if (typeof props.onChange === 'function') {
+            const numberValue = stringValue ? Number(numbro.unformat(stringValue)) : 0;
+            // If the stringvalue is empty, set to empty string so the required error
+            // message is rendered. Otherwise pass the number value for the min/max check.
+            const updateError = displayErrorMessage(!is.empty(stringValue) ? numberValue : '');
+            context.updateState({ value: stringValue, ...updateError }, () => {
+              if (is.fn(props.onChange)) {
                 props.onChange(numberValue, props.id, type);
               }
             });
           };
+
           const handleAdjust = (e) => {
             const direction = (e.currentTarget === upRef.current) ? 'up' : 'down';
             const { type } = e;
-            let numberValue;
             const stringValue = ref.current.value;
-            if (is.empty(stringValue)) {
-              numberValue = 0;
-            } else {
-              numberValue = Number(numbro.unformat(ref.current.value));
+            const numberValue = stringValue ? Number(numbro.unformat(stringValue)) : 0;
+            let newValue;
+            if (direction === 'up') {
+              newValue = Number(numbro(numberValue).add(props.step).format({ mantissa: countDecimals(props.step) }));
+            } else if (direction === 'down') {
+              newValue = Number(numbro(numberValue).subtract(props.step).format({ mantissa: countDecimals(props.step) }));
             }
-            if (is.number(numberValue)) {
-              let newValue;
-              if (direction === 'up') {
-                newValue = Number(numbro(numberValue).add(props.step).format({ mantissa: countDecimals(props.step) }));
-              } else if (direction === 'down') {
-                newValue = Number(numbro(numberValue).subtract(props.step).format({ mantissa: countDecimals(props.step) }));
-              }
-              if ((!hasProperty(props, 'min') || newValue >= props.min) && (!hasProperty(props, 'max') || (newValue <= props.max))) {
-                const { showError, errorMsg } = validNumber(newValue, props.min, props.max);
-                context.updateState({
-                  showError,
-                  errorMsg,
-                  value: toCurrency(newValue, countDecimals(props.step))
-                }, () => {
-                  if (typeof props.onChange === 'function') {
-                    props.onChange(newValue, props.id, type, direction);
-                  }
-                });
-              }
+            if (greaterThanMin(newValue) && lessThanMax(newValue)) {
+              const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : '');
+              context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => {
+                if (is.fn(props.onChange)) {
+                  props.onChange(newValue, props.id, type, direction);
+                }
+              });
             }
           };
+
           const handleKeyDown = (e) => {
             const { type, key } = e;
             const stringValue = ref.current.value;
-            let numberValue;
-            if (is.empty(stringValue)) {
-              numberValue = 0;
-            } else {
-              numberValue = Number(numbro.unformat(stringValue));
-            }
+            const numberValue = stringValue ? Number(numbro.unformat(stringValue)) : 0;
             // default to 0 if defaultValue is NaN
             if (is.number(numberValue) && !is.empty(stringValue)) {
               let newValue = numberValue;
               if (key === 'ArrowDown') {
                 newValue = Number(numbro(numberValue).subtract(props.step).format({ mantissa: countDecimals(props.step) }));
-                if ((!hasProperty(props, 'min') || newValue >= props.min) && (!hasProperty(props, 'max') || (newValue <= props.max))) {
-                  const { showError, errorMsg } = validNumber(newValue, props.min, props.max);
-                  context.updateState({
-                    showError,
-                    errorMsg,
-                    value: toCurrency(newValue, countDecimals(props.step))
-                  }, () => {
-                    if (typeof props.onChange === 'function') {
+                if (greaterThanMin(newValue) && lessThanMax(newValue)) {
+                  const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : '');
+                  context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => {
+                    if (is.fn(props.onChange)) {
                       props.onChange(newValue, props.id, type, key);
                     }
                   });
                 }
               } else if (key === 'ArrowUp') {
                 newValue = Number(numbro(numberValue).add(props.step).format({ mantissa: countDecimals(props.step) }));
-                if ((!hasProperty(props, 'min') || newValue >= props.min) && (!hasProperty(props, 'max') || (newValue <= props.max))) {
-                  const { showError, errorMsg } = validNumber(newValue, props.min, props.max);
-                  context.updateState({
-                    showError,
-                    errorMsg,
-                    value: toCurrency(newValue, countDecimals(props.step))
-                  }, () => {
-                    if (typeof props.onChange === 'function') {
+                if (greaterThanMin(newValue) && lessThanMax(newValue)) {
+                  const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : '');
+                  context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => {
+                    if (is.fn(props.onChange)) {
                       props.onChange(newValue, props.id, type, key);
                     }
                   });
                 }
               }
             }
           };
+
           const handleBlur = () => {
             const inputEl = ref.current;
-            if (is.empty(inputEl.value)) {
+            const stringValue = inputEl.value;
+            // isNotNumber returns true if stringValue is null, undefined or 'NaN'
+            const isNotNumber = !stringValue || isNaN(Number(numbro.unformat(stringValue)));
+            if (isNotNumber) {
               inputEl.setAttribute('placeholder', props.placeholder);
             }
-            const stringValue = inputEl.value;
-            const numberValue = Number(numbro.unformat(stringValue));
+            let newValue = isNotNumber ? '' : Number(numbro.unformat(stringValue));
             if (props.required && is.empty(stringValue)) {
               errorMsg = 'Please enter a value.';
               context.updateState({ showError: true, errorMsg });
             } else if (!is.empty(stringValue)) {
-              let newValue = numberValue;
-              if (hasProperty(props, 'max') && newValue > props.max) {
+              if (!hasNumberProperty(props, 'max') || newValue > props.max) {
                 newValue = props.max;
               }
-              if (hasProperty(props, 'min') && newValue < props.min) {
+              if (!hasNumberProperty(props, 'min') || newValue < props.min) {
                 newValue = props.min;
               }
-              const { showError, errorMsg } = validNumber(newValue, props.min, props.max);
-              context.updateState({ showError, errorMsg, value: toCurrency(newValue, countDecimals(props.step)) }, () => {
-                // invokes custom function if passed in the component
+              const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : '');
+              context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => {
                 if (is.fn(props.onBlur)) {
-                  // context.value won't be immediately changed, so pass new value over.
-                  props.onBlur(numberValue);
+                  props.onBlur(newValue);
                 }
               });
             }
           };
+
           const handleFocus = () => {
             const inputEl = ref.current;
             if (is.empty(inputEl.value)) {
               inputEl.removeAttribute('placeholder');
             }
           };
+
           const inputAttr = {
             className: inputClasses,
             name: props.name,
@@ -207,6 +190,7 @@ const Currency = (props) => {
             value: context.getValue(),
             disabled: props.disabled
           };
+
           return(
             <div className="ma__input-currency">
               <input {...inputAttr} />

diff --git a/react/src/components/atoms/forms/InputNumber/index.js b/react/src/components/atoms/forms/InputNumber/index.js
@@ -52,45 +52,35 @@ const NumberInput = (props) => {
               errorMsg: ''
             };
           };
-          const hasProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && !is.nil(obj[property]);
+
+          const hasNumberProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && is.number(obj[property]);
 
           const handleOnBlur = (e) => {
             e.persist();
             const inputEl = ref.current;
-            let newValue = Number(inputEl.value);
-            if ((hasProperty(props, 'max') && newValue > props.max) || (hasProperty(props, 'min') && newValue < props.min)) {
-              if (hasProperty(props, 'max') && newValue > props.max) {
-                newValue = props.max;
-              }
-              if (hasProperty(props, 'min') && newValue < props.min) {
-                newValue = props.min;
-              }
+            let newValue = inputEl.value ? Number(inputEl.value) : inputEl.value;
+            if (hasNumberProperty(props, 'max') && newValue > props.max) {
+              newValue = props.max;
             }
-            if (!is.empty(inputEl.value)) {
-              inputEl.value = Number(numbro(newValue)
-                .format({ mantissa: countDecimals(props.step) }));
+            if (hasNumberProperty(props, 'min') && newValue < props.min) {
+              newValue = props.min;
+            }
+            if (is.number(newValue)) {
+              // Since to Fixed returns a string, we have to cast it back to a Number
+              newValue = Number(newValue.toFixed(countDecimals(props.step)));
               const updateError = displayErrorMessage(newValue);
-              context.updateState({ value: inputEl.value, ...updateError }, () => {
+              context.updateState({ value: newValue, ...updateError }, () => {
                 if (is.fn(props.onBlur)) {
-                  props.onBlur(e, inputEl.value);
+                  props.onBlur(e, newValue);
                 }
               });
             }
           };
 
           const handleChange = (e) => {
-            const inputEl = ref.current;
             e.persist();
-            let newValue;
-            if (is.empty(inputEl.value)) {
-              newValue = inputEl.value;
-            } else {
-              newValue = Number(inputEl.value);
-              if (is.number(newValue)) {
-                newValue = Number(numbro(inputEl.value)
-                  .format({ mantissa: countDecimals(props.step) }));
-              }
-            }
+            const inputEl = ref.current;
+            const newValue = inputEl.value ? Number(inputEl.value) : inputEl.value;
             const updateError = displayErrorMessage(newValue);
             context.updateState({ value: newValue, ...updateError }, () => {
               if (is.fn(props.onChange)) {
@@ -100,29 +90,33 @@ const NumberInput = (props) => {
           };
 
           const handleAdjust = (e) => {
+            e.persist();
             let direction;
             if (e.currentTarget === upRef.current) {
               direction = 'up';
             } else {
               direction = 'down';
             }
             const inputEl = ref.current;
-            if (direction === 'up' && (!hasProperty(props, 'max') || inputEl.value < props.max)) {
-              if (is.empty(inputEl.value)) {
-                inputEl.value = 1;
-              } else {
-                inputEl.value = Number(numbro(inputEl.value)
-                  .add(props.step).value());
-              }
-              handleChange(e);
-            } else if (direction === 'down' && (!hasProperty(props, 'min') || inputEl.value > props.min)) {
-              if (is.empty(inputEl.value)) {
-                inputEl.value = -1;
-              } else {
-                inputEl.value = Number(numbro(inputEl.value)
-                  .subtract(props.step).value());
-              }
-              handleChange(e);
+            let newValue = inputEl.value ? Number(inputEl.value) : inputEl.value;
+            if (direction === 'up' && (!hasNumberProperty(props, 'max') || newValue < props.max)) {
+              // Since to Fixed returns a string, we have to cast it back to a Number
+              newValue = newValue ? Number((newValue + props.step).toFixed(countDecimals(props.step))) : props.step;
+              const updateError = displayErrorMessage(newValue);
+              context.updateState({ value: newValue, ...updateError }, () => {
+                if (is.fn(props.onChange)) {
+                  props.onChange(e, newValue, props.id);
+                }
+              });
+            } else if (direction === 'down' && (!hasNumberProperty(props, 'min') || newValue > props.min)) {
+              // Since to Fixed returns a string, we have to cast it back to a Number
+              newValue = newValue ? Number((newValue + props.step * -1).toFixed(countDecimals(props.step))) : (props.step * -1);
+              const updateError = displayErrorMessage(newValue);
+              context.updateState({ value: newValue, ...updateError }, () => {
+                if (is.fn(props.onChange)) {
+                  props.onChange(e, newValue, props.id);
+                }
+              });
             }
           };